Skip to content

Loading…

the _escape method fails when called in ruby 1.8.7 #12

Closed
wants to merge 1 commit into from
@elisehuard

to_xs takes an encoding argument in 1.9.2 but doesn't in 1.8.7. This causes this method to fail with an ArgumentError when used in 1.8.7.

@niels

+1

Just ran into this myself & patch looks good.

@bbenezech

+1

Absolutely needed.

I just sent an email to the maintainer, I hope it gets fixed soon.

Does someone knows why is this hitting on us now? Builder hasn't changed since last year...

@niels

Presumably because Rails 3.1 bumped the bundler version it depends on to 3.0, thus automatically exposing many people to this problem.

@bbenezech

@nielsomat What does bundler has to do with that?

@niels

Sorry typo, meant builder of course.

@jimweirich
Owner

Several people have pinged me on this, so I'm looking into it. But for me, all the unit tests in 1.8.6, 1.8.7-p330, 1.8.7-p352, and 1.9.2-p136 are passing. I haven't been able to trigger the problem locally.

Do you have a unit test that exposes the problem code? What version & patch level of Ruby are you using when this happens?

@bbenezech

1.8.7-p334 for me.
just calling to_xml on a collection of records.

@elisehuard

@jimweirich: good point, I will create a test.

@elisehuard

My bad, I was unable to reproduce the situation in tests.
This is (almost certainly) caused by activesupport substituting to_xs by fast_xs in lib/activesupport/core_ext/string/xchar.rb, which takes no arguments.
http://fast-xs.rubyforge.org/String.html#method-i-fast_xs
So builder is not at fault here. We'll work with monkey patching on our project for now ...

@bbenezech

@elisehuard It makes sense. Thanks for investigating.

@jimweirich
Owner

Thanks. Looks like the problem will be handled on the rails end.

@jimweirich jimweirich closed this
@akwiatkowski

Thx for fix.

Thanks!

Thanxxx man!!!! its saved my time... :-)

@NZKoz

@jimweirich this is actually a bug in builder still and the fixes in rails will involve a clusterfuck of monkeypatches to avoid the problem

A Fix in builder to not assume that to_xs takes an argument would be great.

@pdc

At the moment the 3 packages in question are each saying that we should wait for a different package to supply a fix.

It would be more convenient to people using these packages if all three packages issued a fix that avoids the problem whether or not the other packages have, thus reducing the coupling between versions.

@jonkessler

@jimweirich Rails is clearly not patching this. I would really appreciate it if this change were pulled in to builder. Currently, we have to monkey patch _escape in all of our apps because of the conflict.

@jimweirich jimweirich reopened this
@jimweirich
Owner

I reopened this and applied the changes. builder-3.1.0 has the fix. Let me know if it works for you.

@jimweirich jimweirich closed this
@jonkessler

@jimweirich The change looks good, but I just realized that Rails locks you down to ~> 3.0.0, so I am unable to update my builder version. Would it be at all possible to make a patch release (3.0.1, for example), so that Rails users can use the fixed version?

@jimweirich
Owner

I pushed a 3.0.1 version of the gem. It's identical to 3.1.0, but should play nicely with rails requirements.

@jonkessler

Thanks, Jim! Works great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Showing with 5 additions and 1 deletion.
  1. +5 −1 lib/builder/xmlbase.rb
View
6 lib/builder/xmlbase.rb
@@ -132,7 +132,11 @@ def _escape(text)
end
else
def _escape(text)
- text.to_xs((@encoding != 'utf-8' or $KCODE != 'UTF8'))
+ if (text.method(:to_xs).arity == 0)
+ text.to_xs
+ else
+ text.to_xs((@encoding != 'utf-8' or $KCODE != 'UTF8'))
+ end
end
end
Something went wrong with that request. Please try again.