Make sure there's only one slash as path separator

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

5 files changed, 25 insertions, 29 deletions

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)