Merge branch 'CHEF-4671'

3 files changed, 21 insertions, 30 deletions

diff --git a/lib/chef/provider/remote_file/http.rb b/lib/chef/provider/remote_file/http.rb
index 8949e6ab04..f17ab5a56d 100644
--- a/lib/chef/provider/remote_file/http.rb
+++ b/lib/chef/provider/remote_file/http.rb
@@ -56,19 +56,12 @@ class Chef
         end
 
         def fetch
-          tempfile = nil
-          begin
-            http = Chef::HTTP::Simple.new(uri, http_client_opts)
-            tempfile = http.streaming_request(uri, headers)
+          http = Chef::HTTP::Simple.new(uri, http_client_opts)
+          tempfile = http.streaming_request(uri, headers)
+          if tempfile
             update_cache_control_data(tempfile, http.last_response)
-          rescue Net::HTTPRetriableError => e
-            if e.response.is_a? Net::HTTPNotModified
-              tempfile = nil
-            else
-              raise e
-            end
+            tempfile.close
           end
-          tempfile.close if tempfile
           tempfile
         end
 
diff --git a/spec/functional/resource/remote_file_spec.rb b/spec/functional/resource/remote_file_spec.rb
index 9d68389a77..bfc09dccd9 100644
--- a/spec/functional/resource/remote_file_spec.rb
+++ b/spec/functional/resource/remote_file_spec.rb
@@ -83,6 +83,19 @@ describe Chef::Resource::RemoteFile do
       stop_tiny_server
     end
 
+    describe "when redownload isn't necessary" do
+      let(:source) { 'http://localhost:9000/seattle_capo.png' }
+
+      before do
+        @api.get("/seattle_capo.png", 304, "", { 'Etag' => 'abcdef' } )
+      end
+
+      it "does not fetch the file" do
+        resource.run_action(:create)
+      end
+
+    end
+
     context "when using normal encoding" do
       let(:source) { 'http://localhost:9000/nyan_cat.png' }
       let(:expected_content) do
@@ -112,6 +125,7 @@ describe Chef::Resource::RemoteFile do
 
       it_behaves_like "a securable resource with reporting"
     end
+
   end
 
   context "when fetching files over HTTPS" do
diff --git a/spec/unit/provider/remote_file/http_spec.rb b/spec/unit/provider/remote_file/http_spec.rb
index cb7271900d..e763a60e6f 100644
--- a/spec/unit/provider/remote_file/http_spec.rb
+++ b/spec/unit/provider/remote_file/http_spec.rb
@@ -181,26 +181,10 @@ describe Chef::Provider::RemoteFile::HTTP do
 
     describe "and the request does not return new content" do
 
-      it "should propagate non-304 exceptions to the caller" do
-        r = Net::HTTPBadRequest.new("one", "two", "three")
-        e = Net::HTTPServerException.new("fake exception", r)
-        rest.stub!(:streaming_request).and_raise(e)
-        lambda { fetcher.fetch }.should raise_error(Net::HTTPServerException)
-      end
-
-      it "should return HTTPRetriableError when Chef::HTTP::Simple returns a 301" do
-        r = Net::HTTPMovedPermanently.new("one", "two", "three")
-        e = Net::HTTPRetriableError.new("301", r)
-        rest.stub!(:streaming_request).and_raise(e)
-        lambda { fetcher.fetch }.should raise_error(Net::HTTPRetriableError)
-      end
-
       it "should return a nil tempfile for a 304 HTTPNotModifed" do
-        r = Net::HTTPNotModified.new("one", "two", "three")
-        e = Net::HTTPRetriableError.new("304", r)
-        rest.stub!(:streaming_request).and_raise(e)
-        result = fetcher.fetch
-        result.should be_nil
+        # Streaming request returns nil for 304 errors
+        rest.stub(:streaming_request).and_return(nil)
+        fetcher.fetch.should be_nil
       end
 
     end