Support RFC 7239: HTTP Forwarded header

Co-authored-by: Matt Bostock <matt@mattbostock.com>

lib/rack/common_logger.rb

@@ -32,30 +32,31 @@ def initialize(app, logger = nil)
 
     def call(env)
       began_at = Utils.clock_time
-      status, header, body = @app.call(env)
-      header = Utils::HeaderHash.new(header)
-      body = BodyProxy.new(body) { log(env, status, header, began_at) }
-      [status, header, body]
+      status, headers, body = @app.call(env)
+      headers = Utils::HeaderHash.new(headers)
+      body = BodyProxy.new(body) { log(env, status, headers, began_at) }
+      [status, headers, body]
     end
 
     private
 
-    def log(env, status, header, began_at)
-      length = extract_content_length(header)
+    def log(env, status, response_headers, began_at)
+      request = Rack::Request.new(env)
+      length = extract_content_length(response_headers)
 
       msg = FORMAT % [
-        env['HTTP_X_FORWARDED_FOR'] || env["REMOTE_ADDR"] || "-",
-        env["REMOTE_USER"] || "-",
+        request.ip || "-",
+        request.get_header("REMOTE_USER") || "-",
         Time.now.strftime("%d/%b/%Y:%H:%M:%S %z"),
-        env[REQUEST_METHOD],
-        env[PATH_INFO],
-        env[QUERY_STRING].empty? ? "" : "?#{env[QUERY_STRING]}",
-        env[SERVER_PROTOCOL],
+        request.request_method,
+        request.path_info,
+        request.query_string.empty? ? "" : "?#{request.query_string}",
+        request.get_header(SERVER_PROTOCOL),
         status.to_s[0..3],
         length,
         Utils.clock_time - began_at ]
 
-      logger = @logger || env[RACK_ERRORS]
+      logger = @logger || request.get_header(RACK_ERRORS)
       # Standard library logger doesn't support write but it supports << which actually
       # calls to write on the log device without formatting
       if logger.respond_to?(:write)

lib/rack/request.rb

@@ -134,6 +134,7 @@ module Helpers
       # to include the port in a generated URI.
       DEFAULT_PORTS = { 'http' => 80, 'https' => 443, 'coffee' => 80 }
 
+      HTTP_FORWARDED          = 'HTTP_FORWARDED'
       HTTP_X_FORWARDED_SCHEME = 'HTTP_X_FORWARDED_SCHEME'
       HTTP_X_FORWARDED_PROTO  = 'HTTP_X_FORWARDED_PROTO'
       HTTP_X_FORWARDED_HOST   = 'HTTP_X_FORWARDED_HOST'
@@ -238,28 +239,26 @@ def xhr?
       end
 
       def host_with_port
-        if forwarded = get_header(HTTP_X_FORWARDED_HOST)
+        if forwarded = get_http_forwarded(:host)
+          forwarded.last
+        elsif forwarded = get_header(HTTP_X_FORWARDED_HOST)
           forwarded.split(/,\s?/).last
         else
           get_header(HTTP_HOST) || "#{get_header(SERVER_NAME) || get_header(SERVER_ADDR)}:#{get_header(SERVER_PORT)}"
         end
       end
 
       def host
-        # Remove port number.
-        h = host_with_port
-        if colon_index = h.index(":")
-          h[0, colon_index]
-        else
-          h
-        end
+        strip_port(host_with_port)
       end
 
       def port
         if port = extract_port(host_with_port)
           port.to_i
         elsif port = get_header(HTTP_X_FORWARDED_PORT)
           port.to_i
+        elsif get_http_forwarded(:proto)
+          DEFAULT_PORTS[scheme]
         elsif has_header?(HTTP_X_FORWARDED_HOST)
           DEFAULT_PORTS[scheme]
         elsif has_header?(HTTP_X_FORWARDED_PROTO)
@@ -279,7 +278,10 @@ def ip
 
         return remote_addrs.first if remote_addrs.any?
 
-        forwarded_ips = split_ip_addresses(get_header('HTTP_X_FORWARDED_FOR'))
+        rfc7239_forwarded_ips = get_http_forwarded(:for) || []
+        x_forwarded_ips = split_ip_addresses(get_header('HTTP_X_FORWARDED_FOR'))
+
+        forwarded_ips = x_forwarded_ips.concat(rfc7239_forwarded_ips)
           .map { |ip| strip_port(ip) }
 
         return reject_trusted_ip_addresses(forwarded_ips).last || forwarded_ips.first || get_header("REMOTE_ADDR")
@@ -482,6 +484,12 @@ def parse_http_accept_header(header)
         end
       end
 
+      # Get an array of values set in the RFC 7239 `Forwarded` request header.
+      def get_http_forwarded(token)
+        values = Utils.forwarded_values(get_header(HTTP_FORWARDED))
+        values[token] if values
+      end
+
       def query_parser
         Utils.default_query_parser
       end
@@ -522,6 +530,8 @@ def reject_trusted_ip_addresses(ip_addresses)
       end
 
       def forwarded_scheme
+        forwarded_proto = get_http_forwarded(:proto)
+        (forwarded_proto && allowed_scheme(forwarded_proto.first)) ||
         allowed_scheme(get_header(HTTP_X_FORWARDED_SCHEME)) ||
         allowed_scheme(extract_proto_header(get_header(HTTP_X_FORWARDED_PROTO)))
       end

lib/rack/utils.rb

@@ -146,6 +146,21 @@ def q_values(q_value_header)
     end
     module_function :q_values
 
+    FORWARDED_PAIR_REGEX = /\A\s*(by|for|host|proto)\s*=\s*"?([^"]+)"?\s*\Z/i
+
+    def forwarded_values(forwarded_header)
+      return nil unless forwarded_header
+      forwarded_header = forwarded_header.to_s.gsub("\n", ";")
+
+      forwarded_header.split(/\s*;\s*/).each_with_object({}) do |field, values|
+        field.split(/\s*,\s*/).each do |pair|
+          return nil unless pair =~ FORWARDED_PAIR_REGEX
+          (values[$1.downcase.to_sym] ||= []) << $2
+        end
+      end
+    end
+    module_function :forwarded_values
+
     def best_q_match(q_value_header, available_mimes)
       values = q_values(q_value_header)
 

test/spec_common_logger.rb

@@ -38,7 +38,7 @@
     log.string.must_match(/"GET \/ " 200 #{length} /)
   end
 
-  it "work with standartd library logger" do
+  it "work with standard library logger" do
     logdev = StringIO.new
     log = Logger.new(logdev)
     Rack::MockRequest.new(Rack::CommonLogger.new(app, log)).get("/")
@@ -60,6 +60,20 @@
     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

test/spec_request.rb

@@ -122,6 +122,23 @@ class RackRequestTest < Minitest::Spec
       Rack::MockRequest.env_for("/", "SERVER_NAME" => "example.org", "SERVER_PORT" => "9292")
     req.host.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"
@@ -185,6 +202,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
@@ -207,6 +228,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
@@ -541,6 +566,18 @@ def initialize(*)
     request.scheme.must_equal "http"
     request.wont_be :ssl?
 
+    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'))
     request.scheme.must_equal "https"
     request.must_be :ssl?
@@ -1193,6 +1230,21 @@ def ip_app
   it 'deals with proxies' do
     mock = Rack::MockRequest.new(Rack::Lint.new(ip_app))
 
+    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'
@@ -1227,6 +1279,9 @@ def ip_app
     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'
 

test/spec_utils.rb

@@ -378,6 +378,43 @@ def initialize(*)
     ]
   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"