Bumped up nokogiri version.

[kakuzei]

No description provided.

[ghost]

All CLA requirements met.

[scsmith]

@kakuzei since we seem to be lacking a response from MS I think this PR should be changed to include "~> 1.12", ">= 1.12.5".

Since nokogiri uses SemVer no client facing changes will occur without a major version change. This would prevent us being back in this situation when 1.13.0 etc. are released.

[kakuzei]

Thank you @scsmith for your suggestion.

Unless I'm mistaken, if we want to avoid this situation when nokogiri 1.13.0 will be released, the change must be "~> 1", ">= 1.12.5".
Is it acceptable to be so flexible?

[scsmith]

@kakuzei I think "~> 1", ">= 1.12.5" and "~> 1.12", ">= 1.12.5" are identical in this situation. Normally "~> 1.12" means ">= 1.12", "< 2.0" but since we have the greater than CVE restriction too it's the same (https://guides.rubygems.org/patterns/#pessimistic-version-constraint).

Thanks for updating, hopefully it gets approved and merged.

[larvacea]

lgtm. Thank you! fwiw, the version bump mitigates a high-severity vulnerability in nokogiri before 1.12.5, so it would be delightful to get this merged sooner rather than later.

[pjmelling]

@katmsft Can you please merge this? Thank you!!

[hamaker]

Looks good to me!

[fdr]

Not merging this is now blocking a security update in Nokogiri.

[katmsft]

This breaks all ruby 2.4.x support.
Please change it to:

  if RUBY_VERSION < "2.4.0"
    gem "nokogiri",    "~> 1.10.4", :require => false
  elsif RUBY_VERSION < "2.5.0"
    gem "nokogiri",     "~> 1.11.0.rc2", :require => false
  else
    gem "nokogiri",    "~> 1", ">= 1.12.5", :require => false
  end

[katmsft]

Please add 2.4.x version support, as 1.11.0 of nokogiri drops the support for it we need to add special elsif statement for it.

[kakuzei]

@katmsft Thank you for your review, I made the change you suggested to not break Ruby 2.4.x support

[nertzy]

We can simplify things greatly by just ensuring that the user has any version 1.x of nokogiri where x >= 10, regardless of the version of Ruby:

# 😁 GOOD: let users use any nokogiri version above the minimum
gem "nokogiri",    "~> 1.10", :require => false

