Introduce curl helper and uploader classes

This commit introduces a new uploader class for uploading files and
splits up some commonly used functionality between it and the downloader
class into a curl helper library.

lib/vagrant/errors.rb

@@ -830,6 +830,13 @@ module Vagrant
 
     class UploadSourceMissing < VagrantError
       error_key(:upload_source_missing)
+
+    class UploaderError < VagrantError
+      error_key(:uploader_error)
+    end
+
+    class UploaderInterrupted < UploaderError
+      error_key(:uploader_interrupted)
     end
 
     class VagrantInterrupt < VagrantError

lib/vagrant/util/curl_helper.rb

@@ -0,0 +1,96 @@
+module Vagrant
+  module Util
+    class CurlHelper
+
+      # Hosts that do not require notification on redirect
+      SILENCED_HOSTS = [
+        "vagrantcloud.com".freeze,
+        "vagrantup.com".freeze
+      ].freeze
+
+      def self.capture_output_proc(logger, ui, source=nil)
+        progress_data = ""
+        progress_regexp = /^\r\s*(\d.+?)\r/m
+
+        # Setup the proc that'll receive the real-time data from
+        # the downloader.
+        data_proc = Proc.new do |type, data|
+          # Type will always be "stderr" because that is the only
+          # type of data we're subscribed for notifications.
+
+          # Accumulate progress_data
+          progress_data << data
+
+          while true
+            # If the download has been redirected and we are no longer downloading
+            # 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(/(^|[^\w-])Location: (.+?)$/m).flatten.compact.last.to_s.strip
+              if !location.empty?
+                location_uri = URI.parse(location)
+
+                unless location_uri.host.nil?
+                  redirect_notify = false
+                  logger.info("download redirected to #{location}")
+                  source_uri = URI.parse(source)
+                  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}"
+                  end
+                  redirect_notify = true
+                end
+              end
+              progress_data.replace("")
+              break
+            end
+            # If we have a full amount of column data (two "\r") then
+            # we report new progress reports. Otherwise, just keep
+            # accumulating.
+            match = nil
+            check_match = true
+
+            while check_match
+              check_match = progress_regexp.match(progress_data)
+              if check_match
+                data = check_match[1].to_s
+                stop = progress_data.index(data) + data.length
+                progress_data.slice!(0, stop)
+
+                match = check_match
+              end
+            end
+
+            break if !match
+
+            # Ignore the first \r and split by whitespace to grab the columns
+            columns = data.strip.split(/\s+/)
+
+            # COLUMN DATA:
+            #
+            # 0 - % total
+            # 1 - Total size
+            # 2 - % received
+            # 3 - Received size
+            # 4 - % transferred
+            # 5 - Transferred size
+            # 6 - Average download speed
+            # 7 - Average upload speed
+            # 9 - Total time
+            # 9 - Time spent
+            # 10 - Time left
+            # 11 - Current speed
+
+            output = "Progress: #{columns[0]}% (Rate: #{columns[11]}/s, Estimated time remaining: #{columns[10]})"
+            ui.clear_line
+            ui.detail(output, new_line: false)
+          end
+        end
+
+        return data_proc
+      end
+    end
+  end
+end

lib/vagrant/util/downloader.rb

@@ -6,6 +6,7 @@ require "digest/sha1"
 require "vagrant/util/busy"
 require "vagrant/util/platform"
 require "vagrant/util/subprocess"
+require "vagrant/util/curl_helper"
 
 module Vagrant
   module Util
@@ -88,85 +89,7 @@ module Vagrant
           # tell us output so we can parse it out.
           extra_subprocess_opts[:notify] = :stderr
 
