handle Dir.mktmpdir failure outside of filesystem_access block

2 files changed, 53 insertions, 46 deletions

diff --git a/lib/bundler/compact_index_client/updater.rb b/lib/bundler/compact_index_client/updater.rb
index 8dd8de3d06..d2f7da778e 100644
--- a/lib/bundler/compact_index_client/updater.rb
+++ b/lib/bundler/compact_index_client/updater.rb
@@ -26,60 +26,57 @@ module Bundler
       end
 
       def update(local_path, remote_path, retrying = nil)
-        headers = {}
+        local_temp_dir = create_tmpdir
 
-        Bundler::SharedHelpers.filesystem_access(Dir.tmpdir, :write) do
-          Dir.mktmpdir("bundler-compact-index-") do |local_temp_dir|
-            local_temp_path = Pathname.new(local_temp_dir).join(local_path.basename)
-
-            # first try to fetch any new bytes on the existing file
-            if retrying.nil? && local_path.file?
-              FileUtils.cp local_path, local_temp_path
-              headers["If-None-Match"] = etag_for(local_temp_path)
-              headers["Range"] =
-                if local_temp_path.size.nonzero?
-                  # Subtract a byte to ensure the range won't be empty.
-                  # Avoids 416 (Range Not Satisfiable) responses.
-                  "bytes=#{local_temp_path.size - 1}-"
-                else
-                  "bytes=#{local_temp_path.size}-"
-                end
+        headers = {}
+        local_temp_path = Pathname.new(local_temp_dir).join(local_path.basename)
+
+        # first try to fetch any new bytes on the existing file
+        if retrying.nil? && local_path.file?
+          FileUtils.cp local_path, local_temp_path
+          headers["If-None-Match"] = etag_for(local_temp_path)
+          headers["Range"] =
+            if local_temp_path.size.nonzero?
+              # Subtract a byte to ensure the range won't be empty.
+              # Avoids 416 (Range Not Satisfiable) responses.
+              "bytes=#{local_temp_path.size - 1}-"
             else
-              # Fastly ignores Range when Accept-Encoding: gzip is set
-              headers["Accept-Encoding"] = "gzip"
-            end
-
-            response = @fetcher.call(remote_path, headers)
-            return nil if response.is_a?(Net::HTTPNotModified)
-
-            content = response.body
-            if response["Content-Encoding"] == "gzip"
-              content = Zlib::GzipReader.new(StringIO.new(content)).read
+              "bytes=#{local_temp_path.size}-"
             end
+        else
+          # Fastly ignores Range when Accept-Encoding: gzip is set
+          headers["Accept-Encoding"] = "gzip"
+        end
 
-            SharedHelpers.filesystem_access(local_temp_path) do
-              if response.is_a?(Net::HTTPPartialContent) && local_temp_path.size.nonzero?
-                local_temp_path.open("a") {|f| f << slice_body(content, 1..-1) }
-              else
-                local_temp_path.open("w") {|f| f << content }
-              end
-            end
+        response = @fetcher.call(remote_path, headers)
+        return nil if response.is_a?(Net::HTTPNotModified)
 
-            response_etag = (response["ETag"] || "").gsub(%r{\AW/}, "")
-            if etag_for(local_temp_path) == response_etag
-              SharedHelpers.filesystem_access(local_path) do
-                FileUtils.mv(local_temp_path, local_path)
-              end
-              return nil
-            end
+        content = response.body
+        if response["Content-Encoding"] == "gzip"
+          content = Zlib::GzipReader.new(StringIO.new(content)).read
+        end
 
-            if retrying
-              raise MisMatchedChecksumError.new(remote_path, response_etag, etag_for(local_temp_path))
-            end
+        SharedHelpers.filesystem_access(local_temp_path) do
+          if response.is_a?(Net::HTTPPartialContent) && local_temp_path.size.nonzero?
+            local_temp_path.open("a") {|f| f << slice_body(content, 1..-1) }
+          else
+            local_temp_path.open("w") {|f| f << content }
+          end
+        end
 
-            update(local_path, remote_path, :retrying)
+        response_etag = (response["ETag"] || "").gsub(%r{\AW/}, "")
+        if etag_for(local_temp_path) == response_etag
+          SharedHelpers.filesystem_access(local_path) do
+            FileUtils.mv(local_temp_path, local_path)
           end
+          return nil
+        end
+
+        if retrying
+          raise MisMatchedChecksumError.new(remote_path, response_etag, etag_for(local_temp_path))
         end
+
+        update(local_path, remote_path, :retrying)
       end
 
       def etag_for(path)
@@ -104,6 +101,17 @@ module Bundler
           Digest::MD5.hexdigest(IO.read(path))
         end
       end
+
+    private
+
+      def create_tmpdir
+        Dir.mktmpdir("bundler-compact-index-")
+      rescue Errno::EACCES
+        raise Bundler::PermissionError,
+          "Bundler does not have write access to create a temp directory " \
+          "within #{Dir.tmpdir}. Bundler must have write access to your " \
+          "systems temp directory to function properly. " \
+      end
     end
   end
 end
diff --git a/spec/bundler/compact_index_client/updater_spec.rb b/spec/bundler/compact_index_client/updater_spec.rb
index ebec684a26..af24e58d5f 100644
--- a/spec/bundler/compact_index_client/updater_spec.rb
+++ b/spec/bundler/compact_index_client/updater_spec.rb
@@ -34,7 +34,6 @@ RSpec.describe Bundler::CompactIndexClient::Updater do
     let(:remote_path) { double(:remote_path) }
 
     it "Errno::EACCES is raised" do
-      allow(Dir).to receive(:tmpdir) { "/tmp" }
       allow(Dir).to receive(:mktmpdir) { raise Errno::EACCES }
 
       expect do