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
nodejs · May 30, 2022 · bf6d5a1 · bf6d5a1
1 parent 0e64274
commit bf6d5a1
Show file tree

Hide file tree

Showing 7 changed files with 78 additions and 185 deletions.
diff --git 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
@@ -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,20 +60,15 @@ const subresource = [
   ''
 ]
 
-const corsSafeListedResponseHeaderNames = [] // TODO
-
 module.exports = {
   subresource,
-  forbiddenResponseHeaderNames,
-  corsSafeListedResponseHeaderNames,
   forbiddenMethods,
   requestBodyHeader,
   referrerPolicy,
   requestRedirect,
   requestMode,
   requestCredentials,
   requestCache,
-  forbiddenHeaderNames,
   redirectStatus,
   corsSafeListedMethods,
   nullBodyStatus,

diff --git 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
@@ -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,26 +406,25 @@ 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][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
@@ -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,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
@@ -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
@@ -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()
+})