[rubygems/rubygems] Surface entire redirect uri in permanent redirections

https://github.com/rubygems/rubygems/commit/da7837630b

5 files changed, 105 insertions, 43 deletions

diff --git a/lib/rubygems/gemcutter_utilities.rb b/lib/rubygems/gemcutter_utilities.rb
index 1112498357..fb7b9149ba 100644
--- a/lib/rubygems/gemcutter_utilities.rb
+++ b/lib/rubygems/gemcutter_utilities.rb
@@ -213,8 +213,7 @@ module Gem::GemcutterUtilities
         say clean_text(response.body)
       end
     when Net::HTTPPermanentRedirect, Net::HTTPRedirection then
-      message = "The request has redirected permanently to #{Gem::Uri.parse(response['location']).origin}. " \
-        "Please check your defined push host."
+      message = "The request has redirected permanently to #{response['location']}. Please check your defined push host."
       message = "#{error_prefix}: #{message}" if error_prefix
 
       say clean_text(message)
diff --git a/test/rubygems/test_gem_commands_owner_command.rb b/test/rubygems/test_gem_commands_owner_command.rb
index 3d0a265eb1..1f03838614 100644
--- a/test/rubygems/test_gem_commands_owner_command.rb
+++ b/test/rubygems/test_gem_commands_owner_command.rb
@@ -121,17 +121,23 @@ EOF
   def test_show_owners_permanent_redirect
     host = "http://rubygems.example"
     ENV["RUBYGEMS_HOST"] = host
-    @stub_fetcher.data["#{host}/api/v1/gems/freewill/owners.yaml"] = ["", 301, "Moved Permanently"]
+    path = "/api/v1/gems/freewill/owners.yaml"
+    redirected_uri = "https://rubygems.example#{path}"
+
+    @stub_fetcher.data["#{host}#{path}"] = HTTPResponseFactory.create(
+      body: "",
+      code: "301",
+      msg: "Moved Permanently",
+      headers: { "location" => redirected_uri }
+    )
 
     assert_raise Gem::MockGemUi::TermError do
       use_ui @stub_ui do
-        Gem::Uri.stub("parse", URI.parse("https://rubygems.example/")) do
-          @cmd.show_owners("freewill")
-        end
+        @cmd.show_owners("freewill")
       end
     end
 
-    response = "The request has redirected permanently to https://rubygems.example. Please check your defined push host."
+    response = "The request has redirected permanently to #{redirected_uri}. Please check your defined push host."
     assert_match response, @stub_ui.output
   end
 
@@ -178,15 +184,21 @@ EOF
   def test_add_owners_permanent_redirect
     host = "http://rubygems.example"
     ENV["RUBYGEMS_HOST"] = host
-    @stub_fetcher.data["#{host}/api/v1/gems/freewill/owners"] = ["", 308, "Permanent Redirect"]
+    path = "/api/v1/gems/freewill/owners"
+    redirected_uri = "https://rubygems.example#{path}"
+
+    @stub_fetcher.data["#{host}#{path}"] = HTTPResponseFactory.create(
+      body: "",
+      code: "308",
+      msg: "Permanent Redirect",
+      headers: { "location" => redirected_uri }
+    )
 
     use_ui @stub_ui do
-      Gem::Uri.stub("parse", URI.parse("https://rubygems.example/")) do
-        @cmd.add_owners("freewill", ["user-new1@example.com"])
-      end
+      @cmd.add_owners("freewill", ["user-new1@example.com"])
     end
 
-    response = "The request has redirected permanently to https://rubygems.example. Please check your defined push host."
+    response = "The request has redirected permanently to #{redirected_uri}. Please check your defined push host."
     assert_match response, @stub_ui.output
   end
 
@@ -251,15 +263,37 @@ EOF
   def test_remove_owners_permanent_redirect
     host = "http://rubygems.example"
     ENV["RUBYGEMS_HOST"] = host
-    @stub_fetcher.data["#{host}/api/v1/gems/freewill/owners"] = ["", 308, "Permanent Redirect"]
+    path = "/api/v1/gems/freewill/owners"
+    redirected_uri = "https://rubygems.example#{path}"
+    @stub_fetcher.data["#{host}#{path}"] = HTTPResponseFactory.create(
+      body: "",
+      code: "308",
+      msg: "Permanent Redirect",
+      headers: { "location" => redirected_uri }
+    )
 
     use_ui @stub_ui do
-      Gem::Uri.stub("parse", URI.parse("https://rubygems.example/")) do
-        @cmd.remove_owners("freewill", ["user-remove1@example.com"])
-      end
+      @cmd.remove_owners("freewill", ["user-remove1@example.com"])
+    end
+
+    response = "The request has redirected permanently to #{redirected_uri}. Please check your defined push host."
+    assert_match response, @stub_ui.output
+
+    path = "/api/v1/gems/freewill/owners"
+    redirected_uri = "https://rubygems.example#{path}"
+
+    @stub_fetcher.data["#{host}#{path}"] = HTTPResponseFactory.create(
+      body: "",
+      code: "308",
+      msg: "Permanent Redirect",
+      headers: { "location" => redirected_uri }
+    )
+
+    use_ui @stub_ui do
+      @cmd.add_owners("freewill", ["user-new1@example.com"])
     end
 
-    response = "The request has redirected permanently to https://rubygems.example. Please check your defined push host."
+    response = "The request has redirected permanently to #{redirected_uri}. Please check your defined push host."
     assert_match response, @stub_ui.output
   end
 
diff --git a/test/rubygems/test_gem_commands_push_command.rb b/test/rubygems/test_gem_commands_push_command.rb
index c58d3f3692..770ba25681 100644
--- a/test/rubygems/test_gem_commands_push_command.rb
+++ b/test/rubygems/test_gem_commands_push_command.rb
@@ -327,19 +327,17 @@ class TestGemCommandsPushCommand < Gem::TestCase
 
   def test_sending_gem_to_permanent_redirect_host
     @host = "http://rubygems.example"
