add new tests

3 files changed, 202 insertions, 66 deletions

diff --git a/lib/chef/provider/remote_file/ftp.rb b/lib/chef/provider/remote_file/ftp.rb
index bc4e5a5183..995fb0ba72 100644
--- a/lib/chef/provider/remote_file/ftp.rb
+++ b/lib/chef/provider/remote_file/ftp.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Jesse Campbell (<hikeit@gmail.com>)
-# Copyright:: Copyright (c) 2008 Opscode, Inc.
+# Copyright:: Copyright (c) 2013 Jesse Campbell
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -20,64 +20,58 @@ require 'uri'
 require 'tempfile'
 require 'net/ftp'
 require 'chef/provider/remote_file'
- 
+
 class Chef
-	class Provider
-		class RemoteFile
-			class FTP
+  class Provider
+    class RemoteFile
+      class FTP
 
-				# Fetches the file at uri using Net::FTP, returning a Tempfile
-				# Parts shamelessly stolen from open-uri
-				def self.fetch(uri, ftp_active_mode)
-					path = uri.path
-					path = path.sub(%r{\A/}, '%2F') # re-encode the beginning slash because uri library decodes it.
-					directories = path.split(%r{/}, -1)
-					directories.each {|d|
-						d.gsub!(/%([0-9A-Fa-f][0-9A-Fa-f])/) { [$1].pack("H2") }
-					}
-					unless filename = directories.pop
-						raise ArgumentError, "no filename: #{uri.inspect}"
-					end
-					directories.each {|d|
-						if /[\r\n]/ =~ d
-							raise ArgumentError, "invalid directory: #{d.inspect}"
-						end
-					}
-					if /[\r\n]/ =~ filename
-						raise ArgumentError, "invalid filename: #{filename.inspect}"
-					end 
-					typecode = uri.typecode
-					if typecode && /\A[aid]\z/ !~ typecode
-						raise ArgumentError, "invalid typecode: #{typecode.inspect}"
-					end
+        # Fetches the file at uri using Net::FTP, returning a Tempfile
+        # Taken from open-uri
+        def self.fetch(uri, ftp_active_mode)
+          path = uri.path
+          path = path.sub(%r{\A/}, '%2F') # re-encode the beginning slash because uri library decodes it.
+          directories = path.split(%r{/}, -1)
+          directories.each {|d|
+            d.gsub!(/%([0-9A-Fa-f][0-9A-Fa-f])/) { [$1].pack("H2") }
+          }
+          unless filename = directories.pop
+            raise ArgumentError, "no filename: #{uri.inspect}"
+          end
+          if filename.length == 0 || filename.end_with?( "/" )
+            raise ArgumentError, "no filename: #{uri.inspect}"
+          end
+          typecode = uri.typecode
+          # Only support ascii and binary types
+          if typecode && /\A[ai]\z/ !~ typecode
+            raise ArgumentError, "invalid typecode: #{typecode.inspect}"
+          end
 
-					tempfile = Tempfile.new(filename)
+          tempfile = Tempfile.new(filename)
 
-					# The access sequence is defined by RFC 1738
-					ftp = Net::FTP.new
-					ftp.connect(uri.hostname, uri.port)
-					ftp.passive = true if !ftp_active_mode
-					# todo: extract user/passwd from .netrc.
-					user = 'anonymous'
-					passwd = nil
-					if uri.userinfo
-						user = URI.unescape(uri.user)
-						passwd = URI.unescape(uri.password)
-					end
-					ftp.login(user, passwd)
-					directories.each {|cwd|
-						ftp.voidcmd("CWD #{cwd}")
-					}
-					if typecode
-						# xxx: typecode D is not handled.
-						ftp.voidcmd("TYPE #{typecode.upcase}")
-					end
-					ftp.getbinaryfile(filename, tempfile.path)
-					ftp.close
+          # The access sequence is defined by RFC 1738
+          ftp = Net::FTP.new
+          ftp.connect(uri.hostname, uri.port)
+          ftp.passive = !ftp_active_mode
+          user = 'anonymous'
+          passwd = nil
+          if uri.userinfo
+            user = URI.unescape(uri.user)
+            passwd = URI.unescape(uri.password)
+          end
+          ftp.login(user, passwd)
+          directories.each {|cwd|
+            ftp.voidcmd("CWD #{cwd}")
+          }
+          if typecode
+            ftp.voidcmd("TYPE #{typecode.upcase}")
+          end
+          ftp.getbinaryfile(filename, tempfile.path)
+          ftp.close
 
-					tempfile
-				end
-			end
-		end
-	end
+          tempfile
+        end
+      end
+    end
+  end
 end