-          progress_data = ""
+          data_proc = Vagrant::Util::CurlHelper.capture_output_proc(@logger, @ui, @source)
-          progress_regexp = /^\r\s*(\d.+?)\r/m
-
-          # Setup the proc that'll receive the real-time data from
-          # the downloader.
-          data_proc = Proc.new do |type, data|
-            # Type will always be "stderr" because that is the only
-            # type of data we're subscribed for notifications.
-
-            # Accumulate progress_data
-            progress_data << data
-
-            while true
-              # If the download has been redirected and we are no longer downloading
-              # 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(/(^|[^\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.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}"
-                    end
-                    @redirect_notify = true
-                  end
-                end
-                progress_data.replace("")
-                break
-              end
-
-              # If we have a full amount of column data (two "\r") then
-              # we report new progress reports. Otherwise, just keep
-              # accumulating.
-              match = nil
-              check_match = true
-
-              while check_match
-                check_match = progress_regexp.match(progress_data)
-                if check_match
-                  data = check_match[1].to_s
-                  stop = progress_data.index(data) + data.length
-                  progress_data.slice!(0, stop)
-
-                  match = check_match
-                end
-              end
-
-              break if !match
-
-              # Ignore the first \r and split by whitespace to grab the columns
-              columns = data.strip.split(/\s+/)
-
-              # COLUMN DATA:
-              #
-              # 0 - % total
-              # 1 - Total size
-              # 2 - % received
-              # 3 - Received size
-              # 4 - % transferred
-              # 5 - Transferred size
-              # 6 - Average download speed
-              # 7 - Average upload speed
-              # 9 - Total time
-              # 9 - Time spent
-              # 10 - Time left
-              # 11 - Current speed
-
-              output = "Progress: #{columns[0]}% (Rate: #{columns[11]}/s, Estimated time remaining: #{columns[10]})"
-              @ui.clear_line
-              @ui.detail(output, new_line: false)
-            end
-          end
         end
 
         @logger.info("Downloader starting download: ")
@@ -195,8 +118,7 @@ module Vagrant
           # If its any error other than 33, it is an error.
           raise if e.extra_data[:code].to_i != 33
 
-          # Exit code 33 means that the server doesn't support ranges.
+          # Exit code 33 means that the server doesn't support ranges.  # In this case, try again without resume.
-          # In this case, try again without resume.
           @logger.error("Error is server doesn't support byte ranges. Retrying from scratch.")
           @continue = false
           retried = true

lib/vagrant/util/uploader.rb

@@ -0,0 +1,105 @@
+require "uri"
+
+require "log4r"
+require "vagrant/util/busy"
+require "vagrant/util/platform"
+require "vagrant/util/subprocess"
+require "vagrant/util/curl_helper"
+
+module Vagrant
+  module Util
+    # This class uploads files using various protocols by subprocessing
+    # to cURL. cURL is a much more capable and complete download tool than
+    # a hand-rolled Ruby library, so we defer to its expertise.
+    class Uploader
+
+      def initialize(destination, file, options=nil)
+        options ||= {}
+        @logger = Log4r::Logger.new("vagrant::util::uploader")
+        @destination    = destination.to_s
+        @file   = file.to_s
+        @ui     = options[:ui]
+        @request_method = options[:method]
+
+        if !@request_method
+          @request_method = "PUT"
+        end
+      end
+
+      def upload!
+        data_proc = Vagrant::Util::CurlHelper.capture_output_proc(@logger, @ui)
+
+        @logger.info("Uploader starting upload: ")
+        @logger.info("  -- Source: #{@file}")
+        @logger.info("  -- Destination: #{@destination}")
+
+        options = build_options
+        subprocess_options = {notify: :stderr}
+
+        begin
+          execute_curl(options, subprocess_options, &data_proc)
+        rescue Errors::UploaderError => e
+          raise
+        ensure
+          @ui.clear_line if @ui
+        end
+      end
+
+      protected
+
+      def build_options
+        options = [@destination, "--request", @request_method, "--upload-file", @file]
+        return options
+      end
+
+      def execute_curl(options, subprocess_options, &data_proc)
+        options = options.dup
+        options << subprocess_options
+
+        # Create the callback that is called if we are interrupted
+        interrupted  = false
+        int_callback = Proc.new do
+          @logger.info("Uploader interrupted!")
+          interrupted = true
+        end
+
+        # Execute!
+        result = Busy.busy(int_callback) do
+          Subprocess.execute("curl", *options, &data_proc)
+        end
+
+        # If the download was interrupted, then raise a specific error
+        raise Errors::UploaderInterrupted if interrupted
+
+        # If it didn't exit successfully, we need to parse the data and
+        # show an error message.
+        if result.exit_code != 0
+          @logger.warn("Uploader exit code: #{result.exit_code}")
+          check = result.stderr.match(/\n*curl:\s+\((?<code>\d+)\)\s*(?<error>.*)$/)
+          if check && check[:code] == "416"
+            # All good actually. 416 means there is no more bytes to download
+            @logger.warn("Uploader got a 416, but is likely fine. Continuing on...")
+          else
+            if !check
+              err_msg = result.stderr
+            else
+              err_msg = check[:error]
+            end
+
+            raise Errors::UploaderError,
+              exit_code: result.exit_code,
+              message: err_msg
+          end
+        end
+
+        if @ui
+          @ui.clear_line
+          # Windows doesn't clear properly for some reason, so we just
+          # output one more newline.
+          @ui.detail("") if Platform.windows?
+        end
+        result
+      end
+    end
+  end
+end

