Is there a reason Rails 3 no longer URI escapes [ and ]? Example: # Rails 2.3.5 $ script/console Loading development environment (Rails 2.3.5)>> app.url_for(:controller=>''sequel'', ''a[b]''=>1)=> "http://www.example.com/sequel?a%5Bb%5D=1">> app.url_for(:controller=>''sequel'', Rack::Utils.escape(''a[b]'')=>1)=> "http://www.example.com/sequel?a%255Bb%255D=1" # Rails 3.0.0 $ rails console Loading development environment (Rails 3.0.0) irb(main):001:0> app.url_for(:controller=>''sequel'', ''a[b]''=>1) => "http://www.example.com/sequel?a[b]=1" irb(main):002:0> app.url_for(:controller=>''sequel'', Rack::Utils.escape(''a[b]'')=>1) => "http://www.example.com/sequel?a%255Bb%255D=1" I''m not sure it this is intentional or if this is a bug. RFC 3986 (http://tools.ietf.org/rfc/rfc3986.txt) implies that [ and ] are reserved characters in the query part of URIs. reserved = gen-delims / sub-delims gen-delims = ":" / "/" / "?" / "#" / "[" / "]" / "@" In Section 2.2 it says: URI producing applications should percent-encode data octets that correspond to characters in the reserved set unless these characters are specifically allowed by the URI scheme to represent data in that component. The HTTP RFC (RFC 2616) references the older URI RFC (RFC 2396: http://www.ietf.org/rfc/rfc2396.txt), which doesn''t state that [ and ] are reserved characters in the query. But the HTTP RFC doesn''t specifically allow them either. As it predates RFC 3986, I''m not sure what is considered the best practice. Obviously Rails is escaping some characters and not others, and it isn''t using Rack::Utils.escape to do the escaping. Jeremy -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
> Obviously Rails is escaping some characters and not others, and it > isn''t using Rack::Utils.escape to do the escaping.I don''t believe this was done deliberately. A quick glance over the git history didn''t point anything obvious out. Do you want to take a go at rationalising the escaping code a bit? -- Cheers Koz -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
On 09.10.2010 03:56, Michael Koziarski wrote:>> Obviously Rails is escaping some characters and not others, and it >> isn''t using Rack::Utils.escape to do the escaping. >> > I don''t believe this was done deliberately. A quick glance over the > git history didn''t point anything obvious out. Do you want to take a > go at rationalising the escaping code a bit? > > >I already filed this "bug" around two weeks ago at lighthouse: https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/5690-should-be-escaped-in-urls I must admit that my suggested fix (use CGI.escape) does not really work as it escaped the slah (/) too. Corin -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
On Fri, Oct 8, 2010 at 7:56 PM, Michael Koziarski <michael@koziarski.com> wrote:>> Obviously Rails is escaping some characters and not others, and it >> isn''t using Rack::Utils.escape to do the escaping. > > I don''t believe this was done deliberately. A quick glance over the > git history didn''t point anything obvious out. Do you want to take a > go at rationalising the escaping code a bit?After some digging, I can say it was done delibritely: http://github.com/rails/rails/commit/856d2fd874d72dd9f83204affff4edfef3308361 Prehaps josh can enlighten us as to the reason why the change was made, since the commit message does not do so. Reversing that commit and http://github.com/rails/rails/commit/1ee9b40b18a0bed5bb10a0785f7e2730bac983f6 would probably fix things, if you decide to go that route. Jeremy -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
> After some digging, I can say it was done delibritely: > http://github.com/rails/rails/commit/856d2fd874d72dd9f83204affff4edfef3308361 > > Prehaps josh can enlighten us as to the reason why the change was > made, since the commit message does not do so.Nor does the message here: http://github.com/rails/rails/commit/4dee277a9bc05083de6c831cf9aae0846849ecda Those commits don''t really have any justification, so unless josh pipes up I''d be tempted to reverse them. However to play devil''s advocate, the spec isn''t exactly clear that they *should* be escaped, and nothing seems to break with them *not* being escaped. What makes you want them that way? Is it breaking your app some how?> Reversing that commit and > http://github.com/rails/rails/commit/1ee9b40b18a0bed5bb10a0785f7e2730bac983f6 > would probably fix things, if you decide to go that route.-- Cheers Koz -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
On Sun, Oct 10, 2010 at 7:43 PM, Michael Koziarski <michael@koziarski.com> wrote:>> After some digging, I can say it was done delibritely: >> http://github.com/rails/rails/commit/856d2fd874d72dd9f83204affff4edfef3308361 >> >> Prehaps josh can enlighten us as to the reason why the change was >> made, since the commit message does not do so. > > Nor does the message here: > http://github.com/rails/rails/commit/4dee277a9bc05083de6c831cf9aae0846849ecda > > Those commits don''t really have any justification, so unless josh > pipes up I''d be tempted to reverse them. > > However to play devil''s advocate, the spec isn''t exactly clear that > they *should* be escaped, and nothing seems to break with them *not* > being escaped. What makes you want them that way?As RFC 3986 is more recent and implies they should be escaped, and rack and cgi both escape them, that seems like the more logical choice. The current code specifically reverses the escaping done by cgi, so it''s actually doing more work. I can''t see a benefit for escaping the URI with rack and then unescaping just the brackets with a gsub, other than the unescaped URI looks nicer. If a nicer look is more important, why unescape just the brackets, why not unescape other characters where it wouldn''t break things?> Is it breaking your app some how?It breaks some test code, that''s all. Test code that works on 5 other web frameworks and previous versions of Rails. It''s not a big deal either way, but I wanted to know rails-core opinion before I decide whether to update my test code. Thanks, Jeremy -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
>> Is it breaking your app some how? > > It breaks some test code, that''s all. Test code that works on 5 other > web frameworks and previous versions of Rails. > > It''s not a big deal either way, but I wanted to know rails-core > opinion before I decide whether to update my test code. >I''ll see if I can get something a bit more definitive from josh as to why this was changed. Seems a toss up either way, but the current method is certainly a bit messy and at the very least we shouldn''t be escaping / unescaping -- Cheers Koz -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
On Sun, Oct 10, 2010 at 8:36 PM, Michael Koziarski <michael@koziarski.com> wrote:>>> Is it breaking your app some how? >> >> It breaks some test code, that''s all. Test code that works on 5 other >> web frameworks and previous versions of Rails. >> >> It''s not a big deal either way, but I wanted to know rails-core >> opinion before I decide whether to update my test code. >> > > I''ll see if I can get something a bit more definitive from josh as to > why this was changed. Seems a toss up either way, but the current > method is certainly a bit messy and at the very least we shouldn''t be > escaping / unescapingIt''s been a month since this was last discussed, and there doesn''t appear to have been any related changes made. Should I add a issue to lighthouse so that it''s easier to keep track of this? Jeremy -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Fixed here https://github.com/rails/rails/commit/52b71c01fd3c8a87152f55129a8cb3234190734a Thanks you Jeremy ;). On Tue, Nov 16, 2010 at 6:25 PM, Jeremy Evans <jeremyevans0@gmail.com> wrote:> On Sun, Oct 10, 2010 at 8:36 PM, Michael Koziarski > <michael@koziarski.com> wrote: >>>> Is it breaking your app some how? >>> >>> It breaks some test code, that''s all. Test code that works on 5 other >>> web frameworks and previous versions of Rails. >>> >>> It''s not a big deal either way, but I wanted to know rails-core >>> opinion before I decide whether to update my test code. >>> >> >> I''ll see if I can get something a bit more definitive from josh as to >> why this was changed. Seems a toss up either way, but the current >> method is certainly a bit messy and at the very least we shouldn''t be >> escaping / unescaping > > It''s been a month since this was last discussed, and there doesn''t > appear to have been any related changes made. Should I add a issue to > lighthouse so that it''s easier to keep track of this? > > Jeremy > > -- > You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. > To post to this group, send email to rubyonrails-core@googlegroups.com. > To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. > For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. > >-- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.