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.