plugins/commands/cloud/locales/en.yml

@@ -28,14 +28,14 @@ en:
         Updated box %{org}/%{box_name}
     search:
       no_results: |-
-        No results found for %{query}
+        No results found for `%{query}`
     upload:
       no_url: |-
         No URL was provided to upload the provider
         You will need to run the `vagrant cloud provider upload` command to provide a box
     provider:
       upload: |-
-        Uploading provider %{provider_file} ...
+        Uploading box file for '%{org}/%{box_name}' v(%{version}) for provider: '%{provider}'
       upload_success: |-
         Uploaded provider %{provider} on %{org}/%{box_name} for version %{version}
       delete_warn: |-

plugins/commands/cloud/provider/upload.rb

@@ -1,4 +1,5 @@
 require 'optparse'
+require "vagrant/util/uploader"
 
 module VagrantPlugins
   module CloudCommand
@@ -36,7 +37,7 @@ module VagrantPlugins
             box_name = box[1]
             provider_name = argv[1]
             version = argv[2]
-            file = argv[3]
+            file = argv[3] # path expand
 
             upload_provider(org, box_name, provider_name, version, file, @client.token, options)
           end
@@ -50,12 +51,18 @@ module VagrantPlugins
             cloud_version = VagrantCloud::Version.new(box, version, nil, nil, access_token)
             provider = VagrantCloud::Provider.new(cloud_version, provider_name, nil, nil, org, box_name, access_token)
 
+            ul = Vagrant::Util::Uploader.new(provider.upload_url, file, ui: @env.ui)
+            ui = Vagrant::UI::Prefixed.new(@env.ui, "cloud")
+
             begin
-              @env.ui.info(I18n.t("cloud_command.provider.upload", provider_file: file))
+              ui.output(I18n.t("cloud_command.provider.upload", org: org, box_name: box_name, version: version, provider: provider_name))
-              success = provider.upload_file(file)
+              ui.info("Upload File: #{file}")
-              @env.ui.success(I18n.t("cloud_command.provider.upload_success", provider: provider_name, org: org, box_name: box_name, version: version))
+
+              ul.upload!
+
+              ui.success("Successfully uploaded box '#{org}/#{box_name}' (v#{version}) for '#{provider_name}'")
               return 0
-            rescue VagrantCloud::ClientError => e
+            rescue Vagrant::Errors::UploaderError, VagrantCloud::ClientError => e
               @env.ui.error(I18n.t("cloud_command.errors.provider.upload_fail", provider: provider_name, org: org, box_name: box_name, version: version))
               @env.ui.error(e)
               return 1

plugins/commands/cloud/publish.rb

@@ -1,4 +1,5 @@
 require 'optparse'
