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

feat: remove headers filtering #1469

Merged
merged 5 commits into from May 30, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions README.md
Expand Up @@ -288,6 +288,15 @@ const headers = await fetch(url)
.then(res => res.headers)
```

##### Forbidden and Safelisted Header Names

* https://fetch.spec.whatwg.org/#cors-safelisted-response-header-name
* https://fetch.spec.whatwg.org/#forbidden-header-name
* https://fetch.spec.whatwg.org/#forbidden-response-header-name
* https://github.com/wintercg/fetch/issues/6

The [Fetch Standard](https://fetch.spec.whatwg.org) requires implementations to exclude certain headers from requests and responses. In browser environments, some headers are forbidden so the user agent remains in full control over them. In Undici, these constraints are removed to give more control to the user.

KhafraDev marked this conversation as resolved.
Show resolved Hide resolved
### `undici.upgrade([url, options]): Promise`

Upgrade to a different protocol. See [MDN - HTTP - Protocol upgrade mechanism](https://developer.mozilla.org/en-US/docs/Web/HTTP/Protocol_upgrade_mechanism) for more details.
Expand Down
31 changes: 0 additions & 31 deletions lib/fetch/constants.js
@@ -1,28 +1,5 @@
'use strict'

const forbiddenHeaderNames = [
'accept-charset',
'accept-encoding',
'access-control-request-headers',
'access-control-request-method',
'connection',
'content-length',
'cookie',
'cookie2',
'date',
'dnt',
'expect',
'host',
'keep-alive',
'origin',
'referer',
'te',
'trailer',
'transfer-encoding',
'upgrade',
'via'
]

const corsSafeListedMethods = ['GET', 'HEAD', 'POST']

const nullBodyStatus = [101, 204, 205, 304]
Expand Down Expand Up @@ -58,9 +35,6 @@ const requestCache = [
'only-if-cached'
]

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

const requestBodyHeader = [
'content-encoding',
'content-language',
Expand All @@ -86,20 +60,15 @@ const subresource = [
''
]

const corsSafeListedResponseHeaderNames = [] // TODO

module.exports = {
subresource,
forbiddenResponseHeaderNames,
corsSafeListedResponseHeaderNames,
forbiddenMethods,
requestBodyHeader,
referrerPolicy,
requestRedirect,
requestMode,
requestCredentials,
requestCache,
forbiddenHeaderNames,
redirectStatus,
corsSafeListedMethods,
nullBodyStatus,
Expand Down
41 changes: 9 additions & 32 deletions lib/fetch/headers.js
Expand Up @@ -6,10 +6,6 @@ const { validateHeaderName, validateHeaderValue } = require('http')
const { kHeadersList } = require('../core/symbols')
const { kGuard } = require('./symbols')
const { kEnumerableProperty } = require('../core/util')
const {
forbiddenHeaderNames,
forbiddenResponseHeaderNames
} = require('./constants')

const kHeadersMap = Symbol('headers map')
const kHeadersSortedMap = Symbol('headers map sorted')
Expand Down Expand Up @@ -211,21 +207,14 @@ class Headers {
)
}

const normalizedName = normalizeAndValidateHeaderName(String(name))

// Note: undici does not implement forbidden header names
if (this[kGuard] === 'immutable') {
throw new TypeError('immutable')
} else if (
this[kGuard] === 'request' &&
forbiddenHeaderNames.includes(normalizedName)
) {
} else if (this[kGuard] === 'request') {
return
} else if (this[kGuard] === 'request-no-cors') {
// TODO
} else if (
this[kGuard] === 'response' &&
forbiddenResponseHeaderNames.includes(normalizedName)
) {
} else if (this[kGuard] === 'response') {
return
}

Expand All @@ -244,21 +233,14 @@ class Headers {
)
}

const normalizedName = normalizeAndValidateHeaderName(String(name))

// Note: undici does not implement forbidden header names
if (this[kGuard] === 'immutable') {
throw new TypeError('immutable')
} else if (
this[kGuard] === 'request' &&
forbiddenHeaderNames.includes(normalizedName)
) {
} else if (this[kGuard] === 'request') {
return
} else if (this[kGuard] === 'request-no-cors') {
// TODO
} else if (
this[kGuard] === 'response' &&
forbiddenResponseHeaderNames.includes(normalizedName)
) {
} else if (this[kGuard] === 'response') {
return
}

Expand Down Expand Up @@ -307,19 +289,14 @@ class Headers {
)
}

// Note: undici does not implement forbidden header names
if (this[kGuard] === 'immutable') {
throw new TypeError('immutable')
} else if (
this[kGuard] === 'request' &&
forbiddenHeaderNames.includes(String(name).toLocaleLowerCase())
) {
} else if (this[kGuard] === 'request') {
return
} else if (this[kGuard] === 'request-no-cors') {
// TODO
} else if (
this[kGuard] === 'response' &&
forbiddenResponseHeaderNames.includes(String(name).toLocaleLowerCase())
) {
} else if (this[kGuard] === 'response') {
return
}

Expand Down
14 changes: 7 additions & 7 deletions lib/fetch/request.js
Expand Up @@ -383,10 +383,10 @@ class Request {
// 30. Set this’s headers to a new Headers object with this’s relevant
// Realm, whose header list is request’s header list and guard is
// "request".
this[kHeaders] = new Headers()
this[kHeaders] = new Headers(request.headersList)
KhafraDev marked this conversation as resolved.
Show resolved Hide resolved
this[kHeaders][kGuard] = 'request'
this[kHeaders][kHeadersList] = request.headersList
this[kHeaders][kRealm] = this[kRealm]
this[kState].headersList = this[kHeaders][kHeadersList]

// 31. If this’s request’s mode is "no-cors", then:
if (mode === 'no-cors') {
Expand All @@ -406,26 +406,26 @@ class Request {
if (Object.keys(init).length !== 0) {
// 1. Let headers be a copy of this’s headers and its associated header
// list.
let headers = new Headers(this.headers)
let headers = new Headers(this[kHeaders])

// 2. If init["headers"] exists, then set headers to init["headers"].
if (init.headers !== undefined) {
headers = init.headers
}

// 3. Empty this’s headers’s header list.
this[kState].headersList = new HeadersList()
this[kHeaders][kHeadersList] = this[kState].headersList
this[kHeaders] = new Headers()
this[kState].headersList = this[kHeaders][kHeadersList]
KhafraDev marked this conversation as resolved.
Show resolved Hide resolved

// 4. If headers is a Headers object, then for each header in its header
// list, append header’s name/header’s value to this’s headers.
if (headers.constructor.name === 'Headers') {
for (const [key, val] of headers[kHeadersList] || headers) {
for (const [key, val] of headers) {
KhafraDev marked this conversation as resolved.
Show resolved Hide resolved
this[kHeaders].append(key, val)
}
} else {
// 5. Otherwise, fill this’s headers with headers.
fillHeaders(this[kState].headersList, headers)
fillHeaders(this[kHeaders], headers)
}
}

Expand Down
37 changes: 6 additions & 31 deletions lib/fetch/response.js
Expand Up @@ -8,9 +8,7 @@ const { kEnumerableProperty } = util
const { responseURL, isValidReasonPhrase, toUSVString, isCancelled, isAborted, serializeJavascriptValueToJSONString } = require('./util')
const {
redirectStatus,
nullBodyStatus,
forbiddenResponseHeaderNames,
corsSafeListedResponseHeaderNames
nullBodyStatus
} = require('./constants')
const { kState, kHeaders, kGuard, kRealm } = require('./symbols')
const { kHeadersList } = require('../core/symbols')
Expand Down Expand Up @@ -380,28 +378,6 @@ function makeFilteredResponse (response, state) {
})
}

function makeFilteredHeadersList (headersList, filter) {
return new Proxy(headersList, {
get (target, prop) {
// Override methods used by Headers class.
if (prop === 'get' || prop === 'has') {
const defaultReturn = prop === 'has' ? false : null
return (name) => filter(name) ? target[prop](name) : defaultReturn
} else if (prop === Symbol.iterator) {
return function * () {
for (const entry of target) {
if (filter(entry[0])) {
yield entry
}
}
}
} else {
return target[prop]
}
}
})
}

// https://fetch.spec.whatwg.org/#concept-filtered-response
function filterResponse (response, type) {
// Set response to the following filtered response with response as its
Expand All @@ -411,22 +387,21 @@ function filterResponse (response, type) {
// and header list excludes any headers in internal response’s header list
// whose name is a forbidden response-header name.

// Note: undici does not implement forbidden response-header names
return makeFilteredResponse(response, {
type: 'basic',
headersList: makeFilteredHeadersList(
response.headersList,
(name) => !forbiddenResponseHeaderNames.includes(name.toLowerCase())
)
headersList: response.headersList
})
} else if (type === 'cors') {
// A CORS filtered response is a filtered response whose type is "cors"
// and header list excludes any headers in internal response’s header
// list whose name is not a CORS-safelisted response-header name, given
// internal response’s CORS-exposed header-name list.

// Note: undici does not implement CORS-safelisted response-header names
return makeFilteredResponse(response, {
type: 'cors',
headersList: makeFilteredHeadersList(response.headersList, (name) => !corsSafeListedResponseHeaderNames.includes(name))
headersList: response.headersList
})
} else if (type === 'opaque') {
// An opaque filtered response is a filtered response whose type is
Expand All @@ -449,7 +424,7 @@ function filterResponse (response, type) {
type: 'opaqueredirect',
status: 0,
statusText: '',
headersList: makeFilteredHeadersList(response.headersList, () => false),
headersList: [],
body: null
})
} else {
Expand Down
50 changes: 50 additions & 0 deletions test/fetch/cookies.js
@@ -0,0 +1,50 @@
'use strict'

const { once } = require('events')
const { createServer } = require('http')
const { test } = require('tap')
const { fetch, Headers } = require('../..')

test('Can receive set-cookie headers from a server using fetch - issue #1262', async (t) => {
const server = createServer((req, res) => {
res.setHeader('set-cookie', 'name=value; Domain=example.com')
res.end()
}).listen(0)

t.teardown(server.close.bind(server))
await once(server, 'listening')

const response = await fetch(`http://localhost:${server.address().port}`)

t.equal(response.headers.get('set-cookie'), 'name=value; Domain=example.com')

const response2 = await fetch(`http://localhost:${server.address().port}`, {
credentials: 'include'
})

t.equal(response2.headers.get('set-cookie'), 'name=value; Domain=example.com')

t.end()
})

test('Can send cookies to a server with fetch - issue #1463', async (t) => {
const server = createServer((req, res) => {
t.equal(req.headers.cookie, 'value')
res.end()
}).listen(0)

t.teardown(server.close.bind(server))
await once(server, 'listening')

const headersInit = [
new Headers([['cookie', 'value']]),
{ cookie: 'value' },
[['cookie', 'value']]
]

for (const headers of headersInit) {
await fetch(`http://localhost:${server.address().port}`, { headers })
}

t.end()
})