Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Latest Ruby/Rails issue... #22

Closed
RichIsOnRails opened this issue Apr 18, 2013 · 16 comments
Closed

Latest Ruby/Rails issue... #22

RichIsOnRails opened this issue Apr 18, 2013 · 16 comments

Comments

@RichIsOnRails
Copy link

Ruby 1.9.3p392, Rails 3.2.13, I get the following error on some products:

uninitialized constant REXML::Text::Document

The line that causes it is this:
@Items = client.lookup @item.asin

client is client = ASIN::Client.instance

@RichIsOnRails
Copy link
Author

I was able to replicate this without Rails by copy pasting the code from the readme and changing to my credentials, see a partial stacktrace below:

C:/Ruby193/lib/ruby/1.9.1/rexml/text.rb:386:in `block in unnormalize': uninitialized constant REXML::Text::Document (NameError)
        from C:/Ruby193/lib/ruby/1.9.1/rexml/text.rb:384:in `gsub'
        from C:/Ruby193/lib/ruby/1.9.1/rexml/text.rb:384:in `unnormalize'
        from C:/Ruby193/lib/ruby/gems/1.9.1/gems/crack-0.3.2/lib/crack/xml.rb:185:in `unnormalize_xml_entities'
        from C:/Ruby193/lib/ruby/gems/1.9.1/gems/crack-0.3.2/lib/crack/xml.rb:82:in `to_hash'
        from C:/Ruby193/lib/ruby/gems/1.9.1/gems/crack-0.3.2/lib/crack/xml.rb:110:in `merge!'
        from C:/Ruby193/lib/ruby/gems/1.9.1/gems/crack-0.3.2/lib/crack/xml.rb:110:in `block in to_hash'
        from C:/Ruby193/lib/ruby/gems/1.9.1/gems/crack-0.3.2/lib/crack/xml.rb:108:in `each'
        from C:/Ruby193/lib/ruby/gems/1.9.1/gems/crack-0.3.2/lib/crack/xml.rb:108:in `to_hash'
        from C:/Ruby193/lib/ruby/gems/1.9.1/gems/crack-0.3.2/lib/crack/xml.rb:110:in `merge!'
        from C:/Ruby193/lib/ruby/gems/1.9.1/gems/crack-0.3.2/lib/crack/xml.rb:110:in `block in to_hash'
        from C:/Ruby193/lib/ruby/gems/1.9.1/gems/crack-0.3.2/lib/crack/xml.rb:108:in `each'

@phoet
Copy link
Owner

phoet commented Apr 19, 2013

this is an issue with crack jnunemaker/crack#42

@phoet phoet closed this as completed Apr 19, 2013
@welike
Copy link

welike commented Apr 25, 2013

How about instead of just closing the issue, you drop back to a version of crack that does not introduce this regression. This seems to be a common response in open source software these days where the finger is simply pointed elsewhere.

The facts are, asin is broken for people. You have control over what dependencies to use, and which versions of them to use. Could you please reopen and fix this?

@kevinelliott
Copy link

I agree here too.

@phoet
Copy link
Owner

phoet commented Apr 25, 2013

the isse seems to be related to changes in ruby, so what should i do about that?
feel free to provide a patch to fix this issue or the dependency to crack.

kurtfunai commented 16 days ago
Seems related to the changes in ruby-1.9.3-p392 release:
http://www.ruby-lang.org/en/news/2013/02/22/ruby-1-9-3-p392-is-released/
"Entity expansion DoS vulnerability in REXML (XML bomb)"

Changing my local version back to 1.9.3-p385 fixed the issues I was experiencing.

@RichIsOnRails
Copy link
Author

WeLike, while i do agree with your general assessment of certain gem developers finger pointing instead of fixing, phoet is right about ruby breaking stuff. The latest release of Ruby had a security fix that made crack incompatible. The crack team has been unresponsive in fixing this (it's a 1 line code change.)

If you want to fix ASIN on the latest version of ruby, you can use my fork of crack which has the fix for crack.

https://github.com/RichIsOnRails/crack

That's how I ultimately got things running again.

@phoet
Copy link
Owner

phoet commented Apr 27, 2013

guys, this software is opensource and i dedicate my spare time to maintain it.

unless you know me in person, please do not post such stuff in the issues.

it is not an issue with the library, its a thrd party dependency and i am not maintaining windows at all.

so if you want to fix this i am happy to integrate your pull requests as fast as i can.

@kevinelliott
Copy link

I think you are missing our point. Most of us are involved in open source, and can appreciate your time and efforts here. The point we are making is that as a gem maintainer, you have the opportunity to define your dependencies and lock them to known working versions. You have chosen to lock the third-party dependency to a version that is broken, and thus indirectly you are also held accountable for it breaking. It seems strange to me that you feel it's ok that there is a broken dependency that only works in some environments under very strict conditions (a particular ruby version).

You should be working with the third-party dependency's team to resolve this (since as users, we can only complain, or submit a fork's pull request with a gem dependency change), but also in the mean time mitigating the issue by locking to a previous version of the third-party dependency that does not have this regression. As a consumer of the third-party gem, you have more clout than we do as complaining users of your gem. Their lack of attention to the issue is proof of this :(

Again, I'm not being critical of you as a person (despite how I may be presenting myself here), and do appreciate your efforts and time that you have spent to make this project available to all of us.

@phoet
Copy link
Owner

phoet commented Apr 27, 2013

instead of compaining you could just submit a patch with a less strict dependency to the broken library like it was done before #13

@kurtfunai
Copy link
Collaborator

I'm going to jump onto @phoet's side here.
The version of crack used by asin was stable and working. As far as I know, there has not been any recent releases that cause this dependency to break. The last commit to crack was 4 months ago [ https://github.com/jnunemaker/crack/commits/ ].

It was the newest release of ruby that caused an issue with the crack gem, and developers have been discussing this change (and submitted a pull request) to fix the issue. So far that change has not been accepted.
The upgrade from 1.9.3-p385 to 1.9.3-p392 (or to ruby-2.0.0) causes this error.

If you'd like to see this fixed, please make a comment in the pull request on the crack gem.
jnunemaker/crack#42

@rubiii
Copy link
Contributor

rubiii commented May 8, 2013

@phoet savonrb/nori#37. why not require rexml/document yourself? that's an easy fix.

@rubiii
Copy link
Contributor

rubiii commented May 8, 2013

@phoet i could submit a pull request right here from github 😚

@phoet
Copy link
Owner

phoet commented May 8, 2013

@rubiii hey, its nike calling "just do it"

rubiii added a commit to rubiii/asin that referenced this issue May 8, 2013
phoet added a commit that referenced this issue May 8, 2013
require rexml/document to fix #22.
@phoet
Copy link
Owner

phoet commented May 9, 2013

there is a new version here: https://rubygems.org/gems/asin/versions/1.1.2

@phoet
Copy link
Owner

phoet commented May 9, 2013

🎉

@kurtfunai
Copy link
Collaborator

Awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants