Headers with leading underscore being rewritten to dashes

[peterc]

Similar to #317

I'm coding against a (popular) API that claims to support DELETE but which, sadly, does not. It requires you POST and set a _method header to POST instead.

Unfortunately, setting a "method" header results in HTTP::Headers.normalize_header changing the "" to a "-" as part of its normalization process and the header becomes "-Method" which doesn't work.

I believe the first line of normalize_header is in error to only return the existing header name if it is already in canonical form. I believe it should return if it is already compliant instead. Doing this resolves the issue in my case. That is, a simple change from:

return name if name =~ CANONICAL_NAME_RE

to

return name if name =~ COMPLIANT_NAME_RE

Could someone with more knowledge than me verify if my assertions above are correct? If so, I am more than happy to put in a pull request, and even more happy if someone just wants to make the change (or explain why this behavior is desirable, naturally) :-)

[tarcieri]

@peterc at least for responses, we try to canonicalize the headers so things building abstractions on top of this library can rely on the canonical name.

The same does not hold true of requests, however.

[peterc]

Good point, that makes sense for response headers, and the change I proposed may break some uses there.

I am not sure what this means for legitimate (if ugly and increasingly less common) request headers, though. In the interim, however, for anyone who runs into the same issue, I am using the following patch to silently protect leading-underscore headers:

module HTTP
  class Headers
    alias old_normalize_header normalize_header
    def normalize_header(name)
      return name if name.start_with?('_')
      old_normalize_header(name)
    end
  end
end

[ixti]

I would ask why non-standard field is not prefixed with X- as proposed in RFC, but I believe nobody knows ;)) Anyway, one of the main reasons behind "normalization" of headers is, as @tarcieri already pointed out making sure they are easily accessible from whatever stuff on top of http gem.

I agree that we probably should not normalize headers for requests. Alternatively, we can provide a blacklist of normalization instead.

[ixti]

I mean configurable blacklist of course.

[peterc]

Yeah, X-Http-Method-Override is a common (and far easier to deal with) alternative/synonym for _method but appears not to work with the API in question, alas :-( (The API is a financial one almost exclusively used from Java and .NET code, which might explain why I'm mostly discovering these things in the dark 😂)

Given this may be an edge case, I would definitely be totally happy with having a simple flag on HTTP::Headers#add that skipped the normalization, or something along those lines, if you chose not to allow full control over outgoing headers by default.

[ixti]

I'm totally fine with allowing request to have non-normalized headers, but that will most likley need api change. E.g. I know that lots of people do HTTP.headers(:content_encoding => "..."), or at least I do that 😆

Probably we can provide an extra api like:

HTTP.raw_headers("_method" => "DELETE")

that will serve this purpose, but I think it might worth adding a globally known ugly header names that should not be normalized. Or should be denormalized instead:

HTTP::Headers::DENORMALIZED = { "-Method" => "_METHOD" }

[ixti]

Both variants seems good to me, although I lean toward first one ;))

[ixti]

On the other hand, second variant is easier to achieve. :D

[ixti]

@httprb/core RFC

[tarcieri]

I would rather have a generic API than whitelisting people's errata.

@peterc this API really demands a _method header instead of just a param? Seems kinda wacky to me (although RFC 7230 makes it appear valid)

[tarcieri]

Looking at normalize_header, I'm curious why it converts _ to -

[ixti]

@tarcieri We conevrt _ to - in order to make it normalized :-P

headers(:content_length => "123") # Content-Length: 123
headers(:x_what_ever => "abc")    # X-What-Ever: abc

We can actually re-work headers class to store both normalized and non-normalized header names and a flag whenever that header should be sent raw or normalized. I like idea of extra API better as well.

[peterc]

@tarcieri I'll see if it works with a param, but after a cpl hours pulling my hair out the other night, I noticed in their own "API explorer", it was setting that header and using POST. When I replicated this using your library, it also failed, until I printed out exactly what was going out over the wire, then wrote my workaround. So yeah, not the world's best API on their part ;-) Separate to that, though, it is a "legal" HTTP header, if a horribly implemented and uncommon one, so I felt it was legitimate to raise here at least :-)

I have another idea to throw into the mix.

I totally understand why you'd want symbols normalizing back and forth, but what if a string were explicitly supplied? In that case, I'd expect it to remain verbatim. Could Symbol vs String determine whether normalization occurs, perhaps?

[zanker]

Having an API that adds something like raw_headers makes sense for me, since we can't really predict what people come up with (like this _method one). I'd like to stay away from type based APIs though personally, just because that can get confusing.

[ixti]

I also think that type based APIs might cause confusion.