Rails 3 no longer URI escaping [ and ]

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[b]=1"

app.url_for(:controller=>'sequel', Rack::Utils.escape('a[b]')=>1)

=> "http://www.example.com/sequel?a%5Bb%5D=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%5Bb%5D=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

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

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

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?

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

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

Fixed here https://github.com/rails/rails/commit/52b71c01fd3c8a87152f55129a8cb3234190734a
Thanks you Jeremy ;).