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

refactor: port redirect handler to new hooks #3879

Merged
merged 2 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
43 changes: 42 additions & 1 deletion lib/core/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,46 @@ function parseHeaders (headers, obj) {
return obj
}

function normalizeHeaders (headers) {
const ret = {}
if (headers == null) {
// Do nothing...
} else if (Array.isArray(headers)) {
for (let i = 0; i < headers.length; i += 2) {
if (Array.isArray(headers[i]) && headers[i].length === 2) {
const key = headerNameToString(headers[i][0])
const val = ret[key]
if (val == null) {
ret[key] = String(headers[i][1])
} else if (typeof val === 'string') {
ret[key] = [val, String(headers[i][1])]
} else {
val.push(String(headers[i][1]))
}
} else {
const key = headerNameToString(headers[i])
const val = ret[key]
if (val == null) {
ret[key] = String(headers[i + 1])
} else if (typeof val === 'string') {
ret[key] = [val, String(headers[i + 1])]
} else {
val.push(String(headers[i + 1]))
}
}
}
} else if (typeof headers[Symbol.iterator] === 'function') {
for (const [key, val] of headers) {
ret[headerNameToString(key)] = Array.isArray(val) ? val.map(x => String(x)) : String(val)
}
} else {
for (const [key, val] of Object.entries(headers)) {
ret[headerNameToString(key)] = Array.isArray(val) ? val.map(x => String(x)) : String(val)
}
}
return ret
}
Copy link
Member

Choose a reason for hiding this comment

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

This logic seems very complex. why is it needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

To normalize different headers shapes?