diff --git a/spec/unit/provider/remote_file/ftp_spec.rb b/spec/unit/provider/remote_file/ftp_spec.rb
new file mode 100644
index 0000000000..368a7346c4
--- /dev/null
+++ b/spec/unit/provider/remote_file/ftp_spec.rb
@@ -0,0 +1,117 @@
+#
+# Author:: Jesse Campbell (<hikeit@gmail.com>)
+# Copyright:: Copyright (c) 2013 Jesse Campbell
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'spec_helper'
+
+describe Chef::Provider::RemoteFile::FTP, "fetch" do
+  before(:each) do
+    @ftp = mock(Net::FTP, { })
+    Net::FTP.stub!(:new).and_return(@ftp)
+    @ftp.stub!(:connect)
+    @ftp.stub!(:login)
+    @ftp.stub!(:voidcmd)
+    @ftp.stub!(:getbinaryfile)
+    @ftp.stub!(:close)
+    @ftp.stub!(:passive=)
+    @tempfile = Tempfile.new("chef-rspec-ftp_spec-line#{__LINE__}--")
+    Tempfile.stub!(:new).and_return(@tempfile)
+    @uri = URI.parse("ftp://opscode.com/seattle.txt")
+  end
+
+  describe "when parsing the uri" do
+    it "throws an argument exception when no path is given" do
+      @uri.path = ""
+      lambda { Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close! }.should raise_error(ArgumentError)
+    end
+
+    it "throws an argument exception when only a / is given" do
+      @uri.path = "/"
+      lambda { Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close! }.should raise_error(ArgumentError)
+    end
+
+    it "throws an argument exception when no filename is given" do
+      @uri.path = "/the/whole/path/"
+      lambda { Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close! }.should raise_error(ArgumentError)
+    end
+
+    it "throws an argument exception when the typecode is invalid" do
+      @uri.typecode = "d"
+      lambda { Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close! }.should raise_error(ArgumentError)
+    end
+  end
+
+  describe "when connecting to the remote" do
+    it "should connect to the host from the uri on the default port 21" do
+      @ftp.should_receive(:connect).with("opscode.com", 21)
+      Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close!
+    end
+
+    it "should connect on an alternate port when one is provided" do
+      @ftp.should_receive(:connect).with("opscode.com", 8021)
+      Chef::Provider::RemoteFile::FTP.fetch(URI.parse("ftp://opscode.com:8021/seattle.txt"), false).close!
+    end
+
+    it "should set passive true when ftp_active_mode is false" do
+      @ftp.should_receive(:passive=).with(true)
+      Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close!
+    end
+
+    it "should set passive false when ftp_active_mode is false" do
+      @ftp.should_receive(:passive=).with(false)
+      Chef::Provider::RemoteFile::FTP.fetch(@uri, true).close!
+    end
+
+    it "should use anonymous ftp when no userinfo is provided" do
+      @ftp.should_receive(:login).with("anonymous", nil)
+      Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close!
+    end
+
+    it "should use authenticated ftp when userinfo is provided" do
+      @ftp.should_receive(:login).with("the_user", "the_password")
+      Chef::Provider::RemoteFile::FTP.fetch(URI.parse("ftp://the_user:the_password@opscode.com/seattle.txt"), false).close!
+    end
+
+    it "should accept ascii for the typecode" do
+      @uri.typecode = "a"
+      @ftp.should_receive(:voidcmd).with("TYPE A").once
+      Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close!
+    end
+
+    it "should accept image for the typecode" do
+      @uri.typecode = "i"
+      @ftp.should_receive(:voidcmd).with("TYPE I").once
+      Chef::Provider::RemoteFile::FTP.fetch(@uri, false).close!
+    end
+
+    it "should fetch the file from the correct path" do
+      @ftp.should_receive(:voidcmd).with("CWD the").once
+      @ftp.should_receive(:voidcmd).with("CWD whole").once
+      @ftp.should_receive(:voidcmd).with("CWD path").once
+      @ftp.should_receive(:getbinaryfile).with("seattle.txt", @tempfile.path)
+      Chef::Provider::RemoteFile::FTP.fetch(URI.parse("ftp://opscode.com/the/whole/path/seattle.txt"), false).close!
+    end
+  end
+
+  describe "when it finishes downloading" do
+    it "should return a tempfile" do
+      ftpfile = Chef::Provider::RemoteFile::FTP.fetch(@uri, false)
+      ftpfile.should equal @tempfile
+      ftpfile.close!
+    end
+  end
+end
diff --git a/spec/unit/provider/remote_file_spec.rb b/spec/unit/provider/remote_file_spec.rb
index d7fbad52ef..149f0462c3 100644
--- a/spec/unit/provider/remote_file_spec.rb
+++ b/spec/unit/provider/remote_file_spec.rb
@@ -64,7 +64,7 @@ describe Chef::Provider::RemoteFile, "action_create" do
     end
 
     before do
