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

cleanup web symbol removal #3638

Merged
merged 2 commits into from
Sep 23, 2024
Merged
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
6 changes: 3 additions & 3 deletions lib/web/fetch/body.js
Original file line number Diff line number Diff line change
Expand Up @@ -496,14 +496,14 @@ function parseJSONFromBytes (bytes) {

/**
* @see https://fetch.spec.whatwg.org/#concept-body-mime-type
* @param {any} object internal state
* @param {any} requestOrResponse internal state
*/
function bodyMimeType (object) {
function bodyMimeType (requestOrResponse) {
// 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 = object.headersList
const headers = requestOrResponse.headersList

// 4. Let mimeType be the result of extracting a MIME type from headers.
const mimeType = extractMimeType(headers)
Expand Down
121 changes: 65 additions & 56 deletions lib/web/fetch/headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ const { webidl } = require('./webidl')
const assert = require('node:assert')
const util = require('node:util')

// TODO(@KhafraDev): remove
const kHeadersSortedMap = Symbol('headers map sorted')

/**
* @param {number} code
*/
Expand Down Expand Up @@ -119,6 +116,67 @@ function appendHeader (headers, name, value) {
// privileged no-CORS request headers from headers
}

// https://fetch.spec.whatwg.org/#concept-header-list-sort-and-combine
/**
* @param {Headers} target
*/
function headersListSortAndCombine (target) {
const headersList = getHeadersList(target)

if (!headersList) {
return []
}

if (headersList.sortedMap) {
return headersList.sortedMap
}

// 1. Let headers be an empty list of headers with the key being the name
// and value the value.
const headers = []

// 2. Let names be the result of convert header names to a sorted-lowercase
// set with all the names of the headers in list.
const names = headersList.toSortedArray()

const cookies = headersList.cookies

// fast-path
if (cookies === null || cookies.length === 1) {
// Note: The non-null assertion of value has already been done by `HeadersList#toSortedArray`
return (headersList.sortedMap = names)
}

// 3. For each name of names:
for (let i = 0; i < names.length; ++i) {
const { 0: name, 1: value } = names[i]
// 1. If name is `set-cookie`, then:
if (name === 'set-cookie') {
// 1. Let values be a list of all values of headers in list whose name
// is a byte-case-insensitive match for name, in order.

// 2. For each value of values:
// 1. Append (name, value) to headers.
for (let j = 0; j < cookies.length; ++j) {
headers.push([name, cookies[j]])
}
} else {
// 2. Otherwise:

// 1. Let value be the result of getting name from list.

// 2. Assert: value is non-null.
// Note: This operation was done by `HeadersList#toSortedArray`.

// 3. Append (name, value) to headers.
headers.push([name, value])
}
}

// 4. Return headers.
return (headersList.sortedMap = headers)
}

function compareHeaderName (a, b) {
return a[0] < b[0] ? -1 : 1
}
Expand Down Expand Up @@ -548,58 +606,6 @@ class Headers {
return []
}

// https://fetch.spec.whatwg.org/#concept-header-list-sort-and-combine
get [kHeadersSortedMap] () {
if (this.#headersList.sortedMap) {
return this.#headersList.sortedMap
}

// 1. Let headers be an empty list of headers with the key being the name
// and value the value.
const headers = []

// 2. Let names be the result of convert header names to a sorted-lowercase
// set with all the names of the headers in list.
const names = this.#headersList.toSortedArray()

const cookies = this.#headersList.cookies

// 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.sortedMap = names)
}

// 3. For each name of names:
for (let i = 0; i < names.length; ++i) {
const { 0: name, 1: value } = names[i]
// 1. If name is `set-cookie`, then:
if (name === 'set-cookie') {
// 1. Let values be a list of all values of headers in list whose name
// is a byte-case-insensitive match for name, in order.

// 2. For each value of values:
// 1. Append (name, value) to headers.
for (let j = 0; j < cookies.length; ++j) {
headers.push([name, cookies[j]])
}
} else {
// 2. Otherwise:

// 1. Let value be the result of getting name from list.

// 2. Assert: value is non-null.
// Note: This operation was done by `HeadersList#toSortedArray`.

// 3. Append (name, value) to headers.
headers.push([name, value])
}
}

// 4. Return headers.
return (this.#headersList.sortedMap = headers)
}

[util.inspect.custom] (depth, options) {
options.depth ??= depth

Expand All @@ -614,6 +620,9 @@ class Headers {
o.#guard = guard
}

/**
* @param {Headers} o
*/
static getHeadersList (o) {
return o.#headersList
}
Expand All @@ -629,7 +638,7 @@ Reflect.deleteProperty(Headers, 'setHeadersGuard')
Reflect.deleteProperty(Headers, 'getHeadersList')
Reflect.deleteProperty(Headers, 'setHeadersList')

iteratorMixin('Headers', Headers, kHeadersSortedMap, 0, 1)
iteratorMixin('Headers', Headers, headersListSortAndCombine, 0, 1)

Object.defineProperties(Headers.prototype, {
append: kEnumerableProperty,
Expand Down
6 changes: 3 additions & 3 deletions lib/web/fetch/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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|((target: any) => any)} kInternalIterator
* @param {((target: any) => any)} kInternalIterator
* @param {string | number} [keyIndex]
* @param {string | number} [valueIndex]
*/
Expand Down Expand Up @@ -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 = typeof kInternalIterator === 'symbol' ? this.#target[kInternalIterator] : kInternalIterator(this.#target)
const values = kInternalIterator(this.#target)

// 9. Let len be the length of values.
const len = values.length
Expand Down Expand Up @@ -961,7 +961,7 @@ function createIterator (name, kInternalIterator, keyIndex = 0, valueIndex = 1)
* @see https://webidl.spec.whatwg.org/#dfn-iterator-prototype-object
* @param {string} name name of the instance
* @param {any} object class
* @param {symbol} kInternalIterator
* @param {(target: any) => any} kInternalIterator
* @param {string | number} [keyIndex]
* @param {string | number} [valueIndex]
*/
Expand Down
26 changes: 26 additions & 0 deletions test/fetch/spread.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const undici = require('../..')
const { test } = require('node:test')
const assert = require('node:assert')
const { inspect } = require('node:util')

test('spreading web classes yields empty objects', (t) => {
for (const object of [
Expand All @@ -13,3 +14,28 @@ test('spreading web classes yields empty objects', (t) => {
assert.deepStrictEqual({ ...object }, {})
}
})

test('Objects only have an expected set of symbols on their prototypes', (t) => {
const allowedSymbols = [
Symbol.iterator,
Symbol.toStringTag,
inspect.custom
]

for (const object of [
undici.FormData,
undici.Response,
undici.Request,
undici.Headers,
undici.WebSocket,
undici.MessageEvent,
undici.CloseEvent,
undici.ErrorEvent,
undici.EventSource
]) {
const symbols = Object.keys(Object.getOwnPropertyDescriptors(object.prototype))
.filter(v => typeof v === 'symbol')

assert(symbols.every(symbol => allowedSymbols.includes(symbol)))
}
})
19 changes: 12 additions & 7 deletions test/wpt/runner/worker.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,20 @@ import { setFlagsFromString } from 'node:v8'
import { runInNewContext, runInThisContext } from 'node:vm'
import { parentPort, workerData } from 'node:worker_threads'
import {
fetch, FormData, Headers, Request, Response, setGlobalOrigin
fetch,
FormData,
Headers,
Request,
Response,
setGlobalOrigin,
CloseEvent,
WebSocket,
caches,
EventSource
} from '../../../index.js'
import { CloseEvent } from '../../../lib/web/websocket/events.js'
import { WebSocket } from '../../../lib/web/websocket/websocket.js'
// TODO(@KhafraDev): export these in index.js
import { Cache } from '../../../lib/web/cache/cache.js'
import { CacheStorage } from '../../../lib/web/cache/cachestorage.js'
import { kConstruct } from '../../../lib/web/cache/symbols.js'
// TODO(@KhafraDev): move this import once its added to index
import { EventSource } from '../../../lib/web/eventsource/eventsource.js'
import { webcrypto } from 'node:crypto'

const { initScripts, meta, test, url, path } = workerData
Expand Down Expand Up @@ -79,7 +84,7 @@ Object.defineProperties(globalThis, {
},
caches: {
...globalPropertyDescriptors,
value: new CacheStorage(kConstruct)
value: caches
},
Cache: {
...globalPropertyDescriptors,
Expand Down
Loading