From b1e4fb15ae1449ff3b2bedbf8ca55dfc5659fe1d Mon Sep 17 00:00:00 2001 From: Matthias Zirnstein Date: Wed, 5 Nov 2025 10:51:55 +0100 Subject: [PATCH 1/4] fix: Ensure trailing slash is added to source URIs added via gem sources GitHub's private gem registry expects the first path segment after the host to represent the namespace, typically the organization or user name. [1] When adding a source with ``` gem sources --add https://user:password@rubygems.pkg.github.com/my-org ``` without a trailing slash, the last path segment ("my-org") is interpreted as a file and removed during relative path resolution. This causes the resulting URI to become ``` https://user:password@rubygems.pkg.github.com/gems/foo.gem ``` instead of the correct ``` https://user:password@rubygems.pkg.github.com/my-org/gems/foo.gem. [2] ``` Example error: ``` gem source -a https://user:password@rubygems.pkg.github.com/my-org gem install -rf foo.gem rubygems/remote_fetcher.rb:238:in `fetch_http': bad response Not Found 404 (https://user:REDACTED@rubygems.pkg.github.com/gems/foo-0.7.1.gem) (Gem::RemoteFetcher::FetchError) ``` Although this behavior complies with RFC 2396, it's incompatible with GitHub's gem registry requirements. The remote fetcher is just append a relative path without using ./ [3] To address this, we automatically append a trailing slash when adding new gem sources. As illustrated in [4] and [5], given the base URI ``` http://a/b/c/d;p?q ``` and a relative path ``` g/f ``` the resolution process replaces "d;p?q" and yields ``` http://a/b/c/g/f ``` [1] https://docs.github.com/en/packages/working-with-a-github-packages-registry/working-with-the-rubygems-registry#authenticating-with-a-personal-access-token [2] https://github.com/ruby/rubygems/blob/master/lib/rubygems/vendor/uri/lib/uri/generic.rb#L1053 [3] https://github.com/ruby/rubygems/blob/master/lib/rubygems/remote_fetcher.rb#L148 [4] https://www.rfc-editor.org/rfc/rfc2396#section-5.2 [5] https://www.rfc-editor.org/rfc/rfc2396#appendix-C --- lib/rubygems/commands/sources_command.rb | 16 ++++ .../test_gem_commands_sources_command.rb | 84 ++++++++++++++++++- 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/lib/rubygems/commands/sources_command.rb b/lib/rubygems/commands/sources_command.rb index 7e5c2a2465e6..95be9d334b33 100644 --- a/lib/rubygems/commands/sources_command.rb +++ b/lib/rubygems/commands/sources_command.rb @@ -50,6 +50,7 @@ def initialize end def add_source(source_uri) # :nodoc: + source_uri = add_trailing_slash(source_uri) check_rubygems_https source_uri source = Gem::Source.new source_uri @@ -76,6 +77,7 @@ def add_source(source_uri) # :nodoc: end def append_source(source_uri) # :nodoc: + source_uri = add_trailing_slash(source_uri) check_rubygems_https source_uri source = Gem::Source.new source_uri @@ -103,6 +105,7 @@ def append_source(source_uri) # :nodoc: end def prepend_source(source_uri) # :nodoc: + source_uri = add_trailing_slash(source_uri) check_rubygems_https source_uri source = Gem::Source.new source_uri @@ -141,6 +144,19 @@ def check_typo_squatting(source) end end + def add_trailing_slash(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{/+$}, "") + "/" 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 diff --git a/test/rubygems/test_gem_commands_sources_command.rb b/test/rubygems/test_gem_commands_sources_command.rb index 00eb9239940e..62795e5665b9 100644 --- a/test/rubygems/test_gem_commands_sources_command.rb +++ b/test/rubygems/test_gem_commands_sources_command.rb @@ -60,6 +60,81 @@ 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 [@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 setup_fake_source(@new_repo) @@ -583,7 +658,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 +677,12 @@ 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 EOF assert_equal expected, @ui.output From 376bcdf430445bca99b5647baef7a22dac9975cd Mon Sep 17 00:00:00 2001 From: Matthias Zirnstein Date: Wed, 5 Nov 2025 10:52:21 +0100 Subject: [PATCH 2/4] spec: Add missing bad uri test for prepend for gem sources --- test/rubygems/test_gem_commands_sources_command.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/rubygems/test_gem_commands_sources_command.rb b/test/rubygems/test_gem_commands_sources_command.rb index 62795e5665b9..60df6d9a8055 100644 --- a/test/rubygems/test_gem_commands_sources_command.rb +++ b/test/rubygems/test_gem_commands_sources_command.rb @@ -683,6 +683,20 @@ def test_execute_append_bad_uri 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 From 3e81961ef8e186d8f64acf1cf9ee05760016d25c Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Wed, 18 Mar 2026 07:04:46 +0900 Subject: [PATCH 3/4] Refactor source URI normalization in sources command - Rename add_trailing_slash to normalize_source_uri to better reflect that it also removes duplicated trailing slashes - Extract build_source and build_new_source to eliminate duplicated source creation pattern across add_source, append_source, prepend_source, and remove_source - Apply URI normalization to remove_source as well - Use \z instead of $ in trailing slash regex for correctness Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/rubygems/commands/sources_command.rb | 43 ++++++++++++------------ 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/lib/rubygems/commands/sources_command.rb b/lib/rubygems/commands/sources_command.rb index 95be9d334b33..b399af2bd3d5 100644 --- a/lib/rubygems/commands/sources_command.rb +++ b/lib/rubygems/commands/sources_command.rb @@ -50,12 +50,8 @@ def initialize end def add_source(source_uri) # :nodoc: - source_uri = add_trailing_slash(source_uri) - 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 @@ -77,12 +73,8 @@ def add_source(source_uri) # :nodoc: end def append_source(source_uri) # :nodoc: - source_uri = add_trailing_slash(source_uri) - 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 @@ -105,12 +97,8 @@ def append_source(source_uri) # :nodoc: end def prepend_source(source_uri) # :nodoc: - source_uri = add_trailing_slash(source_uri) - 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 @@ -144,13 +132,13 @@ def check_typo_squatting(source) end end - def add_trailing_slash(source_uri) # :nodoc: + 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{/+$}, "") + "/" if uri.path && !uri.path.empty? + 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 @@ -289,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 @@ -344,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 From da8d622c050d6eeb18b015ba6491ea215e1034e8 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Wed, 18 Mar 2026 07:04:55 +0900 Subject: [PATCH 4/4] Fix and improve tests for source URI trailing slash normalization - Fix setup_fake_source double-slash bug: normalize URI before registering spec data to prevent URL mismatch in load_specs - Fix test_execute_add/append_https_rubygems_org: these tests were incorrectly expecting TermError due to the double-slash bug - Fix test_execute_prepend_without_trailing_slash: correct expected source order (prepend adds to front, not end) - Fix test_execute_remove_redundant_source_trailing_slash: path-less URIs are not modified by normalize_source_uri - Use assert_equal for Gem.sources to improve failure diagnostics Co-Authored-By: Claude Opus 4.6 (1M context) --- .../test_gem_commands_sources_command.rb | 66 ++++++++++++------- 1 file changed, 43 insertions(+), 23 deletions(-) diff --git a/test/rubygems/test_gem_commands_sources_command.rb b/test/rubygems/test_gem_commands_sources_command.rb index 60df6d9a8055..71c6d5ce1668 100644 --- a/test/rubygems/test_gem_commands_sources_command.rb +++ b/test/rubygems/test_gem_commands_sources_command.rb @@ -61,7 +61,7 @@ def test_execute_add end def test_execute_add_without_trailing_slash - setup_fake_source('https://rubygems.pkg.github.com/my-org') + setup_fake_source("https://rubygems.pkg.github.com/my-org") @cmd.handle_options %W[--add https://rubygems.pkg.github.com/my-org] @@ -69,7 +69,7 @@ def test_execute_add_without_trailing_slash @cmd.execute end - assert_equal [@gem_repo, 'https://rubygems.pkg.github.com/my-org/'], Gem.sources + 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 @@ -78,8 +78,9 @@ def test_execute_add_without_trailing_slash 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/') + setup_fake_source("https://rubygems.pkg.github.com/my-org/") @cmd.handle_options %W[--add https://rubygems.pkg.github.com/my-org///] @@ -87,7 +88,7 @@ def test_execute_add_multiple_trailing_slash @cmd.execute end - assert_equal [@gem_repo, 'https://rubygems.pkg.github.com/my-org/'], Gem.sources + 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 @@ -98,7 +99,7 @@ def test_execute_add_multiple_trailing_slash end def test_execute_append_without_trailing_slash - setup_fake_source('https://rubygems.pkg.github.com/my-org') + setup_fake_source("https://rubygems.pkg.github.com/my-org") @cmd.handle_options %W[--append https://rubygems.pkg.github.com/my-org] @@ -106,7 +107,7 @@ def test_execute_append_without_trailing_slash @cmd.execute end - assert_equal [@gem_repo, 'https://rubygems.pkg.github.com/my-org/'], Gem.sources + 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 @@ -117,7 +118,7 @@ def test_execute_append_without_trailing_slash end def test_execute_prepend_without_trailing_slash - setup_fake_source('https://rubygems.pkg.github.com/my-org') + setup_fake_source("https://rubygems.pkg.github.com/my-org") @cmd.handle_options %W[--prepend https://rubygems.pkg.github.com/my-org] @@ -125,7 +126,7 @@ def test_execute_prepend_without_trailing_slash @cmd.execute end - assert_equal [@gem_repo, 'https://rubygems.pkg.github.com/my-org/'], Gem.sources + 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 @@ -605,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 @@ -629,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 @@ -872,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] @@ -982,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