From 1e14c3c8525c4e9db6f83da6c037ed94205f65f0 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@gitlab.com>
Date: Fri, 28 Apr 2017 17:53:28 +0200
Subject: Reject paths following namespace for paths including 2 `*`

Reject the part following `/*namespace_id/:project_id` for paths
containing 2 wildcard parameters
---
 app/validators/dynamic_path_validator.rb       |  4 ++--
 spec/validators/dynamic_path_validator_spec.rb | 26 ++++++++++++++------------
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/app/validators/dynamic_path_validator.rb b/app/validators/dynamic_path_validator.rb
index 18680b8975f..96da92b5d76 100644
--- a/app/validators/dynamic_path_validator.rb
+++ b/app/validators/dynamic_path_validator.rb
@@ -70,10 +70,10 @@ class DynamicPathValidator < ActiveModel::EachValidator
   #  'tree' as project name and 'deploy_keys' as route.
   #
   WILDCARD_ROUTES = Set.new(%w[
-    artifacts
     badges
     blame
     blob
+    builds
     commits
     create
     create_dir
@@ -83,10 +83,10 @@ class DynamicPathValidator < ActiveModel::EachValidator
     find_file
     gitlab-lfs/objects
     info/lfs/objects
-    logs_tree
     new
     preview
     raw
+    refs
     tree
     update
     wikis
diff --git a/spec/validators/dynamic_path_validator_spec.rb b/spec/validators/dynamic_path_validator_spec.rb
index 960a6204c90..5053a299bc3 100644
--- a/spec/validators/dynamic_path_validator_spec.rb
+++ b/spec/validators/dynamic_path_validator_spec.rb
@@ -13,29 +13,28 @@ describe DynamicPathValidator do
   # That's not a parameter
   # `/*namespace_id/:project_id/builds/artifacts/*ref_name_and_path`
   #    -> 'builds/artifacts'
-  def path_between_wildcards(path)
+  def path_before_wildcard(path)
     path = path.gsub(STARTING_WITH_NAMESPACE, "")
     path_segments = path.split('/').reject(&:empty?)
-    wildcard_index = path_segments.index { |segment| segment.starts_with?('*') }
+    wildcard_index = path_segments.index { |segment| parameter?(segment) }
 
     segments_before_wildcard = path_segments[0..wildcard_index - 1]
 
-    param_index = segments_before_wildcard.index { |segment| segment.starts_with?(':') }
-    if param_index
-      segments_before_wildcard = segments_before_wildcard[param_index + 1..-1]
-    end
-
     segments_before_wildcard.join('/')
   end
 
+  def parameter?(segment)
+    segment =~ /[*:]/
+  end
+
   # If the path is reserved. Then no conflicting paths can# be created for any
   # route using this reserved word.
   #
-  # Both `builds/artifacts` & `artifacts/file` are covered by reserving the word
-  # `artifacts`
+  # Both `builds/artifacts` & `build` are covered by reserving the word
+  # `build`
   def wildcards_include?(path)
     described_class::WILDCARD_ROUTES.include?(path) ||
-      path.split('/').any? { |segment| described_class::WILDCARD_ROUTES.include?(segment) }
+      described_class::WILDCARD_ROUTES.include?(path.split('/').first)
   end
 
   let(:all_routes) do
@@ -83,7 +82,10 @@ describe DynamicPathValidator do
   # -> ['builds/artifacts', 'info/lfs/objects', 'commits', 'artifacts/file']
   let(:all_wildcard_paths) do
     namespaced_wildcard_routes.map do |route|
-      path_between_wildcards(route)
+      path_before_wildcard(route)
+    end.uniq
+  end
+
     end.uniq
   end
 
@@ -114,7 +116,7 @@ describe DynamicPathValidator do
         to be_truthy
     end
 
-    it 'skips partial path matchies' do
+    it 'skips partial path matches' do
       expect(described_class.contains_path_part?('some/user1/path', 'user')).
         to be_falsy
     end
-- 
cgit v1.2.1