Skip to content

Commit

Permalink
fetch: fix small spec inconsistency (#1158)
Browse files Browse the repository at this point in the history
This condition is not *yet* possible to meet so I couldn't add a test
(sorry!). The responseTainting is always set here or a network error is
returned (.status = 0 is a network error which makes the check never
pass):
https://github.com/nodejs/undici/blob/main/lib/fetch/index.js#L529 and
the condition is always met because response is never set
https://github.com/nodejs/undici/blob/main/lib/fetch/index.js#L475

The spec says that "[a] basic filtered response is a filtered response
whose type is "basic" and header list excludes any headers in internal
response’s header list whose name is a forbidden response-header name."
The library was incorrectly excluding valid headers.

If `data:`, `blob:`, `about:`, or `file:` URIs are ever supported, this
change will be needed.
  • Loading branch information
KhafraDev committed Jan 4, 2022
1 parent 007b17f commit 01302e6
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 2 deletions.
1 change: 1 addition & 0 deletions lib/fetch/constants.js
Expand Up @@ -58,6 +58,7 @@ const requestCache = [
'only-if-cached'
]

// https://fetch.spec.whatwg.org/#forbidden-response-header-name
const forbiddenResponseHeaderNames = ['set-cookie', 'set-cookie2']

const requestBodyHeader = [
Expand Down
5 changes: 3 additions & 2 deletions lib/fetch/response.js
Expand Up @@ -8,7 +8,7 @@ const { responseURL, isValidReasonPhrase, toUSVString } = require('./util')
const {
redirectStatus,
nullBodyStatus,
forbiddenHeaderNames
forbiddenResponseHeaderNames
} = require('./constants')
const { kState, kHeaders, kGuard, kRealm } = require('./symbols')
const { kHeadersList } = require('../core/symbols')
Expand Down Expand Up @@ -366,6 +366,7 @@ function makeNetworkError (reason) {
})
}

// https://fetch.spec.whatwg.org/#concept-filtered-response
function filterResponse (response, type) {
// Set response to the following filtered response with response as its
// internal response, depending on request’s response tainting:
Expand All @@ -376,7 +377,7 @@ function filterResponse (response, type) {

const headers = []
for (let n = 0; n < response.headersList.length; n += 2) {
if (!forbiddenHeaderNames.includes(response.headersList[n])) {
if (!forbiddenResponseHeaderNames.includes(response.headersList[n])) {
headers.push(response.headersList[n + 0], response.headersList[n + 1])
}
}
Expand Down

0 comments on commit 01302e6

Please sign in to comment.