Commit a035ebbe authored by Bob Van Landuyt's avatar Bob Van Landuyt

Update path validation & specs

parent c853dd61
...@@ -140,17 +140,17 @@ class DynamicPathValidator < ActiveModel::EachValidator ...@@ -140,17 +140,17 @@ class DynamicPathValidator < ActiveModel::EachValidator
end end
def self.valid?(path) def self.valid?(path)
path =~ Gitlab::Regex.full_namespace_regex && !reserved?(path) path =~ Gitlab::Regex.full_namespace_regex && !full_path_reserved?(path)
end end
def self.reserved?(path) def self.full_path_reserved?(path)
path = path.to_s.downcase path = path.to_s.downcase
_project_parts, namespace_parts = path.reverse.split('/', 2).map(&:reverse) _project_part, namespace_parts = path.reverse.split('/', 2).map(&:reverse)
wildcard_reserved?(path) || any_reserved?(namespace_parts) wildcard_reserved?(path) || child_reserved?(namespace_parts)
end end
def self.any_reserved?(path) def self.child_reserved?(path)
return false unless path return false unless path
path !~ without_reserved_child_paths_regex path !~ without_reserved_child_paths_regex
...@@ -162,18 +162,23 @@ class DynamicPathValidator < ActiveModel::EachValidator ...@@ -162,18 +162,23 @@ class DynamicPathValidator < ActiveModel::EachValidator
path !~ without_reserved_wildcard_paths_regex path !~ without_reserved_wildcard_paths_regex
end end
delegate :reserved?, delegate :full_path_reserved?,
:any_reserved?, :child_reserved?,
to: :class to: :class
def valid_full_path?(record, value) def path_reserved_for_record?(record, value)
full_path = record.respond_to?(:full_path) ? record.full_path : value full_path = record.respond_to?(:full_path) ? record.full_path : value
case record # For group paths the entire path cannot contain a reserved child word
when Project || User # The path doesn't contain the last `_project_part` so we need to validate
reserved?(full_path) # if the entire path.
# Example:
# A *group* with full path `parent/activity` is reserved.
# A *project* with full path `parent/activity` is allowed.
if record.is_a? Group
child_reserved?(full_path)
else else
any_reserved?(full_path) full_path_reserved?(full_path)
end end
end end
...@@ -182,7 +187,7 @@ class DynamicPathValidator < ActiveModel::EachValidator ...@@ -182,7 +187,7 @@ class DynamicPathValidator < ActiveModel::EachValidator
record.errors.add(attribute, Gitlab::Regex.namespace_regex_message) record.errors.add(attribute, Gitlab::Regex.namespace_regex_message)
end end
if valid_full_path?(record, value) if path_reserved_for_record?(record, value)
record.errors.add(attribute, "#{value} is a reserved name") record.errors.add(attribute, "#{value} is a reserved name")
end end
end end
......
...@@ -144,14 +144,19 @@ describe DynamicPathValidator do ...@@ -144,14 +144,19 @@ describe DynamicPathValidator do
end end
end end
describe '.without_reserved_child_paths_regex' do describe '.regex_excluding_child_paths' do
it 'rejects paths containing a child reserved word' do let(:subject) { described_class.without_reserved_child_paths_regex }
subject = described_class.without_reserved_child_paths_regex
it 'rejects paths containing a child reserved word' do
expect(subject).not_to match('hello/group_members') expect(subject).not_to match('hello/group_members')
expect(subject).not_to match('hello/activity/in-the-middle') expect(subject).not_to match('hello/activity/in-the-middle')
expect(subject).not_to match('foo/bar1/refs/master/logs_tree') expect(subject).not_to match('foo/bar1/refs/master/logs_tree')
end end
it 'allows a child path on the top level' do
expect(subject).to match('activity/foo')
expect(subject).to match('avatar')
end
end end
describe ".valid?" do describe ".valid?" do
...@@ -195,4 +200,36 @@ describe DynamicPathValidator do ...@@ -195,4 +200,36 @@ describe DynamicPathValidator do
expect(described_class.valid?(test_path)).to be_falsey expect(described_class.valid?(test_path)).to be_falsey
end end
end end
describe '#path_reserved_for_record?' do
it 'reserves a sub-group named activity' do
group = build(:group, :nested, path: 'activity')
expect(validator.path_reserved_for_record?(group, 'activity')).to be_truthy
end
it "doesn't reserve a project called activity" do
project = build(:project, path: 'activity')
expect(validator.path_reserved_for_record?(project, 'activity')).to be_falsey
end
end
describe '#validates_each' do
it 'adds a message when the path is not in the correct format' do
group = build(:group)
validator.validate_each(group, :path, "Path with spaces, and comma's!")
expect(group.errors[:path]).to include(Gitlab::Regex.namespace_regex_message)
end
it 'adds a message when the path is not in the correct format' do
group = build(:group, path: 'users')
validator.validate_each(group, :path, 'users')
expect(group.errors[:path]).to include('users is a reserved name')
end
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