diff --git a/lib/core/request.js b/lib/core/request.js index 16a1efffe61..c027fd74689 100644 --- a/lib/core/request.js +++ b/lib/core/request.js @@ -5,7 +5,6 @@ const { NotSupportedError } = require('./errors') const assert = require('node:assert') -const { kHTTP2BuildRequest, kHTTP2CopyHeaders, kHTTP1BuildRequest } = require('./symbols') const util = require('./util') const { channels } = require('./diagnostics.js') const { headerNameLowerCasedRecord } = require('./constants') @@ -149,7 +148,7 @@ class Request { this.contentType = null - this.headers = '' + this.headers = [] // Only for H2 this.expectContinue = expectContinue != null ? expectContinue : false @@ -315,73 +314,6 @@ class Request { processHeader(this, key, value) return this } - - static [kHTTP1BuildRequest] (origin, opts, handler) { - // TODO: Migrate header parsing here, to make Requests - // HTTP agnostic - return new Request(origin, opts, handler) - } - - static [kHTTP2BuildRequest] (origin, opts, handler) { - const headers = opts.headers - opts = { ...opts, headers: null } - - const request = new Request(origin, opts, handler) - - request.headers = {} - - if (Array.isArray(headers)) { - if (headers.length % 2 !== 0) { - throw new InvalidArgumentError('headers array must be even') - } - for (let i = 0; i < headers.length; i += 2) { - processHeader(request, headers[i], headers[i + 1], true) - } - } else if (headers && typeof headers === 'object') { - const keys = Object.keys(headers) - for (let i = 0; i < keys.length; i++) { - const key = keys[i] - processHeader(request, key, headers[key], true) - } - } else if (headers != null) { - throw new InvalidArgumentError('headers must be an object or an array') - } - - return request - } - - static [kHTTP2CopyHeaders] (raw) { - const rawHeaders = raw.split('\r\n') - const headers = {} - - for (const header of rawHeaders) { - const [key, value] = header.split(': ') - - if (value == null || value.length === 0) continue - - if (headers[key]) { - headers[key] += `,${value}` - } else { - headers[key] = value - } - } - - return headers - } -} - -function processHeaderValue (key, val, skipAppend) { - if (val && typeof val === 'object') { - throw new InvalidArgumentError(`invalid ${key} header`) - } - - val = val != null ? `${val}` : '' - - if (headerCharRegex.exec(val) !== null) { - throw new InvalidArgumentError(`invalid ${key} header`) - } - - return skipAppend ? val : `${key}: ${val}\r\n` } function processHeader (request, key, val, skipAppend = false) { @@ -400,10 +332,39 @@ function processHeader (request, key, val, skipAppend = false) { } } - if (request.host === null && headerName === 'host') { + if (Array.isArray(val)) { + const arr = [] + for (let i = 0; i < val.length; i++) { + if (typeof val[i] === 'string') { + if (headerCharRegex.exec(val[i]) !== null) { + throw new InvalidArgumentError(`invalid ${key} header`) + } + arr.push(val[i]) + } else if (val[i] === null) { + arr.push('') + } else if (typeof val[i] === 'object') { + throw new InvalidArgumentError(`invalid ${key} header`) + } else { + arr.push(`${val[i]}`) + } + } + val = arr + } else if (typeof val === 'string') { if (headerCharRegex.exec(val) !== null) { throw new InvalidArgumentError(`invalid ${key} header`) } + } else if (val === null) { + val = '' + } else if (typeof val === 'object') { + throw new InvalidArgumentError(`invalid ${key} header`) + } else { + val = `${val}` + } + + if (request.host === null && headerName === 'host') { + if (typeof val !== 'string') { + throw new InvalidArgumentError('invalid host header') + } // Consumed by Client request.host = val } else if (request.contentLength === null && headerName === 'content-length') { @@ -413,35 +374,22 @@ function processHeader (request, key, val, skipAppend = false) { } } else if (request.contentType === null && headerName === 'content-type') { request.contentType = val - if (skipAppend) request.headers[key] = processHeaderValue(key, val, skipAppend) - else request.headers += processHeaderValue(key, val) + request.headers.push(key, val) } else if (headerName === 'transfer-encoding' || headerName === 'keep-alive' || headerName === 'upgrade') { throw new InvalidArgumentError(`invalid ${headerName} header`) } else if (headerName === 'connection') { const value = typeof val === 'string' ? val.toLowerCase() : null if (value !== 'close' && value !== 'keep-alive') { throw new InvalidArgumentError('invalid connection header') - } else if (value === 'close') { + } + + if (value === 'close') { request.reset = true } } else if (headerName === 'expect') { throw new NotSupportedError('expect header not supported') - } else if (Array.isArray(val)) { - for (let i = 0; i < val.length; i++) { - if (skipAppend) { - if (request.headers[key]) { - request.headers[key] += `,${processHeaderValue(key, val[i], skipAppend)}` - } else { - request.headers[key] = processHeaderValue(key, val[i], skipAppend) - } - } else { - request.headers += processHeaderValue(key, val[i]) - } - } - } else if (skipAppend) { - request.headers[key] = processHeaderValue(key, val, skipAppend) } else { - request.headers += processHeaderValue(key, val) + request.headers.push(key, val) } } diff --git a/lib/core/symbols.js b/lib/core/symbols.js index f3c6e1d5339..02fda9e251d 100644 --- a/lib/core/symbols.js +++ b/lib/core/symbols.js @@ -56,9 +56,6 @@ module.exports = { kMaxResponseSize: Symbol('max response size'), kHTTP2Session: Symbol('http2Session'), kHTTP2SessionState: Symbol('http2Session state'), - kHTTP2BuildRequest: Symbol('http2 build request'), - kHTTP1BuildRequest: Symbol('http1 build request'), - kHTTP2CopyHeaders: Symbol('http2 copy headers'), kRetryHandlerDefaultRetry: Symbol('retry agent default retry'), kConstruct: Symbol('constructable'), kListeners: Symbol('listeners'), diff --git a/lib/core/util.js b/lib/core/util.js index 96e76cc1355..9b331089b2a 100644 --- a/lib/core/util.js +++ b/lib/core/util.js @@ -516,6 +516,7 @@ kEnumerableProperty.enumerable = true module.exports = { kEnumerableProperty, nop, + isDisturbed, isErrored, isReadable, diff --git a/lib/dispatcher/client-h1.js b/lib/dispatcher/client-h1.js index 3cc9b3123b8..df644e0360c 100644 --- a/lib/dispatcher/client-h1.js +++ b/lib/dispatcher/client-h1.js @@ -817,12 +817,12 @@ function writeH1 (client, request) { const [bodyStream, contentType] = extractBody(body) if (request.contentType == null) { - headers += `content-type: ${contentType}\r\n` + headers.push('content-type', contentType) } body = bodyStream.stream contentLength = bodyStream.length } else if (util.isBlobLike(body) && request.contentType == null && body.type) { - headers += `content-type: ${body.type}\r\n` + headers.push('content-type', body.type) } if (body && typeof body.read === 'function') { @@ -922,8 +922,19 @@ function writeH1 (client, request) { header += 'connection: close\r\n' } - if (headers) { - header += headers + if (Array.isArray(headers)) { + for (let n = 0; n < headers.length; n += 2) { + const key = headers[n + 0] + const val = headers[n + 1] + + if (Array.isArray(val)) { + for (let i = 0; i < val.length; i++) { + header += `${key}: ${val[i]}\r\n` + } + } else { + header += `${key}: ${val}\r\n` + } + } } if (channels.sendHeaders.hasSubscribers) { diff --git a/lib/dispatcher/client-h2.js b/lib/dispatcher/client-h2.js index 04d24b741b3..5a6cb2ed91b 100644 --- a/lib/dispatcher/client-h2.js +++ b/lib/dispatcher/client-h2.js @@ -3,7 +3,6 @@ const assert = require('node:assert') const { pipeline } = require('node:stream') const util = require('../core/util.js') -const Request = require('../core/request.js') const { RequestContentLengthMismatchError, RequestAbortedError, @@ -26,7 +25,6 @@ const { // HTTP2 kMaxConcurrentStreams, kHTTP2Session, - kHTTP2CopyHeaders, kResume } = require('../core/symbols.js') @@ -215,10 +213,6 @@ function writeH2 (client, request) { const session = client[kHTTP2Session] const { body, method, path, host, upgrade, expectContinue, signal, headers: reqHeaders } = request - let headers - if (typeof reqHeaders === 'string') headers = Request[kHTTP2CopyHeaders](reqHeaders.trim()) - else headers = reqHeaders - if (upgrade) { errorRequest(client, request, new Error('Upgrade not supported for H2')) return false @@ -228,6 +222,24 @@ function writeH2 (client, request) { return false } + const headers = {} + for (let n = 0; n < reqHeaders.length; n += 2) { + const key = reqHeaders[n + 0] + const val = reqHeaders[n + 1] + + if (Array.isArray(val)) { + for (let i = 0; i < val.length; i++) { + if (headers[key]) { + headers[key] += `,${val[i]}` + } else { + headers[key] = val[i] + } + } + } else { + headers[key] = val + } + } + /** @type {import('node:http2').ClientHttp2Stream} */ let stream diff --git a/lib/dispatcher/client.js b/lib/dispatcher/client.js index 2081a4bcd79..8a4e428f7b4 100644 --- a/lib/dispatcher/client.js +++ b/lib/dispatcher/client.js @@ -60,8 +60,6 @@ const { kHTTPContext, // HTTP2 kMaxConcurrentStreams, - kHTTP2BuildRequest, - kHTTP1BuildRequest, kResume } = require('../core/symbols.js') const connectH1 = require('./client-h1.js') @@ -296,12 +294,7 @@ class Client extends DispatcherBase { [kDispatch] (opts, handler) { const origin = opts.origin || this[kUrl].origin - - // TODO (fix): Why do these need to be - // TODO (fix): This can happen before connect... - const request = this[kHTTPContext]?.version === 'h2' - ? Request[kHTTP2BuildRequest](origin, opts, handler) - : Request[kHTTP1BuildRequest](origin, opts, handler) + const request = new Request(origin, opts, handler) this[kQueue].push(request) if (this[kResuming]) { diff --git a/test/node-test/diagnostics-channel/post-stream.js b/test/node-test/diagnostics-channel/post-stream.js index 49fa0be1a04..881873a7c1c 100644 --- a/test/node-test/diagnostics-channel/post-stream.js +++ b/test/node-test/diagnostics-channel/post-stream.js @@ -33,9 +33,9 @@ test('Diagnostics channel - post stream', (t) => { assert.equal(request.completed, false) assert.equal(request.method, 'POST') assert.equal(request.path, '/') - assert.equal(request.headers, 'bar: bar\r\n') + assert.deepStrictEqual(request.headers, ['bar', 'bar']) request.addHeader('hello', 'world') - assert.equal(request.headers, 'bar: bar\r\nhello: world\r\n') + assert.deepStrictEqual(request.headers, ['bar', 'bar', 'hello', 'world']) assert.deepStrictEqual(request.body, body) }) diff --git a/test/node-test/diagnostics-channel/post.js b/test/node-test/diagnostics-channel/post.js index cddb22ace17..e39ef402316 100644 --- a/test/node-test/diagnostics-channel/post.js +++ b/test/node-test/diagnostics-channel/post.js @@ -31,9 +31,9 @@ test('Diagnostics channel - post', (t) => { assert.equal(request.completed, false) assert.equal(request.method, 'POST') assert.equal(request.path, '/') - assert.equal(request.headers, 'bar: bar\r\n') + assert.equal(request.headers, ['bar', 'bar']) request.addHeader('hello', 'world') - assert.equal(request.headers, 'bar: bar\r\nhello: world\r\n') + assert.equal(request.headers, ['bar', 'bar', 'hello', 'world']) assert.deepStrictEqual(request.body, Buffer.from('hello world')) })