Support RFC 7239: HTTP Forwarded header

Co-authored-by: Matt Bostock <matt@mattbostock.com>
Co-authored-by: Jeremy Evans <code@jeremyevans.net>

3 files changed, 106 insertions, 1 deletions

diff --git a/test/spec_common_logger.rb b/test/spec_common_logger.rb
index 60db7704..3d562ae5 100644
--- a/test/spec_common_logger.rb
+++ b/test/spec_common_logger.rb
@@ -62,6 +62,20 @@ describe Rack::CommonLogger do
     res.errors.must_match(/"GET \/ " 200 - /)
   end
 
+  it "log - records host from X-Forwarded-For header" do
+    res = Rack::MockRequest.new(Rack::CommonLogger.new(app)).get("/", 'HTTP_X_FORWARDED_FOR' => '203.0.113.0')
+
+    res.errors.wont_be :empty?
+    res.errors.must_match(/203\.0\.113\.0 - /)
+  end
+
+  it "log - records host from RFC 7239 forwarded for header" do
+    res = Rack::MockRequest.new(Rack::CommonLogger.new(app)).get("/", 'HTTP_FORWARDED' => 'for=203.0.113.0')
+
+    res.errors.wont_be :empty?
+    res.errors.must_match(/203\.0\.113\.0 - /)
+  end
+
   def with_mock_time(t = 0)
     mc = class << Time; self; end
     mc.send :alias_method, :old_now, :now
diff --git a/test/spec_request.rb b/test/spec_request.rb
index 3818dc1b..f46fcc3e 100644
--- a/test/spec_request.rb
+++ b/test/spec_request.rb
@@ -173,6 +173,23 @@ class RackRequestTest < Minitest::Spec
     req.hostname.must_equal "example.org"
 
     req = make_request \
+      Rack::MockRequest.env_for("/", "HTTP_HOST" => "localhost:81", "HTTP_FORWARDED" => "host=example.org:9292")
+    req.host.must_equal "example.org"
+
+    # Test obfuscated identifier: https://tools.ietf.org/html/rfc7239#section-6.3
+    req = make_request \
+      Rack::MockRequest.env_for("/", "HTTP_HOST" => "localhost:81", "HTTP_FORWARDED" => "host=ObFuScaTeD")
+    req.host.must_equal "ObFuScaTeD"
+
+    req = make_request \
+      Rack::MockRequest.env_for("/", "HTTP_HOST" => "localhost:81", "HTTP_FORWARDED" => "host=example.com; host=example.org:9292")
+    req.host.must_equal "example.org"
+
+    req = make_request \
+      Rack::MockRequest.env_for("/", "HTTP_HOST" => "localhost:81", "HTTP_X_FORWARDED_HOST" => "example.org:9292", "HTTP_FORWARDED" => "host=example.com")
+    req.host.must_equal "example.com"
+
+    req = make_request \
       Rack::MockRequest.env_for("/", "HTTP_HOST" => "localhost:81", "HTTP_X_FORWARDED_HOST" => "example.org:9292")
     req.host.must_equal "example.org"
     req.hostname.must_equal "example.org"
@@ -314,6 +331,10 @@ class RackRequestTest < Minitest::Spec
     req = make_request \
       Rack::MockRequest.env_for("/", "HTTP_HOST" => "localhost", "HTTP_X_FORWARDED_PROTO" => "https,https", "SERVER_PORT" => "80")
     req.port.must_equal 443
+
+    req = make_request \
+      Rack::MockRequest.env_for("/", "HTTP_HOST" => "localhost", "HTTP_FORWARDED" => "proto=https", "HTTP_X_FORWARDED_PROTO" => "http", "SERVER_PORT" => "9393")
+    req.port.must_equal 443
   end
 
   it "figure out the correct host with port" do
@@ -348,6 +369,10 @@ class RackRequestTest < Minitest::Spec
     req = make_request \
       Rack::MockRequest.env_for("/", "HTTP_HOST" => "localhost:81", "HTTP_X_FORWARDED_HOST" => "example.org", "SERVER_PORT" => "9393")
     req.host_with_port.must_equal "example.org"
+
+    req = make_request \
+      Rack::MockRequest.env_for("/", "HTTP_HOST" => "localhost:81", "HTTP_X_FORWARDED_HOST" => "example.org", "HTTP_FORWARDED" => "host=example.com:9292", "SERVER_PORT" => "9393")
+    req.host_with_port.must_equal "example.com:9292"
   end
 
   it "parse the query string" do
@@ -705,6 +730,17 @@ class RackRequestTest < Minitest::Spec
 
     request = make_request(Rack::MockRequest.env_for("/", 'HTTP_X_FORWARDED_PROTO' => 'ws'))
     request.scheme.must_equal "ws"
