Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Unix sockets are not considered proxies, causing useless logging. #353

Merged
merged 1 commit into from

9 participants

@barttenbrinke

When you use the current preferred passenger setup (Nginx => Sockets => Passenger => Rails), Rack will not correctly identify the remote client's ip_address.
This pull request fixes this.

Before in my rails production.log:

Started GET "/users/login" for unix: at Fri Mar 02 15:03:05 +0100 2012
Processing by UsersController#login as HTML
Completed 200 OK in 8ms (Views: 4.7ms | ActiveRecord: 1.6ms)

After:

Started GET "/users/login" for 123.123.123.123: at Fri Mar 02 15:03:05 +0100 2012
Processing by UsersController#login as HTML
Completed 200 OK in 8ms (Views: 4.7ms | ActiveRecord: 1.6ms)
@barttenbrinke

Is there any way I can speed up the merge of this patch? Its hurting us pretty bad in our new Nginx / Passenger setup.

@morenocarullo

+1, this would be helpful.

@daenney

+1

@olafz

+1

@aswen

+1

@foeken

+1

@raggi raggi merged commit d404218 into rack:master
@raggi
Owner

Thanks.

@barttenbrinke

Thank you! Now I can remove my mixins :)

@Mordred

Hi. I think that i should be "^unix" not "^unix$" because it does not match "unix:" string

@barttenbrinke

No, it should be "unix". The "unix:" you see in the example is produced bij the Rails logger. Rack gets "unix".

@barttenbrinke

After typing this I looked, and I couldn't find the ":" in logger.rb, however, I do know that this works as I used the mixin. So either my example was wrong, or this works correctly by accident :)

@Mordred

We are running RoR hosting business and using nginx + passenger standalone connected through sockets. And Rack gets "unix:" with ":" at the end. But I noticed that sometimes it contains another characters (non-ascii) after ":" (e.g "unix:rent^D", "unix:-Compa^D" or "unix: 3.0.1^D").

I think that the regexp can be less strict, because sockets are always local and they can be mark as trusted.

@mlangenberg

Is there a reason why there has not been a Rack release since 6 months?

@mlangenberg

@Mordred is correct by the way, trusted_proxy?(ip) is being called with "unix:", and I have also seen "unix:" + garbage.

The regexp should be less strict, ^unix will do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 8 additions and 1 deletion.
  1. +1 −1  lib/rack/request.rb
  2. +7 −0 test/spec_request.rb
View
2  lib/rack/request.rb
@@ -309,7 +309,7 @@ def accept_encoding
end
def trusted_proxy?(ip)
- ip =~ /^127\.0\.0\.1$|^(10|172\.(1[6-9]|2[0-9]|30|31)|192\.168)\.|^::1$|^fd[0-9a-f]{2}:.+|^localhost$/i
+ ip =~ /^127\.0\.0\.1$|^(10|172\.(1[6-9]|2[0-9]|30|31)|192\.168)\.|^::1$|^fd[0-9a-f]{2}:.+|^localhost$|^unix$/i
end
def ip
View
7 test/spec_request.rb
@@ -892,6 +892,13 @@
res = mock.get '/', 'HTTP_X_FORWARDED_FOR' => '8.8.8.8, fe80::202:b3ff:fe1e:8329'
res.body.should.equal 'fe80::202:b3ff:fe1e:8329'
+
+ # Unix Sockets
+ res = mock.get '/',
+ 'REMOTE_ADDR' => 'unix',
+ 'HTTP_X_FORWARDED_FOR' => '3.4.5.6'
+ res.body.should.equal '3.4.5.6'
+
end
class MyRequest < Rack::Request
Something went wrong with that request. Please try again.