-    @fetcher.data["#{@host}/api/v1/gems"] = ["", 308, "Permanent Redirect"]
+    redirected_uri = "https://rubygems.example/api/v1/gems"
+    @fetcher.data["#{@host}/api/v1/gems"] = HTTPResponseFactory.create(body: "", code: 308, msg: "Permanent Redirect", headers: { "Location" => redirected_uri })
 
     assert_raise Gem::MockGemUi::TermError do
       use_ui @ui do
         @cmd.instance_variable_set :@host, @host
-
-        Gem::Uri.stub("parse", URI.parse("https://rubygems.example/")) do
-          @cmd.send_gem(@path)
-        end
+        @cmd.send_gem(@path)
       end
     end
 
-    response = "The request has redirected permanently to https://rubygems.example. Please check your defined push host."
+    response = "The request has redirected permanently to #{redirected_uri}. Please check your defined push host."
     assert_match response, @ui.output
   end
 
diff --git a/test/rubygems/test_gem_commands_signin_command.rb b/test/rubygems/test_gem_commands_signin_command.rb
index 940e68b3da..78a5f92d18 100644
--- a/test/rubygems/test_gem_commands_signin_command.rb
+++ b/test/rubygems/test_gem_commands_signin_command.rb
@@ -73,22 +73,26 @@ class TestGemCommandsSigninCommand < Gem::TestCase
 
   def test_execute_with_host_permanent_redirect
     host = "http://rubygems.example/"
-    ENV["RUBYGEMS_HOST"]       = host
-    data_key                   = "#{host}/api/v1/api_key"
-    fetcher                    = Gem::FakeFetcher.new
-    fetcher.data[data_key]     = ["", 308, "Moved Permanently"]
+    ENV["RUBYGEMS_HOST"] = host
+    path = "/api/v1/api_key"
+    redirected_uri = "http://rubygems.example#{path}"
+    fetcher = Gem::FakeFetcher.new
+    fetcher.data["#{host}#{path}"] = HTTPResponseFactory.create(
+      body: "",
+      code: "308",
+      msg: "Permanent Redirect",
+      headers: { "location" => redirected_uri }
+    )
     Gem::RemoteFetcher.fetcher = fetcher
     ui = Gem::MockGemUi.new("you@example.com\nsecret\n\n\n\n\n\n\n\n\n")
 
     assert_raise Gem::MockGemUi::TermError do
       use_ui ui do
-        Gem::Uri.stub("parse", URI.parse("https://rubygems.example/")) do
-          @cmd.execute
-        end
+        @cmd.execute
       end
     end
 
-    response = "The request has redirected permanently to https://rubygems.example. Please check your defined push host."
+    response = "The request has redirected permanently to #{redirected_uri}. Please check your defined push host."
     assert_match response, ui.output
   end
 
diff --git a/test/rubygems/utilities.rb b/test/rubygems/utilities.rb
index c01f7acd48..96b197c096 100644
--- a/test/rubygems/utilities.rb
+++ b/test/rubygems/utilities.rb
@@ -61,6 +61,19 @@ class Gem::FakeFetcher
     end
   end
 
+  def create_response(uri)
+    data = find_data(uri)
+    if data.kind_of?(Array)
+      body, code, msg = data
+      HTTPResponseFactory.create(body: body, code: code, msg: msg)
+    elsif data.respond_to?(:call)
+      body, code, msg = data.call
+      HTTPResponseFactory.create(body: body, code: code, msg: msg)
+    else
+      data
+    end
+  end
+
   def fetch_path(path, mtime = nil, head = false)
     data = find_data(path)
 
@@ -86,25 +99,15 @@ class Gem::FakeFetcher
   # Thanks, FakeWeb!
   def open_uri_or_path(path)
     data = find_data(path)
-    body, code, msg = data
 
-    response = Net::HTTPResponse.send(:response_class, code.to_s).new("1.0", code.to_s, msg)
-    response.instance_variable_set(:@body, body)
-    response.instance_variable_set(:@read, true)
-    response
+    create_response(uri)
   end
 
   def request(uri, request_class, last_modified = nil)
-    data = find_data(uri)
-    body, code, msg = (data.respond_to?(:call) ? data.call : data)
-
     @last_request = request_class.new uri.request_uri
     yield @last_request if block_given?
 
-    response = Net::HTTPResponse.send(:response_class, code.to_s).new("1.0", code.to_s, msg)
-    response.instance_variable_set(:@body, body)
-    response.instance_variable_set(:@read, true)
-    response
+    create_response(uri)
   end
 
   def pretty_print(q) # :nodoc:
@@ -164,6 +167,30 @@ class Gem::FakeFetcher
   end
 end
 
+##
+# The HTTPResponseFactory allows easy creation of Net::HTTPResponse instances in RubyGems tests:
+#
+# Example:
+#
+#   HTTPResponseFactory.create(
+#     body: "",
+#     code: 301,
+#     msg: "Moved Permanently",
+#     headers: { "location" => "http://example.com" }
+#   )
+#
+
+class HTTPResponseFactory
+  def self.create(body:, code:, msg:, headers: {})
+    response = Net::HTTPResponse.send(:response_class, code.to_s).new("1.0", code.to_s, msg)
+    response.instance_variable_set(:@body, body)
+    response.instance_variable_set(:@read, true)
+    headers.each {|name, value| response[name] = value }
+
+    response
+  end
+end
+
 # :stopdoc:
 class Gem::RemoteFetcher
   def self.fetcher=(fetcher)