summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorThe Bundler Bot <bot@bundler.io>2018-08-27 17:33:53 +0000
committerColby Swandale <me@colby.fyi>2018-09-14 22:26:18 +1000
commit0e2da805133dd8251f80ba029f5ca27dfa31e646 (patch)
tree1689e58d66b54d8791ddb80e8981e67e2d6eee95
parenteb0f3b30f508ecb3416f1122813f96f5112ac5b4 (diff)
downloadbundler-0e2da805133dd8251f80ba029f5ca27dfa31e646.tar.gz
Auto merge of #6675 - MaxLap:master, r=greysteil
Handle RangeNotSatisfiable for compact index ### What was the end-user problem that led to this PR? On ruby 2.1, bundler would try to install the latest version of psych, which is not compatible with it, instead of using an older version. This is the error message i would get: `Gem::RuntimeRequirementNotMetError: psych requires Ruby version >= 2.2.2. The current ruby version is 2.1.0.` ### What was your diagnosis of the problem? Looking online, i managed to discover that there are 2 API in use, and if bundler can't use the compact index, it will not be able to detect ruby version requirements. (It might be a good idea to have a message that explain that more clearly somewhere, I found that in a random thread somewhere) Using `bundle install --verbose`, I found out that a RangeNotSatisfiable was being returned by the `info/psych` call and it stopped the usage of the compact index. I figured out that this was because the local cache file was too long after reading te code. I also knew that psych did a mistake in one of their release and had to yank versions in the past. ### What is your fix for the problem, implemented in this PR? When receiving the `RangeNotSatisfiable`, instead of generating an exception which bubbles up and forces bundler to switch to the dependency API, I return the response, and test for it in th caller. When the response is that one, I trigger a retry, which reloads the whole while by not requesting a specific range. ### Why did you choose this fix out of the possible options? It seems like the only logical fix to do in the current situation. If the API didn't use the HTTP header stuff, it would have been more efficient to change rubygems to return the full file or just the new etag instead of a RangeNotSatisfiable. Doing so would have solved the issue for all previous versions of Bundler. But the use of caching servers and so on means that it's better to just respect HTTP and do a second query. (cherry picked from commit 4e215b74197581de3b11cf3e2948d604da7ca2d6)
-rw-r--r--lib/bundler/fetcher/downloader.rb15
-rw-r--r--spec/install/gems/compact_index_spec.rb31
-rw-r--r--spec/support/artifice/compact_index_range_not_satisfiable.rb34
3 files changed, 75 insertions, 5 deletions
diff --git a/lib/bundler/fetcher/downloader.rb b/lib/bundler/fetcher/downloader.rb
index cbc5e220bd..e0e0cbf1c9 100644
--- a/lib/bundler/fetcher/downloader.rb
+++ b/lib/bundler/fetcher/downloader.rb
@@ -11,10 +11,10 @@ module Bundler
@redirect_limit = redirect_limit
end
- def fetch(uri, options = {}, counter = 0)
+ def fetch(uri, headers = {}, counter = 0)
raise HTTPError, "Too many redirects" if counter >= redirect_limit
- response = request(uri, options)
+ response = request(uri, headers)
Bundler.ui.debug("HTTP #{response.code} #{response.message} #{uri}")
case response
@@ -26,7 +26,12 @@ module Bundler
new_uri.user = uri.user
new_uri.password = uri.password
end
- fetch(new_uri, options, counter + 1)
+ fetch(new_uri, headers, counter + 1)
+ when Net::HTTPRequestedRangeNotSatisfiable
+ new_headers = headers.dup
+ new_headers.delete("Range")
+ new_headers["Accept-Encoding"] = "gzip"
+ fetch(uri, new_headers)
when Net::HTTPRequestEntityTooLarge
raise FallbackError, response.body
when Net::HTTPUnauthorized
@@ -38,11 +43,11 @@ module Bundler
end
end
- def request(uri, options)
+ def request(uri, headers)
validate_uri_scheme!(uri)
Bundler.ui.debug "HTTP GET #{uri}"
- req = Net::HTTP::Get.new uri.request_uri, options
+ req = Net::HTTP::Get.new uri.request_uri, headers
if uri.user
user = CGI.unescape(uri.user)
password = uri.password ? CGI.unescape(uri.password) : nil
diff --git a/spec/install/gems/compact_index_spec.rb b/spec/install/gems/compact_index_spec.rb
index f633004a3d..02a37a77d5 100644
--- a/spec/install/gems/compact_index_spec.rb
+++ b/spec/install/gems/compact_index_spec.rb
@@ -821,6 +821,37 @@ The checksum of /versions does not match the checksum provided by the server! So
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}"
+ gem 'rack', '0.9.1'
+ G
+
+ rake_info_path = File.join(Bundler.rubygems.user_home, ".bundle", "cache", "compact_index",
+ "localgemserver.test.80.dd34752a738ee965a2a4298dc16db6c5", "info", "rack")
+
+ bundle! :install, :artifice => "compact_index"
+
+ expected_rack_info_content = File.read(rake_info_path)
+
+ # Modify the cache files. We expect them to be reset to the normal ones when we re-run :install
+ File.open(rake_info_path, "w") {|f| f << (expected_rack_info_content + "this is different") }
+
+ # Update the Gemfile so the next install does its normal things
+ gemfile <<-G
+ source "#{source_uri}"
+ gem 'rack', '1.0.0'
+ G
+
+ # The cache files now being longer means the requested range is going to be not satisfiable
+ # Bundler must end up requesting the whole file to fix things up.
+ bundle! :install, :artifice => "compact_index_range_not_satisfiable"
+
+ resulting_rack_info_content = File.read(rake_info_path)
+
+ expect(resulting_rack_info_content).to eq(expected_rack_info_content)
+ end
+
it "fails gracefully when the source URI has an invalid scheme" do
install_gemfile <<-G
source "htps://rubygems.org"
diff --git a/spec/support/artifice/compact_index_range_not_satisfiable.rb b/spec/support/artifice/compact_index_range_not_satisfiable.rb
new file mode 100644
index 0000000000..487be4771a
--- /dev/null
+++ b/spec/support/artifice/compact_index_range_not_satisfiable.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+require File.expand_path("../compact_index", __FILE__)
+
+Artifice.deactivate
+
+class CompactIndexRangeNotSatisfiable < CompactIndexAPI
+ get "/versions" do
+ if env["HTTP_RANGE"]
+ status 416
+ else
+ etag_response do
+ file = tmp("versions.list")
+ file.delete if file.file?
+ file = CompactIndex::VersionsFile.new(file.to_s)
+ file.create(gems)
+ file.contents
+ end
+ end
+ end
+
+ get "/info/:name" do
+ if env["HTTP_RANGE"]
+ status 416
+ else
+ etag_response do
+ gem = gems.find {|g| g.name == params[:name] }
+ CompactIndex.info(gem ? gem.versions : [])
+ end
+ end
+ end
+end
+
+Artifice.activate_with(CompactIndexRangeNotSatisfiable)