Merge pull request #9055 from zirni/fix-gem-sources-trailing-slash

fix: Ensure trailing slash is added to source URIs added via gem sources

Author: hsbt

lib/rubygems/commands/sources_command.rb

@@ -50,11 +50,8 @@ def initialize
   end
 
   def add_source(source_uri) # :nodoc:
-    check_rubygems_https source_uri
-
-    source = Gem::Source.new source_uri
-
-    check_typo_squatting(source)
+    source = build_new_source(source_uri)
+    source_uri = source.uri.to_s
 
     begin
       if Gem.sources.include? source
@@ -76,11 +73,8 @@ def add_source(source_uri) # :nodoc:
   end
 
   def append_source(source_uri) # :nodoc:
-    check_rubygems_https source_uri
-
-    source = Gem::Source.new source_uri
-
-    check_typo_squatting(source)
+    source = build_new_source(source_uri)
+    source_uri = source.uri.to_s
 
     begin
       source.load_specs :released
@@ -103,11 +97,8 @@ def append_source(source_uri) # :nodoc:
   end
 
   def prepend_source(source_uri) # :nodoc:
-    check_rubygems_https source_uri
-
-    source = Gem::Source.new source_uri
-
-    check_typo_squatting(source)
+    source = build_new_source(source_uri)
+    source_uri = source.uri.to_s
 
     begin
       source.load_specs :released
@@ -141,6 +132,19 @@ def check_typo_squatting(source)
     end
   end
 
+  def normalize_source_uri(source_uri) # :nodoc:
+    # Ensure the source URI has a trailing slash for proper RFC 2396 path merging
+    # Without a trailing slash, the last path segment is treated as a file and removed
+    # during relative path resolution (e.g., "/blish" + "gems/foo.gem" = "/gems/foo.gem")
+    # With a trailing slash, it's treated as a directory (e.g., "/blish/" + "gems/foo.gem" = "/blish/gems/foo.gem")
+    uri = Gem::URI.parse(source_uri)
+    uri.path = uri.path.gsub(%r{/+\z}, "") + "/" if uri.path && !uri.path.empty?
+    uri.to_s
+  rescue Gem::URI::Error
+    # If parsing fails, return the original URI and let later validation handle it
+    source_uri
+  end
+
   def check_rubygems_https(source_uri) # :nodoc:
     uri = Gem::URI source_uri
 
@@ -273,7 +277,8 @@ def execute
   end
 
   def remove_source(source_uri) # :nodoc:
-    source = Gem::Source.new source_uri
+    source = build_source(source_uri)
+    source_uri = source.uri.to_s
 
     if configured_sources&.include? source
       Gem.sources.delete source
@@ -328,4 +333,16 @@ def configured_sources
   def config_file_name
     Gem.configuration.config_file_name
   end
+
+  def build_source(source_uri)
+    source_uri = normalize_source_uri(source_uri)
+    Gem::Source.new(source_uri)
+  end
+
+  def build_new_source(source_uri)
+    source = build_source(source_uri)
+    check_rubygems_https(source.uri.to_s)
+    check_typo_squatting(source)
+    source
+  end
 end

test/rubygems/test_gem_commands_sources_command.rb

@@ -60,6 +60,82 @@ def test_execute_add
     assert_equal "", @ui.error
   end
 
