diff --git a/lib/rack/common_logger.rb b/lib/rack/common_logger.rb index a513ff6ea..8bf80d27c 100644 --- a/lib/rack/common_logger.rb +++ b/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) diff --git a/lib/rack/request.rb b/lib/rack/request.rb index 54ea86c4f..8747a058a 100644 --- a/lib/rack/request.rb +++ b/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,7 +239,9 @@ 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)}" @@ -246,13 +249,7 @@ def host_with_port 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 @@ -260,6 +257,8 @@ def 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 diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb index 38d37aaea..f204f037b 100644 --- a/lib/rack/utils.rb +++ b/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) diff --git a/test/spec_common_logger.rb b/test/spec_common_logger.rb index 330a6480b..cfbbbafb5 100644 --- a/test/spec_common_logger.rb +++ b/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 diff --git a/test/spec_request.rb b/test/spec_request.rb index 583a367e3..0a969a3b9 100644 --- a/test/spec_request.rb +++ b/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' diff --git a/test/spec_utils.rb b/test/spec_utils.rb index 6210fd73f..4a9ed02ae 100644 --- a/test/spec_utils.rb +++ b/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"