+
+    request = make_request(Rack::MockRequest.env_for("/", 'HTTP_FORWARDED' => 'proto=https'))
+    request.scheme.must_equal "https"
+    request.must_be :ssl?
+
+    request = make_request(Rack::MockRequest.env_for("/", 'HTTP_FORWARDED' => 'proto=https, proto=http'))
+    request.scheme.must_equal "https"
+    request.must_be :ssl?
+
+    request = make_request(Rack::MockRequest.env_for("/", 'HTTP_FORWARDED' => 'proto=http, proto=https'))
+    request.scheme.must_equal "http"
     request.wont_be :ssl?
 
     request = make_request(Rack::MockRequest.env_for("/", 'HTTPS' => 'on'))
@@ -1364,7 +1400,7 @@ EOF
     res.body.must_equal 'fe80::202:b3ff:fe1e:8329'
 
     res = mock.get '/', 'REMOTE_ADDR' => '1.2.3.4,3.4.5.6'
-    res.body.must_equal '1.2.3.4'
+    res.body.must_equal '3.4.5.6'
 
     res = mock.get '/', 'REMOTE_ADDR' => '127.0.0.1'
     res.body.must_equal '127.0.0.1'
@@ -1378,6 +1414,21 @@ EOF
 
     res = mock.get '/',
       'REMOTE_ADDR' => '1.2.3.4',
+      'HTTP_FORWARDED' => 'for=3.4.5.6'
+    res.body.must_equal '1.2.3.4'
+
+    res = mock.get '/',
+      'HTTP_X_FORWARDED_FOR' => '3.4.5.6',
+      'HTTP_FORWARDED' => 'for=5.6.7.8'
+    res.body.must_equal '5.6.7.8'
+
+    res = mock.get '/',
+      'HTTP_X_FORWARDED_FOR' => '3.4.5.6',
+      'HTTP_FORWARDED' => 'for=5.6.7.8, for=7.8.9.0'
+    res.body.must_equal '7.8.9.0'
+
+    res = mock.get '/',
+      'REMOTE_ADDR' => '1.2.3.4',
       'HTTP_X_FORWARDED_FOR' => '3.4.5.6'
     res.body.must_equal '1.2.3.4'
 
@@ -1410,6 +1461,9 @@ EOF
     res = mock.get '/', 'HTTP_X_FORWARDED_FOR' => '[2001:db8:cafe::17]:47011'
     res.body.must_equal '2001:db8:cafe::17'
 
+    res = mock.get '/', 'HTTP_FORWARDED' => 'for="[2001:db8:cafe::17]:47011"'
+    res.body.must_equal '2001:db8:cafe::17'
+
     res = mock.get '/', 'HTTP_X_FORWARDED_FOR' => '1.2.3.4, [2001:db8:cafe::17]:47011'
     res.body.must_equal '2001:db8:cafe::17'
 
diff --git a/test/spec_utils.rb b/test/spec_utils.rb
index 354ea56f..3f50be7a 100644
--- a/test/spec_utils.rb
+++ b/test/spec_utils.rb
@@ -417,6 +417,43 @@ describe Rack::Utils do
     ]
   end
 
+  it "parses RFC 7239 Forwarded header" do
+    Rack::Utils.forwarded_values('for=3.4.5.6').must_equal({
+      for: [ '3.4.5.6' ],
+    })
+
+    Rack::Utils.forwarded_values(';;;for=3.4.5.6,,').must_equal({
+      for: [ '3.4.5.6' ],
+    })
+
+    Rack::Utils.forwarded_values('for=3.4.5.6').must_equal({
+      for: [ '3.4.5.6' ],
+    })
+
+    Rack::Utils.forwarded_values('for =  3.4.5.6').must_equal({
+      for: [ '3.4.5.6' ],
+    })
+
+    Rack::Utils.forwarded_values('for="3.4.5.6"').must_equal({
+      for: [ '3.4.5.6' ],
+    })
+
+    Rack::Utils.forwarded_values('for=3.4.5.6;proto=https').must_equal({
+      for: [ '3.4.5.6' ],
+      proto: [ 'https' ]
+    })
+
+    Rack::Utils.forwarded_values('for=3.4.5.6; proto=http, proto=https').must_equal({
+      for: [ '3.4.5.6' ],
+      proto: [ 'http', 'https' ]
+    })
+
+    Rack::Utils.forwarded_values('for=3.4.5.6; proto=http, proto=https; for=1.2.3.4').must_equal({
+      for: [ '3.4.5.6', '1.2.3.4' ],
+      proto: [ 'http', 'https' ]
+    })
+  end
+
   it "select best quality match" do
     Rack::Utils.best_q_match("text/html", %w[text/html]).must_equal "text/html"