From 224ce874296b56e31957e053ce897a9327c15cbc Mon Sep 17 00:00:00 2001 From: Adam Wanninger Date: Sun, 20 Aug 2017 09:37:48 -0400 Subject: [PATCH] handle Dir.mktmpdir failure outside of filesystem_access block --- lib/bundler/compact_index_client/updater.rb | 98 ++++++++++--------- .../compact_index_client/updater_spec.rb | 1 - 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 8dd8de3d06f..d2f7da778e8 100644 --- a/lib/bundler/compact_index_client/updater.rb +++ b/lib/bundler/compact_index_client/updater.rb @@ -26,60 +26,57 @@ def initialize(fetcher) 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 @@ def checksum_for_file(path) 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 ebec684a264..af24e58d5f9 100644 --- a/spec/bundler/compact_index_client/updater_spec.rb +++ b/spec/bundler/compact_index_client/updater_spec.rb @@ -34,7 +34,6 @@ 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