Merge pull request #7703 from chef/lcg/gem-version-sanitization

Sanitize inputs to Gem::Version in comparison operation of Package provider superclass

2 files changed, 29 insertions, 2 deletions

diff --git a/lib/chef/provider/package.rb b/lib/chef/provider/package.rb
index 113d33a2fc..58a3f31b50 100644
--- a/lib/chef/provider/package.rb
+++ b/lib/chef/provider/package.rb
@@ -361,9 +361,14 @@ class Chef
       #
       # By default, this function will use Gem::Version comparison. Subclasses can reimplement this method
       # for package-management system specific versions.
+      #
+      # (In other words, pull requests to introduce domain specific mangling of versions into this method
+      # will be closed -- that logic must go into the subclass -- we understand that this is far from perfect
+      # but it is a better default than outright buggy things like v1.to_f <=> v2.to_f)
+      #
       def version_compare(v1, v2)
-        gem_v1 = Gem::Version.new(v1)
-        gem_v2 = Gem::Version.new(v2)
+        gem_v1 = Gem::Version.new(v1.gsub(/\A\s*(#{Gem::Version::VERSION_PATTERN}).*/, '\1'))
+        gem_v2 = Gem::Version.new(v2.gsub(/\A\s*(#{Gem::Version::VERSION_PATTERN}).*/, '\1'))
 
         gem_v1 <=> gem_v2
       end
diff --git a/spec/unit/provider/package_spec.rb b/spec/unit/provider/package_spec.rb
index 688d70d677..36b282c6ac 100644
--- a/spec/unit/provider/package_spec.rb
+++ b/spec/unit/provider/package_spec.rb
@@ -980,4 +980,26 @@ describe "Chef::Provider::Package - Multi" do
       end
     end
   end
+
+  describe "version_compare" do
+    it "tests equality" do
+      expect(provider.version_compare("1.3", "1.3")).to eql(0)
+    end
+
+    it "tests less than" do
+      expect(provider.version_compare("1.2", "1.3")).to eql(-1)
+    end
+
+    it "tests greater than" do
+      expect(provider.version_compare("1.5", "1.3")).to eql(1)
+    end
+
+    it "x.10 is greater than x.2 (so does not do floating point comparisons)" do
+      expect(provider.version_compare("1.10", "1.2")).to eql(1)
+    end
+
+    it "sanitizes inputs" do
+      expect(provider.version_compare("1.3_3", "1.3")).to eql(0)
+    end
+  end
 end