diff --git a/bundler/lib/bundler/compact_index_client/updater.rb b/bundler/lib/bundler/compact_index_client/updater.rb index 1dc70012c288..9348f3f5d082 100644 --- a/bundler/lib/bundler/compact_index_client/updater.rb +++ b/bundler/lib/bundler/compact_index_client/updater.rb @@ -58,8 +58,8 @@ def update(local_path, remote_path, retrying = nil) end end - etag = (response["ETag"] || "").gsub(%r{\AW/}, "") - if etag.length.zero? || etag_for(local_temp_path) == etag + 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 @@ -67,7 +67,7 @@ def update(local_path, remote_path, retrying = nil) end if retrying - raise MisMatchedChecksumError.new(remote_path, etag, etag_for(local_temp_path)) + raise MisMatchedChecksumError.new(remote_path, response_etag, etag_for(local_temp_path)) end update(local_path, remote_path, :retrying) diff --git a/bundler/spec/bundler/compact_index_client/updater_spec.rb b/bundler/spec/bundler/compact_index_client/updater_spec.rb index 4acd7dbc6341..699ce684d839 100644 --- a/bundler/spec/bundler/compact_index_client/updater_spec.rb +++ b/bundler/spec/bundler/compact_index_client/updater_spec.rb @@ -16,11 +16,14 @@ # Regression test for https://github.com/rubygems/bundler/issues/5463 let(:response) { double(:response, :body => "abc123") } - it "treats the response as an update" do - expect(response).to receive(:[]).with("ETag") { nil } - expect(fetcher).to receive(:call) { response } + it "MisMatchedChecksumError is raised" do + # Twice: #update retries on failure + expect(response).to receive(:[]).with("ETag").twice { nil } + expect(fetcher).to receive(:call).twice { response } - updater.update(local_path, remote_path) + expect do + updater.update(local_path, remote_path) + end.to raise_error(Bundler::CompactIndexClient::Updater::MisMatchedChecksumError) end end @@ -56,7 +59,7 @@ begin $VERBOSE = false Encoding.default_internal = "ASCII" - expect(response).to receive(:[]).with("ETag") { nil } + expect(response).to receive(:[]).with("ETag").and_return('"348dd9e974848a61e8abe2dffcfcce93"') expect(fetcher).to receive(:call) { response } updater.update(local_path, remote_path) diff --git a/bundler/spec/install/gems/compact_index_spec.rb b/bundler/spec/install/gems/compact_index_spec.rb index b1f361ebec94..9454508e41e4 100644 --- a/bundler/spec/install/gems/compact_index_spec.rb +++ b/bundler/spec/install/gems/compact_index_spec.rb @@ -826,6 +826,28 @@ def start expect(the_bundle).to include_gems "rack 1.0.0" end + it "performs full update if server endpoints serve partial content responses but don't have incremental content and provide no Etag" do + build_repo4 do + build_gem "rack", "0.9.1" + end + + install_gemfile <<-G, :artifice => "compact_index", :env => { "BUNDLER_SPEC_GEM_REPO" => gem_repo4.to_s } + source "#{source_uri}" + gem 'rack', '0.9.1' + G + + update_repo4 do + build_gem "rack", "1.0.0" + end + + install_gemfile <<-G, :artifice => "compact_index_partial_update_no_etag_not_incremental", :env => { "BUNDLER_SPEC_GEM_REPO" => gem_repo4.to_s } + source "#{source_uri}" + gem 'rack', '1.0.0' + G + + expect(the_bundle).to include_gems "rack 1.0.0" + end + it "performs full update of compact index info cache if range is not satisfiable" do gemfile <<-G source "#{source_uri}" diff --git a/bundler/spec/support/artifice/compact_index_partial_update_no_etag_not_incremental.rb b/bundler/spec/support/artifice/compact_index_partial_update_no_etag_not_incremental.rb new file mode 100644 index 000000000000..e2fa6b41386d --- /dev/null +++ b/bundler/spec/support/artifice/compact_index_partial_update_no_etag_not_incremental.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require_relative "compact_index" + +Artifice.deactivate + +class CompactIndexPartialUpdateNoEtagNotIncremental < CompactIndexAPI + def partial_update_no_etag + response_body = yield + headers "Surrogate-Control" => "max-age=2592000, stale-while-revalidate=60" + content_type "text/plain" + requested_range_for(response_body) + end + + get "/versions" do + partial_update_no_etag do + file = tmp("versions.list") + FileUtils.rm_f(file) + file = CompactIndex::VersionsFile.new(file.to_s) + file.create(gems) + lines = file.contents.split("\n") + + # shuffle versions so new versions are not appended to the end + [*lines[0..-2], lines.last.split(",").reverse.join(",")].join("\n") + end + end + + get "/info/:name" do + partial_update_no_etag do + gem = gems.find {|g| g.name == params[:name] } + lines = CompactIndex.info(gem ? gem.versions : []).split("\n") + + # shuffle versions so new versions are not appended to the end + [lines.first, lines.last, *lines[1..-2]].join("\n") + end + end +end + +Artifice.activate_with(CompactIndexPartialUpdateNoEtagNotIncremental)