Alternatively, if we really do want specific minimum versions per Ruby version (for some reason I don't understand) then we could use:

# 😵‍💫 BAD: specific versions per version of Ruby

  if RUBY_VERSION < "2.4.0"
    gem "nokogiri", "~> 1.10", :require => false
  elsif RUBY_VERSION < "2.5.0"
    gem "nokogiri", "~> 1.11", :require => false
  else
    gem "nokogiri", "~> 1.12", :require => false
  end

See also #196 (comment)

[katmsft]

We can simplify things greatly by just ensuring that the user has any version 1.x of nokogiri where x >= 10, regardless of the version of Ruby:

# 😁 GOOD: let users use any nokogiri version above the minimum
gem "nokogiri",    "~> 1.10", :require => false

Alternatively, if we really do want specific minimum versions per Ruby version (for some reason I don't understand) then we could use:

# 😵‍💫 BAD: specific versions per version of Ruby

  if RUBY_VERSION < "2.4.0"
    gem "nokogiri", "~> 1.10", :require => false
  elsif RUBY_VERSION < "2.5.0"
    gem "nokogiri", "~> 1.11", :require => false
  else
    gem "nokogiri", "~> 1.12", :require => false
  end

See also #196 (comment)

The alternative you provide won't work due to 1.11.0 deprecating 2.4.x support.

And the proposal you gave might regress some customers' nokogiri version back to those versions with security fix not yet applied, which is also not ideal.

[nertzy]

Ruby gems won't install nokogiri 1.11 on Ruby 2.4 because it's gemspec requires Ruby >= 2.5. It doesn't matter what your gem's requirements are, rubygems enforces this automatically.

If you want to allow 1.10 only for Ruby 2.4 and disallow 1.10 for Ruby 2.5+, then go ahead and use the conditional. But please remove the artificial upper bound either way.

Just know that if someone really wants to use nokogiri 1.10 on Ruby 2.5+ alongside your gem, Bundler will just resolve to an earlier version of your gem without the restrictions. By being aggressive with your gem requirements, you are actively preventing some subset of your users from upgrading.

I understand that you don't want your users to downgrade their version of nokogiri because they install your gems. I want you to know that my applications are in this exact position because of the overzealous requirements of the azure storage gems.

Being more straightforward will allow your users the flexibility to upgrade and address security issues quickly, instead of keeping them dependent on your team's release schedule. The irony is that the efforts to force users to upgrade are currently backfiring and preventing us from upgrading.

Thank you for your attention and I hope we can all come to a mutually beneficial solution. I want to do what I can to help improve the ecosystem. We are happy with Azure storage and want to continue to use and trust it.

[katmsft]

Ruby gems won't install nokogiri 1.11 on Ruby 2.4 because it's gemspec requires Ruby >= 2.5. It doesn't matter what your gem's requirements are, rubygems enforces this automatically.

If you want to allow 1.10 only for Ruby 2.4 and disallow 1.10 for Ruby 2.5+, then go ahead and use the conditional. But please remove the artificial upper bound either way.

Just know that if someone really wants to use nokogiri 1.10 on Ruby 2.5+ alongside your gem, Bundler will just resolve to an earlier version of your gem without the restrictions. By being aggressive with your gem requirements, you are actively preventing some subset of your users from upgrading.

I understand that you don't want your users to downgrade their version of nokogiri because they install your gems. I want you to know that my applications are in this exact position because of the overzealous requirements of the azure storage gems.

Being more straightforward will allow your users the flexibility to upgrade and address security issues quickly, instead of keeping them dependent on your team's release schedule. The irony is that the efforts to force users to upgrade are currently backfiring and preventing us from upgrading.

Thank you for your attention and I hope we can all come to a mutually beneficial solution. I want to do what I can to help improve the ecosystem. We are happy with Azure storage and want to continue to use and trust it.

Thank you very much for your input.
I just tried on my PC, it seems that ~>1.11 resolves to 1.12.5, even it does not support 2.4.x version. Maybe it's because the logic of RC releases is 'version x.y.z.rc < version x.y.z', so ~1.11 excludes rc versions.
I believe the current implementation is a good approach for everyone but feel free to share your thoughts if you feel differently.

PS E:\dev\src\ruby\azure-storage-ruby> ruby -v
ruby 2.4.9p362 (2019-10-02 revision 67824) [i386-mingw32]
PS E:\dev\src\ruby\azure-storage-ruby> bundle update nokogiri
[DEPRECATED] This Gemfile does not include an explicit global source. Not using an explicit global source may result in a different lockfile being generated depending on the gems you have installed locally before bundler is run. Instead, define a global source in your Gemfile like this: source "https://rubygems.org".
Fetching gem metadata from https://rubygems.org/......
Resolving dependencies...
Bundler found conflicting requirements for the Ruby version:
  In Gemfile:
    Ruby

    nokogiri (~> 1.11) was resolved to 1.12.5, which depends on
      mini_portile2 (~> 2.6.1) was resolved to 2.6.1, which depends on
        Ruby (>= 2.3.0)

    nokogiri (~> 1.11) was resolved to 1.12.5, which depends on
      Ruby (>= 2.5.0)
PS E:\dev\src\ruby\azure-storage-ruby>

[nertzy]

I believe the current implementation is a good approach for everyone but feel free to share your thoughts if you feel differently.

I agree, if you mean that we should accept this pull request and release a new version of the gems.

Let's get this merged. Thanks!

[fdr]

Given I have an open security flag set, I advise releasing something that looks decent now rather than waiting. You can release again if you must.

[amygurski]

LGTM 🍦 . Resolves nokogiri security vulnerability CVE-2021-41098 and unblocks other updates that depend on nokogiri due to this gem's nokogiri versioning. I see the requested change was agreed it's not needed. Can we get this released? 🚀 ⭐

[lindsaymkelly]

Also LGTM 🌟 Any update on when we can get this merged?

[janmg]

Is this merge blocked because the following 3 gemspecs are missing the RUBY 2.4 support

common/azure-storage-common.gemspec
file/azure-storage-file.gemspec
queue/azure-storage-queue.gemspec

Is adding this stanza to those three gemspecs enough to unblock the merge?

  elsif RUBY_VERSION < "2.5.0"
    s.add_runtime_dependency("nokogiri",                "~> 1.11.0.rc2")

[katmsft]

Yes, that's the main reason, sorry for not posting any update. I'm merging this PR and sending a new one to resolve it instead of waiting for the author's fix. We are also pushing for release soon.