+require "vagrant/util/uploader"
 
 module VagrantPlugins
   module CloudCommand
@@ -18,7 +19,7 @@ module VagrantPlugins
             o.on("--box-version VERSION", String, "Version of box to create") do |v|
               options[:box_version] = v
             end
-            o.on("--url", String, "Valid remote URL to download this provider") do |u|
+            o.on("--url URL", String, "Valid remote URL to download this provider") do |u|
               options[:url] = u
             end
             o.on("-d", "--description DESCRIPTION", String, "Longer description of box") do |d|
@@ -47,7 +48,7 @@ module VagrantPlugins
           # Parse the options
           argv = parse_options(opts)
           return if !argv
-          if argv.empty? || argv.length > 4 || argv.length < 4
+          if argv.empty? || argv.length > 5 || argv.length < 3
             raise Vagrant::Errors::CLIInvalidUsage,
               help: opts.help.chomp
           end
@@ -59,7 +60,7 @@ module VagrantPlugins
           box_name = box[1]
           version = argv[1]
           provider_name = argv[2]
-          box_file = argv[3]
+          box_file = argv[3] # path expand
           publish_box(org, box_name, version, provider_name, box_file, options, @client.token)
         end
 
@@ -67,7 +68,7 @@ module VagrantPlugins
           server_url = VagrantPlugins::CloudCommand::Util.api_server_url
 
           @env.ui.warn("You are about to create a box on Vagrant Cloud with the following options:\n")
-          box_opts = "  #{org}/#{box_name} (#{version}) for #{provider_name}\n"
+          box_opts = "  #{org}/#{box_name}:   (v#{version}) for provider '#{provider_name}'\n"
           box_opts << "  Private:               true\n" if options[:private]
           box_opts << "  Automatic Release:     true\n" if options[:release]
           box_opts << "  Remote Box file:       true\n" if options[:url]
@@ -87,26 +88,29 @@ module VagrantPlugins
           cloud_version = VagrantCloud::Version.new(box, version, nil, options[:version_description], access_token)
           provider = VagrantCloud::Provider.new(cloud_version, provider_name, nil, options[:url], org, box_name, access_token)
 
+          ui = Vagrant::UI::Prefixed.new(@env.ui, "cloud")
           begin
-            @env.ui.info(I18n.t("cloud_command.publish.box_create"))
+            ui.info(I18n.t("cloud_command.publish.box_create"))
             box.create
-            @env.ui.info(I18n.t("cloud_command.publish.version_create"))
+            ui.info(I18n.t("cloud_command.publish.version_create"))
             cloud_version.create_version
-            @env.ui.info(I18n.t("cloud_command.publish.provider_create"))
+            ui.info(I18n.t("cloud_command.publish.provider_create"))
             provider.create_provider
             if !options[:url]
-              @env.ui.info(I18n.t("cloud_command.publish.upload_provider", file: box_file))
+              box_file = File.absolute_path(box_file)
-              provider.upload_file(box_file)
+              ui.info(I18n.t("cloud_command.publish.upload_provider", file: box_file))
+              ul = Vagrant::Util::Uploader.new(provider.upload_url, box_file, ui: @env.ui)
+              ul.upload!
             end
             if options[:release]
-              @env.ui.info(I18n.t("cloud_command.publish.release"))
+              ui.info(I18n.t("cloud_command.publish.release"))
               cloud_version.release
             end
             @env.ui.success(I18n.t("cloud_command.publish.complete", org: org, box_name: box_name))
             success = box.read(org, box_name)
             VagrantPlugins::CloudCommand::Util.format_box_results(success.compact, @env)
             return 0
-          rescue VagrantCloud::ClientError => e
+          rescue Vagrant::Errors::UploaderError, VagrantCloud::ClientError => e
             @env.ui.error(I18n.t("cloud_command.errors.publish.fail", org: org, box_name: box_name))
             @env.ui.error(e)
             return 1

templates/locales/en.yml

@@ -1509,6 +1509,16 @@ en:
         the source location for upload an try again.
 
           Source Path: %{source}
