Just use CGI.escape/unescape. #140

Merged
merged 1 commit into from May 2, 2011

Projects

None yet

6 participants

@jfirebaugh
Contributor

The previous definitions were virtually identical to those provided
by CGI in 1.8.7/1.9.2, except that Rack::Utils.escape failed on non-
UTF8 encoded input.

Tested on 1.8.7, 1.9.2 and JRuby.

See comment thread here: d8a980f

@jfirebaugh jfirebaugh Just use CGI.escape/unescape.
The previous definitions were virtually identical to those provided
by CGI in 1.8.7/1.9.2, except that Rack::Utils.escape failed on non-
UTF8 encoded input.

Tested on 1.8.7, 1.9.2 and JRuby.
0700cd0
@mculp
mculp commented Apr 11, 2011

I just found this page trying to figure out an issue with Capybara:

  Encoding::CompatibilityError:
   incompatible encoding regexp match (UTF-8 regexp with ASCII-8BIT string)

I fixed the issue the same way locally, and will probably end up forking for now. +1

@tenderlove tenderlove merged commit e7c8454 into rack:master May 2, 2011
@josh
Contributor
josh commented May 2, 2011

I think @chneukirchen has turned this down many a times

@jfirebaugh
Contributor

Yeah, we discussed it here: d8a980f

How about backporting URI.encode_www_form_component for 1.8.x?

@tenderlove
Member

Oops. I guess I shouldn't have merged it then. :-(

I would like a rack release soon. Can we please get people who are less green than me to review and reject or accept pull requests. I don't want to make more mistakes. :-(

@josh
Contributor
josh commented May 2, 2011

I'm indifferent, but I just remembered this being controversial.

@tenderlove
Member

From the comments it seemed good. Also, the code looks as if it was taken from cgi.rb in the first place and just slapped a /u on the end (which is wrong). We should probably backport and conditionally include this for 1.8.6 users.

@chneukirchen
Member

:(

@cactus
cactus commented May 2, 2011

it seems the complaint was sourcing the huge CGI library itself (2k LOC?), just to get 4 lines or so.
Maybe just require 'cgi/util' instead of all of cgi (much smaller than all of cgi at 131 LOC without comments)?

@chneukirchen
Member

cactus
reply@reply.github.com
writes:

it seems the complaint was sourcing the huge CGI library itself (2k LOC?), just to get 4 lines or so.
Maybe just require 'cgi/util' instead of all of cgi (much smaller than all of cgi at 131 LOC without comments)?

That will not work on 1.8.

Christian Neukirchen chneukirchen@gmail.com http://chneukirchen.org

@jfirebaugh
Contributor

Yeah, 1.8 has only cgi.rb.

@tenderlove
Member

doh. I will fix

@cactus
cactus commented May 2, 2011

:(

@tenderlove
Member

@cactus DON'T BE SAD! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment