This project is archived and is in readonly mode.
[PATCH] local_request? does not detect local IPv6 connections
Reported by Christopher Owen | September 25th, 2009 @ 02:13 AM | in 3.0.2
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.
Comments and changes to this ticket
-
CancelProfileIsBroken September 25th, 2009 @ 11:50 AM
- Tag changed from ipv6, local_request, patch to bugmash, ipv6, local_request, patch
-
Blue Box Jesse September 27th, 2009 @ 12:54 AM
Test doesn't apply on 2-3-stable.
Test does apply on master.
+1 for being a good idea.
-
Blue Box Chris September 27th, 2009 @ 01:04 AM
+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 September 27th, 2009 @ 03:28 AM
-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 Owen September 28th, 2009 @ 12:22 AM
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.
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 December 9th, 2009 @ 06:55 PM
+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) December 26th, 2009 @ 02:46 PM
I have modified Christopher Owen's patch to simplify things up, as only detecting
127.0.0.1
and::1
as 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
actionpack
inmaster
changed a lot compared to2-3-stable
-
Prem Sichanugrist (sikachu) January 16th, 2010 @ 01:54 PM
Hello,
Just saw this ticket is in bugmash, and realize that the patch doesn't apply on
master
anymore. So here I'm reuploading the patch for2-3-stable
andmaster
Thank you :)
-
Rizwan Reza January 17th, 2010 @ 12:49 AM
- 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 January 17th, 2010 @ 03:55 PM
- Assigned user set to José Valim
-
Repository January 17th, 2010 @ 03:58 PM
(from [eb67532bc1c1ce5c494b71b980c04c8aa83efb74]) Make local_request? to returns true when facing ::1 IPv6 address [#3257 status:resolved]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/eb67532bc1c1ce5c494b71b980c04c... -
José Valim January 17th, 2010 @ 04:02 PM
- State changed from new to incomplete
2-3-stable patch applies, but I have a failure after applying it.
-
Prem Sichanugrist (sikachu) January 18th, 2010 @ 04:53 AM
I've looking into it and fixed the problem. Here is the new patch for 2-3-stable.
Thank you
-
Repository January 18th, 2010 @ 07:31 AM
(from [6012e575bbdd96361ed486516a34fb220b27319f]) Make local_request? to returns true when facing ::1 IPv6 address [#3257 status:resolved]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/6012e575bbdd96361ed486516a34fb... -
José Valim January 18th, 2010 @ 07:36 AM
- State changed from incomplete to committed
And applied! Thanks!
-
John Firebaugh June 4th, 2010 @ 03:19 AM
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) June 4th, 2010 @ 03:31 AM
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 June 4th, 2010 @ 06:29 PM
%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) June 5th, 2010 @ 04:07 AM
- 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) June 6th, 2010 @ 10:10 PM
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
::1
and0:0:0:0:0:0:0:1
will be sent to loopback interface but for IPv4,127.0.0.0/8
could be all referred to the localhost. So I've patched them up as well.I've ran the test on both
2-3-stable
andmaster
, 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
-
Prem Sichanugrist (sikachu) June 8th, 2010 @ 06:34 PM
- Milestone cleared.
Can I get this patch in before RC? Thanks :)
-
Repository June 8th, 2010 @ 06:47 PM
- 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 state:resolved]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/0f44d37d04063bdae12c48e85f3402... -
Jeremy Kemper October 15th, 2010 @ 11:01 PM
- Milestone set to 3.0.2
- Importance changed from to
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Attachments
Tags
Referenced by
- 3257 [PATCH] local_request? does not detect local IPv6 connections [#3257 state:resolved]
- 3257 [PATCH] local_request? does not detect local IPv6 connections (from [eb67532bc1c1ce5c494b71b980c04c8aa83efb74]) Make lo...
- 3257 [PATCH] local_request? does not detect local IPv6 connections (from [6012e575bbdd96361ed486516a34fb220b27319f]) Make lo...