Headers with leading underscore being rewritten to dashes

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) :-)

@peterc

@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.

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

@tarcieri

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.

I mean configurable blacklist of course.

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.

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" }

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

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

@httprb/core RFC

@peterc

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)

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

@tarcieri

@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.

@tarcieri

@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?

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.

I also think that type based APIs might cause confusion.

@tarcieri

@tarcieri Perhaps I should phrase it as "only adding a #raw_headers option will not be sufficient; HTTP::Headers will need changing too :)

The solution you propose will work, I believe, with the caveat that HTTP::Headers won't be able to tell the difference between _method and -method when accessing. This is probably not an issue in practice.

all this trouble so you don't have to write 'Content-Type' => instead of content_type: ? 😄 dual APIs (headers and raw_headers) sounds confusing, and it would exist because of normalisation logic that is buggy and always will be, the solution proposed is to avoid the bug by adding a new API.

i liked string vs symbol being used to opt-in or out, because it's subtle and i never use symbols for header names (i write them as they should be, so 'Content-Type' etc). i also feel normalisation stuff like this hides what is really sent to the server, and doesn't help people trying to learn http.

2c. 😉

any update or fix on this for now? Lots of API make bad choices on headers and the conversion makes it impossible to use this client :(

@tarcieri

I think the next step would be for someone to open a PR based on this discussion. There seems to be sufficient clarity from @tarcieri on what would be acceptable.

I will try to find some time to work on that. If somebody willing to start that earlier - feel free to poke me - will be happy to assist

Hi. I know this is not the intended solution, but I needed something quick and tried the 'don't normalize Strings' option (kind of). If the header name is a String and is already compliant, I use it as it is, otherwise, I go on with the normalization.

This is the monkey patch file in my Rails project, in case it may be useful to someone else. It just removes the .to_s in all the calls to the normalize_header method and adds the two first lines in that method.

module HTTP
  # HTTP Headers container.
  class Headers
    # Removes header.
    #
    # @param [#to_s] name header name
    # @return [void]
    def delete(name)
      name = normalize_header name
      @pile.delete_if { |k, _| k == name }
    end

    # Appends header.
    #
    # @param [#to_s] name header name
    # @param [Array<#to_s>, #to_s] value header value(s) to be appended
    # @return [void]
    def add(name, value)
      name = normalize_header name
      Array(value).each { |v| @pile << [name, v.to_s] }
    end

    # Returns list of header values if any.
    #
    # @return [Array<String>]
    def get(name)
      name = normalize_header name
      @pile.select { |k, _| k == name }.map { |_, v| v }
    end

    # Tells whenever header with given `name` is set or not.
    #
    # @return [Boolean]
    def include?(name)
      name = normalize_header name
      @pile.any? { |k, _| k == name }
    end

    private

    # Transforms `name` to canonical HTTP header capitalization.
    # PATCH: If it is a String and matches COMPLIANT_NAME_RE, then return it as is.
    #
    # @param [String] name
    # @raise [HeaderError] if normalized name does not
    #   match {HEADER_NAME_RE}
    # @return [String] canonical HTTP header name
    def normalize_header(name)
      return name if name.is_a?(String) && name =~ COMPLIANT_NAME_RE

      name = name.to_s
      return name if name =~ CANONICAL_NAME_RE

      normalized = name.split(/[\-_]/).each(&:capitalize!).join("-")

      return normalized if normalized =~ COMPLIANT_NAME_RE

      raise HeaderError, "Invalid HTTP header field name: #{name.inspect}"
    end
  end
end

Just wanted to comment that I got bit by this too. I'm working with an API which requires a custom request header that looks like this: "X-LongNameInCamelCase". Unfortunately the header name seems to be case-sensitive. As far as I know http header field names shouldn't be case-sensitive; nonetheless, it means that I can't use http.rb for this project without patching.

I'm gonna work on this shortly.

@ace

thank @ace
I was considering use other library

The following monkey-patch will prevent normalizing any header that contains an underscore

# Monkeypatch workaround for underscores in headers: https://github.com/httprb/http/issues/337
raise 'This Monkeypatch is only tested for http 4.2.x' unless Gem.loaded_specs['http'].version.to_s.starts_with?('4.2.')

module DontNormalizeUnderscoreHeaders
  def normalize_header(name)
    return name if name.include?('_')

    super
  end
end

module HTTP
  class Headers
    prepend DontNormalizeUnderscoreHeaders
  end
end

This issue was fixed by #576 and can be closed.

@joshuaflanagan

Indeed. Thanks @joshuaflanagan

Navigation Menu

Repository navigation

Headers with leading underscore being rewritten to dashes #337

7 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Headers with leading underscore being rewritten to dashes #337

Description

Activity

tarcieri commented on May 3, 2016

peterc commented on May 3, 2016

ixti commented on May 4, 2016

ixti commented on May 4, 2016

peterc commented on May 4, 2016

ixti commented on May 4, 2016

ixti commented on May 4, 2016

ixti commented on May 4, 2016

ixti commented on May 5, 2016

tarcieri commented on May 5, 2016

tarcieri commented on May 5, 2016

ixti commented on May 5, 2016

peterc commented on May 5, 2016

zanker commented on May 5, 2016

ixti commented on May 5, 2016

7 remaining items

britishtea commented on Feb 9, 2017

altruby commented on Feb 11, 2017

epinault commented on May 3, 2018

mikegee commented on May 3, 2018

ixti commented on May 3, 2018

ace commented on Sep 13, 2018

mofahead commented on Oct 12, 2018

ixti commented on Oct 12, 2018

elit69 commented on Aug 27, 2019

shaicoleman commented on Nov 6, 2019

joshuaflanagan commented on Feb 12, 2020

ixti commented on Feb 13, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions