Skip to content

Revisit the way we use URI.join

In https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/22555#note_111491694 we found that URI.join behaves differently in Ruby 2.4 and 2.5:

Ruby 2.5:

ruby -v -ruri -e 'p URI.join("http://test//", "a").to_s'
ruby 2.5.3p105 (2018-10-18 revision 65156) [x86_64-linux]
"http://test//a"

Ruby 2.4:

ruby -v -ruri -e 'p URI.join("http://test//", "a").to_s'
ruby 2.4.4p296 (2018-03-28 revision 63013) [x86_64-linux-gnu]
"http://test/a"

Looking at all the call sites for URI.join, we need to make sure our codes work the same way both in Ruby 2.4 and 2.5.

This turned out to be non-trivial because we're mixing the intention here. Sometimes we really want URI.join:

# => 'http://192.168.0.18:8888/job'
URI.join('http://192.168.0.18:8888/job/test1', '/job').to_s

The URL was taken from https://docs.gitlab.com/ee/integration/jenkins_deprecated.html and the code was taken from ee/app/models/project_services/jenkins_deprecated_service.rb.

But sometimes we only intend to use / to combine the base URL and path:

# => 'https://gitlab.com/bamboo/rest/api/latest/result/byChangeset/deadbeaf'
URI.join("#{bamboo_url}/", "rest/api/latest/result/byChangeset/deadbeaf").to_s

And we want to make sure whether bamboo_url has a trailing slash or not, it should work, and it should only have a single trailing slash. This could be easily accomplished by:

# => 'https://gitlab.com/bamboo/rest/api/latest/result/byChangeset/deadbeaf'
"#{bamboo_url.chomp('/')}/rest/api/latest/result/byChangeset/deadbeaf"

Thus it works fine both in Ruby 2.4 and 2.5.

The tough thing is that we can't blindly apply this everywhere we're using URI.join because we might really want URI.join! And it's not very easy to see if we'll break compatibility because for URI.join, people could enter things like /path or ../path and they'll give completely different result.

Here's another tough case:

gon.default_avatar_url =
  URI.join(
    Gitlab.config.gitlab.url,
    ActionController::Base.helpers.image_path('no_avatar.png')
  ).to_s

As far as I could tell, Gitlab.config.gitlab.url could be http://example.com/gitlab because we support installing GitLab on relative root. However, using URI.join will make it return the path like: http://example.com/images/no_avatar.png instead of http://example.com/gitlab/images/no_avatar.png.

I suspect we might have bugs for relative path here.

Edited by Lin Jen-Shin