Read about our upcoming Code of Conduct on this issue

Commit c853dd61 authored by Bob Van Landuyt's avatar Bob Van Landuyt
Browse files

Reuse Gitlab::Regex.full_namespace_regex in the DynamicPathValidator

parent 5cb0fccba040
......@@ -15,7 +15,7 @@ class DynamicPathValidator < ActiveModel::EachValidator
#
# the path `api` shouldn't be allowed because it would be masked by `api/*`
#
TOP_LEVEL_ROUTES = Set.new(%w[
TOP_LEVEL_ROUTES = %w[
-
.well-known
abuse_reports
......@@ -59,7 +59,7 @@ class DynamicPathValidator < ActiveModel::EachValidator
unsubscribes
uploads
users
]).freeze
].freeze
# All project routes with wildcard argument must be listed here.
# Otherwise it can lead to routing issues when route considered as project name.
......@@ -70,7 +70,7 @@ class DynamicPathValidator < ActiveModel::EachValidator
# without tree as reserved name routing can match 'group/project' as group name,
# 'tree' as project name and 'deploy_keys' as route.
#
WILDCARD_ROUTES = Set.new(%w[
WILDCARD_ROUTES = %w[
badges
blame
blob
......@@ -91,7 +91,7 @@ class DynamicPathValidator < ActiveModel::EachValidator
tree
update
wikis
]).freeze
].freeze
# These are all the paths that follow `/groups/*id/ or `/groups/*group_id`
# We need to reject these because we have a `/groups/*id` page that is the same
......@@ -100,7 +100,7 @@ class DynamicPathValidator < ActiveModel::EachValidator
# If we would allow a subgroup to be created with the name `activity` then
# this group would not be accessible through `/groups/parent/activity` since
# this would map to the activity-page of it's parent.
GROUP_ROUTES = Set.new(%w[
GROUP_ROUTES = %w[
activity
avatar
edit
......@@ -111,16 +111,16 @@ class DynamicPathValidator < ActiveModel::EachValidator
milestones
projects
subgroups
])
].freeze
CHILD_ROUTES = (WILDCARD_ROUTES | GROUP_ROUTES).freeze
def self.without_reserved_wildcard_paths_regex
@full_path_without_wildcard_regex ||= regex_excluding_child_paths(WILDCARD_ROUTES)
@without_reserved_wildcard_paths_regex ||= regex_excluding_child_paths(WILDCARD_ROUTES)
end
def self.without_reserved_child_paths_regex
@full_path_without_child_routes_regex ||= regex_excluding_child_paths(CHILD_ROUTES)
@without_reserved_child_paths_regex ||= regex_excluding_child_paths(CHILD_ROUTES)
end
# This is used to validate a full path.
......@@ -128,22 +128,19 @@ def self.without_reserved_child_paths_regex
# - Starting with one of the top level words
# - Containing one of the child level words in the middle of a path
def self.regex_excluding_child_paths(child_routes)
reserved_top_level_words = Regexp.union(TOP_LEVEL_ROUTES.to_a)
not_starting_in_reserved_word = %r{^(/?)(?!(#{reserved_top_level_words})(/|$))}
reserved_top_level_words = Regexp.union(TOP_LEVEL_ROUTES)
not_starting_in_reserved_word = %r{\A/?(?!(#{reserved_top_level_words})(/|\z))}
reserved_child_level_words = Regexp.union(child_routes.to_a)
not_containing_reserved_child = %r{(?!(\S+)/(#{reserved_child_level_words})(/|$))}
reserved_child_level_words = Regexp.union(child_routes)
not_containing_reserved_child = %r{(?!\S+/(#{reserved_child_level_words})(/|\z))}
@full_path_regex = %r{
#{not_starting_in_reserved_word}
#{not_containing_reserved_child}
#{Gitlab::Regex::FULL_NAMESPACE_REGEX_STR}}x
%r{#{not_starting_in_reserved_word}
#{not_containing_reserved_child}
#{Gitlab::Regex.full_namespace_regex}}x
end
def self.valid?(path)
path_segments = path.split('/')
!reserved?(path) && path_segments.all? { |value| follow_format?(value) }
path =~ Gitlab::Regex.full_namespace_regex && !reserved?(path)
end
def self.reserved?(path)
......@@ -165,13 +162,9 @@ def self.wildcard_reserved?(path)
path !~ without_reserved_wildcard_paths_regex
end
def self.follow_format?(value)
value =~ Gitlab::Regex.namespace_regex
end
delegate :reserved?,
:any_reserved?,
:follow_format?, to: :class
to: :class
def valid_full_path?(record, value)
full_path = record.respond_to?(:full_path) ? record.full_path : value
......@@ -185,7 +178,7 @@ def valid_full_path?(record, value)
end
def validate_each(record, attribute, value)
unless follow_format?(value)
unless value =~ Gitlab::Regex.namespace_regex
record.errors.add(attribute, Gitlab::Regex.namespace_regex_message)
end
......
......@@ -22,6 +22,10 @@ def namespace_regex
@namespace_regex ||= /\A#{NAMESPACE_REGEX_STR}\z/.freeze
end
def full_namespace_regex
@full_namespace_regex ||= %r{\A#{FULL_NAMESPACE_REGEX_STR}\z}
end
def namespace_route_regex
@namespace_route_regex ||= /#{NAMESPACE_REGEX_STR}/.freeze
end
......
......@@ -45,8 +45,8 @@
it { is_expected.not_to match('foo-') }
end
describe 'FULL_NAMESPACE_REGEX_STR' do
subject { %r{\A#{Gitlab::Regex::FULL_NAMESPACE_REGEX_STR}\z} }
describe '.full_namespace_regex' do
subject { described_class.full_namespace_regex }
it { is_expected.to match('gitlab.org') }
it { is_expected.to match('gitlab.org/gitlab-git') }
......
......@@ -129,6 +129,10 @@ def wildcards_include?(path)
expect(subject).not_to match('dashboard')
end
it 'matches valid paths with a toplevel word in a different place' do
expect(subject).to match('parent/dashboard/project-path')
end
it 'rejects paths containing a wildcard reserved word' do
expect(subject).not_to match('hello/edit')
expect(subject).not_to match('hello/edit/in-the-middle')
......@@ -137,7 +141,6 @@ def wildcards_include?(path)
it 'matches valid paths' do
expect(subject).to match('parent/child/project-path')
expect(subject).to match('/parent/child/project-path')
end
end
......@@ -185,5 +188,11 @@ def wildcards_include?(path)
expect(described_class.valid?(test_path)).to be_falsey
end
it 'rejects paths that are in an incorrect format' do
test_path = 'incorrect/format.git'
expect(described_class.valid?(test_path)).to be_falsey
end
end
end
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment