Skip to content

Commit

Permalink
cleanup web symbol removal (#3638)
Browse files Browse the repository at this point in the history
  • Loading branch information
KhafraDev authored Sep 23, 2024
1 parent 45dceea commit 0d00e46
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 69 deletions.
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

0 comments on commit 0d00e46

Please sign in to comment.