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

perf(fetch): use string comparisons for url schemes #2038

Merged
merged 1 commit into from
Apr 1, 2023
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
18 changes: 9 additions & 9 deletions lib/fetch/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ const {
isErrorLike,
fullyReadBody,
readableStreamClose,
isomorphicEncode
isomorphicEncode,
urlIsLocal,
urlIsHttpHttpsScheme,
urlHasHttpsScheme
} = require('./util')
const { kState, kHeaders, kGuard, kRealm, kHeadersCaseInsensitive } = require('./symbols')
const assert = require('assert')
Expand Down Expand Up @@ -272,7 +275,7 @@ function finalizeAndReportTiming (response, initiatorType = 'other') {
let cacheState = response.cacheState

// 6. If originalURL’s scheme is not an HTTP(S) scheme, then return.
if (!/^https?:/.test(originalURL.protocol)) {
if (!urlIsHttpHttpsScheme(originalURL)) {
return
}

Expand Down Expand Up @@ -530,10 +533,7 @@ async function mainFetch (fetchParams, recursive = false) {

// 3. If request’s local-URLs-only flag is set and request’s current URL is
// not local, then set response to a network error.
if (
request.localURLsOnly &&
!/^(about|blob|data):/.test(requestCurrentURL(request).protocol)
) {
if (request.localURLsOnly && !urlIsLocal(requestCurrentURL(request))) {
response = makeNetworkError('local URLs only')
}

Expand Down Expand Up @@ -623,7 +623,7 @@ async function mainFetch (fetchParams, recursive = false) {
}

// request’s current URL’s scheme is not an HTTP(S) scheme
if (!/^https?:/.test(requestCurrentURL(request).protocol)) {
if (!urlIsHttpHttpsScheme(requestCurrentURL(request))) {
// Return a network error.
return makeNetworkError('URL scheme must be a HTTP(S) scheme')
}
Expand Down Expand Up @@ -1130,7 +1130,7 @@ async function httpRedirectFetch (fetchParams, response) {

// 6. If locationURL’s scheme is not an HTTP(S) scheme, then return a network
// error.
if (!/^https?:/.test(locationURL.protocol)) {
if (!urlIsHttpHttpsScheme(locationURL)) {
return makeNetworkError('URL scheme must be a HTTP(S) scheme')
}

Expand Down Expand Up @@ -1399,7 +1399,7 @@ async function httpNetworkOrCacheFetch (
// header if httpRequest’s header list contains that header’s name.
// TODO: https://github.com/whatwg/fetch/issues/1285#issuecomment-896560129
if (!httpRequest.headersList.contains('accept-encoding')) {
if (/^https:/.test(requestCurrentURL(httpRequest).protocol)) {
if (urlHasHttpsScheme(requestCurrentURL(httpRequest))) {
httpRequest.headersList.append('accept-encoding', 'br, gzip, deflate')
} else {
httpRequest.headersList.append('accept-encoding', 'gzip, deflate')
Expand Down
44 changes: 41 additions & 3 deletions lib/fetch/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ function requestBadPort (request) {

// 2. If url’s scheme is an HTTP(S) scheme and url’s port is a bad port,
// then return blocked.
if (/^https?:/.test(url.protocol) && badPorts.includes(url.port)) {
if (urlIsHttpHttpsScheme(url) && badPorts.includes(url.port)) {
return 'blocked'
}

Expand Down Expand Up @@ -285,7 +285,7 @@ function appendRequestOriginHeader (request) {
case 'strict-origin':
case 'strict-origin-when-cross-origin':
// If request’s origin is a tuple origin, its scheme is "https", and request’s current URL’s scheme is not "https", then set serializedOrigin to `null`.
if (/^https:/.test(request.origin) && !/^https:/.test(requestCurrentURL(request))) {
if (request.origin && urlHasHttpsScheme(request.origin) && !urlHasHttpsScheme(requestCurrentURL(request))) {
serializedOrigin = null
}
break
Expand Down Expand Up @@ -944,6 +944,41 @@ async function readAllBytes (reader, successSteps, failureSteps) {
}
}

/**
* @see https://fetch.spec.whatwg.org/#is-local
* @param {URL} url
*/
function urlIsLocal (url) {
assert('protocol' in url) // ensure it's a url object

const protocol = url.protocol

return protocol === 'about:' || protocol === 'blob:' || protocol === 'data:'
}

/**
* @param {string|URL} url
*/
function urlHasHttpsScheme (url) {
if (typeof url === 'string') {
return url.startsWith('https:')
}

return url.protocol === 'https:'
}

/**
* @see https://fetch.spec.whatwg.org/#http-scheme
* @param {URL} url
*/
function urlIsHttpHttpsScheme (url) {
assert('protocol' in url) // ensure it's a url object

const protocol = url.protocol

return protocol === 'http:' || protocol === 'https:'
}

/**
* Fetch supports node >= 16.8.0, but Object.hasOwn was added in v16.9.0.
*/
Expand Down Expand Up @@ -988,5 +1023,8 @@ module.exports = {
isReadableStreamLike,
readableStreamClose,
isomorphicEncode,
isomorphicDecode
isomorphicDecode,
urlIsLocal,
urlHasHttpsScheme,
urlIsHttpHttpsScheme
}