Azure / azure-storage-ruby

Microsoft Azure Storage Library for Ruby
http://azure.github.io/azure-storage-ruby/
84 stars 150 forks source link

Published azure-storage-blob 2.0.2 still requires Nokogiri 1.11.0.rc2 #205

Closed grncdr closed 1 year ago

grncdr commented 2 years ago

I've seen there were PRs merged to fix this but the actual published gem (downloaded from Rubygems.org) contains the folllowing metadata that makes the Gem incompatible with Nokogiri 1.12:

- !ruby/object:Gem::Dependency
  name: nokogiri
  requirement: !ruby/object:Gem::Requirement
    requirements:
    - - "~>"
      - !ruby/object:Gem::Version
        version: 1.11.0.rc2

I am not super familiar with Rubygem tooling but it appears that the Ruby version used to package the gem is determining the Nokogiri dependency that is "baked in" to the final zip file.

As a a workaround, installing from source will work because the Gemspec is evaluated using the local Ruby version.

git 'https://github.com/Azure/azure-storage-ruby.git' do
  gem 'azure-storage-blob'
end
katmsft commented 2 years ago

When building the gems with gem build azure-storage-blob.gemspec, it resolves the dependency version with currently installed ruby version, which is 2.4.9 on my machine, that uses nokogiri 1.12. There seems to be no way to work around it. Any ideas?

rfeeney0614 commented 2 years ago

I'm a bit confused why the conditional needs to be there. It seems like we had to resolve based on which nokogiri versions allowed certain ruby versions. Bundler can handle that for us if the nokogiri version is relaxed overall.

a blanket gem "nokogiri", "~> 1", ">= 1.10.4" would let each ruby version resolve nokogiri to whatever latest version they were capable of.

unless I'm missing something.

nertzy commented 2 years ago

The gemspec included in the azure-storage-blob gem looks like this, which doesn't match the checked-in version and appears to be generated somehow:

# -*- encoding: utf-8 -*-
# stub: azure-storage-blob 2.0.2 ruby lib

Gem::Specification.new do |s|
  s.name = "azure-storage-blob".freeze
  s.version = "2.0.2"

  s.required_rubygems_version = Gem::Requirement.new(">= 0".freeze) if s.respond_to? :required_rubygems_version=
  s.require_paths = ["lib".freeze]
  s.authors = ["Microsoft Corporation".freeze]
  s.date = "2021-10-28"
  s.description = "Microsoft Azure Storage Blob Client Library for Ruby".freeze
  s.email = "ascl@microsoft.com".freeze
  s.homepage = "http://github.com/azure/azure-storage-ruby".freeze
  s.licenses = ["MIT".freeze]
  s.required_ruby_version = Gem::Requirement.new(">= 2.3.0".freeze)
  s.rubygems_version = "3.2.26".freeze
  s.summary = "Official Ruby client library to consume Azure Storage Blob service".freeze

  s.installed_by_version = "3.2.26" if s.respond_to? :installed_by_version

  if s.respond_to? :specification_version then
    s.specification_version = 4
  end

  if s.respond_to? :add_runtime_dependency then
    s.add_runtime_dependency(%q<azure-storage-common>.freeze, ["~> 2.0"])
    s.add_runtime_dependency(%q<nokogiri>.freeze, ["~> 1.11.0.rc2"])
    s.add_development_dependency(%q<dotenv>.freeze, ["~> 2.0"])
    s.add_development_dependency(%q<minitest>.freeze, ["~> 5"])
    s.add_development_dependency(%q<minitest-reporters>.freeze, ["~> 1"])
    s.add_development_dependency(%q<mocha>.freeze, ["~> 1.0"])
    s.add_development_dependency(%q<rake>.freeze, ["~> 13.0"])
    s.add_development_dependency(%q<timecop>.freeze, ["~> 0.7"])
    s.add_development_dependency(%q<yard>.freeze, ["~> 0.9", ">= 0.9.11"])
  else
    s.add_dependency(%q<azure-storage-common>.freeze, ["~> 2.0"])
    s.add_dependency(%q<nokogiri>.freeze, ["~> 1.11.0.rc2"])
    s.add_dependency(%q<dotenv>.freeze, ["~> 2.0"])
    s.add_dependency(%q<minitest>.freeze, ["~> 5"])
    s.add_dependency(%q<minitest-reporters>.freeze, ["~> 1"])
    s.add_dependency(%q<mocha>.freeze, ["~> 1.0"])
    s.add_dependency(%q<rake>.freeze, ["~> 13.0"])
    s.add_dependency(%q<timecop>.freeze, ["~> 0.7"])
    s.add_dependency(%q<yard>.freeze, ["~> 0.9", ">= 0.9.11"])
  end
end

@rfeeney0614 I agree. It would be best for the dependency code to be simple and allow users to use the latest versions that their Ruby environments allow.

It's frustrating that the Azure team keeps trying to write complex code in order to "require" their users to use more up-to-date versions of Nokogiri, only to find that the code they end up shipping is the only thing holding back users from upgrading Nokogiri.

It's not the responsibility of the Azure team to define which versions of nokogiri I can choose to use, except for the narrow case of requiring at minimum a version that supports their direct usage of nokogiri. I really wish Azure would join the rest of the Ruby ecosystem and simplify their approach.

Until then, it would be great to at the very least get another gem version built and released that includes the checked-in gemspecs instead of these incorrect generated versions.

Also, it would be great to get in a fix for https://github.com/Azure/azure-storage-ruby/issues/196 as well, which is similarly preventing us from upgrading the faraday_middleware gem.

katmsft commented 2 years ago

It's frustrating that the Azure team keeps trying to write complex code in order to "require" their users to use more up-to-date versions of Nokogiri, only to find that the code they end up shipping is the only thing holding back users from upgrading Nokogiri. It's not the responsibility of the Azure team to define which versions of nokogiri I can choose to use, except for the narrow case of requiring at minimum a version that supports their direct usage of nokogiri. I really wish Azure would join the rest of the Ruby ecosystem and simplify their approach.

I'm sorry it feels that way. The branching was aimed to provide a non-breaking behavior for the existing customers, which turns out to be not functioning as expected. None of us wished this to happen. The main concern was the security fixes in nokogiri version 1.10.8 and 1.10.5, writing gem "nokogiri", "~> 1", ">= 1.10.4" would expose them to thoese vulnerabilities again. However, I guess we could have done gem "nokogiri", "~> 1", ">= 1.10.8" instead.

I will push another PR with your suggestion and wait for more community updates/approvals before merging it in. Hopefully, we can get it right together this time.

katmsft commented 2 years ago

PR: https://github.com/Azure/azure-storage-ruby/pull/206

nertzy commented 2 years ago

Thank you, that feels like a reasonable approach to me. I am hopeful it will cover all the cases. I understand it's a bit complex to support so many versions of Ruby. Thanks for working through this.

rfeeney0614 commented 2 years ago

@nertzy maybe a bit off topic, but I'm genuinely curious on the discussion of dependency vulnerabilities and where the responsibility lies. Does it fall on the gem maintainer to protect us or does it fall on the end user to stay up to date? You mentioned the ruby ecosystem supporting the latter. i would love to read up on that. Do you have any threads/blogs/sites I can look at?

katmsft commented 2 years ago

Please try 2.0.3 and see if the issue persisted.

nertzy commented 2 years ago

The issue is solved for me. Thanks!

grncdr commented 1 year ago

Also solved for me with 2.0.3 (sorry for the late response 😅)