fix: lowercase type-providers headers types

[toomuchdesign]

As far as I understand, Fastify converts request headers to lowercase, while Fastify type provider seems to pass down to the type provider libraries the headers with the same casing provided in the route handler schemas.

Here is an example:

fastify.get(
  "/",
  {
    schema: {
      headers: {
        type: "object",
        properties: {
          lowercase: { type: "string" },
          UPPERCASE: { type: "number" },
        },
        required: ["lowercase", "UPPERCASE"],
      } as const,
    },
  },
  (req) => {
    req.headers.lowercase; // Correct types and runtime value
    req.headers.UPPERCASE; // Gets schema types but it's actually undefined and type should be, too
    req.headers.uppercase; // Gets generic headers types but it actually holds the actual request header value
  }
);

This PR is an attempt to align Fastify type provider types with the actual Fastify implementation. Where:

  (req) => {
    req.headers.lowercase; // Correct types and runtime value
    req.headers.UPPERCASE; // Gets schema generic headers types
    req.headers.uppercase; // Gets schema-defined types
  }

Technically speaking we might maybe considering typing the original uppercase version of the header as undefined but it would be probably too much :)

The case conversion happens on the types returned by the type provider libraries, therefore it should be transparent to them.

Open points

• Is there any exception worth considering in Fastify headers lowercase conversion?
• Where I'm supposed to add tests for this type change (couldn't find relevant type tests)
• Should we consider this a fix or a breaking change?

Happy to review this PR based on your suggestions!
Thank you!

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[mcollina]

Thanks for opening a PR! Can you please add a unit test? We use tsd for types.

[toomuchdesign]

Hi @mcollina! Of course, I've just added a few test cases targeting request headers types.

Extra point

I've realised that user provided route schemas can set headers value to any value even though headers can only be of type string | string[] | undefined.

Shall we consider tackling this issue in another PR to try and set request header types different then string | string[] | undefined to never?

Cheers!

[mcollina]

lgtm

[mcollina]

Shall we consider tackling this issue in another PR to try and set request header types different then string | string[] | undefined to never?

I'm not sure I understand this, maybe open an issue first, or you can do a PR and talk there.

[Uzlopak]

Yeah the last point can be an issue, as there imho is no casting in place. The values will be imho just tested if they pass the schema validation.

So if you type a header as number you can expect that the value can be casted to a number, but is still a string.

Well that is what i know so far.

[toomuchdesign]

Yep, good point @Uzlopak :)

[Uzlopak]

@mcollina

Ajv will internally cast it to a number. If it can be casted it will pass. Fastify sets an option of ajv by default to cast the values. But they are only casted for the validation. It will not pass the casted values to the request. So there will be issues...

[mcollina]

Ah, seems like a bummer, how should we address that? Leave things as is?

[Eomm]

. It will not pass the casted values to the request. So there will be issues...

Nope:

const app = require('fastify')({ logger: true })
app.get('/', {
  schema: {
    headers: {
      type: 'object',
      properties: {
        custom: { type: 'number' }
      }
    }
  }
}, async (request, reply) => {
  return { hello: typeof request.headers.custom }
})

app.listen({ port: 8080 })

$ curl -X GET http://localhost:8080/ -H custom:42
{"hello":"number"}

[Uzlopak]

well, then this PR is ready for merging