(fix): Make request.get_headers fully case insensitive

[dmmulroy]

The current implementation of request.get_headers fails in situation where the Header key(s) is/are not lowercase. If you construct a Request object and supply the headers to the constructor directly, the header keys are never lowercased. #53 would add additional api's to help prevent this in most cases by guiding users to use the builder pattern instead.

However this PR changes get_headers to additionally lowercase each headers key while searching.

In addition, I added test coverage for this that validates it failing in main and it passing with this pr/change.

[lpil]

Thank you, but header names must always be lowercased when using gleam_http.

I'll do a quick once over of the docs to see if we can make that clearer.

[dmmulroy]

@lpil good call on updating the docs, I think this PR and #53 will go a long way to preventing this kind of mistake in the common case without needing to refer to docs.

[lpil]

Only lowercase header names are valid, so this PR would make it less clear what the contract of this package is while doing additional needless work.

If we were to accept this we would need to make all operations on headers case insensitive, which would require an opaque type with a smart constructor. This would be a large breaking change and mean pattern matching can no longer be used. I don't think this is a worthwhile trade.

[dmmulroy]

I don't understand why we would need to make the constructor opaque? Why would operations on the header key require that if we always string.lowercase the search key/needle + each header when searching during access to the header list? Is the aim of this library to be compliant with RFC 7230, because if so, headers are noted to be case insensitive https://datatracker.ietf.org/doc/html/rfc7230#section-3.2

[lpil]

Because that's only one of the countless ways one can interact with headers. If we add that here we force the programmer to themselves lowercase every key they interact with as the contract then states they can be either.

This library does not aim to be as close to that document as possible, it instead abstracts over all the HTTP versions in a way that is practical for Gleam.

In HTTP1 header keys are case insensitive. In HTTP2 and HTTP3 they have to be lowercase. Gleam doesn't have case insensitive strings and uppercase violates HTTP2/3 while being more verbose for the programmer, so lowercase is the best option.

[dmmulroy]

In HTTP2 and HTTP3 they have to be lowercase.

Ahhh, TIL. That's the bit I was missing - I haven't had to do much work wrt to http2/http3.

Because that's only one of the countless ways one can interact with headers. If we add that here we force the programmer to themselves lowercase every key they interact with as the contract then states they can be either.

I see what you're saying, and now I see why you would need (want?) an opaque type. That being said, without a major version bump and moving to opaque types/breaking the API I think we're talking about two sides of the same coin.

If we add that here we force the programmer to themselves lowercase every key they interact with as the contract then states they can be either.

Is this not already the case? For libraries/code constructing Requests, there is an expectation/rule only learnable and enforced in documentation, that they must lowercase headers prior to creating a new Request. For libraries/code consuming Requests, like httpc, there is the expectation/rule, that the requests passed in only use lowercase headers. But because that's not enforced or validated, I'd argue that for both sets of programmers/code they still need to lowercase ever key they interact with because the only contract is trust.

That's why I think something like this is more favorable because it at least buys the ability/opportunity for some certainty/guarantees for the libraries/code consuming Requests as long as they use the module functions headers will be lowercase.

Hopefully my tone here is not critical, negative, or aggressive - I very much am asking and discussing in good faith and as always appreciate your time, work, and feedback!

[lpil]

It's not the case today because it's only adding headers (which is typically done with a string literal or by the web server or client itself) that need to be thought about and done in some precise way, while all work that manipulates headers can depend on the format being lowercase.

There's a precise contract for this type so we will not write any code that violates the contract.

[dmmulroy]

I appreciate your stance and time. I fundamentally disagree but I can understand your stance and why you're sticking to it. I'll close both of these PRs as I don't think I'll persuade you hahah

My last thoughts; Gleam's ethos, at least to me, seems to be along the lines of "Be practical, be friendly, do things one way, do things the simple way, and help everyone avoid mistakes".

To me, the current contract/api for this type is at odds with that ethos. Asking me as a user to always remember to lowercase headers prior to using the Request type, or its module, else consumers will fail silently feels bad, and being the forgetful human that I am, I will make this mistake more than once. I feel that this is similar to saying "Hey this API takes Ints, but only the number 7 - but that's on you to make sure you only pass an int with the value 7" and given the power of our type system it feels like we could do better.