Auto merge of #6010 - bundler:seg-remove-failed-gem-download, r=indirect

[Source::Rubygems] Remove .gem if downloaded package is invalid ### What was the end-user problem that led to this PR? The problem was the user could (once) have downloaded a `.gem` file that isn't actually a `.gem`, and that package would poison their cache. Closes #5941. ### What was your diagnosis of the problem? My diagnosis was we should remove the `.gem` right after downloading it if we can't open it. ### What is your fix for the problem, implemented in this PR? My fix `rm_rf`'s the `.gem` on failure. ### Why did you choose this fix out of the possible options? I chose this fix because it won't accidentally nuke existing cache entries for a user, but it should help prevent Bundler propagating an issue.
rubygems · Sep 15, 2017 · 4fc8fe9 · 4fc8fe9
2 parents b019b9b + f420278
commit 4fc8fe9
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 2 deletions.
diff --git a/lib/bundler/source/rubygems.rb b/lib/bundler/source/rubygems.rb
@@ -120,8 +120,14 @@ def install(spec, opts = {})
           uris.uniq!
           Installer.ambiguous_gems << [spec.name, *uris] if uris.length > 1
 
-          s = Bundler.rubygems.spec_from_gem(fetch_gem(spec), Bundler.settings["trust-policy"])
-          spec.__swap__(s)
+          path = fetch_gem(spec)
+          begin
+            s = Bundler.rubygems.spec_from_gem(path, Bundler.settings["trust-policy"])
+            spec.__swap__(s)
+          rescue
+            Bundler.rm_rf(path)
+            raise
+          end
         end
 
         unless Bundler.settings[:no_install]

diff --git a/spec/install/failure_spec.rb b/spec/install/failure_spec.rb
@@ -28,5 +28,24 @@
       activesupport
                      M
     end
+
+    context "because the downloaded .gem was invalid" do
+      before do
+        build_repo4 do
+          build_gem "a"
+        end
+
+        gem_repo4("gems", "a-1.0.gem").open("w") {|f| f << "<html></html>" }
+      end
+
+      it "removes the downloaded .gem" do
+        install_gemfile <<-G
+          source "file:#{gem_repo4}"
+          gem "a"
+        G
+
+        expect(default_bundle_path("cache", "a-1.0.gem")).not_to exist
+      end
+    end
   end
 end