From b719e5e360db1dca1ca1acd4da88721083822119 Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Sun, 5 Apr 2026 09:04:44 +0900 Subject: [PATCH 1/2] fix: improve Response.json() and Response.redirect() spec compliance and efficiency - Throw TypeError for non-JSON-serializable data in Response.json() - Add URL validation with fast-path for common ASCII URLs in Response.redirect() - Remove redundant type assertions in Response.json() --- src/response.ts | 62 ++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 56 insertions(+), 6 deletions(-) diff --git a/src/response.ts b/src/response.ts index 3431ffe..d3926a6 100644 --- a/src/response.ts +++ b/src/response.ts @@ -113,16 +113,63 @@ Object.defineProperty(Response.prototype, Symbol.for('nodejs.util.inspect.custom Object.setPrototypeOf(Response, GlobalResponse) Object.setPrototypeOf(Response.prototype, GlobalResponse.prototype) -// Override Response.json() to return a LightweightResponse so the listener -// fast-path (cacheKey check) is hit instead of falling through to ReadableStream reading. +// Allowed characters in a redirect URL (ASCII subset). +// Covers unreserved + reserved chars per RFC 3986 plus `%` for percent-encoding. +const allowedRedirectUrlChar = new Uint8Array(128) +for (let c = 0x30; c <= 0x39; c++) { + allowedRedirectUrlChar[c] = 1 // 0-9 +} +for (let c = 0x41; c <= 0x5a; c++) { + allowedRedirectUrlChar[c] = 1 // A-Z +} +for (let c = 0x61; c <= 0x7a; c++) { + allowedRedirectUrlChar[c] = 1 // a-z +} +{ + // eslint-disable-next-line quotes + const chars = "-./:?#[]@!$&'()*+,;=%~_" + for (let i = 0; i < chars.length; i++) { + allowedRedirectUrlChar[chars.charCodeAt(i)] = 1 + } +} + +const parseRedirectUrl = (url: string | URL): string => { + if (url instanceof URL) { + return url.href + } + // Fast path: starts with http:// or https:// and all chars are in the allowed set + if ( + url.length > 8 && + url.charCodeAt(0) === 0x68 && // h + (url.charCodeAt(4) === 0x3a || url.charCodeAt(5) === 0x3a) // http: or https: + ) { + let safe = true + for (let i = 0, len = url.length; i < len; i++) { + const c = url.charCodeAt(i) + if (c > 0x7f || allowedRedirectUrlChar[c] === 0) { + safe = false + break + } + } + if (safe) { + return url + } + } + return new URL(url).href +} + +const validRedirectStatuses = new Set([301, 302, 303, 307, 308]) + +// Override Response.json() and Response.redirect() to return a LightweightResponse +// so the listener fast-path (cacheKey check) is hit instead of falling through to ReadableStream reading. Object.defineProperty(Response, 'redirect', { value: function redirect(url: string | URL, status = 302): Response { - if (![301, 302, 303, 307, 308].includes(status)) { + if (!validRedirectStatuses.has(status)) { throw new RangeError('Invalid status code') } return new Response(null, { status, - headers: { location: typeof url === 'string' ? url : url.href }, + headers: { location: parseRedirectUrl(url) }, }) }, writable: true, @@ -132,12 +179,15 @@ Object.defineProperty(Response, 'redirect', { Object.defineProperty(Response, 'json', { value: function json(data?: unknown, init?: ResponseInit): Response { const body = JSON.stringify(data) + if (body === undefined) { + throw new TypeError('The data is not JSON serializable') + } const initHeaders = init?.headers let headers: Record | Headers if (initHeaders) { headers = new Headers(initHeaders) - if (!(headers as Headers).has('content-type')) { - ;(headers as Headers).set('content-type', 'application/json') + if (!headers.has('content-type')) { + headers.set('content-type', 'application/json') } } else { headers = { 'content-type': 'application/json' } From d78a0b2bd44000aad765f4fa659ebf094a6cbdc7 Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Tue, 14 Apr 2026 19:10:34 +0900 Subject: [PATCH 2/2] perf: stricter and faster http(s):// prefix check in parseRedirectUrl --- src/response.ts | 43 ++++++------------------------------------- 1 file changed, 6 insertions(+), 37 deletions(-) diff --git a/src/response.ts b/src/response.ts index d3926a6..549def8 100644 --- a/src/response.ts +++ b/src/response.ts @@ -113,47 +113,16 @@ Object.defineProperty(Response.prototype, Symbol.for('nodejs.util.inspect.custom Object.setPrototypeOf(Response, GlobalResponse) Object.setPrototypeOf(Response.prototype, GlobalResponse.prototype) -// Allowed characters in a redirect URL (ASCII subset). -// Covers unreserved + reserved chars per RFC 3986 plus `%` for percent-encoding. -const allowedRedirectUrlChar = new Uint8Array(128) -for (let c = 0x30; c <= 0x39; c++) { - allowedRedirectUrlChar[c] = 1 // 0-9 -} -for (let c = 0x41; c <= 0x5a; c++) { - allowedRedirectUrlChar[c] = 1 // A-Z -} -for (let c = 0x61; c <= 0x7a; c++) { - allowedRedirectUrlChar[c] = 1 // a-z -} -{ - // eslint-disable-next-line quotes - const chars = "-./:?#[]@!$&'()*+,;=%~_" - for (let i = 0; i < chars.length; i++) { - allowedRedirectUrlChar[chars.charCodeAt(i)] = 1 - } -} - +// Fast path regex: matches http:// or https:// followed by RFC 3986 allowed chars. +// Character class covers unreserved + reserved chars plus `%` for percent-encoding. +// ! #-; = ?-[ ] _ a-z ~ A-Z (A-Z is within ?-[ range but listed for clarity) +const validRedirectUrl = /^https?:\/\/[!#-;=?-[\]_a-z~A-Z]+$/ const parseRedirectUrl = (url: string | URL): string => { if (url instanceof URL) { return url.href } - // Fast path: starts with http:// or https:// and all chars are in the allowed set - if ( - url.length > 8 && - url.charCodeAt(0) === 0x68 && // h - (url.charCodeAt(4) === 0x3a || url.charCodeAt(5) === 0x3a) // http: or https: - ) { - let safe = true - for (let i = 0, len = url.length; i < len; i++) { - const c = url.charCodeAt(i) - if (c > 0x7f || allowedRedirectUrlChar[c] === 0) { - safe = false - break - } - } - if (safe) { - return url - } + if (validRedirectUrl.test(url)) { + return url } return new URL(url).href }