From 6d987a84721853c3a5d6a39f57956cd2f3f0770e Mon Sep 17 00:00:00 2001 From: Khafra Date: Fri, 20 Sep 2024 14:00:12 -0400 Subject: [PATCH 1/5] remove kSignal, kDispatcher --- lib/web/fetch/headers.js | 63 +++++++++++++++++++++------------------- lib/web/fetch/index.js | 6 ++-- lib/web/fetch/request.js | 63 +++++++++++++++++++++++++++++++++------- lib/web/fetch/symbols.js | 4 +-- test/fetch/issue-3630.js | 4 +-- 5 files changed, 91 insertions(+), 49 deletions(-) diff --git a/lib/web/fetch/headers.js b/lib/web/fetch/headers.js index 6f51f36a58b..1092ba7b8fe 100644 --- a/lib/web/fetch/headers.js +++ b/lib/web/fetch/headers.js @@ -13,7 +13,7 @@ const { webidl } = require('./webidl') const assert = require('node:assert') const util = require('node:util') -const kHeadersMap = Symbol('headers map') +// TODO(@KhafraDev): remove const kHeadersSortedMap = Symbol('headers map sorted') /** @@ -127,14 +127,17 @@ class HeadersList { /** @type {[string, string][]|null} */ cookies = null + sortedMap + headersMap + constructor (init) { if (init instanceof HeadersList) { - this[kHeadersMap] = new Map(init[kHeadersMap]) - this[kHeadersSortedMap] = init[kHeadersSortedMap] + this.headersMap = new Map(init.headersMap) + this.sortedMap = init.sortedMap this.cookies = init.cookies === null ? null : [...init.cookies] } else { - this[kHeadersMap] = new Map(init) - this[kHeadersSortedMap] = null + this.headersMap = new Map(init) + this.sortedMap = null } } @@ -148,12 +151,12 @@ class HeadersList { // contains a header whose name is a byte-case-insensitive // match for name. - return this[kHeadersMap].has(isLowerCase ? name : name.toLowerCase()) + return this.headersMap.has(isLowerCase ? name : name.toLowerCase()) } clear () { - this[kHeadersMap].clear() - this[kHeadersSortedMap] = null + this.headersMap.clear() + this.sortedMap = null this.cookies = null } @@ -164,22 +167,22 @@ class HeadersList { * @param {boolean} isLowerCase */ append (name, value, isLowerCase) { - this[kHeadersSortedMap] = null + this.sortedMap = null // 1. If list contains name, then set name to the first such // header’s name. const lowercaseName = isLowerCase ? name : name.toLowerCase() - const exists = this[kHeadersMap].get(lowercaseName) + const exists = this.headersMap.get(lowercaseName) // 2. Append (name, value) to list. if (exists) { const delimiter = lowercaseName === 'cookie' ? '; ' : ', ' - this[kHeadersMap].set(lowercaseName, { + this.headersMap.set(lowercaseName, { name: exists.name, value: `${exists.value}${delimiter}${value}` }) } else { - this[kHeadersMap].set(lowercaseName, { name, value }) + this.headersMap.set(lowercaseName, { name, value }) } if (lowercaseName === 'set-cookie') { @@ -194,7 +197,7 @@ class HeadersList { * @param {boolean} isLowerCase */ set (name, value, isLowerCase) { - this[kHeadersSortedMap] = null + this.sortedMap = null const lowercaseName = isLowerCase ? name : name.toLowerCase() if (lowercaseName === 'set-cookie') { @@ -205,7 +208,7 @@ class HeadersList { // the first such header to value and remove the // others. // 2. Otherwise, append header (name, value) to list. - this[kHeadersMap].set(lowercaseName, { name, value }) + this.headersMap.set(lowercaseName, { name, value }) } /** @@ -214,14 +217,14 @@ class HeadersList { * @param {boolean} isLowerCase */ delete (name, isLowerCase) { - this[kHeadersSortedMap] = null + this.sortedMap = null if (!isLowerCase) name = name.toLowerCase() if (name === 'set-cookie') { this.cookies = null } - this[kHeadersMap].delete(name) + this.headersMap.delete(name) } /** @@ -235,12 +238,12 @@ class HeadersList { // 2. Return the values of all headers in list whose name // is a byte-case-insensitive match for name, // separated from each other by 0x2C 0x20, in order. - return this[kHeadersMap].get(isLowerCase ? name : name.toLowerCase())?.value ?? null + return this.headersMap.get(isLowerCase ? name : name.toLowerCase())?.value ?? null } * [Symbol.iterator] () { // use the lowercased name - for (const { 0: name, 1: { value } } of this[kHeadersMap]) { + for (const { 0: name, 1: { value } } of this.headersMap) { yield [name, value] } } @@ -248,8 +251,8 @@ class HeadersList { get entries () { const headers = {} - if (this[kHeadersMap].size !== 0) { - for (const { name, value } of this[kHeadersMap].values()) { + if (this.headersMap.size !== 0) { + for (const { name, value } of this.headersMap.values()) { headers[name] = value } } @@ -258,14 +261,14 @@ class HeadersList { } rawValues () { - return this[kHeadersMap].values() + return this.headersMap.values() } get entriesList () { const headers = [] - if (this[kHeadersMap].size !== 0) { - for (const { 0: lowerName, 1: { name, value } } of this[kHeadersMap]) { + if (this.headersMap.size !== 0) { + for (const { 0: lowerName, 1: { name, value } } of this.headersMap) { if (lowerName === 'set-cookie') { for (const cookie of this.cookies) { headers.push([name, cookie]) @@ -281,7 +284,7 @@ class HeadersList { // https://fetch.spec.whatwg.org/#convert-header-names-to-a-sorted-lowercase-set toSortedArray () { - const size = this[kHeadersMap].size + const size = this.headersMap.size const array = new Array(size) // In most cases, you will use the fast-path. // fast-path: Use binary insertion sort for small arrays. @@ -292,7 +295,7 @@ class HeadersList { } // Improve performance by unrolling loop and avoiding double-loop. // Double-loop-less version of the binary insertion sort. - const iterator = this[kHeadersMap][Symbol.iterator]() + const iterator = this.headersMap[Symbol.iterator]() const firstValue = iterator.next().value // set [name, value] to first index. array[0] = [firstValue[0], firstValue[1].value] @@ -342,7 +345,7 @@ class HeadersList { // This case would be a rare occurrence. // slow-path: fallback let i = 0 - for (const { 0: name, 1: { value } } of this[kHeadersMap]) { + for (const { 0: name, 1: { value } } of this.headersMap) { array[i++] = [name, value] // https://fetch.spec.whatwg.org/#concept-header-list-sort-and-combine // 3.2.2. Assert: value is non-null. @@ -547,8 +550,8 @@ class Headers { // https://fetch.spec.whatwg.org/#concept-header-list-sort-and-combine get [kHeadersSortedMap] () { - if (this.#headersList[kHeadersSortedMap]) { - return this.#headersList[kHeadersSortedMap] + if (this.#headersList.sortedMap) { + return this.#headersList.sortedMap } // 1. Let headers be an empty list of headers with the key being the name @@ -564,7 +567,7 @@ class Headers { // fast-path if (cookies === null || cookies.length === 1) { // Note: The non-null assertion of value has already been done by `HeadersList#toSortedArray` - return (this.#headersList[kHeadersSortedMap] = names) + return (this.#headersList.sortedMap = names) } // 3. For each name of names: @@ -594,7 +597,7 @@ class Headers { } // 4. Return headers. - return (this.#headersList[kHeadersSortedMap] = headers) + return (this.#headersList.sortedMap = headers) } [util.inspect.custom] (depth, options) { diff --git a/lib/web/fetch/index.js b/lib/web/fetch/index.js index 08fb8c23604..f650c3c01ee 100644 --- a/lib/web/fetch/index.js +++ b/lib/web/fetch/index.js @@ -10,7 +10,7 @@ const { fromInnerResponse } = require('./response') const { HeadersList } = require('./headers') -const { Request, cloneRequest } = require('./request') +const { Request, cloneRequest, getRequestDispatcher } = require('./request') const zlib = require('node:zlib') const { bytesMatch, @@ -46,7 +46,7 @@ const { createInflate, extractMimeType } = require('./util') -const { kState, kDispatcher } = require('./symbols') +const { kState } = require('./symbols') const assert = require('node:assert') const { safelyExtractBody, extractBody } = require('./body') const { @@ -243,7 +243,7 @@ function fetch (input, init = undefined) { request, processResponseEndOfBody: handleFetchDone, processResponse, - dispatcher: requestObject[kDispatcher] // undici + dispatcher: getRequestDispatcher(requestObject) // undici }) // 14. Return p. diff --git a/lib/web/fetch/request.js b/lib/web/fetch/request.js index 92c9ee501fb..0fcbeb5ef73 100644 --- a/lib/web/fetch/request.js +++ b/lib/web/fetch/request.js @@ -23,7 +23,7 @@ const { requestDuplex } = require('./constants') const { kEnumerableProperty, normalizedMethodRecordsBase, normalizedMethodRecords } = util -const { kHeaders, kSignal, kState, kDispatcher } = require('./symbols') +const { kHeaders, kState } = require('./symbols') const { webidl } = require('./webidl') const { URLSerializer } = require('./data-url') const { kConstruct } = require('../../core/symbols') @@ -80,6 +80,12 @@ let patchMethodWarning = false // https://fetch.spec.whatwg.org/#request-class class Request { + /** @type {AbortSignal} */ + #signal + + /** @type {import('../../dispatcher/dispatcher')} */ + #dispatcher + // https://fetch.spec.whatwg.org/#dom-request constructor (input, init = undefined) { if (input === kConstruct) { @@ -106,7 +112,7 @@ class Request { // 5. If input is a string, then: if (typeof input === 'string') { - this[kDispatcher] = init.dispatcher + this.#dispatcher = init.dispatcher // 1. Let parsedURL be the result of parsing input with baseURL. // 2. If parsedURL is failure, then throw a TypeError. @@ -131,8 +137,6 @@ class Request { // 5. Set fallbackMode to "cors". fallbackMode = 'cors' } else { - this[kDispatcher] = init.dispatcher || input[kDispatcher] - // 6. Otherwise: // 7. Assert: input is a Request object. @@ -142,7 +146,9 @@ class Request { request = input[kState] // 9. Set signal to input’s signal. - signal = input[kSignal] + signal = input.#signal + + this.#dispatcher = init.dispatcher || input.#dispatcher } // 7. Let origin be this’s relevant settings object’s origin. @@ -396,7 +402,7 @@ class Request { // TODO: could this be simplified with AbortSignal.any // (https://dom.spec.whatwg.org/#dom-abortsignal-any) const ac = new AbortController() - this[kSignal] = ac.signal + this.#signal = ac.signal // 29. If signal is not null, then make this’s signal follow signal. if (signal != null) { @@ -721,7 +727,7 @@ class Request { webidl.brandCheck(this, Request) // The signal getter steps are to return this’s signal. - return this[kSignal] + return this.#signal } get body () { @@ -775,7 +781,7 @@ class Request { } // 4. Return clonedRequestObject. - return fromInnerRequest(clonedRequest, this[kDispatcher], ac.signal, getHeadersGuard(this[kHeaders])) + return fromInnerRequest(clonedRequest, this.#dispatcher, ac.signal, getHeadersGuard(this[kHeaders])) } [nodeUtil.inspect.custom] (depth, options) { @@ -805,8 +811,37 @@ class Request { return `Request ${nodeUtil.formatWithOptions(options, properties)}` } + + /** + * @param {Request} request + * @param {AbortSignal} newSignal + */ + static setRequestSignal (request, newSignal) { + request.#signal = newSignal + return request + } + + /** + * @param {Request} request + */ + static getRequestDispatcher (request) { + return request.#dispatcher + } + + /** + * @param {Request} request + * @param {import('../../dispatcher/dispatcher')} newDispatcher + */ + static setRequestDispatcher (request, newDispatcher) { + request.#dispatcher = newDispatcher + } } +const { setRequestSignal, getRequestDispatcher, setRequestDispatcher } = Request +Reflect.deleteProperty(Request, 'setRequestSignal') +Reflect.deleteProperty(Request, 'getRequestDispatcher') +Reflect.deleteProperty(Request, 'setRequestDispatcher') + mixinBody(Request) // https://fetch.spec.whatwg.org/#requests @@ -883,8 +918,8 @@ function cloneRequest (request) { function fromInnerRequest (innerRequest, dispatcher, signal, guard) { const request = new Request(kConstruct) request[kState] = innerRequest - request[kDispatcher] = dispatcher - request[kSignal] = signal + setRequestDispatcher(request, dispatcher) + setRequestSignal(request, signal) request[kHeaders] = new Headers(kConstruct) setHeadersList(request[kHeaders], innerRequest.headersList) setHeadersGuard(request[kHeaders], guard) @@ -1018,4 +1053,10 @@ webidl.converters.RequestInit = webidl.dictionaryConverter([ } ]) -module.exports = { Request, makeRequest, fromInnerRequest, cloneRequest } +module.exports = { + Request, + makeRequest, + fromInnerRequest, + cloneRequest, + getRequestDispatcher +} diff --git a/lib/web/fetch/symbols.js b/lib/web/fetch/symbols.js index 38043a04051..d78145d7764 100644 --- a/lib/web/fetch/symbols.js +++ b/lib/web/fetch/symbols.js @@ -2,7 +2,5 @@ module.exports = { kHeaders: Symbol('headers'), - kSignal: Symbol('signal'), - kState: Symbol('state'), - kDispatcher: Symbol('dispatcher') + kState: Symbol('state') } diff --git a/test/fetch/issue-3630.js b/test/fetch/issue-3630.js index d03b7185fa5..d40a5c64d99 100644 --- a/test/fetch/issue-3630.js +++ b/test/fetch/issue-3630.js @@ -3,10 +3,10 @@ const { test } = require('node:test') const assert = require('node:assert') const { Request, Agent } = require('../..') -const { kDispatcher } = require('../../lib/web/fetch/symbols') +const { getRequestDispatcher } = require('../../lib/web/fetch/request') test('Cloned request should inherit its dispatcher', () => { const agent = new Agent() const request = new Request('https://a', { dispatcher: agent }) - assert.strictEqual(request[kDispatcher], agent) + assert.strictEqual(getRequestDispatcher(request), agent) }) From f30fe2e311eb23b3bbcde3c81ea94ef9702a0e6b Mon Sep 17 00:00:00 2001 From: Khafra Date: Fri, 20 Sep 2024 15:38:21 -0400 Subject: [PATCH 2/5] remove formdata kState --- lib/web/fetch/body.js | 4 +-- lib/web/fetch/formdata.js | 52 ++++++++++++++++++++++++++------------- lib/web/fetch/util.js | 4 +-- 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/lib/web/fetch/body.js b/lib/web/fetch/body.js index 9de2a7e41b9..2dc0d2b17e0 100644 --- a/lib/web/fetch/body.js +++ b/lib/web/fetch/body.js @@ -9,7 +9,7 @@ const { extractMimeType, utf8DecodeBytes } = require('./util') -const { FormData } = require('./formdata') +const { FormData, setFormDataState } = require('./formdata') const { kState } = require('./symbols') const { webidl } = require('./webidl') const { Blob } = require('node:buffer') @@ -375,7 +375,7 @@ function bodyMixinMethods (instance) { // 3. Return a new FormData object, appending each entry, // resulting from the parsing operation, to its entry list. const fd = new FormData() - fd[kState] = parsed + setFormDataState(fd, parsed) return fd } diff --git a/lib/web/fetch/formdata.js b/lib/web/fetch/formdata.js index 71644898585..4909e6dbc98 100644 --- a/lib/web/fetch/formdata.js +++ b/lib/web/fetch/formdata.js @@ -1,7 +1,6 @@ 'use strict' const { iteratorMixin } = require('./util') -const { kState } = require('./symbols') const { kEnumerableProperty } = require('../../core/util') const { webidl } = require('./webidl') const { File: NativeFile } = require('node:buffer') @@ -12,6 +11,8 @@ const File = globalThis.File ?? NativeFile // https://xhr.spec.whatwg.org/#formdata class FormData { + #state = [] + constructor (form) { if (form !== undefined) { throw webidl.errors.conversionFailed({ @@ -20,8 +21,6 @@ class FormData { types: ['undefined'] }) } - - this[kState] = [] } append (name, value, filename = undefined) { @@ -49,7 +48,7 @@ class FormData { const entry = makeEntry(name, value, filename) // 3. Append entry to this’s entry list. - this[kState].push(entry) + this.#state.push(entry) } delete (name) { @@ -62,7 +61,7 @@ class FormData { // The delete(name) method steps are to remove all entries whose name // is name from this’s entry list. - this[kState] = this[kState].filter(entry => entry.name !== name) + this.#state = this.#state.filter(entry => entry.name !== name) } get (name) { @@ -75,14 +74,14 @@ class FormData { // 1. If there is no entry whose name is name in this’s entry list, // then return null. - const idx = this[kState].findIndex((entry) => entry.name === name) + const idx = this.#state.findIndex((entry) => entry.name === name) if (idx === -1) { return null } // 2. Return the value of the first entry whose name is name from // this’s entry list. - return this[kState][idx].value + return this.#state[idx].value } getAll (name) { @@ -97,7 +96,7 @@ class FormData { // then return the empty list. // 2. Return the values of all entries whose name is name, in order, // from this’s entry list. - return this[kState] + return this.#state .filter((entry) => entry.name === name) .map((entry) => entry.value) } @@ -112,7 +111,7 @@ class FormData { // The has(name) method steps are to return true if there is an entry // whose name is name in this’s entry list; otherwise false. - return this[kState].findIndex((entry) => entry.name === name) !== -1 + return this.#state.findIndex((entry) => entry.name === name) !== -1 } set (name, value, filename = undefined) { @@ -144,21 +143,21 @@ class FormData { // 3. If there are entries in this’s entry list whose name is name, then // replace the first such entry with entry and remove the others. - const idx = this[kState].findIndex((entry) => entry.name === name) + const idx = this.#state.findIndex((entry) => entry.name === name) if (idx !== -1) { - this[kState] = [ - ...this[kState].slice(0, idx), + this.#state = [ + ...this.#state.slice(0, idx), entry, - ...this[kState].slice(idx + 1).filter((entry) => entry.name !== name) + ...this.#state.slice(idx + 1).filter((entry) => entry.name !== name) ] } else { // 4. Otherwise, append entry to this’s entry list. - this[kState].push(entry) + this.#state.push(entry) } } [nodeUtil.inspect.custom] (depth, options) { - const state = this[kState].reduce((a, b) => { + const state = this.#state.reduce((a, b) => { if (a[b.name]) { if (Array.isArray(a[b.name])) { a[b.name].push(b.value) @@ -180,9 +179,28 @@ class FormData { // remove [Object null prototype] return `FormData ${output.slice(output.indexOf(']') + 2)}` } + + /** + * @param {FormData} formData + */ + static getFormDataState (formData) { + return formData.#state + } + + /** + * @param {FormData} formData + * @param {any[]} newState + */ + static setFormDataState (formData, newState) { + formData.#state = newState + } } -iteratorMixin('FormData', FormData, kState, 'name', 'value') +const { getFormDataState, setFormDataState } = FormData +Reflect.deleteProperty(FormData, 'getFormDataState') +Reflect.deleteProperty(FormData, 'setFormDataState') + +iteratorMixin('FormData', FormData, getFormDataState, 'name', 'value') Object.defineProperties(FormData.prototype, { append: kEnumerableProperty, @@ -238,4 +256,4 @@ function makeEntry (name, value, filename) { return { name, value } } -module.exports = { FormData, makeEntry } +module.exports = { FormData, makeEntry, setFormDataState } diff --git a/lib/web/fetch/util.js b/lib/web/fetch/util.js index 9bead826aa9..b85e754a1d9 100644 --- a/lib/web/fetch/util.js +++ b/lib/web/fetch/util.js @@ -825,7 +825,7 @@ const esIteratorPrototype = Object.getPrototypeOf(Object.getPrototypeOf([][Symbo /** * @see https://webidl.spec.whatwg.org/#dfn-iterator-prototype-object * @param {string} name name of the instance - * @param {symbol} kInternalIterator + * @param {symbol|((target: any) => any)} kInternalIterator * @param {string | number} [keyIndex] * @param {string | number} [valueIndex] */ @@ -867,7 +867,7 @@ function createIterator (name, kInternalIterator, keyIndex = 0, valueIndex = 1) // 7. Let kind be object’s kind. // 8. Let values be object’s target's value pairs to iterate over. const index = this.#index - const values = this.#target[kInternalIterator] + const values = typeof kInternalIterator === 'symbol' ? this.#target[kInternalIterator] : kInternalIterator(this.#target) // 9. Let len be the length of values. const len = values.length From ce50babb6bae6ca120a60191b0004fcef9395e4d Mon Sep 17 00:00:00 2001 From: Khafra Date: Fri, 20 Sep 2024 15:49:02 -0400 Subject: [PATCH 3/5] remove kHeaders --- lib/web/fetch/request.js | 43 +++++++++++++++++++++++++-------------- lib/web/fetch/response.js | 43 ++++++++++++++++++++++++++++++--------- lib/web/fetch/symbols.js | 1 - test/fetch/response.js | 16 --------------- 4 files changed, 61 insertions(+), 42 deletions(-) diff --git a/lib/web/fetch/request.js b/lib/web/fetch/request.js index 0fcbeb5ef73..74a4b906eff 100644 --- a/lib/web/fetch/request.js +++ b/lib/web/fetch/request.js @@ -23,7 +23,7 @@ const { requestDuplex } = require('./constants') const { kEnumerableProperty, normalizedMethodRecordsBase, normalizedMethodRecords } = util -const { kHeaders, kState } = require('./symbols') +const { kState } = require('./symbols') const { webidl } = require('./webidl') const { URLSerializer } = require('./data-url') const { kConstruct } = require('../../core/symbols') @@ -86,6 +86,9 @@ class Request { /** @type {import('../../dispatcher/dispatcher')} */ #dispatcher + /** @type {Headers} */ + #headers + // https://fetch.spec.whatwg.org/#dom-request constructor (input, init = undefined) { if (input === kConstruct) { @@ -440,9 +443,9 @@ class Request { // 30. Set this’s headers to a new Headers object with this’s relevant // Realm, whose header list is request’s header list and guard is // "request". - this[kHeaders] = new Headers(kConstruct) - setHeadersList(this[kHeaders], request.headersList) - setHeadersGuard(this[kHeaders], 'request') + this.#headers = new Headers(kConstruct) + setHeadersList(this.#headers, request.headersList) + setHeadersGuard(this.#headers, 'request') // 31. If this’s request’s mode is "no-cors", then: if (mode === 'no-cors') { @@ -455,13 +458,13 @@ class Request { } // 2. Set this’s headers’s guard to "request-no-cors". - setHeadersGuard(this[kHeaders], 'request-no-cors') + setHeadersGuard(this.#headers, 'request-no-cors') } // 32. If init is not empty, then: if (initHasKey) { /** @type {HeadersList} */ - const headersList = getHeadersList(this[kHeaders]) + const headersList = getHeadersList(this.#headers) // 1. Let headers be a copy of this’s headers and its associated header // list. // 2. If init["headers"] exists, then set headers to init["headers"]. @@ -480,7 +483,7 @@ class Request { headersList.cookies = headers.cookies } else { // 5. Otherwise, fill this’s headers with headers. - fillHeaders(this[kHeaders], headers) + fillHeaders(this.#headers, headers) } } @@ -515,8 +518,8 @@ class Request { // 3, If Content-Type is non-null and this’s headers’s header list does // not contain `Content-Type`, then append `Content-Type`/Content-Type to // this’s headers. - if (contentType && !getHeadersList(this[kHeaders]).contains('content-type', true)) { - this[kHeaders].append('content-type', contentType, true) + if (contentType && !getHeadersList(this.#headers).contains('content-type', true)) { + this.#headers.append('content-type', contentType, true) } } @@ -595,7 +598,7 @@ class Request { webidl.brandCheck(this, Request) // The headers getter steps are to return this’s headers. - return this[kHeaders] + return this.#headers } // Returns the kind of resource requested by request, e.g., "document" @@ -781,7 +784,7 @@ class Request { } // 4. Return clonedRequestObject. - return fromInnerRequest(clonedRequest, this.#dispatcher, ac.signal, getHeadersGuard(this[kHeaders])) + return fromInnerRequest(clonedRequest, this.#dispatcher, ac.signal, getHeadersGuard(this.#headers)) } [nodeUtil.inspect.custom] (depth, options) { @@ -835,12 +838,21 @@ class Request { static setRequestDispatcher (request, newDispatcher) { request.#dispatcher = newDispatcher } + + /** + * @param {Request} request + * @param {Headers} newHeaders + */ + static setRequestHeaders (request, newHeaders) { + request.#headers = newHeaders + } } -const { setRequestSignal, getRequestDispatcher, setRequestDispatcher } = Request +const { setRequestSignal, getRequestDispatcher, setRequestDispatcher, setRequestHeaders } = Request Reflect.deleteProperty(Request, 'setRequestSignal') Reflect.deleteProperty(Request, 'getRequestDispatcher') Reflect.deleteProperty(Request, 'setRequestDispatcher') +Reflect.deleteProperty(Request, 'setRequestHeaders') mixinBody(Request) @@ -920,9 +932,10 @@ function fromInnerRequest (innerRequest, dispatcher, signal, guard) { request[kState] = innerRequest setRequestDispatcher(request, dispatcher) setRequestSignal(request, signal) - request[kHeaders] = new Headers(kConstruct) - setHeadersList(request[kHeaders], innerRequest.headersList) - setHeadersGuard(request[kHeaders], guard) + const headers = new Headers(kConstruct) + setRequestHeaders(request, headers) + setHeadersList(headers, innerRequest.headersList) + setHeadersGuard(headers, guard) return request } diff --git a/lib/web/fetch/response.js b/lib/web/fetch/response.js index 9e0ab867841..4e8efaac1ae 100644 --- a/lib/web/fetch/response.js +++ b/lib/web/fetch/response.js @@ -18,7 +18,7 @@ const { redirectStatusSet, nullBodyStatus } = require('./constants') -const { kState, kHeaders } = require('./symbols') +const { kState } = require('./symbols') const { webidl } = require('./webidl') const { FormData } = require('./formdata') const { URLSerializer } = require('./data-url') @@ -30,6 +30,9 @@ const textEncoder = new TextEncoder('utf-8') // https://fetch.spec.whatwg.org/#response-class class Response { + /** @type {Headers} */ + #headers + // Creates network error Response. static error () { // The static error() method steps are to return the result of creating a @@ -125,9 +128,9 @@ class Response { // 2. Set this’s headers to a new Headers object with this’s relevant // Realm, whose header list is this’s response’s header list and guard // is "response". - this[kHeaders] = new Headers(kConstruct) - setHeadersGuard(this[kHeaders], 'response') - setHeadersList(this[kHeaders], this[kState].headersList) + this.#headers = new Headers(kConstruct) + setHeadersGuard(this.#headers, 'response') + setHeadersList(this.#headers, this[kState].headersList) // 3. Let bodyWithType be null. let bodyWithType = null @@ -208,7 +211,7 @@ class Response { webidl.brandCheck(this, Response) // The headers getter steps are to return this’s headers. - return this[kHeaders] + return this.#headers } get body () { @@ -240,7 +243,7 @@ class Response { // 3. Return the result of creating a Response object, given // clonedResponse, this’s headers’s guard, and this’s relevant Realm. - return fromInnerResponse(clonedResponse, getHeadersGuard(this[kHeaders])) + return fromInnerResponse(clonedResponse, getHeadersGuard(this.#headers)) } [nodeUtil.inspect.custom] (depth, options) { @@ -264,8 +267,27 @@ class Response { return `Response ${nodeUtil.formatWithOptions(options, properties)}` } + + /** + * @param {Response} response + */ + static getResponseHeaders (response) { + return response.#headers + } + + /** + * @param {Response} response + * @param {Headers} newHeaders + */ + static setResponseHeaders (response, newHeaders) { + response.#headers = newHeaders + } } +const { getResponseHeaders, setResponseHeaders } = Response +Reflect.deleteProperty(Response, 'getResponseHeaders') +Reflect.deleteProperty(Response, 'setResponseHeaders') + mixinBody(Response) Object.defineProperties(Response.prototype, { @@ -473,7 +495,7 @@ function initializeResponse (response, init, body) { // 5. If init["headers"] exists, then fill response’s headers with init["headers"]. if ('headers' in init && init.headers != null) { - fill(response[kHeaders], init.headers) + fill(getResponseHeaders(response), init.headers) } // 6. If body was given, then: @@ -506,9 +528,10 @@ function initializeResponse (response, init, body) { function fromInnerResponse (innerResponse, guard) { const response = new Response(kConstruct) response[kState] = innerResponse - response[kHeaders] = new Headers(kConstruct) - setHeadersList(response[kHeaders], innerResponse.headersList) - setHeadersGuard(response[kHeaders], guard) + const headers = new Headers(kConstruct) + setResponseHeaders(response, headers) + setHeadersList(headers, innerResponse.headersList) + setHeadersGuard(headers, guard) if (hasFinalizationRegistry && innerResponse.body?.stream) { // If the target (response) is reclaimed, the cleanup callback may be called at some point with diff --git a/lib/web/fetch/symbols.js b/lib/web/fetch/symbols.js index d78145d7764..3d9c4c4784e 100644 --- a/lib/web/fetch/symbols.js +++ b/lib/web/fetch/symbols.js @@ -1,6 +1,5 @@ 'use strict' module.exports = { - kHeaders: Symbol('headers'), kState: Symbol('state') } diff --git a/test/fetch/response.js b/test/fetch/response.js index 92470218991..230e70061d0 100644 --- a/test/fetch/response.js +++ b/test/fetch/response.js @@ -9,9 +9,6 @@ const { Response, FormData } = require('../../') -const { fromInnerResponse, makeResponse } = require('../../lib/web/fetch/response') -const { kState, kHeaders } = require('../../lib/web/fetch/symbols') -const { getHeadersGuard, getHeadersList } = require('../../lib/web/fetch/headers') test('arg validation', async () => { // constructor @@ -272,19 +269,6 @@ test('Check the Content-Type of invalid formData', async (t) => { }) }) -test('fromInnerResponse', () => { - const innerResponse = makeResponse({ - urlList: [new URL('http://asd')] - }) - - const response = fromInnerResponse(innerResponse, 'immutable') - - // check property - assert.strictEqual(response[kState], innerResponse) - assert.strictEqual(getHeadersList(response[kHeaders]), innerResponse.headersList) - assert.strictEqual(getHeadersGuard(response[kHeaders]), 'immutable') -}) - test('clone body garbage collection', async () => { if (typeof global.gc === 'undefined') { throw new Error('gc is not available. Run with \'--expose-gc\'.') From 8f94a76c72a680bcc3d7840622d0740a68782128 Mon Sep 17 00:00:00 2001 From: Khafra Date: Fri, 20 Sep 2024 16:31:05 -0400 Subject: [PATCH 4/5] remove kState --- index.js | 2 +- lib/web/cache/cache.js | 29 +++++++------ lib/web/cache/cachestorage.js | 2 +- lib/web/cache/symbols.js | 5 --- lib/web/fetch/body.js | 53 ++++++++++++----------- lib/web/fetch/index.js | 10 ++--- lib/web/fetch/request.js | 79 ++++++++++++++++++++++------------- lib/web/fetch/response.js | 67 ++++++++++++++++++----------- lib/web/fetch/symbols.js | 5 --- test/node-fetch/response.js | 5 --- 10 files changed, 143 insertions(+), 114 deletions(-) delete mode 100644 lib/web/cache/symbols.js delete mode 100644 lib/web/fetch/symbols.js diff --git a/index.js b/index.js index 444706560ae..35d3d0428bc 100644 --- a/index.js +++ b/index.js @@ -124,7 +124,7 @@ module.exports.setGlobalOrigin = setGlobalOrigin module.exports.getGlobalOrigin = getGlobalOrigin const { CacheStorage } = require('./lib/web/cache/cachestorage') -const { kConstruct } = require('./lib/web/cache/symbols') +const { kConstruct } = require('./lib/core/symbols') // Cache & CacheStorage are tightly coupled with fetch. Even if it may run // in an older version of Node, it doesn't have any use without fetch. diff --git a/lib/web/cache/cache.js b/lib/web/cache/cache.js index 6a1d16fa28b..597de52bb4a 100644 --- a/lib/web/cache/cache.js +++ b/lib/web/cache/cache.js @@ -1,12 +1,11 @@ 'use strict' -const { kConstruct } = require('./symbols') +const { kConstruct } = require('../../core/symbols') const { urlEquals, getFieldValues } = require('./util') const { kEnumerableProperty, isDisturbed } = require('../../core/util') const { webidl } = require('../fetch/webidl') -const { Response, cloneResponse, fromInnerResponse } = require('../fetch/response') -const { Request, fromInnerRequest } = require('../fetch/request') -const { kState } = require('../fetch/symbols') +const { Response, cloneResponse, fromInnerResponse, getResponseState } = require('../fetch/response') +const { Request, fromInnerRequest, getRequestState } = require('../fetch/request') const { fetching } = require('../fetch/index') const { urlIsHttpHttpsScheme, createDeferredPromise, readAllBytes } = require('../fetch/util') const assert = require('node:assert') @@ -115,7 +114,7 @@ class Cache { } // 3.1 - const r = request[kState] + const r = getRequestState(request) // 3.2 if (!urlIsHttpHttpsScheme(r.url) || r.method !== 'GET') { @@ -133,7 +132,7 @@ class Cache { // 5. for (const request of requests) { // 5.1 - const r = new Request(request)[kState] + const r = getRequestState(new Request(request)) // 5.2 if (!urlIsHttpHttpsScheme(r.url)) { @@ -270,9 +269,9 @@ class Cache { // 2. if (request instanceof Request) { - innerRequest = request[kState] + innerRequest = getRequestState(request) } else { // 3. - innerRequest = new Request(request)[kState] + innerRequest = getRequestState(new Request(request)) } // 4. @@ -284,7 +283,7 @@ class Cache { } // 5. - const innerResponse = response[kState] + const innerResponse = getResponseState(response) // 6. if (innerResponse.status === 206) { @@ -402,7 +401,7 @@ class Cache { let r = null if (request instanceof Request) { - r = request[kState] + r = getRequestState(request) if (r.method !== 'GET' && !options.ignoreMethod) { return false @@ -410,7 +409,7 @@ class Cache { } else { assert(typeof request === 'string') - r = new Request(request)[kState] + r = getRequestState(new Request(request)) } /** @type {CacheBatchOperation[]} */ @@ -469,14 +468,14 @@ class Cache { // 2.1 if (request instanceof Request) { // 2.1.1 - r = request[kState] + r = getRequestState(request) // 2.1.2 if (r.method !== 'GET' && !options.ignoreMethod) { return [] } } else if (typeof request === 'string') { // 2.2 - r = new Request(request)[kState] + r = getRequestState(new Request(request)) } } @@ -751,7 +750,7 @@ class Cache { if (request !== undefined) { if (request instanceof Request) { // 2.1.1 - r = request[kState] + r = getRequestState(request) // 2.1.2 if (r.method !== 'GET' && !options.ignoreMethod) { @@ -759,7 +758,7 @@ class Cache { } } else if (typeof request === 'string') { // 2.2.1 - r = new Request(request)[kState] + r = getRequestState(new Request(request)) } } diff --git a/lib/web/cache/cachestorage.js b/lib/web/cache/cachestorage.js index cc773b94b49..3e35200b626 100644 --- a/lib/web/cache/cachestorage.js +++ b/lib/web/cache/cachestorage.js @@ -1,9 +1,9 @@ 'use strict' -const { kConstruct } = require('./symbols') const { Cache } = require('./cache') const { webidl } = require('../fetch/webidl') const { kEnumerableProperty } = require('../../core/util') +const { kConstruct } = require('../../core/symbols') class CacheStorage { /** diff --git a/lib/web/cache/symbols.js b/lib/web/cache/symbols.js deleted file mode 100644 index 9271fb61267..00000000000 --- a/lib/web/cache/symbols.js +++ /dev/null @@ -1,5 +0,0 @@ -'use strict' - -module.exports = { - kConstruct: require('../../core/symbols').kConstruct -} diff --git a/lib/web/fetch/body.js b/lib/web/fetch/body.js index 2dc0d2b17e0..899f2216f90 100644 --- a/lib/web/fetch/body.js +++ b/lib/web/fetch/body.js @@ -10,7 +10,6 @@ const { utf8DecodeBytes } = require('./util') const { FormData, setFormDataState } = require('./formdata') -const { kState } = require('./symbols') const { webidl } = require('./webidl') const { Blob } = require('node:buffer') const assert = require('node:assert') @@ -307,7 +306,7 @@ function throwIfAborted (state) { } } -function bodyMixinMethods (instance) { +function bodyMixinMethods (instance, getInternalState) { const methods = { blob () { // The blob() method steps are to return the result of @@ -316,7 +315,7 @@ function bodyMixinMethods (instance) { // contents are bytes and whose type attribute is this’s // MIME type. return consumeBody(this, (bytes) => { - let mimeType = bodyMimeType(this) + let mimeType = bodyMimeType(getInternalState(this)) if (mimeType === null) { mimeType = '' @@ -327,7 +326,7 @@ function bodyMixinMethods (instance) { // Return a Blob whose contents are bytes and type attribute // is mimeType. return new Blob([bytes], { type: mimeType }) - }, instance) + }, instance, getInternalState) }, arrayBuffer () { @@ -337,19 +336,19 @@ function bodyMixinMethods (instance) { // whose contents are bytes. return consumeBody(this, (bytes) => { return new Uint8Array(bytes).buffer - }, instance) + }, instance, getInternalState) }, text () { // The text() method steps are to return the result of running // consume body with this and UTF-8 decode. - return consumeBody(this, utf8DecodeBytes, instance) + return consumeBody(this, utf8DecodeBytes, instance, getInternalState) }, json () { // The json() method steps are to return the result of running // consume body with this and parse JSON from bytes. - return consumeBody(this, parseJSONFromBytes, instance) + return consumeBody(this, parseJSONFromBytes, instance, getInternalState) }, formData () { @@ -357,7 +356,7 @@ function bodyMixinMethods (instance) { // consume body with this and the following step given a byte sequence bytes: return consumeBody(this, (value) => { // 1. Let mimeType be the result of get the MIME type with this. - const mimeType = bodyMimeType(this) + const mimeType = bodyMimeType(getInternalState(this)) // 2. If mimeType is non-null, then switch on mimeType’s essence and run // the corresponding steps: @@ -401,7 +400,7 @@ function bodyMixinMethods (instance) { throw new TypeError( 'Content-Type was not one of "multipart/form-data" or "application/x-www-form-urlencoded".' ) - }, instance) + }, instance, getInternalState) }, bytes () { @@ -410,33 +409,36 @@ function bodyMixinMethods (instance) { // result of creating a Uint8Array from bytes in this’s relevant realm. return consumeBody(this, (bytes) => { return new Uint8Array(bytes) - }, instance) + }, instance, getInternalState) } } return methods } -function mixinBody (prototype) { - Object.assign(prototype.prototype, bodyMixinMethods(prototype)) +function mixinBody (prototype, getInternalState) { + Object.assign(prototype.prototype, bodyMixinMethods(prototype, getInternalState)) } /** * @see https://fetch.spec.whatwg.org/#concept-body-consume-body - * @param {Response|Request} object + * @param {any} object internal state * @param {(value: unknown) => unknown} convertBytesToJSValue - * @param {Response|Request} instance + * @param {any} instance + * @param {(target: any) => any} getInternalState */ -async function consumeBody (object, convertBytesToJSValue, instance) { +async function consumeBody (object, convertBytesToJSValue, instance, getInternalState) { webidl.brandCheck(object, instance) + const state = getInternalState(object) + // 1. If object is unusable, then return a promise rejected // with a TypeError. - if (bodyUnusable(object)) { + if (bodyUnusable(state)) { throw new TypeError('Body is unusable: Body has already been read') } - throwIfAborted(object[kState]) + throwIfAborted(state) // 2. Let promise be a new promise. const promise = createDeferredPromise() @@ -458,22 +460,25 @@ async function consumeBody (object, convertBytesToJSValue, instance) { // 5. If object’s body is null, then run successSteps with an // empty byte sequence. - if (object[kState].body == null) { + if (state.body == null) { successSteps(Buffer.allocUnsafe(0)) return promise.promise } // 6. Otherwise, fully read object’s body given successSteps, // errorSteps, and object’s relevant global object. - await fullyReadBody(object[kState].body, successSteps, errorSteps) + fullyReadBody(state.body, successSteps, errorSteps) // 7. Return promise. return promise.promise } -// https://fetch.spec.whatwg.org/#body-unusable +/** + * @see https://fetch.spec.whatwg.org/#body-unusable + * @param {any} object internal state + */ function bodyUnusable (object) { - const body = object[kState].body + const body = object.body // An object including the Body interface mixin is // said to be unusable if its body is non-null and @@ -491,14 +496,14 @@ function parseJSONFromBytes (bytes) { /** * @see https://fetch.spec.whatwg.org/#concept-body-mime-type - * @param {import('./response').Response|import('./request').Request} requestOrResponse + * @param {any} object internal state */ -function bodyMimeType (requestOrResponse) { +function bodyMimeType (object) { // 1. Let headers be null. // 2. If requestOrResponse is a Request object, then set headers to requestOrResponse’s request’s header list. // 3. Otherwise, set headers to requestOrResponse’s response’s header list. /** @type {import('./headers').HeadersList} */ - const headers = requestOrResponse[kState].headersList + const headers = object.headersList // 4. Let mimeType be the result of extracting a MIME type from headers. const mimeType = extractMimeType(headers) diff --git a/lib/web/fetch/index.js b/lib/web/fetch/index.js index f650c3c01ee..b2c95d94a09 100644 --- a/lib/web/fetch/index.js +++ b/lib/web/fetch/index.js @@ -7,10 +7,11 @@ const { makeAppropriateNetworkError, filterResponse, makeResponse, - fromInnerResponse + fromInnerResponse, + getResponseState } = require('./response') const { HeadersList } = require('./headers') -const { Request, cloneRequest, getRequestDispatcher } = require('./request') +const { Request, cloneRequest, getRequestDispatcher, getRequestState } = require('./request') const zlib = require('node:zlib') const { bytesMatch, @@ -46,7 +47,6 @@ const { createInflate, extractMimeType } = require('./util') -const { kState } = require('./symbols') const assert = require('node:assert') const { safelyExtractBody, extractBody } = require('./body') const { @@ -143,7 +143,7 @@ function fetch (input, init = undefined) { } // 3. Let request be requestObject’s request. - const request = requestObject[kState] + const request = getRequestState(requestObject) // 4. If requestObject’s signal’s aborted flag is set, then: if (requestObject.signal.aborted) { @@ -342,7 +342,7 @@ function abortFetch (p, request, responseObject, error) { } // 4. Let response be responseObject’s response. - const response = responseObject[kState] + const response = getResponseState(responseObject) // 5. If response’s body is not null and is readable, then error response’s // body with error. diff --git a/lib/web/fetch/request.js b/lib/web/fetch/request.js index 74a4b906eff..b6a0d6dfdfb 100644 --- a/lib/web/fetch/request.js +++ b/lib/web/fetch/request.js @@ -23,7 +23,6 @@ const { requestDuplex } = require('./constants') const { kEnumerableProperty, normalizedMethodRecordsBase, normalizedMethodRecords } = util -const { kState } = require('./symbols') const { webidl } = require('./webidl') const { URLSerializer } = require('./data-url') const { kConstruct } = require('../../core/symbols') @@ -89,6 +88,8 @@ class Request { /** @type {Headers} */ #headers + #state + // https://fetch.spec.whatwg.org/#dom-request constructor (input, init = undefined) { if (input === kConstruct) { @@ -146,7 +147,7 @@ class Request { assert(input instanceof Request) // 8. Set request to input’s request. - request = input[kState] + request = input.#state // 9. Set signal to input’s signal. signal = input.#signal @@ -398,7 +399,7 @@ class Request { } // 27. Set this’s request to request. - this[kState] = request + this.#state = request // 28. Set this’s signal to a new AbortSignal object with this’s relevant // Realm. @@ -489,7 +490,7 @@ class Request { // 33. Let inputBody be input’s request’s body if input is a Request // object; otherwise null. - const inputBody = input instanceof Request ? input[kState].body : null + const inputBody = input instanceof Request ? input.#state.body : null // 34. If either init["body"] exists and is non-null or inputBody is // non-null, and request’s method is `GET` or `HEAD`, then throw a @@ -554,7 +555,7 @@ class Request { // 40. If initBody is null and inputBody is non-null, then: if (initBody == null && inputBody != null) { // 1. If input is unusable, then throw a TypeError. - if (bodyUnusable(input)) { + if (bodyUnusable(input.#state)) { throw new TypeError( 'Cannot construct a Request with a Request object that has already been used.' ) @@ -572,7 +573,7 @@ class Request { } // 41. Set this’s request’s body to finalBody. - this[kState].body = finalBody + this.#state.body = finalBody } // Returns request’s HTTP method, which is "GET" by default. @@ -580,7 +581,7 @@ class Request { webidl.brandCheck(this, Request) // The method getter steps are to return this’s request’s method. - return this[kState].method + return this.#state.method } // Returns the URL of request as a string. @@ -588,7 +589,7 @@ class Request { webidl.brandCheck(this, Request) // The url getter steps are to return this’s request’s URL, serialized. - return URLSerializer(this[kState].url) + return URLSerializer(this.#state.url) } // Returns a Headers object consisting of the headers associated with request. @@ -607,7 +608,7 @@ class Request { webidl.brandCheck(this, Request) // The destination getter are to return this’s request’s destination. - return this[kState].destination + return this.#state.destination } // Returns the referrer of request. Its value can be a same-origin URL if @@ -620,18 +621,18 @@ class Request { // 1. If this’s request’s referrer is "no-referrer", then return the // empty string. - if (this[kState].referrer === 'no-referrer') { + if (this.#state.referrer === 'no-referrer') { return '' } // 2. If this’s request’s referrer is "client", then return // "about:client". - if (this[kState].referrer === 'client') { + if (this.#state.referrer === 'client') { return 'about:client' } // Return this’s request’s referrer, serialized. - return this[kState].referrer.toString() + return this.#state.referrer.toString() } // Returns the referrer policy associated with request. @@ -641,7 +642,7 @@ class Request { webidl.brandCheck(this, Request) // The referrerPolicy getter steps are to return this’s request’s referrer policy. - return this[kState].referrerPolicy + return this.#state.referrerPolicy } // Returns the mode associated with request, which is a string indicating @@ -651,15 +652,17 @@ class Request { webidl.brandCheck(this, Request) // The mode getter steps are to return this’s request’s mode. - return this[kState].mode + return this.#state.mode } // Returns the credentials mode associated with request, // which is a string indicating whether credentials will be sent with the // request always, never, or only when sent to a same-origin URL. get credentials () { + webidl.brandCheck(this, Request) + // The credentials getter steps are to return this’s request’s credentials mode. - return this[kState].credentials + return this.#state.credentials } // Returns the cache mode associated with request, @@ -669,7 +672,7 @@ class Request { webidl.brandCheck(this, Request) // The cache getter steps are to return this’s request’s cache mode. - return this[kState].cache + return this.#state.cache } // Returns the redirect mode associated with request, @@ -680,7 +683,7 @@ class Request { webidl.brandCheck(this, Request) // The redirect getter steps are to return this’s request’s redirect mode. - return this[kState].redirect + return this.#state.redirect } // Returns request’s subresource integrity metadata, which is a @@ -691,7 +694,7 @@ class Request { // The integrity getter steps are to return this’s request’s integrity // metadata. - return this[kState].integrity + return this.#state.integrity } // Returns a boolean indicating whether or not request can outlive the @@ -700,7 +703,7 @@ class Request { webidl.brandCheck(this, Request) // The keepalive getter steps are to return this’s request’s keepalive. - return this[kState].keepalive + return this.#state.keepalive } // Returns a boolean indicating whether or not request is for a reload @@ -710,7 +713,7 @@ class Request { // The isReloadNavigation getter steps are to return true if this’s // request’s reload-navigation flag is set; otherwise false. - return this[kState].reloadNavigation + return this.#state.reloadNavigation } // Returns a boolean indicating whether or not request is for a history @@ -720,7 +723,7 @@ class Request { // The isHistoryNavigation getter steps are to return true if this’s request’s // history-navigation flag is set; otherwise false. - return this[kState].historyNavigation + return this.#state.historyNavigation } // Returns the signal associated with request, which is an AbortSignal @@ -736,13 +739,13 @@ class Request { get body () { webidl.brandCheck(this, Request) - return this[kState].body ? this[kState].body.stream : null + return this.#state.body ? this.#state.body.stream : null } get bodyUsed () { webidl.brandCheck(this, Request) - return !!this[kState].body && util.isDisturbed(this[kState].body.stream) + return !!this.#state.body && util.isDisturbed(this.#state.body.stream) } get duplex () { @@ -756,12 +759,12 @@ class Request { webidl.brandCheck(this, Request) // 1. If this is unusable, then throw a TypeError. - if (bodyUnusable(this)) { + if (bodyUnusable(this.#state)) { throw new TypeError('unusable') } // 2. Let clonedRequest be the result of cloning this’s request. - const clonedRequest = cloneRequest(this[kState]) + const clonedRequest = cloneRequest(this.#state) // 3. Let clonedRequestObject be the result of creating a Request object, // given clonedRequest, this’s headers’s guard, and this’s relevant Realm. @@ -846,15 +849,32 @@ class Request { static setRequestHeaders (request, newHeaders) { request.#headers = newHeaders } + + /** + * @param {Request} request + */ + static getRequestState (request) { + return request.#state + } + + /** + * @param {Request} request + * @param {any} newState + */ + static setRequestState (request, newState) { + request.#state = newState + } } -const { setRequestSignal, getRequestDispatcher, setRequestDispatcher, setRequestHeaders } = Request +const { setRequestSignal, getRequestDispatcher, setRequestDispatcher, setRequestHeaders, getRequestState, setRequestState } = Request Reflect.deleteProperty(Request, 'setRequestSignal') Reflect.deleteProperty(Request, 'getRequestDispatcher') Reflect.deleteProperty(Request, 'setRequestDispatcher') Reflect.deleteProperty(Request, 'setRequestHeaders') +Reflect.deleteProperty(Request, 'getRequestState') +Reflect.deleteProperty(Request, 'setRequestState') -mixinBody(Request) +mixinBody(Request, getRequestState) // https://fetch.spec.whatwg.org/#requests function makeRequest (init) { @@ -929,7 +949,7 @@ function cloneRequest (request) { */ function fromInnerRequest (innerRequest, dispatcher, signal, guard) { const request = new Request(kConstruct) - request[kState] = innerRequest + setRequestState(request, innerRequest) setRequestDispatcher(request, dispatcher) setRequestSignal(request, signal) const headers = new Headers(kConstruct) @@ -1071,5 +1091,6 @@ module.exports = { makeRequest, fromInnerRequest, cloneRequest, - getRequestDispatcher + getRequestDispatcher, + getRequestState } diff --git a/lib/web/fetch/response.js b/lib/web/fetch/response.js index 4e8efaac1ae..904752f6c98 100644 --- a/lib/web/fetch/response.js +++ b/lib/web/fetch/response.js @@ -18,7 +18,6 @@ const { redirectStatusSet, nullBodyStatus } = require('./constants') -const { kState } = require('./symbols') const { webidl } = require('./webidl') const { FormData } = require('./formdata') const { URLSerializer } = require('./data-url') @@ -33,6 +32,8 @@ class Response { /** @type {Headers} */ #headers + #state + // Creates network error Response. static error () { // The static error() method steps are to return the result of creating a @@ -98,13 +99,13 @@ class Response { const responseObject = fromInnerResponse(makeResponse({}), 'immutable') // 5. Set responseObject’s response’s status to status. - responseObject[kState].status = status + responseObject.#state.status = status // 6. Let value be parsedURL, serialized and isomorphic encoded. const value = isomorphicEncode(URLSerializer(parsedURL)) // 7. Append `Location`/value to responseObject’s response’s header list. - responseObject[kState].headersList.append('location', value, true) + responseObject.#state.headersList.append('location', value, true) // 8. Return responseObject. return responseObject @@ -123,14 +124,14 @@ class Response { init = webidl.converters.ResponseInit(init) // 1. Set this’s response to a new response. - this[kState] = makeResponse({}) + this.#state = makeResponse({}) // 2. Set this’s headers to a new Headers object with this’s relevant // Realm, whose header list is this’s response’s header list and guard // is "response". this.#headers = new Headers(kConstruct) setHeadersGuard(this.#headers, 'response') - setHeadersList(this.#headers, this[kState].headersList) + setHeadersList(this.#headers, this.#state.headersList) // 3. Let bodyWithType be null. let bodyWithType = null @@ -150,14 +151,14 @@ class Response { webidl.brandCheck(this, Response) // The type getter steps are to return this’s response’s type. - return this[kState].type + return this.#state.type } // Returns response’s URL, if it has one; otherwise the empty string. get url () { webidl.brandCheck(this, Response) - const urlList = this[kState].urlList + const urlList = this.#state.urlList // The url getter steps are to return the empty string if this’s // response’s URL is null; otherwise this’s response’s URL, @@ -177,7 +178,7 @@ class Response { // The redirected getter steps are to return true if this’s response’s URL // list has more than one item; otherwise false. - return this[kState].urlList.length > 1 + return this.#state.urlList.length > 1 } // Returns response’s status. @@ -185,7 +186,7 @@ class Response { webidl.brandCheck(this, Response) // The status getter steps are to return this’s response’s status. - return this[kState].status + return this.#state.status } // Returns whether response’s status is an ok status. @@ -194,7 +195,7 @@ class Response { // The ok getter steps are to return true if this’s response’s status is an // ok status; otherwise false. - return this[kState].status >= 200 && this[kState].status <= 299 + return this.#state.status >= 200 && this.#state.status <= 299 } // Returns response’s status message. @@ -203,7 +204,7 @@ class Response { // The statusText getter steps are to return this’s response’s status // message. - return this[kState].statusText + return this.#state.statusText } // Returns response’s headers as Headers. @@ -217,13 +218,13 @@ class Response { get body () { webidl.brandCheck(this, Response) - return this[kState].body ? this[kState].body.stream : null + return this.#state.body ? this.#state.body.stream : null } get bodyUsed () { webidl.brandCheck(this, Response) - return !!this[kState].body && util.isDisturbed(this[kState].body.stream) + return !!this.#state.body && util.isDisturbed(this.#state.body.stream) } // Returns a clone of response. @@ -231,7 +232,7 @@ class Response { webidl.brandCheck(this, Response) // 1. If this is unusable, then throw a TypeError. - if (bodyUnusable(this)) { + if (bodyUnusable(this.#state)) { throw webidl.errors.exception({ header: 'Response.clone', message: 'Body has already been consumed.' @@ -239,7 +240,7 @@ class Response { } // 2. Let clonedResponse be the result of cloning this’s response. - const clonedResponse = cloneResponse(this[kState]) + const clonedResponse = cloneResponse(this.#state) // 3. Return the result of creating a Response object, given // clonedResponse, this’s headers’s guard, and this’s relevant Realm. @@ -282,13 +283,30 @@ class Response { static setResponseHeaders (response, newHeaders) { response.#headers = newHeaders } + + /** + * @param {Response} response + */ + static getResponseState (response) { + return response.#state + } + + /** + * @param {Response} response + * @param {any} newState + */ + static setResponseState (response, newState) { + response.#state = newState + } } -const { getResponseHeaders, setResponseHeaders } = Response +const { getResponseHeaders, setResponseHeaders, getResponseState, setResponseState } = Response Reflect.deleteProperty(Response, 'getResponseHeaders') Reflect.deleteProperty(Response, 'setResponseHeaders') +Reflect.deleteProperty(Response, 'getResponseState') +Reflect.deleteProperty(Response, 'setResponseState') -mixinBody(Response) +mixinBody(Response, getResponseState) Object.defineProperties(Response.prototype, { type: kEnumerableProperty, @@ -485,12 +503,12 @@ function initializeResponse (response, init, body) { // 3. Set response’s response’s status to init["status"]. if ('status' in init && init.status != null) { - response[kState].status = init.status + getResponseState(response).status = init.status } // 4. Set response’s response’s status message to init["statusText"]. if ('statusText' in init && init.statusText != null) { - response[kState].statusText = init.statusText + getResponseState(response).statusText = init.statusText } // 5. If init["headers"] exists, then fill response’s headers with init["headers"]. @@ -509,12 +527,12 @@ function initializeResponse (response, init, body) { } // 2. Set response's body to body's body. - response[kState].body = body.body + getResponseState(response).body = body.body // 3. If body's type is non-null and response's header list does not contain // `Content-Type`, then append (`Content-Type`, body's type) to response's header list. - if (body.type != null && !response[kState].headersList.contains('content-type', true)) { - response[kState].headersList.append('content-type', body.type, true) + if (body.type != null && !getResponseState(response).headersList.contains('content-type', true)) { + getResponseState(response).headersList.append('content-type', body.type, true) } } } @@ -527,7 +545,7 @@ function initializeResponse (response, init, body) { */ function fromInnerResponse (innerResponse, guard) { const response = new Response(kConstruct) - response[kState] = innerResponse + setResponseState(response, innerResponse) const headers = new Headers(kConstruct) setResponseHeaders(response, headers) setHeadersList(headers, innerResponse.headersList) @@ -610,5 +628,6 @@ module.exports = { filterResponse, Response, cloneResponse, - fromInnerResponse + fromInnerResponse, + getResponseState } diff --git a/lib/web/fetch/symbols.js b/lib/web/fetch/symbols.js deleted file mode 100644 index 3d9c4c4784e..00000000000 --- a/lib/web/fetch/symbols.js +++ /dev/null @@ -1,5 +0,0 @@ -'use strict' - -module.exports = { - kState: Symbol('state') -} diff --git a/test/node-fetch/response.js b/test/node-fetch/response.js index 84b3e9c471d..b67c786aa9f 100644 --- a/test/node-fetch/response.js +++ b/test/node-fetch/response.js @@ -6,15 +6,12 @@ const stream = require('node:stream') const { Response } = require('../../index.js') const TestServer = require('./utils/server.js') const { Blob } = require('node:buffer') -const { kState } = require('../../lib/web/fetch/symbols.js') describe('Response', () => { const local = new TestServer() - let base before(async () => { await local.start() - base = `http://${local.hostname}:${local.port}/` }) after(async () => { @@ -115,11 +112,9 @@ describe('Response', () => { status: 346, statusText: 'production' }) - res[kState].urlList = [new URL(base)] const cl = res.clone() assert.strictEqual(cl.headers.get('a'), '1') assert.strictEqual(cl.type, 'default') - assert.strictEqual(cl.url, base) assert.strictEqual(cl.status, 346) assert.strictEqual(cl.statusText, 'production') assert.strictEqual(cl.ok, false) From cee38b16499f63929a7b5a6a63b33ebab39f2143 Mon Sep 17 00:00:00 2001 From: Khafra Date: Fri, 20 Sep 2024 16:35:37 -0400 Subject: [PATCH 5/5] add test --- test/fetch/spread.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 test/fetch/spread.js diff --git a/test/fetch/spread.js b/test/fetch/spread.js new file mode 100644 index 00000000000..f684cb18f40 --- /dev/null +++ b/test/fetch/spread.js @@ -0,0 +1,15 @@ +'use strict' + +const undici = require('../..') +const { test } = require('node:test') +const assert = require('node:assert') + +test('spreading web classes yields empty objects', (t) => { + for (const object of [ + new undici.FormData(), + new undici.Response(null), + new undici.Request('http://a') + ]) { + assert.deepStrictEqual({ ...object }, {}) + } +})