undefined method `bytesize' for nil:NilClass in 3.0.8 and 3.0.9 with Rakismet gem

[bnferguson]

Hello,

Upon upgrading to 3.0.8 (3.0.9 is also affected) I found that the spam checking using Rakismet no longer worked. Was getting breaks with this exception when calling the "spam?" instance method:

https://gist.github.com/de84c1242fdcffb98492 (this is in a stalker job, looks like it also threw hoptoad for a loop when trying to handle the error)

Would normally look to these gems for the fix but considering that this was a tiny version increment and it works in 3.0.7 and below I figured this might be the place.

[bnferguson]

Looks like the CGI.escape in this bit from Rakismet is causing the problem:

      akismet = URI.parse(call_url(function))
      _, response = Net::HTTP.start(akismet.host) do |http|
        data = args.map { |k,v| "#{k}=#{CGI.escape(v.to_s)}" }.join('&')
        http.post(akismet.path, data, Rakismet.headers)
      end

[dmathieu]

Are you sure it doesn't fail when you're in 3.0.7 and it's not some akismet unavailability ?
It seems the value provided in CGI.escape is nil, which is causing the error. But that wouldn't be impacted by any rails upgrade.

BTW I use rakismet in a 3.1 app and it works properly.

[tardate]

Looks familiar;-) Seems to be the same SafeBuffer issue as @dolzenko found with Rack::Utils.escape (see #1555)

The stack trace indicates that you have a SafeBuffer going in ( output_safety.rb:123:in `gsub' ), and internally CGI.escape is doing a gsub that attempts to substitute $1 (doesn't work with SafeBuffer).

I think v.to_str should ensure you are sending 'unsafe' strings into CGI.escape

[bnferguson]

@tardate - That does look to be the issue! May have to submit a patch for Rakismet.

@dmathieu - Baffling that it works with 3.1. It is indeed linked to rails version, currently running 3.0.7 with no hitches. May try patching Rakismet and see if that fixes it.

I'm going to go ahead and close this since it looks like it's a dupe. Thanks guys!

[dmathieu]

Actually you should try running with 3-0-stable git. There has been several changes to SafeBuffer since the last releases.
My application works with 3-1-stable git. I haven't tried it on the released rc.

[jrochkind]

Shouldn't a SafeBuffer masquerade just like a String?

I am running into this same issue in rails 3.2.6. Have a SafeBuffer, which I'm used to being able to treat just like a string, which winds up being passed to an external library as a String, and CGI.escape is called on it.

Indeed "undefined method `bytesize' for nil:NilClass"

This is not a bug in SafeBuffer, that you can't call stdlib CGI.escape on it, if it happens to be passed to a library that wants to do that?

internally CGI.escape is doing a gsub that attempts to substitute $1 (doesn't work with SafeBuffer).

How come?

[jrochkind]

To make matters worse, you'd think calling "#to_s" on anything ought to give you a String, right?

So if you're not sure if you have a SafeBuffer or not, just call #to_s on it, and you'll have something you can pass safely to CGI.escape.

Nope, calling #to_s on a SafeBuffer, you still have a SafeBuffer. Argh.

There seems to be no clear way to turn a SafeBuffer back into an ordinary string?

[tardate]

@jrochkind I think you are looking for #to_str (not #to_s) to get an ordinary string back from a safebuffer

String.new.html_safe.to_s.class
=> ActiveSupport::SafeBuffer 
String.new.html_safe.to_str.class
=> String 

PS: see #1555 if you want more details of why these things break with SafeBuffer.

[jrochkind]

Thanks @tardate.

Believe it or not, this is not the first time I've run into trouble with gsub and magic $1 variables, grr, the gsub $1-in-block magic is very fragile, and breaks easily. I guess that's really the bug, it's a mistaken feature in ruby (IMO) if it's going to be so fragile to extension and sub-classing etc. (It also breaks when you try to pass a &block by reference to gsub, instead of using a literal block, which is where I ran into this general problem area before.) Grr.

[jrochkind]

Hmm, but this is weird @tardate, the disposition on #1555 (which I see I commented on a year ago too!) says that calling gsub on a SafeBuffer should raise with an immediate "unsupported method" exception. Which is exactly what it did a year ago when I commented on the issue.

But now with Rails 3.2.6, it's does not seem to be doing so -- or CGI.escape would cause that exception to be raised (at least immediately revealing the problem, that it was a SafeBuffer), instead of succesfully doing the gsub but then having $1/$2/etc break, right?

So what's up, you think?

[tardate]

@jrochkind oh, yeah, we've been here before;-)

looks like the unsupported method exception went in on #2248 and #2931 but then was taken out soon after. Seems to have caused havoc in a whole range of subsystems, and this was never resolved.

commit e05d4ce
Author: Vijay Dev vijaydev.cse@gmail.com
Date: Fri Sep 9 01:05:07 2011 +0530

revert the changes from c60995f3 - related to marking sub,gsub as unavailable to use with safe strings

commit 6b010c2
Author: José Valim jose.valim@gmail.com
Date: Thu Sep 8 20:49:08 2011 +0200

Revert removing gsub and sub from safe buffer.

[jrochkind]

Ugh, so gsub is still there, but doesn't actually work if you use
the block form (at least not if you try to use $1 etc). What a mess! I
liked it better when the method wasn't there, at least it would fail
with a clear exception, instead of a mystery unexpected nil error.
SafeBuffer's are ordinarily so interchangeable with Strings, it's
quite easy to lose track of which you have. I still blame ruby's magic
$1 variables in combination with gsub block form for being impossible
to support in any even slightly unusual circumstances, but it seems
like there's got to be something better Rails can do. I dunno.

It would be nice if, when reverting a commit, someone would at least
put in their revert commit message WHY they were reverting it.

On Jul 25, 2012, at 8:46 PM, Paul Gallagher wrote:

@jrochkind oh, yeah, we've been here before;-)

looks like the unsupported method exception went in on #2248 and
#2931 but then was taken out soon after. Seems to have caused havoc
in a whole range of subsystems, and this was never resolved.

commit e05d4ce
Author: Vijay Dev vijaydev.cse@gmail.com
Date: Fri Sep 9 01:05:07 2011 +0530

revert the changes from c60995f - related to marking sub,gsub as
unavailable to use with safe strings

commit 6b010c2
Author: José Valim jose.valim@gmail.com
Date: Thu Sep 8 20:49:08 2011 +0200

Revert removing gsub and sub from safe buffer.

---

Reply to this email directly or view it on GitHub:
#1739 (comment)

[grosser]

Patch to solve it for CGI and Rack::Utils http://grosser.it/2012/08/16/rackutils-escape-unescape-cgi-escapeunescapeescapehtml-vs-undefined-method-bytesize-for-nil/