Preserve header casing

The original behavior was to normalize all header names so that they
were broken up into words, delimited by `-` or `_`, capitalize each word,
and then join the words together with a `-`.

This made it impossible to make a request with an underscore in the header
name, or with a different casing (ex: all caps).

However, the normalized name made it possible to access (or delete) headers,
without having to know the exact casing.

The new behavior is based on the following rules (as specified in #524 (comment))

1) Fail if a header name is not specified as a String or Symbol
2) If the header name is specified as a Symbol, normalize it when writing it in a request.
If the header name is specified as a String, preserve it as-is when writing it in a request.
3) Allow lookup of any header using the normalized form of the name

I implemented this behavior by storing three elements for each header value:
1) normalized header name
2) header name as it will be written in a request
3) header value

Element 2 is the new addition. I considered just storing the header value
as it would be written, and only doing normalization during lookup, but
it seemed wasteful to potentially normalize the same value over and over
when searching through the list for various lookups. This way we only
normalize each name once, and can continue to use that value for lookups.
However, whenever asked for the contents (ex: via `each` or `keys`) we
return the new, non-normalized name.

Fixes: #524

Author: joshuaflanagan

lib/http/headers.rb

@@ -21,6 +21,10 @@ class Headers
 
     # Class constructor.
     def initialize
+      # The @pile stores each header value using a three element array:
+      #  0 - the normalized header key, used for lookup
+      #  1 - the header key as it will be sent with a request
+      #  2 - the value
       @pile = []
     end
 
@@ -49,16 +53,30 @@ def delete(name)
     # @param [Array<#to_s>, #to_s] value header value(s) to be appended
     # @return [void]
     def add(name, value)
-      name = normalize_header name.to_s
-      Array(value).each { |v| @pile << [name, validate_value(v)] }
+      lookup_name = normalize_header(name.to_s)
+      wire_name = case name
+                  when String
+                    name
+                  when Symbol
+                    lookup_name
+                  else
+                    raise HTTP::HeaderError, "HTTP header must be a String or Symbol: #{name.inspect}"
+                  end
+      Array(value).each do |v|
+        @pile << [
+          lookup_name,
+          wire_name,
+          validate_value(v)
+        ]
+      end
     end
 
     # Returns list of header values if any.
     #
     # @return [Array<String>]
     def get(name)
       name = normalize_header name.to_s
-      @pile.select { |k, _| k == name }.map { |_, v| v }
+      @pile.select { |k, _| k == name }.map { |_, _, v| v }
     end
 
     # Smart version of {#get}.
@@ -96,7 +114,7 @@ def to_h
     #
     # @return [Array<[String, String]>]
     def to_a
-      @pile.map { |pair| pair.map(&:dup) }
+      @pile.map { |item| item[1..2] }
     end
 
     # Returns human-readable representation of `self` instance.
@@ -110,7 +128,7 @@ def inspect
     #
     # @return [Array<String>]
     def keys
-      @pile.map { |k, _| k }.uniq
+      @pile.map { |_, k, _| k }.uniq
     end
 
     # Compares headers to another Headers or Array of key/value pairs
@@ -119,7 +137,7 @@ def keys
     def ==(other)
       return false unless other.respond_to? :to_a
 
-      @pile == other.to_a
+      to_a == other.to_a
     end
 
     # Calls the given block once for each key/value pair in headers container.
@@ -129,7 +147,7 @@ def ==(other)
     def each
       return to_enum(__method__) unless block_given?
 
-      @pile.each { |arr| yield(arr) }
+      @pile.each { |item| yield(item[1..2]) }
       self
     end
 
@@ -152,7 +170,7 @@ def each
     # @api private
     def initialize_copy(orig)
       super
-      @pile = to_a
+      @pile = @pile.map(&:dup)
     end
 
     # Merges `other` headers into `self`.

spec/lib/http/connection_spec.rb

@@ -20,14 +20,17 @@
         <<-RESPONSE.gsub(/^\s*\| */, "").gsub(/\n/, "\r\n")
         | HTTP/1.1 200 OK
         | Content-Type: text
+        | foo_bar: 123
         |
         RESPONSE
       end
     end
 
-    it "reads data in parts" do
+    it "populates headers collection, preserving casing" do
       connection.read_headers!
