From: "NARUSE, Yui" <naruse@...>
Date: 2009-12-17T11:20:32+09:00
Subject: [ruby-core:27204] Re: [PATCH] fix CGI::escape to work with blocks, 	avoid dollar variables

--000e0cd1132acaa3ef047ae341f1
Content-Type: text/plain; charset=ISO-8859-1

On Thu, Dec 17, 2009 at 12:28 AM, Gaston Ramos <ramos.gaston@gmail.com>wrote:

> Hi Ruby-Core, I attach a path that solve this problem:
>
> class String
>  alias :old_gsub :gsub
>    def gsub(*args, &block)
>       old_gsub(*args, &block)
>    end
> end
>
> require 'cgi'
> CGI.escape("An Interview with Criteri&#8230;")
>
> NoMethodError: undefined method `size' for nil:NilClass
>  from /usr/lib/ruby/1.8/cgi.rb:343:in `escape'
>  from (irb):4:in `old_gsub'
>  from (irb):4:in `gsub'
>  from /usr/lib/ruby/1.8/cgi.rb:342:in `escape'
> from (irb):9
>
> As you can see CGI.escape won't work well when we use gsub and blocks, this
> issue was discussed in this thread:
>
>
> http://www.justskins.com/forums/bug-when-rerouting-string-gsub-with-a-block-using-1-a-52852.html


Even if you patch cgi.rb, you can use only matched string ($&) which is
passed as block parameter.
There are $~ (Regexp.last_match), $` (pre_match), $' (post_match) and $1,
$2, $3 and so on.
So your patch is fortunately adopted to cgi.rb, but not universal.

So I'm against such a sectional patch.


> where Matz say "Then don't use ugly dollar variables"


When Matz say so?
That statement doesn't face up to real situation, described above.
dollar variables may be ugly but required.


> so, dollar variables are
> problematic in this case, I changed the dollar variables by a block
> argument
> variable and the problem was solved, I added 2 tests for CGI.escape and
> CGI.unescape that reproduce this "problem", If you agree with this patch I
> could
> do other new patch in order to avoid all dollar variables inside CGI lib.
>
> And there is a bug in ActiveSupport realted with this issue:
>
> https://rails.lighthouseapp.com/projects/8994/tickets/3475-activesupportmultibytecharsgsub-fails-while-stringgsub-works
>

So current answer is "Don't change String#gsub when you controll all
things".

See also this ticket (but this is in Japanese...)
http://redmine.ruby-lang.org/issues/show/546

-- 
NARUSE, Yui
naruse@airemix.jp

--000e0cd1132acaa3ef047ae341f1
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

<div class=3D"gmail_quote">On Thu, Dec 17, 2009 at 12:28 AM, Gaston Ramos <=
span dir=3D"ltr">&lt;<a href=3D"mailto:ramos.gaston@gmail.com">ramos.gaston=
@gmail.com</a>&gt;</span> wrote:<br><blockquote class=3D"gmail_quote" style=
=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Hi Ruby-Core, I attach a path that solve this problem:<br>
<br>
class String<br>
 =A0alias :old_gsub :gsub<br>
 =A0 =A0def gsub(*args, &amp;block)<br>
 =A0 =A0 =A0 old_gsub(*args, &amp;block)<br>
 =A0 =A0end<br>
end<br>
<br>
require &#39;cgi&#39;<br>
CGI.escape(&quot;An Interview with Criteri&amp;#8230;&quot;)<br>
<br>
NoMethodError: undefined method `size&#39; for nil:NilClass<br>
 =A0from /usr/lib/ruby/1.8/cgi.rb:343:in `escape&#39;<br>
 =A0from (irb):4:in `old_gsub&#39;<br>
 =A0from (irb):4:in `gsub&#39;<br>
 =A0from /usr/lib/ruby/1.8/cgi.rb:342:in `escape&#39;<br>
from (irb):9<br>
<br>
As you can see CGI.escape won&#39;t work well when we use gsub and blocks, =
this<br>
issue was discussed in this thread:<br>
<br>
<a href=3D"http://www.justskins.com/forums/bug-when-rerouting-string-gsub-w=
ith-a-block-using-1-a-52852.html" target=3D"_blank">http://www.justskins.co=
m/forums/bug-when-rerouting-string-gsub-with-a-block-using-1-a-52852.html</=
a></blockquote>
<div><br></div><div><div>Even if you patch cgi.rb, you can use only matched=
 string ($&amp;) which is passed as block parameter.</div><div>There are $~=
 (Regexp.last_match), $` (pre_match), $&#39; (post_match) and $1, $2, $3 an=
d so on.</div>
<div>So your patch is fortunately adopted to cgi.rb, but not universal.</di=
v><div><br></div><div>So I&#39;m against such a sectional patch.</div></div=
><div>=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex=
;border-left:1px #ccc solid;padding-left:1ex;">

where Matz say &quot;Then don&#39;t use ugly dollar variables&quot;</blockq=
uote><div><br></div><div>When Matz say so?</div><div>That statement doesn&#=
39;t face up to real situation, described above.</div><div>dollar variables=
 may be ugly but required.</div>
<div>=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;=
border-left:1px #ccc solid;padding-left:1ex;">so, dollar variables are<br>
problematic in this case, I changed the dollar variables by a block argumen=
t<br>
variable and the problem was solved, I added 2 tests for CGI.escape and<br>
CGI.unescape that reproduce this &quot;problem&quot;, If you agree with thi=
s patch I could<br>
do other new patch in order to avoid all dollar variables inside CGI lib.<b=
r>
<br>
And there is a bug in ActiveSupport realted with this issue:<br>
<a href=3D"https://rails.lighthouseapp.com/projects/8994/tickets/3475-activ=
esupportmultibytecharsgsub-fails-while-stringgsub-works" target=3D"_blank">=
https://rails.lighthouseapp.com/projects/8994/tickets/3475-activesupportmul=
tibytecharsgsub-fails-while-stringgsub-works</a><br>
</blockquote></div><div><br></div>So current answer is &quot;Don&#39;t chan=
ge String#gsub when you controll all things&quot;.<div><br></div><div>See a=
lso this ticket (but this is in Japanese...)</div><div><a href=3D"http://re=
dmine.ruby-lang.org/issues/show/546">http://redmine.ruby-lang.org/issues/sh=
ow/546</a><br clear=3D"all">
<br>-- <br>NARUSE, Yui<br><a href=3D"mailto:naruse@airemix.jp">naruse@airem=
ix.jp</a><br>
</div>

--000e0cd1132acaa3ef047ae341f1--