Merge pull request #127 from CocoaPods/validate_before_dl

Switches where we check for invalid input, to move it inside the download function

Author: dnkoutso

lib/cocoapods-downloader/base.rb

@@ -77,6 +77,7 @@ def name
       # @return [void]
       #
       def download
+        validate_input
         ui_action("#{name} download") do
           target_path.mkpath
           download!
@@ -121,6 +122,14 @@ def checkout_options
         raise 'Abstract method'
       end
 
+      # Provides a before-download check for safety of the options in the
+      # concrete downloader.
+      #
+      # @return [void]
+      #
+      def validate_input
+      end
+
       # Returns a User-Agent string that itentifies http network requests as
       # originating from CocoaPods.
       # Contains version numbers from the CocoaPods Gem and the cocoapods-downloader Gem.

lib/cocoapods-downloader/git.rb

@@ -21,7 +21,6 @@ def checkout_options
       end
 
       def self.preprocess_options(options)
-        validate_input options
         return options unless options[:branch]
 
         command = ['ls-remote',
@@ -58,13 +57,7 @@ def self.commit_from_ls_remote(output, branch_name)
         match[1] unless match.nil?
       end
 
-      def self.validate_input(options)
-        input = [options[:git], options[:branch], options[:commit], options[:tag]].map(&:to_s)
-        invalid = input.compact.any? { |value| value.start_with?('--') || value.include?(' --') }
-        raise DownloaderError, "Provided unsafe input for git #{options}." if invalid
-      end
-
-      private_class_method :commit_from_ls_remote, :validate_input
+      private_class_method :commit_from_ls_remote
 
       private
 
@@ -160,6 +153,12 @@ def checkout_commit
       def target_git(*args)
         git!(['-C', target_path] + args)
       end
+
+      def validate_input
+        input = [url, options[:branch], options[:commit], options[:tag]].map(&:to_s)
+        invalid = input.compact.any? { |value| value.start_with?('--') || value.include?(' --') }
+        raise DownloaderError, "Provided unsafe input for git #{options}." if invalid
+      end
     end
   end
 end

lib/cocoapods-downloader/mercurial.rb

@@ -18,19 +18,6 @@ def checkout_options
         end
       end
 
-      def self.preprocess_options(options)
-        validate_input options
-        options
-      end
-
-      def self.validate_input(options)
-        input = [options[:hg], options[:revision], options[:branch], options[:tag]].map(&:to_s)
-        invalid = input.compact.any? { |value| value.start_with?('--') || value.include?(' --') }
-        raise DownloaderError, "Provided unsafe input for hg #{options}." if invalid
-      end
-
-      private_class_method :validate_input
-
       private
 
       executable :hg
@@ -62,6 +49,12 @@ def download_tag!
       def download_branch!
         hg! 'clone', url, '--updaterev', options[:branch], @target_path
       end
+
+      def validate_input
+        input = [url, options[:revision], options[:branch], options[:tag]].map(&:to_s)
+        invalid = input.compact.any? { |value| value.start_with?('--') || value.include?(' --') }
+        raise DownloaderError, "Provided unsafe input for hg #{options}." if invalid
+      end
     end
   end
 end

spec/git_spec.rb

@@ -294,19 +294,19 @@ def ensure_only_one_ref(folder)
       describe ':bad input' do
         it 'bails when you provide a bad input' do
           options = { :git => '--upload-pack=touch ./HELLO1;', :branch => 'foo' }
-          e = lambda { Downloader.preprocess_options(options) }.should.raise DownloaderError
+          e = lambda { Downloader.for_target(tmp_folder, options).download }.should.raise DownloaderError
           e.message.should.match /Provided unsafe input/
         end
 
         it 'bails when you provide a bad input after valid input' do
           options = { :git => 'github.com --upload-pack=touch ./HELLO1;', :branch => 'foo' }
-          e = lambda { Downloader.preprocess_options(options) }.should.raise DownloaderError
+          e = lambda { Downloader.for_target(tmp_folder, options).download }.should.raise DownloaderError
           e.message.should.match /Provided unsafe input/
         end
 
         it 'bails with other fields' do
           options = { :branch => '--upload-pack=touch ./HELLO1;', :git => 'foo' }
-          e = lambda { Downloader.preprocess_options(options) }.should.raise DownloaderError
+          e = lambda { Downloader.for_target(tmp_folder, options).download }.should.raise DownloaderError
           e.message.should.match /Provided unsafe input/
         end
       end

spec/mercurial_spec.rb

@@ -110,7 +110,13 @@ module Downloader
     describe ':bad input' do
       it 'bails when you provide a bad input' do
         options = { :hg => '--config=alias.clone=!touch ./HELLO2;' }
-        e = lambda { Downloader.preprocess_options(options) }.should.raise DownloaderError
+        e = lambda { Downloader.for_target(tmp_folder, options).download }.should.raise DownloaderError
+        e.message.should.match /Provided unsafe input/
+      end
+
+      it 'bails when you provide a bad input2' do
+        options = { :hg => 'foo/bar', :revision => '--config=alias.clone=!touch ./HELLO3;' }
+        e = lambda { Downloader.for_target(tmp_folder, options).download }.should.raise DownloaderError
         e.message.should.match /Provided unsafe input/
       end
     end