Skip to content

Commit

Permalink
fix(fetch): send headers in the case that they were sent (nodejs#1784)
Browse files Browse the repository at this point in the history
* fix(fetch): treat headers as case sensitive

* fix: mark test as flaky
  • Loading branch information
KhafraDev authored and anonrig committed Apr 4, 2023
1 parent c4c0636 commit 7a23f81
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 18 deletions.
32 changes: 22 additions & 10 deletions lib/fetch/headers.js
Expand Up @@ -3,7 +3,7 @@
'use strict'

const { kHeadersList } = require('../core/symbols')
const { kGuard } = require('./symbols')
const { kGuard, kHeadersCaseInsensitive } = require('./symbols')
const { kEnumerableProperty } = require('../core/util')
const {
makeIterator,
Expand Down Expand Up @@ -96,27 +96,27 @@ class HeadersList {

// 1. If list contains name, then set name to the first such
// header’s name.
name = name.toLowerCase()
const exists = this[kHeadersMap].get(name)
const lowercaseName = name.toLowerCase()
const exists = this[kHeadersMap].get(lowercaseName)

// 2. Append (name, value) to list.
if (exists) {
this[kHeadersMap].set(name, `${exists}, ${value}`)
this[kHeadersMap].set(lowercaseName, { name: exists.name, value: `${exists.value}, ${value}` })
} else {
this[kHeadersMap].set(name, `${value}`)
this[kHeadersMap].set(lowercaseName, { name, value })
}
}

// https://fetch.spec.whatwg.org/#concept-header-list-set
set (name, value) {
this[kHeadersSortedMap] = null
name = name.toLowerCase()
const lowercaseName = name.toLowerCase()

// 1. If list contains name, then set the value of
// the first such header to value and remove the
// others.
// 2. Otherwise, append header (name, value) to list.
return this[kHeadersMap].set(name, value)
return this[kHeadersMap].set(lowercaseName, { name, value })
}

// https://fetch.spec.whatwg.org/#concept-header-list-delete
Expand All @@ -137,14 +137,26 @@ class HeadersList {
// 2. Return the values of all headers in list whose name
// is a byte-case-insensitive match for name,
// separated from each other by 0x2C 0x20, in order.
return this[kHeadersMap].get(name.toLowerCase()) ?? null
return this[kHeadersMap].get(name.toLowerCase())?.value ?? null
}

* [Symbol.iterator] () {
for (const pair of this[kHeadersMap]) {
yield pair
// use the lowercased name
for (const [name, { value }] of this[kHeadersMap]) {
yield [name, value]
}
}

get [kHeadersCaseInsensitive] () {
/** @type {string[]} */
const flatList = []

for (const { name, value } of this[kHeadersMap].values()) {
flatList.push(name, value)
}

return flatList
}
}

// https://fetch.spec.whatwg.org/#headers-class
Expand Down
10 changes: 5 additions & 5 deletions lib/fetch/index.js
Expand Up @@ -39,7 +39,7 @@ const {
readableStreamClose,
isomorphicEncode
} = require('./util')
const { kState, kHeaders, kGuard, kRealm } = require('./symbols')
const { kState, kHeaders, kGuard, kRealm, kHeadersCaseInsensitive } = require('./symbols')
const assert = require('assert')
const { safelyExtractBody } = require('./body')
const {
Expand Down Expand Up @@ -843,8 +843,8 @@ async function schemeFetch (fetchParams) {
const response = makeResponse({
statusText: 'OK',
headersList: [
['content-length', length],
['content-type', type]
['content-length', { name: 'Content-Length', value: length }],
['content-type', { name: 'Content-Type', value: type }]
]
})

Expand Down Expand Up @@ -873,7 +873,7 @@ async function schemeFetch (fetchParams) {
return makeResponse({
statusText: 'OK',
headersList: [
['content-type', mimeType]
['content-type', { name: 'Content-Type', value: mimeType }]
],
body: safelyExtractBody(dataURLStruct.body)[0]
})
Expand Down Expand Up @@ -1941,7 +1941,7 @@ async function httpNetworkFetch (
origin: url.origin,
method: request.method,
body: fetchParams.controller.dispatcher.isMockActive ? request.body && request.body.source : body,
headers: [...request.headersList].flat(),
headers: request.headersList[kHeadersCaseInsensitive],
maxRedirections: 0,
bodyTimeout: 300_000,
headersTimeout: 300_000
Expand Down
3 changes: 2 additions & 1 deletion lib/fetch/symbols.js
Expand Up @@ -6,5 +6,6 @@ module.exports = {
kSignal: Symbol('signal'),
kState: Symbol('state'),
kGuard: Symbol('guard'),
kRealm: Symbol('realm')
kRealm: Symbol('realm'),
kHeadersCaseInsensitive: Symbol('headers case insensitive')
}
17 changes: 17 additions & 0 deletions test/wpt/server/server.mjs
Expand Up @@ -316,6 +316,23 @@ const server = createServer(async (req, res) => {
res.end('none')
break
}
case '/xhr/resources/echo-headers.py': {
res.statusCode = 200
res.setHeader('Content-Type', 'text/plain')

// wpt runner sends this as 1 chunk
let body = ''

for (let i = 0; i < req.rawHeaders.length; i += 2) {
const key = req.rawHeaders[i]
const value = req.rawHeaders[i + 1]

body += `${key}: ${value}`
}

res.end(body)
break
}
default: {
res.statusCode = 200
res.end('body')
Expand Down
4 changes: 2 additions & 2 deletions test/wpt/status/fetch.status.json
Expand Up @@ -49,12 +49,12 @@
"header-value-combining.any.js": {
"fail": [
"response.headers.get('content-length') expects 0, 0",
"response.headers.get('double-trouble') expects , ",
"response.headers.get('foo-test') expects 1, 2, 3",
"response.headers.get('heya') expects , \\x0B\f, 1, , , 2"
],
"flaky": [
"response.headers.get('content-length') expects 0",
"response.headers.get('double-trouble') expects , ",
"response.headers.get('www-authenticate') expects 1, 2, 3, 4"
]
},
Expand Down Expand Up @@ -207,4 +207,4 @@
"fetch() with value %1F"
]
}
}
}
13 changes: 13 additions & 0 deletions test/wpt/tests/fetch/api/basic/request-headers-case.any.js
@@ -0,0 +1,13 @@
// META: global=window,worker

promise_test(() => {
return fetch("/xhr/resources/echo-headers.py", {headers: [["THIS-is-A-test", 1], ["THIS-IS-A-TEST", 2]] }).then(res => res.text()).then(body => {
assert_regexp_match(body, /THIS-is-A-test: 1, 2/)
})
}, "Multiple headers with the same name, different case (THIS-is-A-test first)")

promise_test(() => {
return fetch("/xhr/resources/echo-headers.py", {headers: [["THIS-IS-A-TEST", 1], ["THIS-is-A-test", 2]] }).then(res => res.text()).then(body => {
assert_regexp_match(body, /THIS-IS-A-TEST: 1, 2/)
})
}, "Multiple headers with the same name, different case (THIS-IS-A-TEST first)")

0 comments on commit 7a23f81

Please sign in to comment.