fetch: don't re-lowercase HeadersList

[tsctx]

No description provided.

[mcollina]

lgtm

[KhafraDev]

No they're not, and if they are, they shouldn't be.

[KhafraDev]

Sorry, this should be reverted:

import { fetch, Headers } from './index.js'
import { createServer } from 'http'
import { once } from 'events'

const server = createServer((req, res) => {
  console.log(req.rawHeaders)
  // Actual: ['content-type', 'text/plain']
  // Expected: ['Content-Type', 'text/plain']

  res.end()
}).listen(0)

await once(server, 'listening')

const headers = new Headers([['Content-Type', 'text/plain']])

await fetch(`http://localhost:${server.address().port}`, {
  headers
}).finally(() => server.close())

[KhafraDev]

I spent a little time on this, and this Pr wasn't the issue, there's actually a much deeper issue due to us not following the spec regarding Headers.

The way that the webidl converter for HeadersInit works for any input V:

• If V is not a webidl Object (non-null), throw an error.
• If V has a Symbol.iterator, convert that to a sequence<sequence<ByteString>> (a 2d array)
• Otherwise, try converting it to a record<ByteString, ByteString> (an object)

The issue is in that second step, when V is a Headers object, which has a Symbol.iterator. The Headers' Symbol.iterator returns lowercased values only, basically the normalized versions, similar to IncomingHttpRequest.headers in Node. However, we need it to use HeadersList.entries, which returns the not-really-normalized-but-kinda-normalized object with case-sensitive Headers. But you can also override a Headers' Symbol.iterator with your own, which the webidl implementation is expected to use (this is tested for in the WPTs...).

We can fix it with the following diff, but as you can see, is pretty terrible:

diff --git a/lib/web/fetch/headers.js b/lib/web/fetch/headers.js
index 2235f125..7c91fc20 100644
--- a/lib/web/fetch/headers.js
+++ b/lib/web/fetch/headers.js
@@ -602,6 +602,10 @@ webidl.converters.HeadersInit = function (V) {
   if (webidl.util.Type(V) === 'Object') {
     const iterator = Reflect.get(V, Symbol.iterator)

+    if (!util.types.isProxy(V) && kHeadersList in V && iterator === Headers.prototype.entries) { // Headers object
+      return V[kHeadersList].entries
+    }
+
     if (typeof iterator === 'function') {
       return webidl.converters['sequence<sequence<ByteString>>'](V, iterator.bind(V))
     }

On the other hand, if we can't/have no intentions to support sending the raw header names to the server, we should probably drop support for it totally and mark the tests as failing. Deno sends all header names as lowercased, but it's the only platform that does so.

[KhafraDev]

This is a perfect example of why I am very stringent on PRs that don't follow or skip steps in the spec. If we followed the spec this wouldn't have been an issue. 😐

[tsctx]

@KhafraDev It can be reproduced without my changes. (This is a fetch-specific problem.)

[KhafraDev]

Yeah, that's what I was saying.

this Pr wasn't the issue, there's actually a much deeper issue due to us not following the spec regarding Headers.

I re-added your changes in 034e3f0 but it seems like when the PRs got squashed it removed the credit I gave to you.