Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix bug in `ActionController::Request#remote_ip` #4202

Merged
merged 1 commit into from

2 participants

@dasch

If HTTP_X_FORWARDED_FOR only contains whitespace, don't try to extract a
list of IP addresses from it. Previously, calling #remote_ip when HTTP_X_FORWARDED_FOR was e.g. "" would cause "NoMethodError: undefined method strip for nil:NilClass".

@dasch dasch Make Request#remote_ip return nil when HTTP_X_FORWARDED_FOR is empty
If HTTP_X_FORWARDED_FOR only contains whitespace, don't try to extract a
list of IP addresses from it.
cd2136a
@tenderlove
Owner

@dasch this commit targets 2-3-stable, is this a backport? Does this bug exist in the other branches?

@dasch

@tenderlove we (Zendesk) experienced it on 2.3, and I wrote the patch directly on 2-3-stable. I can check if the bug is also present in latest stable tomorrow.

@tenderlove
Owner

@dasch please do. I'd rather apply this to master and backport than only commit to the release branch.

@dasch

@tenderlove it seems this stuff has been moved to Rack in 3.0, Rails simply delegates to Rack::Request.

This passes on master:

    request = stub_request 'HTTP_X_FORWARDED_FOR' => ''
    assert_nil request.remote_ip

I think this is just a 2.3 issue.

@tenderlove
Owner

Okay. I'll merge this to 2-3-stable, but please note that we have no plans of releasing another version of 2.3.

@tenderlove tenderlove merged commit 2eb197e into rails:2-3-stable
@dasch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 27, 2011
  1. @dasch

    Make Request#remote_ip return nil when HTTP_X_FORWARDED_FOR is empty

    dasch authored
    If HTTP_X_FORWARDED_FOR only contains whitespace, don't try to extract a
    list of IP addresses from it.
This page is out of date. Refresh to see the latest.
View
2  actionpack/lib/action_controller/request.rb
@@ -225,7 +225,7 @@ def remote_ip
not_trusted_addrs = remote_addr_list.reject {|addr| addr =~ TRUSTED_PROXIES}
return not_trusted_addrs.first unless not_trusted_addrs.empty?
end
- remote_ips = @env['HTTP_X_FORWARDED_FOR'] && @env['HTTP_X_FORWARDED_FOR'].split(',')
+ remote_ips = @env['HTTP_X_FORWARDED_FOR'].present? && @env['HTTP_X_FORWARDED_FOR'].split(',')
if @env.include? 'HTTP_CLIENT_IP'
if ActionController::Base.ip_spoofing_check && remote_ips && !remote_ips.include?(@env['HTTP_CLIENT_IP'])
View
3  actionpack/test/controller/request_test.rb
@@ -20,6 +20,9 @@ def test_remote_ip
'HTTP_X_FORWARDED_FOR' => '3.4.5.6'
assert_equal '1.2.3.4', request.remote_ip
+ request = stub_request 'HTTP_X_FORWARDED_FOR' => ''
+ assert_nil request.remote_ip
+
request = stub_request 'REMOTE_ADDR' => '127.0.0.1',
'HTTP_X_FORWARDED_FOR' => '3.4.5.6'
assert_equal '3.4.5.6', request.remote_ip
Something went wrong with that request. Please try again.