-      @resource.source("http://localhost:9000/seattle.txt")
+      @resource.source("http://opscode.com/seattle.txt")
     end
 
     describe "and the target location's enclosing directory does not exist" do
@@ -121,7 +121,7 @@ describe Chef::Provider::RemoteFile, "action_create" do
         end
 
         it "does not download the file" do
-          RestClient::Request.should_not_receive(:execute).with("http://localhost:9000/seattle.txt").and_return(@tempfile)
+          RestClient::Request.should_not_receive(:execute).with("http://opscode.com/seattle.txt").and_return(@tempfile)
           @provider.run_action(:create)
         end
 
@@ -138,7 +138,7 @@ describe Chef::Provider::RemoteFile, "action_create" do
         end
 
         it "should not download the file if the checksum is a partial match from the beginning" do
-          @rawresp.should_not_receive(:fetch).with("http://localhost:9000/seattle.txt").and_return(@tempfile)
+          @rawresp.should_not_receive(:fetch).with("http://opscode.com/seattle.txt").and_return(@tempfile)
           @provider.run_action(:create)
         end
 
@@ -152,7 +152,7 @@ describe Chef::Provider::RemoteFile, "action_create" do
       describe "and the existing file doesn't match the given checksum" do
         it "downloads the file" do
           @resource.checksum("this hash doesn't match")
-          RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://localhost:9000/seattle.txt", :raw_response => true).and_return(@rawresp)
+          RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :raw_response => true).and_return(@rawresp)
           @provider.stub!(:update_new_file_state)
           @provider.run_action(:create)
         end
@@ -160,7 +160,7 @@ describe Chef::Provider::RemoteFile, "action_create" do
         it "does not consider the checksum a match if the matching string is offset" do
           # i.e., the existing file is      "0fd012fdc96e96f8f7cf2046522a54aed0ce470224513e45da6bc1a17a4924aa"
           @resource.checksum("fd012fd")
-          RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://localhost:9000/seattle.txt", :raw_response => true).and_return(@rawresp)
+          RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :raw_response => true).and_return(@rawresp)
           @provider.stub!(:update_new_file_state)
           @provider.run_action(:create)
         end
@@ -171,7 +171,7 @@ describe Chef::Provider::RemoteFile, "action_create" do
     describe "and the resource doesn't specify a checksum" do
       it "should download the file from the remote URL" do
         @resource.checksum(nil)
-        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://localhost:9000/seattle.txt", :raw_response => true).and_return(@rawresp)
+        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :raw_response => true).and_return(@rawresp)
         @provider.run_action(:create)
       end
     end
@@ -188,7 +188,7 @@ describe Chef::Provider::RemoteFile, "action_create" do
     context "and the target file is a tarball" do
       before do
         @resource.path(File.expand_path(File.join(CHEF_SPEC_DATA, "seattle.tar.gz")))
-        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://localhost:9000/seattle.txt", :raw_response => true).and_return(@rawresp)
+        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :raw_response => true).and_return(@rawresp)
       end
 
       it "disables gzip in the http client" do
@@ -208,6 +208,34 @@ describe Chef::Provider::RemoteFile, "action_create" do
       end
     end
 
+    context "and the uri scheme is ftp" do
+      before do
+        @resource.source("ftp://opscode.com/seattle.txt")
+      end
+
+      it "should fetch with ftp in passive mode" do
+        Chef::Provider::RemoteFile::FTP.should_receive(:fetch).with(URI.parse("ftp://opscode.com/seattle.txt"), false).and_return(@tempfile)
+        @provider.run_action(:create)
+      end
+
+      it "should fetch with ftp in active mode" do
+        @resource.ftp_active_mode true
+        Chef::Provider::RemoteFile::FTP.should_receive(:fetch).with(URI.parse("ftp://opscode.com/seattle.txt"), true).and_return(@tempfile)
+        @provider.run_action(:create)
+      end
+    end
+
+    context "and the uri scheme is file" do
+      before do
+        @resource.source("file:///nyan_cat.png")
+      end
+
+      it "should load the local file" do
+        File.should_receive(:new).with("/nyan_cat.png", "r").and_return(File.open(File.join(CHEF_SPEC_DATA, "remote_file", "nyan_cat.png"), "r"))
+        @provider.run_action(:create)
+      end
+    end
+
     it "should raise an exception if it's any other kind of retriable response than 304" do
       r = Net::HTTPMovedPermanently.new("one", "two", "three")
       e = Net::HTTPRetriableError.new("301", r)
@@ -314,9 +342,6 @@ describe Chef::Provider::RemoteFile, "action_create" do
         @provider.should_receive(:set_all_access_controls).and_return(true)
         @provider.run_action(:create)
       end
-
-
     end
-
   end
 end