diff options
author | Dominic Cleal <dominic@cleal.org> | 2016-05-10 11:37:44 +0100 |
---|---|---|
committer | Dominic Cleal <dominic@cleal.org> | 2016-05-10 19:12:58 +0100 |
commit | 97bf5934ac426c047450d7b00c5929fd8ac5a215 (patch) | |
tree | 28413756348d6649f8e22504d181c4489d9b4b11 /spec | |
parent | 8d3eb4aa0a3286d219c78a05bf1ab47c784fc25c (diff) | |
download | bundler-97bf5934ac426c047450d7b00c5929fd8ac5a215.tar.gz |
Safely store concurrent compact index downloads
When bundler is run concurrently using the same bundle dir in $HOME,
the versions file can be updated from two processes at once. The
download has been changed to a temporary file, which is securely moved
into place over the original.
If retrying the update operation, the original file is no longer
immediately deleted and instead a full download is performed, later
overwriting the original file if successful.
If two processes are updating in parallel, this should ensure the
original file isn't corrupted and that both processes succeed.
- Fixes #4519
Diffstat (limited to 'spec')
-rw-r--r-- | spec/install/gems/compact_index_spec.rb | 18 | ||||
-rw-r--r-- | spec/support/artifice/compact_index.rb | 10 | ||||
-rw-r--r-- | spec/support/artifice/compact_index_concurrent_download.rb | 31 |
3 files changed, 58 insertions, 1 deletions
diff --git a/spec/install/gems/compact_index_spec.rb b/spec/install/gems/compact_index_spec.rb index ef3bc5aedc..e96936f01d 100644 --- a/spec/install/gems/compact_index_spec.rb +++ b/spec/install/gems/compact_index_spec.rb @@ -656,4 +656,22 @@ The checksum of /versions does not match the checksum provided by the server! So bundle! :install, :artifice => "compact_index_forbidden" end end + + it "performs partial update while local cache is updated by another process" do + gemfile <<-G + source "#{source_uri}" + gem 'rack' + G + + # Create an empty file to trigger a partial download + versions = File.join(Bundler.rubygems.user_home, ".bundle", "cache", "compact_index", + "localgemserver.test.80.dd34752a738ee965a2a4298dc16db6c5", "versions") + FileUtils.mkdir_p(File.dirname(versions)) + FileUtils.touch(versions) + + bundle! :install, :artifice => "compact_index_concurrent_download" + + expect(File.read(versions)).to start_with("created_at") + should_be_installed "rack 1.0.0" + end end diff --git a/spec/support/artifice/compact_index.rb b/spec/support/artifice/compact_index.rb index 88706c3590..a9fe7112b0 100644 --- a/spec/support/artifice/compact_index.rb +++ b/spec/support/artifice/compact_index.rb @@ -40,7 +40,7 @@ class CompactIndexAPI < Endpoint if ranges status 206 - body ranges.map! {|range| response_body.byteslice(range) }.join + body ranges.map! {|range| slice_body(response_body, range) }.join else status 200 body response_body @@ -55,6 +55,14 @@ class CompactIndexAPI < Endpoint value ? value.split(/, ?/).select {|s| s.sub!(/"(.*)"/, '\1') } : [] end + def slice_body(body, range) + if body.respond_to?(:byteslice) + body.byteslice(range) + else # pre-1.9.3 + body.unpack("@#{range.first}a#{range.end + 1}").first + end + end + def gems(gem_repo = gem_repo1) @gems ||= {} @gems[gem_repo] ||= begin diff --git a/spec/support/artifice/compact_index_concurrent_download.rb b/spec/support/artifice/compact_index_concurrent_download.rb new file mode 100644 index 0000000000..30a2171a30 --- /dev/null +++ b/spec/support/artifice/compact_index_concurrent_download.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true +require File.expand_path("../compact_index", __FILE__) + +Artifice.deactivate + +class CompactIndexConcurrentDownload < CompactIndexAPI + get "/versions" do + versions = File.join(Bundler.rubygems.user_home, ".bundle", "cache", "compact_index", + "localgemserver.test.80.dd34752a738ee965a2a4298dc16db6c5", "versions") + + # Verify the original (empty) content hasn't been deleted, e.g. on a retry + File.read(versions) == "" || raise("Original file should be present and empty") + + # Verify this is only requested once for a partial download + env["HTTP_RANGE"] || raise("Missing Range header for expected partial download") + + # Overwrite the file in parallel, which should be then overwritten + # after a successful download to prevent corruption + File.open(versions, "w") {|f| f.puts "another process" } + + etag_response do + file = tmp("versions.list") + file.delete if file.file? + file = CompactIndex::VersionsFile.new(file.to_s) + file.update_with(gems) + CompactIndex.versions(file, nil, {}) + end + end +end + +Artifice.activate_with(CompactIndexConcurrentDownload) |