Merge pull request jaswope#52 from jjb/stripped-headers-option

new stripped_headers option & retire preserve_encoding option

Author: waterlink

CHANGELOG.md

@@ -3,6 +3,9 @@
 ## 1.0.0 (UNRELEASED)
 
 - Breaking Change: Never modify Location headers that are only paths without hosts. [John Bachir](https://github.com/jjb) [#46](https://github.com/waterlink/rack-reverse-proxy/pull/46)
+- Breaking Change: Previously, the Accept-Encoding header was stripped by default, unless the
+  `preserve_encoding` option was set to true. Now, no headers are stripped by default, and an array
+  of headers that should be stripped can be specified with the `stripped_headers` option.
 - Bugfix: Fix rack response body for https redirects [John Bachir](https://github.com/jjb) [#43](https://github.com/waterlink/rack-reverse-proxy/pull/43)
 
 ## 0.12.0

README.md

@@ -88,7 +88,8 @@ Right now if more than one rule matches any given route, it throws an exception
 * `:force_ssl` redirects to ssl version, if not already using it (requires `:replace_response_host`). Default: false.
 * `:verify_mode` the `OpenSSL::SSL` verify mode passed to Net::HTTP. Default: `OpenSSL::SSL::VERIFY_PEER`.
 * `:x_forwarded_headers` sets up proper `X-Forwarded-*` headers. Default: true.
-* `:preserve_encoding` Set to true to pass Accept-Encoding header to proxy server. Default: false.
+* `:stripped_headers` Array of headers that should be stripped before forwarding reqeust. Default: nil.
+  e.g. `stripped_headers: ["Accept-Encoding", "Foo-Bar"]`
 
 ## Note on Patches/Pull Requests
 * Fork the project.

lib/rack_reverse_proxy/middleware.rb

@@ -13,7 +13,7 @@ def initialize(app = nil, &b)
       @rules = []
       @global_options = {
         :preserve_host => true,
-        :preserve_encoding => false,
+        :stripped_headers => nil,
         :x_forwarded_headers => true,
         :matching => :all,
         :replace_response_host => false

lib/rack_reverse_proxy/roundtrip.rb

@@ -84,9 +84,11 @@ def preserve_host
       target_request_headers["HOST"] = host_header
     end
 
-    def preserve_encoding
-      return if options[:preserve_encoding]
-      target_request_headers.delete("Accept-Encoding")
+    def strip_headers
+      return unless options[:stripped_headers]
+      options[:stripped_headers].each do |header|
+        target_request_headers.delete(header)
+      end
     end
 
     def host_header
@@ -184,7 +186,7 @@ def need_replace_location?
 
     def setup_request
       preserve_host
-      preserve_encoding
+      strip_headers
       set_forwarded_headers
       initialize_http_header
       set_basic_auth

spec/rack/reverse_proxy_spec.rb

@@ -159,20 +159,6 @@ def app
       expect(last_response.headers["Content-Length"]).to eq(body.length.to_s)
     end
 
-    it "does not include Accept-Encoding header" do
-      stub_request(:any, "http://example.com/test")
-
-      get "/test", {}, "HTTP_ACCEPT_ENCODING" => "gzip, deflate"
-
-      expect(
-        a_request(:get, "http://example.com/test").with(
-          :headers => { "Accept-Encoding" => "gzip, deflate" }
-        )
-      ).not_to have_been_made
-
-      expect(a_request(:get, "http://example.com/test")).to have_been_made
-    end
-
     describe "with non-default port" do
       def app
         Rack::ReverseProxy.new(dummy_app) do
@@ -212,23 +198,53 @@ def app
       end
     end
 
-    describe "with preserve encoding turned on" do
-      def app
-        Rack::ReverseProxy.new(dummy_app) do
-          reverse_proxy "/test", "http://example.com/", :preserve_encoding => true
-        end
+    context "stripped_headers option" do
+      subject do
+        stub_request(:any, "http://example.com/test")
+        get "/test", {}, "HTTP_ACCEPT_ENCODING" => "gzip, deflate", "HTTP_FOO_BAR" => "baz"
       end
 
-      it "sets the Accept-Encoding header" do
-        stub_request(:any, "http://example.com/test")
+      describe "with stripped_headers not set" do
+        def app
+          Rack::ReverseProxy.new(dummy_app) do
+            reverse_proxy "/test", "http://example.com/"
+          end
+        end
 
-        get "/test", {}, "HTTP_ACCEPT_ENCODING" => "gzip, deflate"
+        it "forwards the headers" do
+          subject
+          expect(
+            a_request(:get, "http://example.com/test").with(
+              :headers => { "Accept-Encoding" => "gzip, deflate", "Foo-Bar" => "baz" }
+            )
+          ).to have_been_made
+        end
+      end
 
-        expect(
-          a_request(:get, "http://example.com/test").with(
-            :headers => { "Accept-Encoding" => "gzip, deflate" }
-          )
-        ).to have_been_made
+      describe "with stripped_headers set" do
+        before do
+          @stripped_headers = ["Accept-Encoding", "Foo-Bar"]
+          def app
+            # so the value is constant in the closure below
+            stripped_headers = @stripped_headers
+            Rack::ReverseProxy.new(dummy_app) do
+              reverse_proxy "/test", "http://example.com/", :stripped_headers => stripped_headers
+            end
+          end
+        end
+
+        it "removes the stripped headers" do
+          subject
+          expect(
+            a_request(:get, "http://example.com/test").with{ |req|
+              req.headers.each do |header, value|
+                if @stripped_headers.include?(header)
+                  fail "expected #{header} to not be present"
+                end
+              end
+            }
+          ).to have_been_made
+        end
       end
     end