Header casing

[delvedor]

The headers can be either Uppercased o lowercased, based on how they have been written. I think we should lowercase every key by default, to avoid confusion.

test('headers casing', (t) => {
  t.plan(4)

  const server = createServer((req, res) => {
    res.writeHead(200, {
      'content-type': 'text/plain',
      Foo: 'Bar'
    })
    res.end('hello')
  })
  t.tearDown(server.close.bind(server))

  server.listen(0, () => {
    const client = new Client(`http://localhost:${server.address().port}`)
    t.tearDown(client.close.bind(client))

    client.request({ path: '/', method: 'GET' }, (err, { statusCode, headers, body }) => {
      t.error(err)
      t.strictEqual(headers['content-type'], 'text/plain')
      t.strictEqual(headers.foo, 'Bar') // This fails
      const bufs = []
      body.on('data', (buf) => {
        bufs.push(buf)
      })
      body.on('end', () => {
        t.strictEqual('hello', Buffer.concat(bufs).toString('utf8'))
      })
    })
  })
})

[ronag]

This has performance implications and HTTP header handling should be case insensitive. I'm unsure.

[ronag]

Should this be an option?

[delvedor]

I agree that this might cause some performance slowdown, but if we don't lowercase all headers by default (in the response), then all of our users will need to log the headers before writing their code because they can't know in advance the casing of the headers.
Finally, I think we should behave in the same way as Node.js core, as I believe that keeping the code as-is, will bite us in the future.

[delvedor]

In fact, I've noticed this behavior because I was trying to port the current test in the Elasticsearch client to use undici, and every header check started failing.

[ronag]

So an option which is enabled by default?

[delvedor]

An option enabled by default could work, but I don't think anyone will ever change this, because it might save you a few milliseconds (did we measure how many?), but it adds a lot of complexity afterward, as you don't really know the casing of the headers.

[delvedor]

In Fastify we are lowercasing everything by default, and I don't think it causes a degradation in performances that justifies the complexity of supporting different solutions: https://github.com/fastify/fastify/blob/8b8485ac5ff56d1ead387f6dd76384f996a2cf8e/lib/reply.js#L162-L210

[delvedor]

I did some benchmarks:

With key.toLowerCase():

~/Development/undici  master ✗                                                        4h17m ⚑  ⍉
▶ node benchmarks/index.js
http - keepalive - pipe x 8,719 ops/sec ±7.94% (71 runs sampled)
undici - request - pipe x 11,290 ops/sec ±12.67% (73 runs sampled)
undici - pipeline - pipe x 9,305 ops/sec ±4.41% (75 runs sampled)
undici - stream - pipe x 12,794 ops/sec ±3.98% (76 runs sampled)

~/Development/undici  master ✗                                                         4h18m ⚑
▶ node benchmarks/index.js
http - keepalive - pipe x 9,167 ops/sec ±4.51% (72 runs sampled)
undici - request - pipe x 11,624 ops/sec ±6.34% (71 runs sampled)
undici - pipeline - pipe x 9,289 ops/sec ±10.42% (76 runs sampled)
undici - stream - pipe x 12,814 ops/sec ±4.25% (75 runs sampled)

Without key.toLowerCase():

~/Development/undici  master ✗                                                         4h18m ⚑
▶ node benchmarks/index.js
http - keepalive - pipe x 9,504 ops/sec ±4.29% (73 runs sampled)
undici - request - pipe x 11,948 ops/sec ±4.65% (71 runs sampled)
undici - pipeline - pipe x 9,391 ops/sec ±6.52% (75 runs sampled)
undici - stream - pipe x 13,161 ops/sec ±3.71% (80 runs sampled)

~/Development/undici  master ✗                                                         4h19m ⚑
▶ node benchmarks/index.js
http - keepalive - pipe x 9,254 ops/sec ±4.39% (74 runs sampled)
undici - request - pipe x 10,315 ops/sec ±13.93% (68 runs sampled)
undici - pipeline - pipe x 8,941 ops/sec ±7.72% (71 runs sampled)
undici - stream - pipe x 12,642 ops/sec ±4.01% (74 runs sampled)

The server code:

const { createServer } = require('http')

createServer((req, res) => {
  res.writeHead(200, {
    'content-type': 'text/plain',
    'X-Foo': 'Bar'
  })
  res.end('hello world')
}).listen(3000)

I don't see a sensible difference :)