Make sure there's only one slash as path separatorsh-53180-append-path

In Ruby 2.4, `URI.join("http://test//", "a").to_s` will
remove the double slash, however it's not the case in
Ruby 2.5. Using chomp should work better for the intention,
as we're not trying to allow things like ../ or / paths
resolution.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/53180

13 files changed, 43 insertions, 54 deletions

diff --git a/app/helpers/import_helper.rb b/app/helpers/import_helper.rb
index 3d0eb3d0d51..49171df1433 100644
--- a/app/helpers/import_helper.rb
+++ b/app/helpers/import_helper.rb
@@ -83,7 +83,7 @@ module ImportHelper
   private
 
   def github_project_url(full_path)
-    URI.join(github_root_url, full_path).to_s
+    Gitlab::Utils.append_path(github_root_url, full_path)
   end
 
   def github_root_url
@@ -95,6 +95,6 @@ module ImportHelper
   end
 
   def gitea_project_url(full_path)
-    URI.join(@gitea_host_url, full_path).to_s
+    Gitlab::Utils.append_path(@gitea_host_url, full_path)
   end
 end
diff --git a/app/models/project_services/bamboo_service.rb b/app/models/project_services/bamboo_service.rb
index d121d088ff6..a252052200a 100644
--- a/app/models/project_services/bamboo_service.rb
+++ b/app/models/project_services/bamboo_service.rb
@@ -86,14 +86,16 @@ class BambooService < CiService
   end
 
   def read_build_page(response)
-    if response.code != 200 || response.dig('results', 'results', 'size') == '0'
-      # If actual build link can't be determined, send user to build summary page.
-      URI.join("#{bamboo_url}/", "browse/#{build_key}").to_s
-    else
-      # If actual build link is available, go to build result page.
-      result_key = response.dig('results', 'results', 'result', get_build_result_index, 'planResultKey', 'key')
-      URI.join("#{bamboo_url}/", "browse/#{result_key}").to_s
-    end
+    key =
+      if response.code != 200 || response.dig('results', 'results', 'size') == '0'
+        # If actual build link can't be determined, send user to build summary page.
+        build_key
+      else
+        # If actual build link is available, go to build result page.
+        response.dig('results', 'results', 'result', get_build_result_index, 'planResultKey', 'key')
+      end
+
+    build_url("browse/#{key}")
   end
 
   def read_commit_status(response)
@@ -117,7 +119,7 @@ class BambooService < CiService
   end
 
   def build_url(path)
-    URI.join("#{bamboo_url}/", path).to_s
+    Gitlab::Utils.append_path(bamboo_url, path)
   end
 
   def get_path(path, query_params = {})
diff --git a/app/models/project_services/drone_ci_service.rb b/app/models/project_services/drone_ci_service.rb
index 158ae0bf255..5ccc2f019cb 100644
--- a/app/models/project_services/drone_ci_service.rb
+++ b/app/models/project_services/drone_ci_service.rb
@@ -39,11 +39,9 @@ class DroneCiService < CiService
   end
 
   def commit_status_path(sha, ref)
-    url = [drone_url,
-           "gitlab/#{project.full_path}/commits/#{sha}",
-           "?branch=#{URI.encode(ref.to_s)}&access_token=#{token}"]
-
-    URI.join(*url).to_s
+    Gitlab::Utils.append_path(
+      drone_url,
+      "gitlab/#{project.full_path}/commits/#{sha}?branch=#{URI.encode(ref.to_s)}&access_token=#{token}")
   end
 
   def commit_status(sha, ref)
@@ -74,11 +72,9 @@ class DroneCiService < CiService
   end
 
   def build_page(sha, ref)
-    url = [drone_url,
-           "gitlab/#{project.full_path}/redirect/commits/#{sha}",
-           "?branch=#{URI.encode(ref.to_s)}"]
-
-    URI.join(*url).to_s
+    Gitlab::Utils.append_path(
+      drone_url,
+      "gitlab/#{project.full_path}/redirect/commits/#{sha}?branch=#{URI.encode(ref.to_s)}")
   end
 
   def title
diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb
index 5a38f48c542..9066a0b7f1d 100644
--- a/app/models/project_services/jira_service.rb
+++ b/app/models/project_services/jira_service.rb
@@ -54,7 +54,7 @@ class JiraService < IssueTrackerService
     {
       username: self.username,
       password: self.password,
-      site: URI.join(url, '/').to_s,
+      site: URI.join(url, '/').to_s, # Intended to find the root
       context_path: url.path.chomp('/'),
       auth_type: :basic,
       read_timeout: 120,
diff --git a/app/models/project_services/mock_ci_service.rb b/app/models/project_services/mock_ci_service.rb
index 6883976f0c8..d8bba58dcbf 100644
--- a/app/models/project_services/mock_ci_service.rb
+++ b/app/models/project_services/mock_ci_service.rb
@@ -34,10 +34,9 @@ class MockCiService < CiService
   #   http://jenkins.example.com:8888/job/test1/scm/bySHA1/12d65c
   #
   def build_page(sha, ref)
-    url = [mock_service_url,
-           "#{project.namespace.path}/#{project.path}/status/#{sha}"]
-
-    URI.join(*url).to_s
+    Gitlab::Utils.append_path(
+      mock_service_url,
+      "#{project.namespace.path}/#{project.path}/status/#{sha}")
   end
 
   # Return string with build status or :error symbol
@@ -61,10 +60,9 @@ class MockCiService < CiService
   end
 
   def commit_status_path(sha)
-    url = [mock_service_url,
-           "#{project.namespace.path}/#{project.path}/status/#{sha}.json"]
-
-    URI.join(*url).to_s
+    Gitlab::Utils.append_path(
+      mock_service_url,
+      "#{project.namespace.path}/#{project.path}/status/#{sha}.json")
   end
 
   def read_commit_status(response)
diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb
index eeeff5e802a..b8e17087db5 100644
--- a/app/models/project_services/teamcity_service.rb
+++ b/app/models/project_services/teamcity_service.rb
@@ -132,7 +132,7 @@ class TeamcityService < CiService
   end
 
   def build_url(path)
-    URI.join("#{teamcity_url}/", path).to_s
+    Gitlab::Utils.append_path(teamcity_url, path)
   end
 
   def get_path(path)
diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml
index 3aff5538813..de768696fe9 100644
--- a/app/views/projects/edit.html.haml
+++ b/app/views/projects/edit.html.haml
@@ -165,7 +165,7 @@
               .input-group
                 .input-group-prepend
                   .input-group-text
-                    #{URI.join(root_url, @project.namespace.full_path)}/
+                    #{Gitlab::Utils.append_path(root_url, @project.namespace.full_path)}/
                 = f.text_field :path, class: 'form-control'
             %ul
               %li Be careful. Renaming a project's repository can have unintended side effects.
diff --git a/changelogs/unreleased/sh-53180-append-path.yml b/changelogs/unreleased/sh-53180-append-path.yml
new file mode 100644
index 00000000000..64fae5522d8
--- /dev/null
+++ b/changelogs/unreleased/sh-53180-append-path.yml
@@ -0,0 +1,5 @@
+---
+title: Make sure there's only one slash as path separator
+merge_request: 22954
+author:
+type: other
diff --git a/lib/banzai/filter/absolute_link_filter.rb b/lib/banzai/filter/absolute_link_filter.rb
index 04ec568eee3..a9bdb004c4b 100644
--- a/lib/banzai/filter/absolute_link_filter.rb
+++ b/lib/banzai/filter/absolute_link_filter.rb
@@ -29,6 +29,7 @@ module Banzai
       end
 
       def absolute_link_attr(uri)
+        # Here we really want to expand relative path to absolute path
         URI.join(Gitlab.config.gitlab.url, uri).to_s
       end
     end
diff --git a/lib/bitbucket_server/connection.rb b/lib/bitbucket_server/connection.rb
index 45a437844bd..7efcdcf8619 100644
--- a/lib/bitbucket_server/connection.rb
+++ b/lib/bitbucket_server/connection.rb
@@ -88,35 +88,19 @@ module BitbucketServer
     def build_url(path)
       return path if path.starts_with?(root_url)
 
-      url_join_paths(root_url, path)
+      Gitlab::Utils.append_path(root_url, path)
     end
 
     def root_url
-      url_join_paths(base_uri, "/rest/api/#{api_version}")
+      Gitlab::Utils.append_path(base_uri, "rest/api/#{api_version}")
     end
 
     def delete_url(resource, path)
       if resource == :branches
-        url_join_paths(base_uri, "/rest/branch-utils/#{api_version}#{path}")
+        Gitlab::Utils.append_path(base_uri, "rest/branch-utils/#{api_version}#{path}")
       else
         build_url(path)
       end
     end
-
-    # URI.join is stupid in that slashes are important:
-    #
-    # # URI.join('http://example.com/subpath', 'hello')
-    # => http://example.com/hello
-    #
-    # We really want http://example.com/subpath/hello
-    #
-    def url_join_paths(*paths)
-      paths.map { |path| strip_slashes(path) }.join(SEPARATOR)
-    end
-
-    def strip_slashes(path)
-      path = path[1..-1] if path.starts_with?(SEPARATOR)
-      path.chomp(SEPARATOR)
-    end
   end
 end
diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb
index 860c39feb64..15137140639 100644
--- a/lib/gitlab/gon_helper.rb
+++ b/lib/gitlab/gon_helper.rb
@@ -8,7 +8,10 @@ module Gitlab
 
     def add_gon_variables
       gon.api_version            = 'v4'
-      gon.default_avatar_url     = URI.join(Gitlab.config.gitlab.url, ActionController::Base.helpers.image_path('no_avatar.png')).to_s
+      gon.default_avatar_url     =
+        Gitlab::Utils.append_path(
+          Gitlab.config.gitlab.url,
+          ActionController::Base.helpers.image_path('no_avatar.png'))
       gon.max_file_size          = Gitlab::CurrentSettings.max_attachment_size
       gon.asset_host             = ActionController::Base.asset_host
       gon.webpack_public_path    = webpack_public_path
diff --git a/lib/gitlab/manifest_import/manifest.rb b/lib/gitlab/manifest_import/manifest.rb
index 4d6034fb956..b69b9ac4b64 100644
--- a/lib/gitlab/manifest_import/manifest.rb
+++ b/lib/gitlab/manifest_import/manifest.rb
@@ -63,7 +63,7 @@ module Gitlab
       end
 
       def repository_url(name)
-        URI.join(remote, name).to_s
+        Gitlab::Utils.append_path(remote, name)
       end
 
       def remote
diff --git a/lib/gitlab/middleware/go.rb b/lib/gitlab/middleware/go.rb
index 1fd8f147b44..6943567fb6d 100644
--- a/lib/gitlab/middleware/go.rb
+++ b/lib/gitlab/middleware/go.rb
@@ -38,7 +38,7 @@ module Gitlab
 
       def go_body(path)
         config = Gitlab.config
-        project_url = URI.join(config.gitlab.url, path)
+        project_url = Gitlab::Utils.append_path(config.gitlab.url, path)
         import_prefix = strip_url(project_url.to_s)
 
         repository_url = if Gitlab::CurrentSettings.enabled_git_access_protocol == 'ssh'