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 version of gem relies on a very old version of Nokogiri #499

Closed
bobbytables opened this issue Apr 14, 2019 · 13 comments
Closed

Latest version of gem relies on a very old version of Nokogiri #499

bobbytables opened this issue Apr 14, 2019 · 13 comments

Comments

@bobbytables
Copy link

In the latest ruby gem release for 1.10.1, I think there's a regression for the Nokogiri version. https://rubygems.org/gems/ruby-saml/versions/1.10.1

It now requires Nokogiri ~1.5, whereas version 1.10 of this gem required nokogiri >= 1.8.2

I discovered this trying to install the gem into a Rails project I have:

Bundler could not find compatible versions for gem "nokogiri":
  In Gemfile:
    rails (~> 5.2.0) was resolved to 5.2.2.1, which depends on
      actionmailer (= 5.2.2.1) was resolved to 5.2.2.1, which depends on
        rails-dom-testing (~> 2.0) was resolved to 2.0.3, which depends on
          nokogiri (>= 1.6)

    ruby-saml (~> 1.10, >= 1.10.1) was resolved to 1.10.1, which depends on
      nokogiri (<= 1.5.11)
@bobbytables bobbytables changed the title Latest Version of gem relies on a very old version of Nokogiri Latest version of gem relies on a very old version of Nokogiri Apr 14, 2019
@pitbulk
Copy link
Collaborator

pitbulk commented Apr 15, 2019

That the recent commit that forced to use at least version 1.8.2
325a444#diff-7cb020d23c0a838b83644b2dad8ad05c

But if you installed ruby-saml using the nokogiri-1.5.gemfile then you will have the conflict you see.

@jpalumickas
Copy link

Have the same problem:

NEW https://rubygems.org/gems/ruby-saml/versions/1.10.1
1.10.1 requires nokogiri <= 1.5.11

PREVIOUS https://rubygems.org/gems/ruby-saml/versions/1.10.0
1.10.0 requires nokogiri >= 1.8.2

@chiting
Copy link

chiting commented Apr 29, 2019

Hi,

The gem cli also shows the same issue:

$ gem dependency -r ruby-saml -v '1.10.1' --source https://rubygems.org
Gem ruby-saml-1.10.1
  minitest (~> 5.5, development)
  mocha (~> 0.14, development)
  nokogiri (<= 1.5.11)
  rake (~> 10, development)
  ruby-debug (~> 0.10.4, development)
  shoulda (~> 2.11, development)
  simplecov (>= 0, development)
  systemu (~> 2, development)
  timecop (<= 0.6.0, development)
  uuid (>= 0)

Now, when I bundle install in the cloned repo of ruby-saml, I don't have the issue, which would mean that the gemspec is OK but whatever is returned by rubygems is not (didn't dig much further).

I tested with gem version 3.0.3 and 2.7.3 with ruby 2.6.3 and 2.5.3

@pitbulk
Copy link
Collaborator

pitbulk commented Apr 29, 2019

That is very strange.

Forcing nokogiri to be 1.8.2, when possible, was introduced on the 1.10.1 version:
325a444

No idea why that change affected Rubygem.

I guess that Rubygem takes the lower ruby version supported (1.8.7) and then verifies its dependencies where nokogiri < 1.5.11

Not sure why 1.10.0 returns nokogiri 1.8.2 and 1.10.1 returns <= 1.5.11

Maybe something changed on Rubygem?

@chiting
Copy link

chiting commented Apr 29, 2019

I dug a bit more and the metadata packed with the gem might be the culprit:

[...]
- !ruby/object:Gem::Dependency
  name: nokogiri
  prerelease: false
  requirement: &id002 !ruby/object:Gem::Requirement
    requirements:
    - - <=
      - !ruby/object:Gem::Version
        version: 1.5.11
[...]

I'm not sure how these metadata are generated but it would be the issue I think.

To compare with the previous version's metadata:

--- !ruby/object:Gem::Specification
name: ruby-saml
version: !ruby/object:Gem::Version
  version: 1.10.0
platform: ruby
authors:
- OneLogin LLC
autorequire:
bindir: bin
cert_chain: []
date: 2019-03-21 00:00:00.000000000 Z
dependencies:
- !ruby/object:Gem::Dependency
  name: nokogiri
  requirement: !ruby/object:Gem::Requirement
    requirements:
    - - ">="
      - !ruby/object:Gem::Version
        version: 1.8.2
  type: :runtime
  prerelease: false
[...]

@pitbulk
Copy link
Collaborator

pitbulk commented Apr 29, 2019

ok, I fixed it on 1.10.2

gem dependency -r ruby-saml -v '1.10.1' --source https://rubygems.org

Gem ruby-saml-1.10.2
  minitest (~> 5.5, development)
  mocha (~> 0.14, development)
  nokogiri (>= 1.5.10)
  pry-byebug (>= 0, development)
  rake (~> 10, development)
  shoulda (~> 2.11, development)
  simplecov (>= 0, development)
  systemu (~> 2, development)
  timecop (<= 0.6.0, development)