+      uploader_error: |-
+        An error occurred while uploading the file. The error
+        message, if any, is reproduced below. Please fix this error and try
+        again.
+
+        exit code: %{exit_code}
+        %{message}
+      uploader_interrupted: |-
+        The upload was interrupted by an external signal. It did not
+        complete.
       vagrantfile_exists: |-
         `Vagrantfile` already exists in this directory. Remove it before
         running `vagrant init`.

test/unit/plugins/commands/cloud/provider/upload_test.rb

@@ -21,6 +21,7 @@ describe VagrantPlugins::CloudCommand::ProviderCommand::Command::Upload do
   let(:box) { double("box") }
   let(:version) { double("version") }
   let(:provider) { double("provider") }
+  let(:uploader) { double("uploader") }
 
   before do
     allow(iso_env).to receive(:action_runner).and_return(action_runner)
@@ -50,10 +51,13 @@ describe VagrantPlugins::CloudCommand::ProviderCommand::Command::Upload do
       allow(VagrantCloud::Provider).to receive(:new).
         with(version, "virtualbox", nil, nil, "vagrant", "box-name", client.token).
         and_return(provider)
+      allow(provider).to receive(:upload_url).
+        and_return("http://upload.here/there")
+      allow(Vagrant::Util::Uploader).to receive(:new).
+        with("http://upload.here/there", "path/to/box.box", {ui: anything}).
+        and_return(uploader)
 
-      expect(provider).to receive(:upload_file).
+      expect(uploader).to receive(:upload!)
-        with("path/to/box.box").
-        and_return({})
       expect(subject.execute).to eq(0)
     end
 
@@ -61,9 +65,14 @@ describe VagrantPlugins::CloudCommand::ProviderCommand::Command::Upload do
       allow(VagrantCloud::Provider).to receive(:new).
         with(version, "virtualbox", nil, nil, "vagrant", "box-name", client.token).
         and_return(provider)
+      allow(provider).to receive(:upload_url).
+        and_return("http://upload.here/there")
+      allow(Vagrant::Util::Uploader).to receive(:new).
+        with("http://upload.here/there", "path/to/box.box", {ui: anything}).
+        and_return(uploader)
 
-      allow(provider).to receive(:upload_file).
+      allow(uploader).to receive(:upload!).
-        and_raise(VagrantCloud::ClientError.new("Fail Message", "Message"))
+        and_raise(Vagrant::Errors::UploaderError.new(exit_code: 1, message: "Error"))
       expect(subject.execute).to eq(1)
     end
   end

test/unit/plugins/commands/cloud/publish_test.rb

@@ -18,10 +18,12 @@ describe VagrantPlugins::CloudCommand::Command::Publish do
   subject { described_class.new(argv, iso_env) }
 
   let(:action_runner) { double("action_runner") }
+  let(:box_path) { "path/to/the/virtualbox.box" }
 
   let(:box) { double("box", create: true, read: {}) }
   let(:version) { double("version", create_version: true, release: true) }
   let(:provider) { double("provider", create_provider: true, upload_file: true) }
+  let(:uploader) { double("uploader") }
 
   before do
     allow(iso_env).to receive(:action_runner).and_return(action_runner)
@@ -34,6 +36,8 @@ describe VagrantPlugins::CloudCommand::Command::Publish do
     allow(VagrantCloud::Box).to receive(:new).and_return(box)
     allow(VagrantCloud::Version).to receive(:new).and_return(version)
     allow(VagrantCloud::Provider).to receive(:new).and_return(provider)
+
+    allow(File).to receive(:absolute_path).and_return("/full/#{box_path}")
   end
 
   context "with no arguments" do
@@ -44,14 +48,24 @@ describe VagrantPlugins::CloudCommand::Command::Publish do
   end
 
   context "with arguments" do
-    let(:argv) { ["vagrant/box", "1.0.0", "virtualbox", "path/to/the/virtualbox.box"] }
+    let(:argv) { ["vagrant/box", "1.0.0", "virtualbox", box_path] }
 
     it "publishes a box given options" do
