Skip to content

remote_ip was failing under unix socks (puma) #7234

Closed
wants to merge 1 commit into from

5 participants

@SamSaffron

was crashing under puma, needed to ensure we always get an ip even if we are being served over a unix sock where remote_addr is empty

@carlosantoniodasilva
Ruby on Rails member

Can you add tests for it, just making sure remote_addrs is empty and client ip address is used, etc?

/cc @pixeltrix

@frodsan
frodsan commented Oct 26, 2012

@SamSaffron ping! Could you add the tests that Carlos mentioned? Thanks.

@steveklabnik
Ruby on Rails member

@evanphx hey do you have any thoughts on this? When I add this patch, I get failures like this:

 1) Failure:
test_0002_remote ip(RequestTest) [/Users/steve/src/rails/actionpack/test/dispatch/request_test.rb:51]:
Expected: nil
  Actual: "172.16.0.1"

  2) Failure:
test_0004_remote ip v6(RequestTest) [/Users/steve/src/rails/actionpack/test/dispatch/request_test.rb:146]:
Expected: nil
  Actual: "::1"

  3) Failure:
test_0007_remote ip v6 with user specified trusted proxies String(RequestTest) [/Users/steve/src/rails/actionpack/test/dispatch/request_test.rb:235]:
--- expected
+++ actual
@@ -1 +1 @@
-nil
+"fe80:0000:0000:0000:0202:b3ff:fe1e:8329"


  4) Failure:
test_0008_remote ip with user specified trusted proxies Regexp(RequestTest) [/Users/steve/src/rails/actionpack/test/dispatch/request_test.rb:246]:
Expected: nil
  Actual: "67.205.106.73"

  5) Failure:
test_0009_remote ip v6 with user specified trusted proxies Regexp(RequestTest) [/Users/steve/src/rails/actionpack/test/dispatch/request_test.rb:257]:
--- expected
+++ actual
@@ -1 +1 @@
-nil
+"fe80:0000:0000:0000:0202:b3ff:fe1e:8329"

They seem to imply that we do expect nil in certain cases. Should puma be assuming that we always get something back?

@SamSaffron

Sorry guys, I am fine to close this PR, since this happened I moved of puma and started using thin, sockets are not really an option for me cause I am using an off box load balancer that needs to pipe websockets back to the app.

I think the bug is not going to impact anyone in production and only a very small handful of devs.

@steveklabnik
Ruby on Rails member

Okay then. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.