Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accept responses with no etag header #3865

Merged
merged 3 commits into from Aug 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions bundler/lib/bundler/compact_index_client/updater.rb
Expand Up @@ -66,16 +66,16 @@ def update(local_path, remote_path, retrying = nil)
end
end

response_etag = (response["ETag"] || "").gsub(%r{\AW/}, "")
if etag_for(local_temp_path) == response_etag
etag = (response["ETag"] || "").gsub(%r{\AW/}, "")
if etag.length.zero? || etag_for(local_temp_path) == 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))
raise MisMatchedChecksumError.new(remote_path, etag, etag_for(local_temp_path))
end

update(local_path, remote_path, :retrying)
Expand Down
19 changes: 8 additions & 11 deletions bundler/spec/bundler/compact_index_client/updater_spec.rb
Expand Up @@ -6,25 +6,21 @@

RSpec.describe Bundler::CompactIndexClient::Updater do
let(:fetcher) { double(:fetcher) }
let(:local_path) { Pathname("/tmp/localpath") }
let(:local_path) { Pathname.new Dir.mktmpdir("localpath") }
let(:remote_path) { double(:remote_path) }

let!(:updater) { described_class.new(fetcher) }

context "when the ETag header is missing" do
# Regression test for https://github.com/rubygems/bundler/issues/5463
let(:response) { double(:response, :body => "abc123") }

let(:response) { double(:response, :body => "") }

it "MisMatchedChecksumError is raised" do
# Twice: #update retries on failure
expect(response).to receive(:[]).with("Content-Encoding").twice { "" }
expect(response).to receive(:[]).with("ETag").twice { nil }
expect(fetcher).to receive(:call).twice { response }
it "treats the response as an update" do
expect(response).to receive(:[]).with("Content-Encoding") { "" }
expect(response).to receive(:[]).with("ETag") { nil }
expect(fetcher).to receive(:call) { response }

expect do
updater.update(local_path, remote_path)
end.to raise_error(Bundler::CompactIndexClient::Updater::MisMatchedChecksumError)
updater.update(local_path, remote_path)
end
end

Expand All @@ -43,6 +39,7 @@

context "when bundler doesn't have permissions on Dir.tmpdir" do
it "Errno::EACCES is raised" do
local_path # create local path before stubbing mktmpdir
allow(Dir).to receive(:mktmpdir) { raise Errno::EACCES }

expect do
Expand Down