This project is archived and is in readonly mode.
[PATCH] local_request? does not detect local IPv6 connections
When deciding how to handle server exceptions, the implementation of ActionDispatch::ShowExceptions#local_request? tries to match the request IP address to '127.0.0.1'. This approach does not consider IPv6 loopback connections (from ::1) to be local.
The main ramification is that when developing locally against a rails environment where config.action_controller.consider_all_requests_local is false, local connections from 127.0.0.1 still exhibit development error view rendering, whereas connections from ::1 use production error rendering.
This can be confusing when looking at an error using Safari, which uses the IPv6 interface if available, versus Firefox which uses the IPv4 loopback interface. Safari will render the non-local error view while FF will use the local/development view.
I've attached a patch and test for a version of local_request? that identifies all connections from a loopback address. It also uses a strictly precise definition for IPv4 connections such that any request from the 127.0.0.0/8 subnet is considered local. I'm a little concerned that such a precise treatment could have unintended consequences in a production environment -- I'd welcome any discussion in that regard.
-
local_request_ipv6_fix.diff
3.13 KB
-
CancelProfileIsBroken- Tag changed from ipv6, local_request, patch to bugmash, ipv6, local_request, patch
-
Blue Box JesseTest doesn't apply on 2-3-stable.
Test does apply on master.
+1 for being a good idea.
-
Blue Box Chris
+1 good idea as Ipv6 becomes more popular in the months to come.
Applying Fix local_request? for ipv6 connections
.dotest/patch:63: trailing whitespace. { .dotest/patch:64: trailing whitespace.:IPv4 => ['127.0.0.1', '127.0.0.34', '127.0.0.254'],.dotest/patch:65: trailing whitespace.
:IPv6 => ['::1', '0000::1', '0:0:0:0:0:0:0:1'].dotest/patch:66: trailing whitespace. }.each do |addr_family, addrs| .dotest/patch:67: trailing whitespace.
addrs.to_a.each do |addr|warning: 5 lines add whitespace errors.
-
John Pignata
-1 on the patch as it doesn't apply
Some thoughts on the idea -- preparing for ipv6 in general is very good.
I don't think you should treat all traffic originating from 127/8 as local - while I don't think it is harmful, I don't see a case where it is useful. The converse of traffic being destined to 127/8 makes more sense.
I also think to actually have this be a problem in development takes a bunch of stars to be in alignment including -b ::1 being passed to script/server explicitly, the system resolver returning ::1 for localhost and treat all hosts as local being set to false in development. Seems like an unlikely occurrence. As systems will support IPv4 for quite a while, I'm not sure if this fix is necessary.
-
Christopher OwenI also think to actually have this be a problem in development takes a bunch of stars to be in alignment including -b ::1 being passed to script/server explicitly, the system resolver returning ::1 for localhost and treat all hosts as local being set to false in development. Seems like an unlikely occurrence. As systems will support IPv4 for quite a while, I'm not sure if this fix is necessary.
Just FYI: My setup is Mac OS X 1.5 running development environments under Apache HTTPD via Passenger. The reported behaviour is the default behaviour under this environment, I didn't really have to jump through hoops to make it happen. I agree that it isn't a major issue and it's prudent to be cautious in this case. It would be nice if the behaviour was correct and consistent though; it would have saved some time and confusion.
-
jerrett
+1 on fixing this. I ran into it because snow leopard puts ::1 localhost in hosts file by default.
No "stars have to align" for this to happen, the only thing that has to happen is to have an entry in your hosts file that points localhost to ::1, which if anything will become more common, not less. Seems checking IPv4 and IPv6 is an easy change, and futureproofing is never a bad idea.
-
Prem Sichanugrist (sikachu)I have modified Christopher Owen's patch to simplify things up, as only detecting
127.0.0.1and::1as a local address. I don't really think we need to complicate thing up here.I have separated the patch into two files, since the structure of
actionpackinmasterchanged a lot compared to2-3-stable -
Prem Sichanugrist (sikachu)
Hello,
Just saw this ticket is in bugmash, and realize that the patch doesn't apply on
masteranymore. So here I'm reuploading the patch for2-3-stableandmasterThank you :)
-
Rizwan Reza- Tag changed from bugmash, ipv6, local_request, patch to bugmash, ipv6, local_request, patch, review
- Title changed from local_request? does not detect local IPv6 connections to [PATCH] local_request? does not detect local IPv6 connections
-
José Valim- Assigned user set to José Valim
-
Repository
(from [eb67532bc1c1ce5c494b71b980c04c8aa83efb74]) Make local_request? to returns true when facing ::1 IPv6 address [#3257 [PATCH] local_request? does not detect local IPv6 connections status:resolved]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/eb67532bc1c1ce5c494b71b980c04c... -
José Valim
- State changed from new to incomplete
2-3-stable patch applies, but I have a failure after applying it.
-
Prem Sichanugrist (sikachu)
I've looking into it and fixed the problem. Here is the new patch for 2-3-stable.
Thank you
-
Repository
(from [6012e575bbdd96361ed486516a34fb220b27319f]) Make local_request? to returns true when facing ::1 IPv6 address [#3257 [PATCH] local_request? does not detect local IPv6 connections status:resolved]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/6012e575bbdd96361ed486516a34fb... -
-
John Firebaugh
These patches don't work for me; I'm getting '0:0:0:0:0:0:0:1%0' as the remote address.
-
Prem Sichanugrist (sikachu)
I see. So what's that trailing %0 mean?
Will try to read spec and provide a patch
Sent from my iPhone
On 4 มิ.ย. 2553, at 9:19, "Lighthouse" <no-
reply@lighthouseapp.com> wrote: -
John Firebaugh
%0 is apparently a scope zone ID:
http://tools.ietf.org/html/rfc4007#section-11
I don't know why it's included for me, nor why I get the expanded zeros format rather than ::1. This is with JRuby 1.5 on Snow Leopard, FF or Chrome connecting to http://localhost:3000/. If I connect to http://0.0.0.0/ the remote address is 127.0.0.1.
-
Prem Sichanugrist (sikachu)
- Milestone set to 2.3.9
- State changed from committed to verified
- Assigned user changed from José Valim to Prem Sichanugrist (sikachu)
Ok, I have tested and can reproduce this bug. Got this one from script/server log
Processing TopicsController#index (for 0:0:0:0:0:0:0:1%0 at 2010-06-05 10:05:26) [GET] Rendering topics/index Completed in 124ms (View: 114 | 200 OK [http://localhost/topics]Will try to 'catch all' of the local request. Will provide the patch soon.
-
Prem Sichanugrist (sikachu)
OK, I've provide the patch to support both full notation and short notation of the IPv6.
Also, after I've read the spec here: http://en.wikipedia.org/wiki/Localhost and here: http://tools.ietf.org/html/rfc4291#section-2.5.3 It seems like for IPv6 only
::1and0:0:0:0:0:0:0:1will be sent to loopback interface but for IPv4,127.0.0.0/8could be all referred to the localhost. So I've patched them up as well.I've ran the test on both
2-3-stableandmaster, both seems to be fine. However, at the moment I can't get 100% tests to pass on the actionpack because somebody has break something. Just FYI.I'm expected this to be committed before RailsConf, Thanks! :D
-
-
Repository
- State changed from verified to resolved
(from [0f44d37d04063bdae12c48e85f34024775c6a9f9]) Make sure that rails recognized the full notation of IPv6 loopback address, and recognize 127.0.0.0/8 in IPv4
[#3257 [PATCH] local_request? does not detect local IPv6 connections state:resolved]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/0f44d37d04063bdae12c48e85f3402... -