/**
* @param {Buffer[]} headers
* @returns {string[]}
Expand Down Expand Up @@ -903,5 +943,6 @@ module.exports = {
nodeMajor,
nodeMinor,
safeHTTPMethods: Object.freeze(['GET', 'HEAD', 'OPTIONS', 'TRACE']),
wrapRequestBody
wrapRequestBody,
normalizeHeaders
}
4 changes: 1 addition & 3 deletions lib/handler/cache-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ class CacheHandler {
* @param {import('../../types/cache-interceptor.d.ts').default.CacheKey} cacheKey
* @param {import('../../types/dispatcher.d.ts').default.DispatchHandler} handler
*/
constructor (opts, cacheKey, handler) {
const { store } = opts

constructor ({ store }, cacheKey, handler) {
this.#store = store
this.#cacheKey = cacheKey
this.#handler = handler
Expand Down
3 changes: 3 additions & 0 deletions lib/handler/decorator-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

const assert = require('node:assert')

/**
* @deprecated
*/
module.exports = class DecoratorHandler {
#handler
#onCompleteCalled = false
Expand Down
88 changes: 28 additions & 60 deletions lib/handler/redirect-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ class RedirectHandler {

this.dispatch = dispatch
this.location = null
this.abort = null
this.opts = { ...opts, maxRedirections: 0 } // opts must be a copy
this.maxRedirections = maxRedirections
this.handler = handler
Expand Down Expand Up @@ -83,20 +82,16 @@ class RedirectHandler {
}
}

onConnect (abort) {
this.abort = abort
this.handler.onConnect(abort, { history: this.history })
}

onUpgrade (statusCode, headers, socket) {
this.handler.onUpgrade(statusCode, headers, socket)
onRequestStart (controller, context) {
this.location = null
this.handler.onRequestStart?.(controller, { ...context, history: this.history })
}

onError (error) {
this.handler.onError(error)
onRequestUpgrade (controller, statusCode, headers, socket) {
this.handler.onRequestUpgrade?.(controller, statusCode, headers, socket)
}

onHeaders (statusCode, rawHeaders, resume, statusText) {
onResponseStart (controller, statusCode, statusMessage, headers) {
if (this.opts.throwOnMaxRedirect && this.history.length >= this.maxRedirections) {
throw new Error('max redirects')
}
Expand All @@ -122,16 +117,17 @@ class RedirectHandler {
this.opts.body = null
}

this.location = this.history.length >= this.maxRedirections || util.isDisturbed(this.opts.body)
this.location = this.history.length >= this.maxRedirections || util.isDisturbed(this.opts.body) || redirectableStatusCodes.indexOf(statusCode) === -1
? null
: parseLocation(statusCode, rawHeaders)
: headers.location

if (this.opts.origin) {
this.history.push(new URL(this.opts.path, this.opts.origin))
}

if (!this.location) {
return this.handler.onHeaders(statusCode, rawHeaders, resume, statusText)
this.handler.onResponseStart?.(controller, statusCode, statusMessage, headers)
return
}

const { origin, pathname, search } = util.parseURL(new URL(this.location, this.opts.origin && new URL(this.opts.path, this.opts.origin)))
Expand All @@ -140,14 +136,16 @@ class RedirectHandler {
// Remove headers referring to the original URL.
// By default it is Host only, unless it's a 303 (see below), which removes also all Content-* headers.
// https://tools.ietf.org/html/rfc7231#section-6.4
this.opts.headers = cleanRequestHeaders(this.opts.headers, statusCode === 303, this.opts.origin !== origin)
this.opts.headers = this.opts.headers
? cleanRequestHeaders(this.opts.headers, statusCode === 303, this.opts.origin !== origin)
: this.opts.headers
this.opts.path = path
this.opts.origin = origin
this.opts.maxRedirections = 0
this.opts.query = null
}

onData (chunk) {
onResponseData (controller, chunk) {
if (this.location) {
/*
https://tools.ietf.org/html/rfc7231#section-6.4
Expand All @@ -167,11 +165,11 @@ class RedirectHandler {
servers and browsers implementors, we ignore the body as there is no specified way to eventually parse it.
*/
} else {
return this.handler.onData(chunk)
this.handler.onResponseData?.(controller, chunk)
}
}

onComplete (trailers) {
onResponseEnd (controller, trailers) {
if (this.location) {
/*
https://tools.ietf.org/html/rfc7231#section-6.4
Expand All @@ -181,68 +179,38 @@ class RedirectHandler {
See comment on onData method above for more detailed information.
*/

this.location = null
this.abort = null

this.dispatch(this.opts, this)
} else {
this.handler.onComplete(trailers)
this.handler.onResponseEnd(controller, trailers)
}
}

onBodySent (chunk) {
if (this.handler.onBodySent) {
this.handler.onBodySent(chunk)
}
}
}

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

for (let i = 0; i < rawHeaders.length; i += 2) {
if (rawHeaders[i].length === 8 && util.headerNameToString(rawHeaders[i]) === 'location') {
return rawHeaders[i + 1]
}
onResponseError (controller, error) {
this.handler.onResponseError?.(controller, error)
}
}

// https://tools.ietf.org/html/rfc7231#section-6.4.4
function shouldRemoveHeader (header, removeContent, unknownOrigin) {
if (header.length === 4) {
return util.headerNameToString(header) === 'host'
function shouldRemoveHeader (name, removeContent, unknownOrigin) {
if (name.length === 4) {
return name === 'host'
}
if (removeContent && util.headerNameToString(header).startsWith('content-')) {
if (removeContent && name.startsWith('content-')) {
return true
}
if (unknownOrigin && (header.length === 13 || header.length === 6 || header.length === 19)) {
const name = util.headerNameToString(header)
if (unknownOrigin && (name.length === 13 || name.length === 6 || name.length === 19)) {
return name === 'authorization' || name === 'cookie' || name === 'proxy-authorization'
}
return false
}

// https://tools.ietf.org/html/rfc7231#section-6.4
function cleanRequestHeaders (headers, removeContent, unknownOrigin) {
const ret = []
if (Array.isArray(headers)) {
for (let i = 0; i < headers.length; i += 2) {
if (!shouldRemoveHeader(headers[i], removeContent, unknownOrigin)) {
ret.push(headers[i], headers[i + 1])
}
}
} else if (headers && typeof headers === 'object') {
const entries = typeof headers[Symbol.iterator] === 'function' ? headers : Object.entries(headers)
for (const [key, value] of entries) {
if (!shouldRemoveHeader(key, removeContent, unknownOrigin)) {
ret.push(key, value)
}
const ret = util.normalizeHeaders(headers)
for (const name of Object.keys(ret)) {
if (shouldRemoveHeader(name, removeContent, unknownOrigin)) {
delete ret[name]
}
} else {
assert(headers == null, 'headers must be an object or an array')
}
return ret
}
Expand Down
2 changes: 1 addition & 1 deletion lib/handler/unwrap-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ module.exports = class UnwrapHandler {
}

onError (err) {
if (!this.#handler.onError) {
if (!this.#handler.onResponseError) {
throw new InvalidArgumentError('invalid onError method')
}

Expand Down
Loading