Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

replace instanceof in brand checks with isPrototypeOf #3692

Merged
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
15 changes: 9 additions & 6 deletions lib/web/cache/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ 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, getResponseState } = require('../fetch/response')
const { 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')
Expand Down Expand Up @@ -268,7 +268,7 @@ class Cache {
let innerRequest = null

// 2.
if (request instanceof Request) {
if (webidl.is.Request(request)) {
innerRequest = getRequestState(request)
} else { // 3.
innerRequest = getRequestState(new Request(request))
Expand Down Expand Up @@ -400,7 +400,7 @@ class Cache {
*/
let r = null

if (request instanceof Request) {
if (webidl.is.Request(request)) {
r = getRequestState(request)

if (r.method !== 'GET' && !options.ignoreMethod) {
Expand Down Expand Up @@ -466,7 +466,7 @@ class Cache {
// 2.
if (request !== undefined) {
// 2.1
if (request instanceof Request) {
if (webidl.is.Request(request)) {
// 2.1.1
r = getRequestState(request)

Expand Down Expand Up @@ -748,7 +748,7 @@ class Cache {

// 2.
if (request !== undefined) {
if (request instanceof Request) {
if (webidl.is.Request(request)) {
// 2.1.1
r = getRequestState(request)

Expand Down Expand Up @@ -847,7 +847,10 @@ webidl.converters.MultiCacheQueryOptions = webidl.dictionaryConverter([
}
])

webidl.converters.Response = webidl.interfaceConverter(Response)
webidl.converters.Response = webidl.interfaceConverter(
webidl.is.Response,
'Response'
)

webidl.converters['sequence<RequestInfo>'] = webidl.sequenceConverter(
webidl.converters.RequestInfo
Expand Down
12 changes: 5 additions & 7 deletions lib/web/cookies/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ const { stringify } = require('./util')
const { webidl } = require('../fetch/webidl')
const { Headers } = require('../fetch/headers')

const headersToBrandCheck = globalThis.Headers
? [Headers, globalThis.Headers]
: [Headers]
const brandChecks = webidl.brandCheckMultiple([Headers, globalThis.Headers].filter(Boolean))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if you run node with --no-experimental-fetch? Then globalThis.Headers will be undefined. Thus breaking the code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's what the .filter(Boolean) is for

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right


/**
* @typedef {Object} Cookie
Expand All @@ -30,7 +28,7 @@ const headersToBrandCheck = globalThis.Headers
function getCookies (headers) {
webidl.argumentLengthCheck(arguments, 1, 'getCookies')

webidl.brandCheckMultiple(headers, headersToBrandCheck)
brandChecks(headers)

const cookie = headers.get('cookie')

Expand All @@ -57,7 +55,7 @@ function getCookies (headers) {
* @returns {void}
*/
function deleteCookie (headers, name, attributes) {
webidl.brandCheckMultiple(headers, headersToBrandCheck)
brandChecks(headers)

const prefix = 'deleteCookie'
webidl.argumentLengthCheck(arguments, 2, prefix)
Expand All @@ -82,7 +80,7 @@ function deleteCookie (headers, name, attributes) {
function getSetCookies (headers) {
webidl.argumentLengthCheck(arguments, 1, 'getSetCookies')

webidl.brandCheckMultiple(headers, headersToBrandCheck)
brandChecks(headers)

const cookies = headers.getSetCookie()

Expand All @@ -101,7 +99,7 @@ function getSetCookies (headers) {
function setCookie (headers, cookie) {
webidl.argumentLengthCheck(arguments, 2, 'setCookie')

webidl.brandCheckMultiple(headers, headersToBrandCheck)
brandChecks(headers)

cookie = webidl.converters.Cookie(cookie)

Expand Down
16 changes: 8 additions & 8 deletions lib/web/fetch/body.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ function extractBody (object, keepalive = false) {
let stream = null

// 2. If object is a ReadableStream object, then set stream to object.
if (object instanceof ReadableStream) {
if (webidl.is.ReadableStream(object)) {
stream = object
} else if (object instanceof Blob) {
} else if (webidl.is.Blob(object)) {
// 3. Otherwise, if object is a Blob object, set stream to the
// result of running object’s get stream.
stream = object.stream()
Expand All @@ -64,7 +64,7 @@ function extractBody (object, keepalive = false) {
}

// 5. Assert: stream is a ReadableStream object.
assert(stream instanceof ReadableStream)
assert(webidl.is.ReadableStream(stream))

// 6. Let action be null.
let action = null
Expand All @@ -86,7 +86,7 @@ function extractBody (object, keepalive = false) {

// Set type to `text/plain;charset=UTF-8`.
type = 'text/plain;charset=UTF-8'
} else if (object instanceof URLSearchParams) {
} else if (webidl.is.URLSearchParams(object)) {
// URLSearchParams

// spec says to run application/x-www-form-urlencoded on body.list
Expand All @@ -109,7 +109,7 @@ function extractBody (object, keepalive = false) {

// Set source to a copy of the bytes held by object.
source = new Uint8Array(object.buffer.slice(object.byteOffset, object.byteOffset + object.byteLength))
} else if (object instanceof FormData) {
} else if (webidl.is.FormData(object)) {
const boundary = `----formdata-undici-0${`${Math.floor(Math.random() * 1e11)}`.padStart(11, '0')}`
const prefix = `--${boundary}\r\nContent-Disposition: form-data`

Expand Down Expand Up @@ -178,7 +178,7 @@ function extractBody (object, keepalive = false) {
// followed by the multipart/form-data boundary string generated
// by the multipart/form-data encoding algorithm.
type = `multipart/form-data; boundary=${boundary}`
} else if (object instanceof Blob) {
} else if (webidl.is.Blob(object)) {
// Blob

// Set source to object.
Expand Down Expand Up @@ -206,7 +206,7 @@ function extractBody (object, keepalive = false) {
}

stream =
object instanceof ReadableStream ? object : ReadableStreamFrom(object)
webidl.is.ReadableStream(object) ? object : ReadableStreamFrom(object)
}

// 11. If source is a byte sequence, then set action to a
Expand Down Expand Up @@ -265,7 +265,7 @@ function safelyExtractBody (object, keepalive = false) {
// a byte sequence or BodyInit object object, run these steps:

// 1. If object is a ReadableStream object, then:
if (object instanceof ReadableStream) {
if (webidl.is.ReadableStream(object)) {
// Assert: object is neither disturbed nor locked.
// istanbul ignore next
assert(!util.isDisturbed(object), 'The body has already been consumed.')
Expand Down
3 changes: 2 additions & 1 deletion lib/web/fetch/formdata-parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const { isUSVString, bufferToLowerCasedHeaderName } = require('../../core/util')
const { utf8DecodeBytes } = require('./util')
const { HTTP_TOKEN_CODEPOINTS, isomorphicDecode } = require('./data-url')
const { makeEntry } = require('./formdata')
const { webidl } = require('./webidl')
const assert = require('node:assert')
const { File: NodeFile } = require('node:buffer')

Expand Down Expand Up @@ -204,7 +205,7 @@ function multipartFormDataParser (input, mimeType) {

// 5.12. Assert: name is a scalar value string and value is either a scalar value string or a File object.
assert(isUSVString(name))
assert((typeof value === 'string' && isUSVString(value)) || value instanceof File)
assert((typeof value === 'string' && isUSVString(value)) || webidl.is.File(value))

// 5.13. Create an entry with name and value, and append it to entry list.
entryList.push(makeEntry(name, value, filename))
Expand Down
8 changes: 5 additions & 3 deletions lib/web/fetch/formdata.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class FormData {

name = webidl.converters.USVString(name)

if (arguments.length === 3 || value instanceof Blob) {
if (arguments.length === 3 || webidl.is.Blob(value)) {
value = webidl.converters.Blob(value, prefix, 'value')

if (filename !== undefined) {
Expand Down Expand Up @@ -122,7 +122,7 @@ class FormData {

name = webidl.converters.USVString(name)

if (arguments.length === 3 || value instanceof Blob) {
if (arguments.length === 3 || webidl.is.Blob(value)) {
value = webidl.converters.Blob(value, prefix, 'value')

if (filename !== undefined) {
Expand Down Expand Up @@ -235,7 +235,7 @@ function makeEntry (name, value, filename) {

// 1. If value is not a File object, then set value to a new File object,
// representing the same bytes, whose name attribute value is "blob"
if (!(value instanceof File)) {
if (!webidl.is.File(value)) {
value = new File([value], 'blob', { type: value.type })
}

Expand All @@ -256,4 +256,6 @@ function makeEntry (name, value, filename) {
return { name, value }
}

webidl.is.FormData = webidl.util.MakeTypeAssertion(FormData.prototype)

module.exports = { FormData, makeEntry, setFormDataState }
4 changes: 2 additions & 2 deletions lib/web/fetch/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,7 @@ function schemeFetch (fetchParams) {

// 2. If request’s method is not `GET`, blobURLEntry is null, or blobURLEntry’s
// object is not a Blob object, then return a network error.
if (request.method !== 'GET' || !(blob instanceof Blob)) {
if (request.method !== 'GET' || !webidl.is.Blob(blob)) {
return Promise.resolve(makeNetworkError('invalid method'))
}

Expand Down Expand Up @@ -1439,7 +1439,7 @@ async function httpNetworkOrCacheFetch (
// 11. If httpRequest’s referrer is a URL, then append
// `Referer`/httpRequest’s referrer, serialized and isomorphic encoded,
// to httpRequest’s header list.
if (httpRequest.referrer instanceof URL) {
if (webidl.is.URL(httpRequest.referrer)) {
httpRequest.headersList.append('referer', isomorphicEncode(httpRequest.referrer.href), true)
}

Expand Down
12 changes: 5 additions & 7 deletions lib/web/fetch/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ class Request {
// 6. Otherwise:

// 7. Assert: input is a Request object.
assert(input instanceof Request)
assert(webidl.is.Request(input))

// 8. Set request to input’s request.
request = input.#state
Expand Down Expand Up @@ -490,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.#state.body : null
const inputBody = webidl.is.Request(input) ? 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
Expand Down Expand Up @@ -986,23 +986,21 @@ Object.defineProperties(Request.prototype, {
}
})

webidl.is.Request = webidl.util.MakeTypeAssertion(Request.prototype)

// https://fetch.spec.whatwg.org/#requestinfo
webidl.converters.RequestInfo = function (V, prefix, argument) {
if (typeof V === 'string') {
return webidl.converters.USVString(V)
}

if (V instanceof Request) {
if (webidl.is.Request(V)) {
return V
}

return webidl.converters.USVString(V)
}

webidl.converters.AbortSignal = webidl.interfaceConverter(
AbortSignal
)

// https://fetch.spec.whatwg.org/#requestinit
webidl.converters.RequestInit = webidl.dictionaryConverter([
{
Expand Down
11 changes: 6 additions & 5 deletions lib/web/fetch/response.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ const {
nullBodyStatus
} = require('./constants')
const { webidl } = require('./webidl')
const { FormData } = require('./formdata')
const { URLSerializer } = require('./data-url')
const { kConstruct } = require('../../core/symbols')
const assert = require('node:assert')
Expand Down Expand Up @@ -569,19 +568,19 @@ webidl.converters.XMLHttpRequestBodyInit = function (V, prefix, name) {
return webidl.converters.USVString(V, prefix, name)
}

if (V instanceof Blob) {
if (webidl.is.Blob(V)) {
return V
}

if (ArrayBuffer.isView(V) || types.isArrayBuffer(V)) {
return V
}

if (V instanceof FormData) {
if (webidl.is.FormData(V)) {
return V
}

if (V instanceof URLSearchParams) {
if (webidl.is.URLSearchParams(V)) {
return V
}

Expand All @@ -590,7 +589,7 @@ webidl.converters.XMLHttpRequestBodyInit = function (V, prefix, name) {

// https://fetch.spec.whatwg.org/#bodyinit
webidl.converters.BodyInit = function (V, prefix, argument) {
if (V instanceof ReadableStream) {
if (webidl.is.ReadableStream(V)) {
return V
}

Expand Down Expand Up @@ -620,6 +619,8 @@ webidl.converters.ResponseInit = webidl.dictionaryConverter([
}
])

webidl.is.Response = webidl.util.MakeTypeAssertion(Response.prototype)

module.exports = {
isNetworkError,
makeNetworkError,
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 @@ -399,7 +399,7 @@ function determineRequestsReferrer (request) {

// note: we need to clone it as it's mutated
referrerSource = new URL(globalOrigin)
} else if (request.referrer instanceof URL) {
} else if (webidl.is.URL(request.referrer)) {
// Let referrerSource be request’s referrer.
referrerSource = request.referrer
}
Expand Down Expand Up @@ -476,7 +476,7 @@ function determineRequestsReferrer (request) {
*/
function stripURLForReferrer (url, originOnly) {
// 1. Assert: url is a URL.
assert(url instanceof URL)
assert(webidl.is.URL(url))

url = new URL(url)

Expand Down Expand Up @@ -508,7 +508,7 @@ function stripURLForReferrer (url, originOnly) {
}

function isURLPotentiallyTrustworthy (url) {
if (!(url instanceof URL)) {
if (!webidl.is.URL(url)) {
return false
}

Expand Down
Loading
Loading