It seems that the dependencies listed on the metadata of the gem depend on the env where you built the gem, and I generated it on an old env 1.8.7 that requires nokogiri <= 1.5.11 and that requirement was added to Rubygem.

@chiting
Copy link

chiting commented Apr 29, 2019

ok, I fixed it on 1.10.2

gem dependency -r ruby-saml -v '1.10.1' --source https://rubygems.org

Gem ruby-saml-1.10.2
  minitest (~> 5.5, development)
  mocha (~> 0.14, development)
  nokogiri (>= 1.5.10)
  pry-byebug (>= 0, development)
  rake (~> 10, development)
  shoulda (~> 2.11, development)
  simplecov (>= 0, development)
  systemu (~> 2, development)
  timecop (<= 0.6.0, development)

It seems that the dependencies listed on the metadata of the gem depend on the env where you built the gem, and I generated it on an old env 1.8.7 that requires nokogiri <= 1.5.11 and that requirement was added to Rubygem.

Now I'm thinking about this, wouldn't it defeat the purpose of enforcing a version of nokogiri per ruby version(s) unless one uses a git repo as source, as the metadata is generated for one version of ruby only and no matter what version of ruby you'd use, it would pull the same metadata file?

@pitbulk
Copy link
Collaborator

pitbulk commented Apr 30, 2019

I think is different the restriction that came from Rubygem, that the restriction defined on the ruby-saml.gemspec itself.

For example if you review Travis, you will see that:

So I only need to take care and not generate the gem with a restriction too strict, as happened before

@chiting
Copy link

chiting commented Apr 30, 2019

I totally agree that if you install the gem with

gem 'ruby-saml', :github => 'onelogin/ruby-saml'

No matter what version of ruby you use, you'll end up with the correct version of Nokogiri or at least the one you've defined in your gemspec.

And I believe your CI is installing the right version of Nokogiri for each ruby version because when you bundle install within the source, it'll take the gemspec and that works fine.

Correct me if I'm wrong but let's take JRuby < 9.2.0.0 for this example:

gem 'ruby-saml', '~> 1.10.2'

This dependency will never be met because when you generate the gem for one version, you can only do it with a specific version of ruby and the dependencies in the metadata reflects this.

Additionally, I'm not entirely sure but I suspect that for ruby < 2.1, this requirement won't be met either and will pull the latest version of Nokogiri to date (1.10.3) and would be above the highest version you specified (<= 1.6.8.1), as the metadata is specifying nokogiri (>= 1.5.10).

What do you think?

Edit: Sorry, I think this issue is fixed anyway and if what I've raised above is worth raising another issue for you, I'll be happy to put it in a new one.

@pitbulk
Copy link
Collaborator

pitbulk commented Apr 30, 2019

Yes, you are right, if in a Ruby 2.0 environment I try to install the ruby-saml gem with a:

gem install ruby-saml

I will get a

Fetching: nokogiri-1.10.3.gem (100%)
ERROR:  Error installing ruby-saml:
	nokogiri requires Ruby version >= 2.3.0.

But If I install the required nokogiri version that appears on the gemspec for that version:

gem install nokogiri -v 1.6.8.1

later I'm able to install ruby-saml

gem install ruby-saml
Fetching: ruby-saml-1.10.2.gem (100%)
Successfully installed ruby-saml-1.10.2

Do you know a way to improve the current way to handle this?

@chiting
Copy link

chiting commented May 1, 2019

I don't have much experience in that area so take it with a grain of salt but from what I've seen from other projects, I can suggest the following option:

If you want to keep supporting all these ruby versions, having a major version per requirement would be the way to go (a bit like rails with their stable branches). That way, you can still backport patches until you decide you want to remove the support for older ruby versions.

It's more complicated to maintain but you might have your reasons as to why you want to support these old ruby versions.

It would be easier to support only versions of ruby down to those which are still in a security maintenance status at least (2.4 as of today). This solution has some downsides though, some people might not be able to update to a more recent version of ruby and are relying on this paying service to get security patches (I can't remember the name). If they want to use your gem, they might be left out in that case.

Ultimately, it's up to you as you are spending your time to maintain this gem for the community. I don't know how much time you are willing to spend on this so having to spend even more time because of a more complicated versioning model, might not be an option for you.

@rmkanda
Copy link

rmkanda commented May 1, 2019

» bundle audit                                                            
Name: nokogiri
Version: 1.10.2
Advisory: CVE-2019-11068
Criticality: Unknown
URL: https://github.com/sparklemotion/nokogiri/issues/1892
Title: Nokogiri gem, via libxslt, is affected by improper access control vulnerability
Solution: upgrade to >= 1.10.3

@johnnyshields
Copy link
Collaborator

This can be closed. Recently the gemspec nokogiri versions were increased. That being said, ultimately this gem should force project maintainers to keep Nokogiri at the latest version--that's the project maintainers' job.

@pitbulk pitbulk closed this as completed Feb 11, 2021
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

6 participants