Skip to content

Commit

Permalink
Use sets and reusable TextEncoder/TextDecoder instances (nodejs#2368)
Browse files Browse the repository at this point in the history
* Use sets and reusable TextEncoder/TextDecoder instances

* Do not reuse streaming decoder
  • Loading branch information
kibertoad authored and crysmags committed Feb 27, 2024
1 parent eb94539 commit 04fbc08
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 30 deletions.
21 changes: 12 additions & 9 deletions lib/fetch/body.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ let ReadableStream = globalThis.ReadableStream

/** @type {globalThis['File']} */
const File = NativeFile ?? UndiciFile
const textEncoder = new TextEncoder()
const textDecoder = new TextDecoder()

// https://fetch.spec.whatwg.org/#concept-bodyinit-extract
function extractBody (object, keepalive = false) {
Expand All @@ -49,7 +51,7 @@ function extractBody (object, keepalive = false) {
stream = new ReadableStream({
async pull (controller) {
controller.enqueue(
typeof source === 'string' ? new TextEncoder().encode(source) : source
typeof source === 'string' ? textEncoder.encode(source) : source
)
queueMicrotask(() => readableStreamClose(controller))
},
Expand Down Expand Up @@ -119,21 +121,20 @@ function extractBody (object, keepalive = false) {
// - That the content-length is calculated in advance.
// - And that all parts are pre-encoded and ready to be sent.

const enc = new TextEncoder()
const blobParts = []
const rn = new Uint8Array([13, 10]) // '\r\n'
length = 0
let hasUnknownSizeValue = false

for (const [name, value] of object) {
if (typeof value === 'string') {
const chunk = enc.encode(prefix +
const chunk = textEncoder.encode(prefix +
`; name="${escape(normalizeLinefeeds(name))}"` +
`\r\n\r\n${normalizeLinefeeds(value)}\r\n`)
blobParts.push(chunk)
length += chunk.byteLength
} else {
const chunk = enc.encode(`${prefix}; name="${escape(normalizeLinefeeds(name))}"` +
const chunk = textEncoder.encode(`${prefix}; name="${escape(normalizeLinefeeds(name))}"` +
(value.name ? `; filename="${escape(value.name)}"` : '') + '\r\n' +
`Content-Type: ${
value.type || 'application/octet-stream'
Expand All @@ -147,7 +148,7 @@ function extractBody (object, keepalive = false) {
}
}

const chunk = enc.encode(`--${boundary}--`)
const chunk = textEncoder.encode(`--${boundary}--`)
blobParts.push(chunk)
length += chunk.byteLength
if (hasUnknownSizeValue) {
Expand Down Expand Up @@ -443,14 +444,16 @@ function bodyMixinMethods (instance) {
let text = ''
// application/x-www-form-urlencoded parser will keep the BOM.
// https://url.spec.whatwg.org/#concept-urlencoded-parser
const textDecoder = new TextDecoder('utf-8', { ignoreBOM: true })
// Note that streaming decoder is stateful and cannot be reused
const streamingDecoder = new TextDecoder('utf-8', { ignoreBOM: true })

for await (const chunk of consumeBody(this[kState].body)) {
if (!isUint8Array(chunk)) {
throw new TypeError('Expected Uint8Array chunk')
}
text += textDecoder.decode(chunk, { stream: true })
text += streamingDecoder.decode(chunk, { stream: true })
}
text += textDecoder.decode()
text += streamingDecoder.decode()
entries = new URLSearchParams(text)
} catch (err) {
// istanbul ignore next: Unclear when new URLSearchParams can fail on a string.
Expand Down Expand Up @@ -565,7 +568,7 @@ function utf8DecodeBytes (buffer) {

// 3. Process a queue with an instance of UTF-8’s
// decoder, ioQueue, output, and "replacement".
const output = new TextDecoder().decode(buffer)
const output = textDecoder.decode(buffer)

// 4. Return output.
return output
Expand Down
17 changes: 16 additions & 1 deletion lib/fetch/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
const { MessageChannel, receiveMessageOnPort } = require('worker_threads')

const corsSafeListedMethods = ['GET', 'HEAD', 'POST']
const corsSafeListedMethodsSet = new Set(corsSafeListedMethods)

const nullBodyStatus = [101, 204, 205, 304]

const redirectStatus = [301, 302, 303, 307, 308]
const redirectStatusSet = new Set(redirectStatus)

// https://fetch.spec.whatwg.org/#block-bad-port
const badPorts = [
Expand All @@ -18,6 +20,8 @@ const badPorts = [
'10080'
]

const badPortsSet = new Set(badPorts)

// https://w3c.github.io/webappsec-referrer-policy/#referrer-policies
const referrerPolicy = [
'',
Expand All @@ -30,10 +34,12 @@ const referrerPolicy = [
'strict-origin-when-cross-origin',
'unsafe-url'
]
const referrerPolicySet = new Set(referrerPolicy)

const requestRedirect = ['follow', 'manual', 'error']

const safeMethods = ['GET', 'HEAD', 'OPTIONS', 'TRACE']
const safeMethodsSet = new Set(safeMethods)

const requestMode = ['navigate', 'same-origin', 'no-cors', 'cors']

Expand Down Expand Up @@ -68,6 +74,7 @@ const requestDuplex = [

// http://fetch.spec.whatwg.org/#forbidden-method
const forbiddenMethods = ['CONNECT', 'TRACE', 'TRACK']
const forbiddenMethodsSet = new Set(forbiddenMethods)

const subresource = [
'audio',
Expand All @@ -83,6 +90,7 @@ const subresource = [
'xslt',
''
]
const subresourceSet = new Set(subresource)

/** @type {globalThis['DOMException']} */
const DOMException = globalThis.DOMException ?? (() => {
Expand Down Expand Up @@ -132,5 +140,12 @@ module.exports = {
nullBodyStatus,
safeMethods,
badPorts,
requestDuplex
requestDuplex,
subresourceSet,
badPortsSet,
redirectStatusSet,
corsSafeListedMethodsSet,
safeMethodsSet,
forbiddenMethodsSet,
referrerPolicySet
}
3 changes: 2 additions & 1 deletion lib/fetch/file.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const { isBlobLike } = require('./util')
const { webidl } = require('./webidl')
const { parseMIMEType, serializeAMimeType } = require('./dataURL')
const { kEnumerableProperty } = require('../core/util')
const encoder = new TextEncoder()

class File extends Blob {
constructor (fileBits, fileName, options = {}) {
Expand Down Expand Up @@ -280,7 +281,7 @@ function processBlobParts (parts, options) {
}

// 3. Append the result of UTF-8 encoding s to bytes.
bytes.push(new TextEncoder().encode(s))
bytes.push(encoder.encode(s))
} else if (
types.isAnyArrayBuffer(element) ||
types.isTypedArray(element)
Expand Down
17 changes: 9 additions & 8 deletions lib/fetch/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ const { kState, kHeaders, kGuard, kRealm } = require('./symbols')
const assert = require('assert')
const { safelyExtractBody } = require('./body')
const {
redirectStatus,
redirectStatusSet,
nullBodyStatus,
safeMethods,
safeMethodsSet,
requestBodyHeader,
subresource,
subresourceSet,
DOMException
} = require('./constants')
const { kHeadersList } = require('../core/symbols')
Expand All @@ -62,6 +62,7 @@ const { TransformStream } = require('stream/web')
const { getGlobalDispatcher } = require('../global')
const { webidl } = require('./webidl')
const { STATUS_CODES } = require('http')
const GET_OR_HEAD = ['GET', 'HEAD']

/** @type {import('buffer').resolveObjectURL} */
let resolveObjectURL
Expand Down Expand Up @@ -509,7 +510,7 @@ function fetching ({
}

// 15. If request is a subresource request, then:
if (subresource.includes(request.destination)) {
if (subresourceSet.has(request.destination)) {
// TODO
}

Expand Down Expand Up @@ -1063,7 +1064,7 @@ async function httpFetch (fetchParams) {
}

// 8. If actualResponse’s status is a redirect status, then:
if (redirectStatus.includes(actualResponse.status)) {
if (redirectStatusSet.has(actualResponse.status)) {
// 1. If actualResponse’s status is not 303, request’s body is not null,
// and the connection uses HTTP/2, then user agents may, and are even
// encouraged to, transmit an RST_STREAM frame.
Expand Down Expand Up @@ -1181,7 +1182,7 @@ function httpRedirectFetch (fetchParams, response) {
if (
([301, 302].includes(actualResponse.status) && request.method === 'POST') ||
(actualResponse.status === 303 &&
!['GET', 'HEAD'].includes(request.method))
!GET_OR_HEAD.includes(request.method))
) {
// then:
// 1. Set request’s method to `GET` and request’s body to null.
Expand Down Expand Up @@ -1465,7 +1466,7 @@ async function httpNetworkOrCacheFetch (
// responses in httpCache, as per the "Invalidation" chapter of HTTP
// Caching, and set storedResponse to null. [HTTP-CACHING]
if (
!safeMethods.includes(httpRequest.method) &&
!safeMethodsSet.has(httpRequest.method) &&
forwardResponse.status >= 200 &&
forwardResponse.status <= 399
) {
Expand Down Expand Up @@ -2025,7 +2026,7 @@ async function httpNetworkFetch (

const willFollow = request.redirect === 'follow' &&
location &&
redirectStatus.includes(status)
redirectStatusSet.has(status)

// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Encoding
if (request.method !== 'HEAD' && request.method !== 'CONNECT' && !nullBodyStatus.includes(status) && !willFollow) {
Expand Down
8 changes: 4 additions & 4 deletions lib/fetch/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ const {
makePolicyContainer
} = require('./util')
const {
forbiddenMethods,
corsSafeListedMethods,
forbiddenMethodsSet,
corsSafeListedMethodsSet,
referrerPolicy,
requestRedirect,
requestMode,
Expand Down Expand Up @@ -319,7 +319,7 @@ class Request {
throw TypeError(`'${init.method}' is not a valid HTTP method.`)
}

if (forbiddenMethods.indexOf(method.toUpperCase()) !== -1) {
if (forbiddenMethodsSet.has(method.toUpperCase())) {
throw TypeError(`'${init.method}' HTTP method is unsupported.`)
}

Expand Down Expand Up @@ -404,7 +404,7 @@ class Request {
if (mode === 'no-cors') {
// 1. If this’s request’s method is not a CORS-safelisted method,
// then throw a TypeError.
if (!corsSafeListedMethods.includes(request.method)) {
if (!corsSafeListedMethodsSet.has(request.method)) {
throw new TypeError(
`'${request.method} is unsupported in no-cors mode.`
)
Expand Down
7 changes: 4 additions & 3 deletions lib/fetch/response.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const {
isomorphicEncode
} = require('./util')
const {
redirectStatus,
redirectStatusSet,
nullBodyStatus,
DOMException
} = require('./constants')
Expand All @@ -28,6 +28,7 @@ const assert = require('assert')
const { types } = require('util')

const ReadableStream = globalThis.ReadableStream || require('stream/web').ReadableStream
const textEncoder = new TextEncoder('utf-8')

// https://fetch.spec.whatwg.org/#response-class
class Response {
Expand Down Expand Up @@ -57,7 +58,7 @@ class Response {
}

// 1. Let bytes the result of running serialize a JavaScript value to JSON bytes on data.
const bytes = new TextEncoder('utf-8').encode(
const bytes = textEncoder.encode(
serializeJavascriptValueToJSONString(data)
)

Expand Down Expand Up @@ -102,7 +103,7 @@ class Response {
}

// 3. If status is not a redirect status, then throw a RangeError.
if (!redirectStatus.includes(status)) {
if (!redirectStatusSet.has(status)) {
throw new RangeError('Invalid status code ' + status)
}

Expand Down
8 changes: 4 additions & 4 deletions lib/fetch/util.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict'

const { redirectStatus, badPorts, referrerPolicy: referrerPolicyTokens } = require('./constants')
const { redirectStatusSet, referrerPolicySet: referrerPolicyTokens, badPortsSet } = require('./constants')
const { getGlobalOrigin } = require('./global')
const { performance } = require('perf_hooks')
const { isBlobLike, toUSVString, ReadableStreamFrom } = require('../core/util')
Expand Down Expand Up @@ -29,7 +29,7 @@ function responseURL (response) {
// https://fetch.spec.whatwg.org/#concept-response-location-url
function responseLocationURL (response, requestFragment) {
// 1. If response’s status is not a redirect status, then return null.
if (!redirectStatus.includes(response.status)) {
if (!redirectStatusSet.has(response.status)) {
return null
}

Expand Down Expand Up @@ -64,7 +64,7 @@ function requestBadPort (request) {

// 2. If url’s scheme is an HTTP(S) scheme and url’s port is a bad port,
// then return blocked.
if (urlIsHttpHttpsScheme(url) && badPorts.includes(url.port)) {
if (urlIsHttpHttpsScheme(url) && badPortsSet.has(url.port)) {
return 'blocked'
}

Expand Down Expand Up @@ -206,7 +206,7 @@ function setRequestReferrerPolicyOnRedirect (request, actualResponse) {
// The left-most policy is the fallback.
for (let i = policyHeader.length; i !== 0; i--) {
const token = policyHeader[i - 1].trim()
if (referrerPolicyTokens.includes(token)) {
if (referrerPolicyTokens.has(token)) {
policy = token
break
}
Expand Down

0 comments on commit 04fbc08

Please sign in to comment.