Merge branch 'sh-53180-append-path' into 'master'

Make sure there's only one slash as path separator

Closes #53180

See merge request gitlab-org/gitlab-ce!22954

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

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 = {})

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

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,

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)

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)

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.

changelogs/unreleased/sh-53180-append-path.yml

+---
+title: Make sure there's only one slash as path separator
+merge_request: 22954
+author:
+type: other

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

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

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

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

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'