-      expect(connection.headers).to eq("Content-Type" => "text")
+      expect(connection.headers).to eq("Content-Type" => "text", "foo_bar" => "123")
+      expect(connection.headers["Foo-Bar"]).to eq("123")
+      expect(connection.headers["foo_bar"]).to eq("123")
     end
   end
 

spec/lib/http/headers_spec.rb

@@ -13,7 +13,7 @@
       expect(headers["Accept"]).to eq "application/json"
     end
 
-    it "normalizes header name" do
+    it "allows retrieval via normalized header name" do
       headers.set :content_type, "application/json"
       expect(headers["Content-Type"]).to eq "application/json"
     end
@@ -54,7 +54,7 @@
       expect(headers["Accept"]).to eq "application/json"
     end
 
-    it "normalizes header name" do
+    it "allows retrieval via normalized header name" do
       headers[:content_type] = "application/json"
       expect(headers["Content-Type"]).to eq "application/json"
     end
@@ -80,7 +80,7 @@
       expect(headers["Content-Type"]).to be_nil
     end
 
-    it "normalizes header name" do
+    it "removes header that matches normalized version of specified name" do
       headers.delete :content_type
       expect(headers["Content-Type"]).to be_nil
     end
@@ -104,13 +104,13 @@
       expect(headers["Accept"]).to eq "application/json"
     end
 
-    it "normalizes header name" do
+    it "allows retrieval via normalized header name" do
       headers.add :content_type, "application/json"
       expect(headers["Content-Type"]).to eq "application/json"
     end
 
     it "appends new value if header exists" do
-      headers.add :set_cookie, "hoo=ray"
+      headers.add "Set-Cookie", "hoo=ray"
       headers.add :set_cookie, "woo=hoo"
       expect(headers["Set-Cookie"]).to eq %w[hoo=ray woo=hoo]
     end
@@ -137,6 +137,11 @@
       expect { headers.add "foo", "bar\nEvil-Header: evil-value" }.
         to raise_error HTTP::HeaderError
     end
+
+    it "fails when header name is not a String or Symbol" do
+      expect { headers.add 2, "foo" }.
+        to raise_error HTTP::HeaderError
+    end
   end
 
   describe "#get" do
@@ -314,6 +319,17 @@
       )
     end
 
+    it "yields header keys specified as symbols in normalized form" do
+      keys = headers.each.map(&:first)
+      expect(keys).to eq(["Set-Cookie", "Content-Type", "Set-Cookie"])
+    end
+
+    it "yields headers specified as strings without conversion" do
+      headers.add "X_kEy", "value"
+      keys = headers.each.map(&:first)
+      expect(keys).to eq(["Set-Cookie", "Content-Type", "Set-Cookie", "X_kEy"])
+    end
+
     it "returns self instance if block given" do
       expect(headers.each { |*| }).to be headers
     end
@@ -490,14 +506,15 @@
     end
 
     context "with duplicate header keys (mixed case)" do
-      let(:headers) { {"Set-Cookie" => "hoo=ray", "set-cookie" => "woo=hoo"} }
+      let(:headers) { {"Set-Cookie" => "hoo=ray", "set_cookie" => "woo=hoo", :set_cookie => "ta=da"} }
 
       it "adds all headers" do
         expect(described_class.coerce(headers).to_a).
           to match_array(
             [
               %w[Set-Cookie hoo=ray],
-              %w[Set-Cookie woo=hoo]
+              %w[set_cookie woo=hoo],
+              %w[Set-Cookie ta=da]
             ]
           )
       end

spec/lib/http/options/headers_spec.rb

@@ -8,7 +8,7 @@
   end
 
   it "may be specified with with_headers" do
-    opts2 = opts.with_headers("accept" => "json")
+    opts2 = opts.with_headers(:accept => "json")
     expect(opts.headers).to be_empty
     expect(opts2.headers).to eq([%w[Accept json]])
   end

spec/lib/http/request/writer_spec.rb

@@ -22,6 +22,18 @@
       end
     end
 
+    context "when headers are specified as strings with mixed case" do
+      let(:headers) { HTTP::Headers.coerce "content-Type" => "text", "X_MAX" => "200" }
+
+      it "writes the headers with the same casing" do
+        writer.stream
+        expect(io.string).to eq [
+          "#{headerstart}\r\n",
+          "content-Type: text\r\nX_MAX: 200\r\nContent-Length: 0\r\n\r\n"
+        ].join
+      end
+    end
+
     context "when body is nonempty" do
       let(:body) { HTTP::Request::Body.new("content") }