HTTP header names mangled even if conformant to RFC

oliver-gramberg commented 6 years ago

I am (indirectly) using pact-mockservice to test against a REST service that I do not control that requires HTTP header names of the form /[a-z]+([a-z]+)/, e.g., 'my_header'. These tests fail, and I find that somewhere between the client sending the request and the pact mock receiving it, header names get rewritten to the form /[A_Z][a-z](-[A-Z][a-z])/, e.g., 'My-Header'. While the latter form is most common in the wild, the former seems still to be according to the following definitions from RFC 7230 (https://tools.ietf.org/html/rfc7230#page-22):

3.2.  Header Fields

   Each header field consists of a case-insensitive field name followed
   by a colon (":"), optional leading whitespace, the field value, and
   optional trailing whitespace.

     header-field   = field-name ":" OWS field-value OWS

     field-name     = token

     ...

     token          = 1*tchar

     tchar          = "!" / "#" / "$" / "%" / "&" / "'" / "*"
                    / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~"
                    / DIGIT / ALPHA
                    ; any VCHAR, except delimiters

In https://github.com/pact-foundation/pact-mock_service/blob/7d4fd1f8ec6b6f6f118aae56fe6373ebc4f83972/lib/pact/consumer/mock_service/rack_request_helper.rb, I find:

   def standardise_header header
     header.gsub(/^HTTP_/, '').split("_").collect{|word| word[0] + word[1..-1].downcase}.join("-")
   end

All is well if I change the function to read,

      def standardise_header header
        header.gsub(/^HTTP_/, '').downcase
      end

I do not completely understand what is happening. What is the purpose of the original transformation? Can we do with my version?

Thank you!

bethesque commented 6 years ago

The reason this transformation is going on is that the Ruby Rack protocol passes the headers from the "outside world" to all Rack compatible apps by producing a single environment hash that has all the HTTP header and request information in it. Here is an example:

{
  "SERVER_SOFTWARE"=>"thin 1.4.1 codename Chromeo",
  "SERVER_NAME"=>"localhost",
  "rack.input"=>#<StringIO:0x007fa1bce039f8>,
  "rack.version"=>[1, 0],
  "rack.errors"=>#<IO:<STDERR>>,
  "rack.multithread"=>false,
  "rack.multiprocess"=>false,
  "rack.run_once"=>false,
  "REQUEST_METHOD"=>"GET",
  "REQUEST_PATH"=>"/favicon.ico",
  "PATH_INFO"=>"/favicon.ico",
  "REQUEST_URI"=>"/favicon.ico",
  "HTTP_VERSION"=>"HTTP/1.1",
  "HTTP_HOST"=>"localhost:8080",
  "HTTP_CONNECTION"=>"keep-alive",
  "HTTP_ACCEPT"=>"*/*",
  "HTTP_USER_AGENT"=>
  "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_4) AppleWebKit/536.11 (KHTML, like Gecko) Chrome/20.0.1132.47 Safari/536.11",
  "HTTP_ACCEPT_ENCODING"=>"gzip,deflate,sdch",
  "HTTP_ACCEPT_LANGUAGE"=>"en-US,en;q=0.8",
  "HTTP_ACCEPT_CHARSET"=>"ISO-8859-1,utf-8;q=0.7,*;q=0.3",
  "HTTP_COOKIE"=> "_gauges_unique_year=1;  _gauges_unique_month=1",
  "GATEWAY_INTERFACE"=>"CGI/1.2",
  "SERVER_PORT"=>"8080",
  "QUERY_STRING"=>"",
  "SERVER_PROTOCOL"=>"HTTP/1.1",
  "rack.url_scheme"=>"http",
  "SCRIPT_NAME"=>"",
  "REMOTE_ADDR"=>"127.0.0.1",
  "async.callback"=>#<Method: Thin::Connection#post_process>,
  "async.close"=>#<EventMachine::DefaultDeferrable:0x007fa1bce35b88
}

All the HTTP headers are turned into keys named HTTP_{uppercase underscored header name}. The code you have found turns the Rack headers back into their conventional values. Unfortunately, the code you propose will break the existing implementation for everyone else.

This is a tricky one, because pact terms only work on header values, not header names. I'm going to have to give it some thought. What package of the mock service are you using? (ie. pure ruby, or one of the wrapped implementations, or docker)

oliver-gramberg commented 6 years ago

I am using @pact-foundation/pact-node.

Thanks for the hint with Rack. I found a page explaining that Rack does this to stay compatible with CGI. I am not a Ruby person. Is CGI used here at all? If not, maybe we can ask Rack to provide an option to not mangle the names in the first place?

bethesque commented 6 years ago

Rack is the Ruby defacto standard for interfacing between web applications and their http servers. They won't be changing their interface for anyone. I haven't giving up on working out a more elegant solution yet, but the best I can think of at the moment is giving you a way to monkey patch the code at runtime.

bethesque commented 6 years ago

I may have come up with a work around, ping me if I don't reply in few days.

bethesque commented 6 years ago

I've got something in the works that will allow you to monkeypatch the method you need to fix.

oliver-gramberg commented 6 years ago

I try to help the guys over there with this fork/PR: https://github.com/pact-foundation/pact-node/pull/60. Maybe you can try your changes against it already?

oliver-gramberg commented 6 years ago

I upgraded to the current versions of involved libraries, copied from your test to my patch file, and all works like a charm. A big thanks, also to @mefellows for merging https://github.com/pact-foundation/pact-node/pull/60 there!

I believe the other issue you mentioned in https://github.com/pact-foundation/pact-js/issues/123 which you said you did not remember must be #38. (The OP of it says in https://github.com/pact-foundation/pact-node/issues/24 that they changed the format of their headers in the meantime.)

pact-foundation / pact-mock_service

HTTP header names mangled even if conformant to RFC #78