+  def test_execute_add_without_trailing_slash
+    setup_fake_source("https://rubygems.pkg.github.com/my-org")
+
+    @cmd.handle_options %W[--add https://rubygems.pkg.github.com/my-org]
+
+    use_ui @ui do
+      @cmd.execute
+    end
+
+    assert_equal [@gem_repo, "https://rubygems.pkg.github.com/my-org/"], Gem.sources
+
+    expected = <<-EOF
+https://rubygems.pkg.github.com/my-org/ added to sources
+    EOF
+
+    assert_equal expected, @ui.output
+    assert_equal "", @ui.error
+  end
+
+  def test_execute_add_multiple_trailing_slash
+    setup_fake_source("https://rubygems.pkg.github.com/my-org/")
+
+    @cmd.handle_options %W[--add https://rubygems.pkg.github.com/my-org///]
+
+    use_ui @ui do
+      @cmd.execute
+    end
+
+    assert_equal [@gem_repo, "https://rubygems.pkg.github.com/my-org/"], Gem.sources
+
+    expected = <<-EOF
+https://rubygems.pkg.github.com/my-org/ added to sources
+    EOF
+
+    assert_equal expected, @ui.output
+    assert_equal "", @ui.error
+  end
+
+  def test_execute_append_without_trailing_slash
+    setup_fake_source("https://rubygems.pkg.github.com/my-org")
+
+    @cmd.handle_options %W[--append https://rubygems.pkg.github.com/my-org]
+
+    use_ui @ui do
+      @cmd.execute
+    end
+
+    assert_equal [@gem_repo, "https://rubygems.pkg.github.com/my-org/"], Gem.sources
+
+    expected = <<-EOF
+https://rubygems.pkg.github.com/my-org/ added to sources
+    EOF
+
+    assert_equal expected, @ui.output
+    assert_equal "", @ui.error
+  end
+
+  def test_execute_prepend_without_trailing_slash
+    setup_fake_source("https://rubygems.pkg.github.com/my-org")
+
+    @cmd.handle_options %W[--prepend https://rubygems.pkg.github.com/my-org]
+
+    use_ui @ui do
+      @cmd.execute
+    end
+
+    assert_equal ["https://rubygems.pkg.github.com/my-org/", @gem_repo], Gem.sources
+
+    expected = <<-EOF
+https://rubygems.pkg.github.com/my-org/ added to sources
+    EOF
+
+    assert_equal expected, @ui.output
+    assert_equal "", @ui.error
+  end
+
   def test_execute_append
     setup_fake_source(@new_repo)
 
@@ -530,17 +606,14 @@ def test_execute_add_https_rubygems_org
 
     @cmd.handle_options %W[--add #{https_rubygems_org}]
 
-    ui = Gem::MockGemUi.new "n"
-
-    use_ui ui do
-      assert_raise Gem::MockGemUi::TermError do
-        @cmd.execute
-      end
+    use_ui @ui do
+      @cmd.execute
     end
 
-    assert_equal [@gem_repo], Gem.sources
+    assert_equal [@gem_repo, https_rubygems_org], Gem.sources
 
     expected = <<-EXPECTED
+#{https_rubygems_org} added to sources
     EXPECTED
 
     assert_equal expected, @ui.output
@@ -554,17 +627,14 @@ def test_execute_append_https_rubygems_org
 
     @cmd.handle_options %W[--append #{https_rubygems_org}]
 
-    ui = Gem::MockGemUi.new "n"
-
-    use_ui ui do
-      assert_raise Gem::MockGemUi::TermError do
-        @cmd.execute
-      end
+    use_ui @ui do
+      @cmd.execute
     end
 
-    assert_equal [@gem_repo], Gem.sources
+    assert_equal [@gem_repo, https_rubygems_org], Gem.sources
 
     expected = <<-EXPECTED
+#{https_rubygems_org} added to sources
     EXPECTED
 
     assert_equal expected, @ui.output
@@ -583,7 +653,7 @@ def test_execute_add_bad_uri
     assert_equal [@gem_repo], Gem.sources
 
     expected = <<-EOF
-beta-gems.example.com is not a URI
+beta-gems.example.com/ is not a URI
     EOF
 
     assert_equal expected, @ui.output
@@ -602,7 +672,26 @@ def test_execute_append_bad_uri
     assert_equal [@gem_repo], Gem.sources
 
     expected = <<-EOF
-beta-gems.example.com is not a URI
+beta-gems.example.com/ is not a URI
+    EOF
+
+    assert_equal expected, @ui.output
+    assert_equal "", @ui.error
+  end
+
+  def test_execute_prepend_bad_uri
+    @cmd.handle_options %w[--prepend beta-gems.example.com]
+
+    use_ui @ui do
+      assert_raise Gem::MockGemUi::TermError do
+        @cmd.execute
+      end
+    end
+
+    assert_equal [@gem_repo], Gem.sources
+
+    expected = <<-EOF
+beta-gems.example.com/ is not a URI
     EOF
 
     assert_equal expected, @ui.output
@@ -778,6 +867,31 @@ def test_execute_remove_redundant_source_trailing_slash
     Gem.configuration.sources = nil
   end
 
+  def test_execute_remove_without_trailing_slash
+    source_uri = "https://rubygems.pkg.github.com/my-org/"
+
+    Gem.configuration.sources = [source_uri]
+
+    setup_fake_source(source_uri)
+
+    @cmd.handle_options %W[--remove https://rubygems.pkg.github.com/my-org]
+
+    use_ui @ui do
+      @cmd.execute
+    end
+
+    assert_equal [], Gem.sources
+
+    expected = <<-EOF
+#{source_uri} removed from sources
+    EOF
+
+    assert_equal expected, @ui.output
+    assert_equal "", @ui.error
+  ensure
+    Gem.configuration.sources = nil
+  end
+
   def test_execute_update
     @cmd.handle_options %w[--update]
 
@@ -888,6 +1002,6 @@ def setup_fake_source(uri)
       Marshal.dump specs, io
     end
 
-    @fetcher.data["#{uri}/specs.#{@marshal_version}.gz"] = specs_dump_gz.string
+    @fetcher.data["#{uri.chomp("/")}/specs.#{@marshal_version}.gz"] = specs_dump_gz.string
   end
 end