+      allow(provider).to receive(:upload_url).and_return("http://upload.here/there")
+      allow(Vagrant::Util::Uploader).to receive(:new).
+        with("http://upload.here/there", "/full/path/to/the/virtualbox.box", {ui: anything}).
+        and_return(uploader)
+      allow(uploader).to receive(:upload!)
       expect(VagrantPlugins::CloudCommand::Util).to receive(:format_box_results)
       expect(subject.execute).to eq(0)
     end
 
     it "catches a ClientError if something goes wrong" do
+      allow(provider).to receive(:upload_url).and_return("http://upload.here/there")
+      allow(Vagrant::Util::Uploader).to receive(:new).
+        with("http://upload.here/there", "/full/path/to/the/virtualbox.box", {ui: anything}).
+        and_return(uploader)
+      allow(uploader).to receive(:upload!)
       allow(box).to receive(:create).
         and_raise(VagrantCloud::ClientError.new("Fail Message", "Message"))
       expect(subject.execute).to eq(1)
@@ -59,9 +73,14 @@ describe VagrantPlugins::CloudCommand::Command::Publish do
   end
 
   context "with arguments and releasing a box" do
-    let(:argv) { ["vagrant/box", "1.0.0", "virtualbox", "path/to/the/virtualbox.box", "--release"] }
+    let(:argv) { ["vagrant/box", "1.0.0", "virtualbox", box_path, "--release"] }
 
     it "releases the box" do
+      allow(provider).to receive(:upload_url).and_return("http://upload.here/there")
+      allow(Vagrant::Util::Uploader).to receive(:new).
+        with("http://upload.here/there", "/full/path/to/the/virtualbox.box", {ui: anything}).
+        and_return(uploader)
+      allow(uploader).to receive(:upload!)
       expect(VagrantPlugins::CloudCommand::Util).to receive(:format_box_results)
       expect(version).to receive(:release)
       expect(subject.execute).to eq(0)

test/unit/vagrant/util/curl_helper_test.rb

@@ -0,0 +1,6 @@
+require File.expand_path("../../../base", __FILE__)
+
+require "vagrant/util/curl_helper"
+
+describe Vagrant::Util::CurlHelper do
+end

test/unit/vagrant/util/uploader_test.rb

@@ -0,0 +1,50 @@
+require File.expand_path("../../../base", __FILE__)
+
+require "vagrant/util/uploader"
+
+describe Vagrant::Util::Uploader do
+  let(:destination) { "fake" }
+  let(:file) { "my/file.box" }
+  let(:curl_options) { [destination, "--request", "PUT", "--upload-file", file, {notify: :stderr}] }
+
+  let(:subprocess_result) do
+    double("subprocess_result").tap do |result|
+      allow(result).to receive(:exit_code).and_return(exit_code)
+      allow(result).to receive(:stderr).and_return("")
+    end
+  end
+
+  subject { described_class.new(destination, file, options) }
+
+  before :each do
+    allow(Vagrant::Util::Subprocess).to receive(:execute).and_return(subprocess_result)
+  end
+
+  describe "#upload!" do
+    context "with a good exit status" do
+      let(:options) { {} }
+      let(:exit_code) { 0 }
+
+      it "uploads the file and returns true" do
+        expect(Vagrant::Util::Subprocess).to receive(:execute).
+          with("curl", *curl_options).
+          and_return(subprocess_result)
+
+        expect(subject.upload!).to be
+      end
+    end
+
+    context "with a bad exit status" do
+      let(:options) { {} }
+      let(:exit_code) { 1 }
+      it "raises an exception" do
+        expect(Vagrant::Util::Subprocess).to receive(:execute).
+          with("curl", *curl_options).
+          and_return(subprocess_result)
+
+        expect { subject.upload! }.
+          to raise_error(Vagrant::Errors::UploaderError)
+      end
+    end
+  end
+end