Merge pull request jaswope#6 from waterlink/fix/response-headers-arrays-vs-strings

Fix/response headers arrays vs strings

Author: waterlink

lib/rack/reverse_proxy.rb

@@ -18,7 +18,7 @@ def initialize(app = nil, &b)
 
     def call(env)
       rackreq = Rack::Request.new(env)
-      matcher = get_matcher(rackreq.fullpath, extract_http_request_headers(rackreq.env), rackreq)
+      matcher = get_matcher(rackreq.fullpath, Proxy.extract_http_request_headers(rackreq.env), rackreq)
       return @app.call(env) if matcher.nil?
 
       if @global_options[:newrelic_instrumentation]
@@ -44,7 +44,7 @@ def proxy(env, source_request, matcher)
       target_request = Net::HTTP.const_get(source_request.request_method.capitalize).new(uri.request_uri)
 
       # Setup headers
-      target_request_headers = extract_http_request_headers(source_request.env)
+      target_request_headers = Proxy.extract_http_request_headers(source_request.env)
 
       if options[:preserve_host]
         if uri.port == uri.default_port
@@ -82,12 +82,12 @@ def proxy(env, source_request, matcher)
       target_response.use_ssl = "https" == uri.scheme
 
       # Let rack set the transfer-encoding header
-      response_headers = format_headers(target_response.headers)
+      response_headers = Rack::Utils::HeaderHash.new Proxy.normalize_headers(format_headers(target_response.headers))
       response_headers.delete('Transfer-Encoding')
 
       # Replace the location header with the proxy domain
       if response_headers['Location'] && options[:replace_response_host]
-        response_location = URI(response_headers['Location'][0])
+        response_location = URI(response_headers['location'])
         response_location.host = source_request.host
         response_location.port = source_request.port
         response_headers['Location'] = response_location.to_s
@@ -96,26 +96,6 @@ def proxy(env, source_request, matcher)
       [target_response.status, response_headers, target_response.body]
     end
 
-    def extract_http_request_headers(env)
-      headers = env.reject do |k, v|
-        !(/^HTTP_[A-Z_]+$/ === k) || v.nil?
-      end.map do |k, v|
-        [reconstruct_header_name(k), v]
-      end.inject(Utils::HeaderHash.new) do |hash, k_v|
-        k, v = k_v
-        hash[k] = v
-        hash
-      end
-
-      x_forwarded_for = (headers["X-Forwarded-For"].to_s.split(/, +/) << env["REMOTE_ADDR"]).join(", ")
-
-      headers.merge!("X-Forwarded-For" =>  x_forwarded_for)
-    end
-
-    def reconstruct_header_name(name)
-      name.sub(/^HTTP_/, "").gsub("_", "-")
-    end
-
     def get_matcher(path, headers, rackreq)
       matches = @matchers.select do |matcher|
         matcher.match?(path, headers, rackreq)

spec/rack/reverse_proxy_spec.rb

@@ -31,6 +31,19 @@ def app
       last_response.body.should == "Proxied App"
     end
 
+    it "should produce a response header of type HeaderHash" do
+      stub_request(:get, 'http://example.com/test')
+      get '/test'
+      last_response.headers.should be_an_instance_of Rack::Utils::HeaderHash
+    end
+
+    it "should parse the headers as a Hash with values of type String" do
+      stub_request(:get, 'http://example.com/test').to_return({:headers => {'cache-control'=> 'max-age=300, public'} })
+      get '/test'
+      last_response.headers['cache-control'].should be_an_instance_of String
+      last_response.headers['cache-control'].should == 'max-age=300, public'
+    end
+
     it "should proxy requests to a lambda url when a pattern is matched" do
       stub_request(:get, 'http://example.com/2test').to_return({:body => "Proxied App2"})
       get '/2test'
@@ -166,7 +179,6 @@ def app
       it "should replace the location response header" do
         stub_request(:get, "http://example.com/test/stuff").to_return(:headers => {"location" => "http://test.com/bar"})
         get 'http://example.com/test/stuff'
-        # puts last_response.headers.inspect
         last_response.headers['location'].should == "http://example.com/bar"
       end