Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP header names mangled even if conformant to RFC #78

Closed
oliver-gramberg opened this issue Nov 1, 2017 · 7 comments
Closed

HTTP header names mangled even if conformant to RFC #78

oliver-gramberg opened this issue Nov 1, 2017 · 7 comments

Comments

@oliver-gramberg
Copy link

I am (indirectly) using pact-mock_service 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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

bethesque commented Nov 5, 2017 via email

@bethesque
Copy link
Member

bethesque commented Nov 6, 2017 via email

@bethesque
Copy link
Member

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

@oliver-gramberg
Copy link
Author

I try to help the guys over there with this fork/PR: pact-foundation/pact-js-core#60. Maybe you can try your changes against it already?

@oliver-gramberg
Copy link
Author

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 pact-foundation/pact-js-core#60 there!

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants