Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: always parse headers #3408

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions docs/docs/api/DiagnosticsChannel.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ diagnosticsChannel.channel('undici:request:create').subscribe(({ request }) => {
console.log('path', request.path)
console.log('headers') // array of strings, e.g: ['foo', 'bar']
request.addHeader('hello', 'world')
console.log('headers', request.headers) // e.g. ['foo', 'bar', 'hello', 'world']
console.log('headers', request.headers) // e.g. {'foo': 'bar', 'hello': 'world']
})
```

Expand Down Expand Up @@ -49,8 +49,8 @@ diagnosticsChannel.channel('undici:request:headers').subscribe(({ request, respo
// request is the same object undici:request:create
console.log('statusCode', response.statusCode)
console.log(response.statusText)
// response.headers are buffers.
console.log(response.headers.map((x) => x.toString()))
// response.headers are an object.
console.log(response.headers)
})
```

Expand All @@ -64,8 +64,8 @@ import diagnosticsChannel from 'diagnostics_channel'
diagnosticsChannel.channel('undici:request:trailers').subscribe(({ request, trailers }) => {
// request is the same object undici:request:create
console.log('completed', request.completed)
// trailers are buffers.
console.log(trailers.map((x) => x.toString()))
// trailers are an object.
console.log(trailers)
})
```

Expand Down
6 changes: 3 additions & 3 deletions docs/docs/api/Dispatcher.md
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,9 @@ Returns: `Boolean` - `false` if dispatcher is busy and further dispatch calls wo

* **onConnect** `(abort: () => void, context: object) => void` - Invoked before request is dispatched on socket. May be invoked multiple times when a request is retried when the request at the head of the pipeline fails.
* **onError** `(error: Error) => void` - Invoked when an error has occurred. May not throw.
* **onUpgrade** `(statusCode: number, headers: Buffer[], socket: Duplex) => void` (optional) - Invoked when request is upgraded. Required if `DispatchOptions.upgrade` is defined or `DispatchOptions.method === 'CONNECT'`.
* **onUpgrade** `(statusCode: number, headers: Record<string, string>, socket: Duplex) => void` (optional) - Invoked when request is upgraded. Required if `DispatchOptions.upgrade` is defined or `DispatchOptions.method === 'CONNECT'`.
* **onResponseStarted** `() => void` (optional) - Invoked when response is received, before headers have been read.
* **onHeaders** `(statusCode: number, headers: Buffer[], resume: () => void, statusText: string) => boolean` - Invoked when statusCode and headers have been received. May be invoked multiple times due to 1xx informational headers. Not required for `upgrade` requests.
* **onHeaders** `(statusCode: number, headers: Record<string, string>, resume: () => void, statusText: string) => boolean` - Invoked when statusCode and headers have been received. May be invoked multiple times due to 1xx informational headers. Not required for `upgrade` requests.
* **onData** `(chunk: Buffer) => boolean` - Invoked when response payload data is received. Not required for `upgrade` requests.
* **onComplete** `(trailers: Buffer[]) => void` - Invoked when response payload and trailers have been received and the request has completed. Not required for `upgrade` requests.
* **onBodySent** `(chunk: string | Buffer | Uint8Array) => void` - Invoked when a body chunk is sent to the server. Not required. For a stream or iterable body this will be invoked for every chunk. For other body types, it will be invoked once after the body is sent.
Expand Down Expand Up @@ -975,7 +975,7 @@ const client = new Client("http://example.com").compose(
})
);

// or
// or
client.dispatch(
{
path: "/",
Expand Down
14 changes: 5 additions & 9 deletions lib/api/api-connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
const assert = require('node:assert')
const { AsyncResource } = require('node:async_hooks')
const { InvalidArgumentError, SocketError } = require('../core/errors')
const util = require('../core/util')
const { addSignal, removeSignal } = require('./abort-signal')

class ConnectHandler extends AsyncResource {
Expand All @@ -22,10 +21,13 @@ class ConnectHandler extends AsyncResource {
throw new InvalidArgumentError('signal must be an EventEmitter or EventTarget')
}

if (responseHeaders != null) {
throw new InvalidArgumentError('responseHeaders must be nully')
}

super('UNDICI_CONNECT')

this.opaque = opaque || null
this.responseHeaders = responseHeaders || null
this.callback = callback
this.abort = null

Expand All @@ -48,19 +50,13 @@ class ConnectHandler extends AsyncResource {
throw new SocketError('bad connect', null)
}

onUpgrade (statusCode, rawHeaders, socket) {
onUpgrade (statusCode, headers, socket) {
const { callback, opaque, context } = this

removeSignal(this)

this.callback = null

let headers = rawHeaders
// Indicates is an HTTP2Session
if (headers != null) {
headers = this.responseHeaders === 'raw' ? util.parseRawHeaders(rawHeaders) : util.parseHeaders(rawHeaders)
}

this.runInAsyncScope(callback, null, null, {
statusCode,
headers,
Expand Down
9 changes: 5 additions & 4 deletions lib/api/api-pipeline.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,17 @@ class PipelineHandler extends AsyncResource {
throw new InvalidArgumentError('invalid method')
}

if (responseHeaders != null) {
throw new InvalidArgumentError('responseHeaders must be nully')
}

if (onInfo && typeof onInfo !== 'function') {
throw new InvalidArgumentError('invalid onInfo callback')
}

super('UNDICI_PIPELINE')

this.opaque = opaque || null
this.responseHeaders = responseHeaders || null
this.handler = handler
this.abort = null
this.context = null
Expand Down Expand Up @@ -158,12 +161,11 @@ class PipelineHandler extends AsyncResource {
this.context = context
}

onHeaders (statusCode, rawHeaders, resume) {
onHeaders (statusCode, headers, resume) {
const { opaque, handler, context } = this

if (statusCode < 200) {
if (this.onInfo) {
const headers = this.responseHeaders === 'raw' ? util.parseRawHeaders(rawHeaders) : util.parseHeaders(rawHeaders)
this.onInfo({ statusCode, headers })
}
return
Expand All @@ -174,7 +176,6 @@ class PipelineHandler extends AsyncResource {
let body
try {
this.handler = null
const headers = this.responseHeaders === 'raw' ? util.parseRawHeaders(rawHeaders) : util.parseHeaders(rawHeaders)
body = this.runInAsyncScope(handler, null, {
statusCode,
headers,
Expand Down
18 changes: 9 additions & 9 deletions lib/api/api-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ class RequestHandler extends AsyncResource {
throw new InvalidArgumentError('invalid onInfo callback')
}

if (responseHeaders != null) {
throw new InvalidArgumentError('responseHeaders must be nully')
}

super('UNDICI_REQUEST')
} catch (err) {
if (util.isStream(body)) {
Expand All @@ -45,7 +49,6 @@ class RequestHandler extends AsyncResource {
}

this.method = method
this.responseHeaders = responseHeaders || null
this.opaque = opaque || null
this.callback = callback
this.res = null
Expand Down Expand Up @@ -85,10 +88,8 @@ class RequestHandler extends AsyncResource {
this.context = context
}

onHeaders (statusCode, rawHeaders, resume, statusMessage) {
const { callback, opaque, abort, context, responseHeaders, highWaterMark } = this

const headers = responseHeaders === 'raw' ? util.parseRawHeaders(rawHeaders) : util.parseHeaders(rawHeaders)
onHeaders (statusCode, headers, resume, statusMessage) {
const { callback, opaque, abort, context, highWaterMark } = this

if (statusCode < 200) {
if (this.onInfo) {
Expand All @@ -97,9 +98,8 @@ class RequestHandler extends AsyncResource {
return
}

const parsedHeaders = responseHeaders === 'raw' ? util.parseHeaders(rawHeaders) : headers
const contentType = parsedHeaders['content-type']
const contentLength = parsedHeaders['content-length']
const contentType = headers['content-type']
const contentLength = headers['content-length']
const res = new Readable({
resume,
abort,
Expand Down Expand Up @@ -140,7 +140,7 @@ class RequestHandler extends AsyncResource {
}

onComplete (trailers) {
util.parseHeaders(trailers, this.trailers)
Object.assign(this.trailers, trailers)
this.res.push(null)
}

Expand Down
16 changes: 8 additions & 8 deletions lib/api/api-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ class StreamHandler extends AsyncResource {
throw new InvalidArgumentError('invalid onInfo callback')
}

if (responseHeaders != null) {
throw new InvalidArgumentError('responseHeaders must be nully')
}

super('UNDICI_STREAM')
} catch (err) {
if (util.isStream(body)) {
Expand All @@ -45,7 +49,6 @@ class StreamHandler extends AsyncResource {
throw err
}

this.responseHeaders = responseHeaders || null
this.opaque = opaque || null
this.factory = factory
this.callback = callback
Expand Down Expand Up @@ -78,10 +81,8 @@ class StreamHandler extends AsyncResource {
this.context = context
}

onHeaders (statusCode, rawHeaders, resume, statusMessage) {
const { factory, opaque, context, callback, responseHeaders } = this

const headers = responseHeaders === 'raw' ? util.parseRawHeaders(rawHeaders) : util.parseHeaders(rawHeaders)
onHeaders (statusCode, headers, resume, statusMessage) {
const { factory, opaque, context, callback } = this

if (statusCode < 200) {
if (this.onInfo) {
Expand All @@ -95,8 +96,7 @@ class StreamHandler extends AsyncResource {
let res

if (this.throwOnError && statusCode >= 400) {
const parsedHeaders = responseHeaders === 'raw' ? util.parseHeaders(rawHeaders) : headers
const contentType = parsedHeaders['content-type']
const contentType = headers['content-type']
res = new PassThrough()

this.callback = null
Expand Down Expand Up @@ -168,7 +168,7 @@ class StreamHandler extends AsyncResource {
return
}

this.trailers = util.parseHeaders(trailers)
this.trailers = trailers

res.end()
}
Expand Down
9 changes: 5 additions & 4 deletions lib/api/api-upgrade.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
const { InvalidArgumentError, SocketError } = require('../core/errors')
const { AsyncResource } = require('node:async_hooks')
const assert = require('node:assert')
const util = require('../core/util')
const { addSignal, removeSignal } = require('./abort-signal')

class UpgradeHandler extends AsyncResource {
Expand All @@ -22,9 +21,12 @@ class UpgradeHandler extends AsyncResource {
throw new InvalidArgumentError('signal must be an EventEmitter or EventTarget')
}

if (responseHeaders != null) {
throw new InvalidArgumentError('responseHeaders must be nully')
}

super('UNDICI_UPGRADE')

this.responseHeaders = responseHeaders || null
this.opaque = opaque || null
this.callback = callback
this.abort = null
Expand All @@ -49,15 +51,14 @@ class UpgradeHandler extends AsyncResource {
throw new SocketError('bad upgrade', null)
}

onUpgrade (statusCode, rawHeaders, socket) {
onUpgrade (statusCode, headers, socket) {
const { callback, opaque, context } = this

assert.strictEqual(statusCode, 101)

removeSignal(this)

this.callback = null
const headers = this.responseHeaders === 'raw' ? util.parseRawHeaders(rawHeaders) : util.parseHeaders(rawHeaders)
this.runInAsyncScope(callback, null, null, {
headers,
socket,
Expand Down
15 changes: 10 additions & 5 deletions lib/core/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ const {
buildURL,
validateHandler,
getServerName,
normalizedMethodRecords
normalizedMethodRecords,
parseHeaders
} = require('./util')
const { channels } = require('./diagnostics.js')
const { headerNameLowerCasedRecord } = require('./constants')
Expand Down Expand Up @@ -232,10 +233,12 @@ class Request {
return this[kHandler].onResponseStarted?.()
}

onHeaders (statusCode, headers, resume, statusText) {
onHeaders (statusCode, rawHeaders, resume, statusText) {
assert(!this.aborted)
assert(!this.completed)

const headers = parseHeaders(rawHeaders)

if (channels.headers.hasSubscribers) {
channels.headers.publish({ request: this, response: { statusCode, headers, statusText } })
}
Expand All @@ -259,18 +262,20 @@ class Request {
}
}

onUpgrade (statusCode, headers, socket) {
onUpgrade (statusCode, rawHeaders, socket) {
assert(!this.aborted)
assert(!this.completed)

return this[kHandler].onUpgrade(statusCode, headers, socket)
return this[kHandler].onUpgrade(statusCode, parseHeaders(rawHeaders), socket)
}

onComplete (trailers) {
onComplete (rawTrailers) {
this.onFinally()

assert(!this.aborted)

const trailers = parseHeaders(rawTrailers)

this.completed = true
if (channels.trailers.hasSubscribers) {
channels.trailers.publish({ request: this, trailers })
Expand Down
5 changes: 3 additions & 2 deletions lib/core/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ function bufferToLowerCasedHeaderName (value) {
*/
function parseHeaders (headers, obj) {
if (obj === undefined) obj = {}
if (headers == null) return
for (let i = 0; i < headers.length; i += 2) {
const key = headerNameToString(headers[i])
let val = obj[key]
Expand All @@ -341,13 +342,13 @@ function parseHeaders (headers, obj) {
val = [val]
obj[key] = val
}
val.push(headers[i + 1].toString('utf8'))
val.push(headers[i + 1].toString('latin1'))
} else {
const headersValue = headers[i + 1]
if (typeof headersValue === 'string') {
obj[key] = headersValue
} else {
obj[key] = Array.isArray(headersValue) ? headersValue.map(x => x.toString('utf8')) : headersValue.toString('utf8')
obj[key] = Array.isArray(headersValue) ? headersValue.map(x => x.toString('latin1')) : headersValue.toString('latin1')
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adjusted the tests to align with the change

}
}
}
Expand Down
12 changes: 3 additions & 9 deletions lib/handler/redirect-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,15 +191,9 @@ class RedirectHandler {
}

function parseLocation (statusCode, headers) {
if (redirectableStatusCodes.indexOf(statusCode) === -1) {
return null
}

for (let i = 0; i < headers.length; i += 2) {
if (headers[i].length === 8 && util.headerNameToString(headers[i]) === 'location') {
return headers[i + 1]
}
}
return redirectableStatusCodes.indexOf(statusCode) === -1
? null
: headers.location
}

// https://tools.ietf.org/html/rfc7231#section-6.4.4
Expand Down
Loading
Loading