Merge pull request #10041 from chrisroberts/f-downloader-mtch

Prevent matching on other Location headers

lib/vagrant/util/downloader.rb

@@ -105,15 +105,15 @@ module Vagrant
               # from the original host, notify the user that the target host has
               # changed from the source.
               if progress_data.include?("Location")
-                location = progress_data.scan(/Location: (.+?)$/m).flatten.compact.first.to_s.strip
+                location = progress_data.scan(/(^|[^\w-])Location: (.+?)$/m).flatten.compact.last.to_s.strip
                 if !location.empty?
                   location_uri = URI.parse(location)
 
                   unless location_uri.host.nil?
                     @logger.info("download redirected to #{location}")
                     source_uri = URI.parse(source)
-                    source_host = source_uri.host.split(".", 2).last
-                    location_host = location_uri.host.split(".", 2).last
+                    source_host = source_uri.host.to_s.split(".", 2).last
+                    location_host = location_uri.host.to_s.split(".", 2).last
                     if !@redirect_notify && location_host != source_host && !SILENCED_HOSTS.include?(location_host)
                       @ui.clear_line
                       @ui.detail "Download redirected to host: #{location_uri.host}"

test/unit/vagrant/util/downloader_test.rb

@@ -6,6 +6,7 @@ describe Vagrant::Util::Downloader do
   let(:source) { "foo" }
   let(:destination) { "bar" }
   let(:exit_code) { 0 }
+  let(:options) { {} }
 
   let(:subprocess_result) do
     double("subprocess_result").tap do |result|
@@ -14,7 +15,7 @@ describe Vagrant::Util::Downloader do
     end
   end
 
-  subject { described_class.new(source, destination) }
+  subject { described_class.new(source, destination, options) }
 
   before :each do
     allow(Vagrant::Util::Subprocess).to receive(:execute).and_return(subprocess_result)
@@ -27,6 +28,69 @@ describe Vagrant::Util::Downloader do
        "--output", destination, source, {}]
     }
 
+    context "with UI" do
+      let(:ui) { double("ui") }
+      let(:options) { {ui: ui} }
+      let(:source) { "http://example.org/vagrant.box" }
+      let(:redirect) { nil }
+      let(:progress_data) { "Location: #{redirect}" }
+
+      before do
+        allow(ui).to receive(:clear_line)
+        allow(ui).to receive(:detail)
+      end
+
+      after do
+        expect(subject).to receive(:execute_curl) do |*_, &data_proc|
+          expect(data_proc).not_to be_nil
+          data_proc.call(:stderr, progress_data)
+        end
+        subject.download!
+      end
+
+      context "with Location header at same host" do
+        let(:redirect) { "http://example.org/other-vagrant.box" }
+
+        it "should not output redirection information" do
+          expect(ui).not_to receive(:detail)
+        end
+      end
+
+      context "with Location header at different host" do
+        let(:redirect) { "http://example.com/vagrant.box" }
+
+        it "should output redirection information" do
+          expect(ui).to receive(:detail).with(/example.com/)
+        end
+      end
+
+      context "with Location header at different subdomain" do
+        let(:redirect) { "http://downloads.example.org/vagrant.box" }
+
+        it "should output redirection information" do
+          expect(ui).to receive(:detail).with(/downloads.example.org/)
+        end
+      end
+
+      context "with custom header including Location name" do
+        let(:custom_redirect) { "http://example.com/vagrant.box" }
+        let(:progress_data) { "X-Custom-Location: #{custom_redirect}" }
+
+        it "should not output redirection information" do
+          expect(ui).not_to receive(:detail)
+        end
+
+        context "with Location header at different host" do
+          let(:redirect) { "http://downloads.example.com/vagrant.box" }
+          let(:progress_data) { "X-Custom-Location: #{custom_redirect}\nLocation: #{redirect}" }
+
+          it "should output redirection information" do
+            expect(ui).to receive(:detail).with(/downloads.example.com/)
+          end
+        end
+      end
+    end
+
     context "with a good exit status" do
       let(:exit_code) { 0 }