From 25b6674dd28156d8420fbb2eb93ebe5e3df679c4 Mon Sep 17 00:00:00 2001 From: Khafra <42794878+KhafraDev@users.noreply.github.com> Date: Sun, 29 May 2022 21:44:43 -0400 Subject: [PATCH] feat: remove headers filtering (#1469) * feat: remove headers filtering * add wintercg issue to README * update README * fix(Headers): properly use/set "this's headers" * fix: remove broken guard checks --- README.md | 9 +++++ lib/fetch/constants.js | 31 ----------------- lib/fetch/headers.js | 46 +++++------------------- lib/fetch/request.js | 11 +++--- lib/fetch/response.js | 37 ++++---------------- test/fetch/cookies.js | 50 ++++++++++++++++++++++++++ test/fetch/headers.js | 79 ------------------------------------------ 7 files changed, 78 insertions(+), 185 deletions(-) create mode 100644 test/fetch/cookies.js diff --git a/README.md b/README.md index b7a5ef1acd4..c8dc260e12a 100644 --- a/README.md +++ b/README.md @@ -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. + ### `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. diff --git a/lib/fetch/constants.js b/lib/fetch/constants.js index 75c9265e8c1..4358cd3ae08 100644 --- a/lib/fetch/constants.js +++ b/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] @@ -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', @@ -86,12 +60,8 @@ const subresource = [ '' ] -const corsSafeListedResponseHeaderNames = [] // TODO - module.exports = { subresource, - forbiddenResponseHeaderNames, - corsSafeListedResponseHeaderNames, forbiddenMethods, requestBodyHeader, referrerPolicy, @@ -99,7 +69,6 @@ module.exports = { requestMode, requestCredentials, requestCache, - forbiddenHeaderNames, redirectStatus, corsSafeListedMethods, nullBodyStatus, diff --git a/lib/fetch/headers.js b/lib/fetch/headers.js index eb4eb47c4ec..0c6e5584d33 100644 --- a/lib/fetch/headers.js +++ b/lib/fetch/headers.js @@ -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') @@ -115,6 +111,11 @@ class HeadersList { } } + clear () { + this[kHeadersMap].clear() + this[kHeadersSortedMap] = null + } + append (name, value) { this[kHeadersSortedMap] = null @@ -211,22 +212,11 @@ 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) - ) { - return } else if (this[kGuard] === 'request-no-cors') { // TODO - } else if ( - this[kGuard] === 'response' && - forbiddenResponseHeaderNames.includes(normalizedName) - ) { - return } return this[kHeadersList].append(String(name), String(value)) @@ -244,22 +234,11 @@ 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) - ) { - return } else if (this[kGuard] === 'request-no-cors') { // TODO - } else if ( - this[kGuard] === 'response' && - forbiddenResponseHeaderNames.includes(normalizedName) - ) { - return } return this[kHeadersList].delete(String(name)) @@ -307,20 +286,11 @@ 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()) - ) { - return } else if (this[kGuard] === 'request-no-cors') { // TODO - } else if ( - this[kGuard] === 'response' && - forbiddenResponseHeaderNames.includes(String(name).toLocaleLowerCase()) - ) { - return } return this[kHeadersList].set(String(name), String(value)) diff --git a/lib/fetch/request.js b/lib/fetch/request.js index 661f5814ba2..42c7eb447a2 100644 --- a/lib/fetch/request.js +++ b/lib/fetch/request.js @@ -384,8 +384,8 @@ class Request { // Realm, whose header list is request’s header list and guard is // "request". this[kHeaders] = new Headers() - this[kHeaders][kGuard] = 'request' this[kHeaders][kHeadersList] = request.headersList + this[kHeaders][kGuard] = 'request' this[kHeaders][kRealm] = this[kRealm] // 31. If this’s request’s mode is "no-cors", then: @@ -406,7 +406,7 @@ 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) { @@ -414,18 +414,17 @@ class Request { } // 3. Empty this’s headers’s header list. - this[kState].headersList = new HeadersList() - this[kHeaders][kHeadersList] = this[kState].headersList + this[kHeaders][kHeadersList].clear() // 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) { this[kHeaders].append(key, val) } } else { // 5. Otherwise, fill this’s headers with headers. - fillHeaders(this[kState].headersList, headers) + fillHeaders(this[kHeaders], headers) } } diff --git a/lib/fetch/response.js b/lib/fetch/response.js index 03db7eb488e..582876e050a 100644 --- a/lib/fetch/response.js +++ b/lib/fetch/response.js @@ -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') @@ -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 @@ -411,12 +387,10 @@ 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" @@ -424,9 +398,10 @@ function filterResponse (response, type) { // 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 @@ -449,7 +424,7 @@ function filterResponse (response, type) { type: 'opaqueredirect', status: 0, statusText: '', - headersList: makeFilteredHeadersList(response.headersList, () => false), + headersList: [], body: null }) } else { diff --git a/test/fetch/cookies.js b/test/fetch/cookies.js new file mode 100644 index 00000000000..d17a8827df5 --- /dev/null +++ b/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() +}) diff --git a/test/fetch/headers.js b/test/fetch/headers.js index f84c6c23839..05abc4339cf 100644 --- a/test/fetch/headers.js +++ b/test/fetch/headers.js @@ -8,12 +8,6 @@ const { fill } = require('../../lib/fetch/headers') const { kGuard } = require('../../lib/fetch/symbols') -const { - forbiddenHeaderNames, - forbiddenResponseHeaderNames -} = require('../../lib/fetch/constants') -const { createServer } = require('http') -const { fetch } = require('../../index') tap.test('Headers initialization', t => { t.plan(7) @@ -661,76 +655,3 @@ tap.test('request-no-cors guard', (t) => { t.doesNotThrow(() => { headers.delete('key') }) t.end() }) - -tap.test('request guard', (t) => { - const headers = new Headers(forbiddenHeaderNames.map(k => [k, 'v'])) - headers[kGuard] = 'request' - headers.set('set-cookie', 'val') - - for (const name of forbiddenHeaderNames) { - headers.set(name, '1') - headers.append(name, '1') - t.equal(headers.get(name), 'v') - headers.delete(name) - t.equal(headers.has(name), true) - } - - t.equal(headers.get('set-cookie'), 'val') - t.equal(headers.has('set-cookie'), true) - - t.end() -}) - -tap.test('response guard', (t) => { - const headers = new Headers(forbiddenResponseHeaderNames.map(k => [k, 'v'])) - headers[kGuard] = 'response' - headers.set('key', 'val') - headers.set('keep-alive', 'val') - - for (const name of forbiddenResponseHeaderNames) { - headers.set(name, '1') - headers.append(name, '1') - t.equal(headers.get(name), 'v') - headers.delete(name) - t.equal(headers.has(name), true) - } - - t.equal(headers.get('keep-alive'), 'val') - t.equal(headers.has('keep-alive'), true) - - t.end() -}) - -tap.test('set-cookie[2] in Headers constructor', (t) => { - const headers = new Headers(forbiddenResponseHeaderNames.map(k => [k, 'v'])) - - for (const header of forbiddenResponseHeaderNames) { - t.ok(headers.has(header)) - t.equal(headers.get(header), 'v') - } - - t.end() -}) - -// https://github.com/nodejs/undici/issues/1328 -tap.test('set-cookie[2] received from server - issue #1328', (t) => { - const server = createServer((req, res) => { - res.setHeader('set-cookie', 'my-cookie; wow') - res.end('Goodbye!') - }).unref() - t.teardown(server.close.bind(server)) - - server.listen(0, async () => { - const { headers } = await fetch(`http://localhost:${server.address().port}`) - - t.notOk(headers.has('set-cookie')) - t.notOk(headers.has('Set-cookie')) - t.notOk(headers.has('sEt-CoOkIe')) - - t.equal(headers.get('set-cookie'), null) - t.equal(headers.get('Set-cookie'), null) - t.equal(headers.get('sEt-CoOkIe'), null) - - t.end() - }) -})