Merge pull request #10671 from briancain/err-msg-for-bad-version

Fixes #10663: Ignore boxes in collection with malformed version numbers

lib/vagrant/box_collection.rb

@@ -233,14 +233,23 @@ module Vagrant
             version = versiondir.basename.to_s
 
             versiondir.children(true).each do |provider|
+              # Ensure version of box is correct before continuing
+              if !Gem::Version.correct?(version)
+                ui = Vagrant::UI::Prefixed.new(Vagrant::UI::Colored.new, "vagrant")
+                ui.warn(I18n.t("vagrant.box_version_malformed",
+                              version: version, box_name: box_name))
+                @logger.debug("Invalid version #{version} for box #{box_name}")
+                next
+              end
+
               # Verify this is a potentially valid box. If it looks
               # correct enough then include it.
               if provider.directory? && provider.join("metadata.json").file?
                 provider_name = provider.basename.to_s.to_sym
-                @logger.debug("Box: #{box_name} (#{provider_name})")
+                @logger.debug("Box: #{box_name} (#{provider_name}, #{version})")
                 results << [box_name, version, provider_name]
               else
-                @logger.debug("Invalid box, ignoring: #{provider}")
+                @logger.debug("Invalid box #{box_name}, ignoring: #{provider}")
               end
             end
           end

templates/locales/en.yml

@@ -82,6 +82,8 @@ en:
       * '%{name}' for '%{provider}' (v%{version}) is up to date
     box_up_to_date_single: |-
       Box '%{name}' (v%{version}) is running the latest version.
+    box_version_malformed:
+      Invalid version '%{version}' for '%{box_name}', ignoring...
     cfengine_bootstrapping: |-
       Bootstrapping CFEngine with policy server: %{policy_server}...
     cfengine_bootstrapping_policy_hub: |-

test/unit/vagrant/box_collection_test.rb

@@ -16,6 +16,8 @@ describe Vagrant::BoxCollection, :skip_windows do
   end
 
   describe "#all" do
+    let(:ui) { double("ui", warn: true) }
+
     it "should return an empty array when no boxes are there" do
       expect(subject.all).to eq([])
     end
@@ -38,6 +40,27 @@ describe Vagrant::BoxCollection, :skip_windows do
       expect(results.include?(["foo:colon", "1.0", :virtualbox])).to be
     end
 
+    it "should return the boxes and their providers even if box has wrong version" do
+      allow(Vagrant::UI::Prefixed).to receive(:new).and_return(ui)
+      # Create some boxes
+      environment.box3("foo", "fake-invalid-version", :virtualbox)
+      environment.box3("foo", "1.0", :vmware)
+      environment.box3("bar", "0", :ec2)
+      environment.box3("foo-VAGRANTSLASH-bar", "1.0", :virtualbox)
+      environment.box3("foo-VAGRANTCOLON-colon", "1.0", :virtualbox)
+
+      expect(ui).to receive(:warn).once
+
+      # Verify some output
+      results = subject.all
+      expect(results.length).to eq(4)
+      expect(results.include?(["foo", "1.0", :virtualbox])).not_to be
+      expect(results.include?(["foo", "1.0", :vmware])).to be
+      expect(results.include?(["bar", "0", :ec2])).to be
+      expect(results.include?(["foo/bar", "1.0", :virtualbox])).to be
+      expect(results.include?(["foo:colon", "1.0", :virtualbox])).to be
+    end
+
     it 'does not raise an exception when a file appears in the boxes dir' do
       Tempfile.open('vagrant-a_file', environment.boxes_dir) do
         expect { subject.all }.to_not raise_error