Commit 2b0a85c1 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Adjust `PathRegex` to validate files in the `public` directory

And reports when too many words are rejected.
parent d964816b
...@@ -36,9 +36,10 @@ describe Gitlab::PathRegex, lib: true do ...@@ -36,9 +36,10 @@ describe Gitlab::PathRegex, lib: true do
described_class::PROJECT_WILDCARD_ROUTES.include?(path.split('/').first) described_class::PROJECT_WILDCARD_ROUTES.include?(path.split('/').first)
end end
def failure_message(missing_words, constant_name, migration_helper) def failure_message(constant_name, migration_helper, missing_words:, additional_words: [])
missing_words = Array(missing_words) missing_words = Array(missing_words)
<<-MSG additional_words = Array(additional_words)
message = <<-MSG
Found new routes that could cause conflicts with existing namespaced routes Found new routes that could cause conflicts with existing namespaced routes
for groups or projects. for groups or projects.
...@@ -52,6 +53,18 @@ describe Gitlab::PathRegex, lib: true do ...@@ -52,6 +53,18 @@ describe Gitlab::PathRegex, lib: true do
Make sure to make a note of the renamed records in the release blog post. Make sure to make a note of the renamed records in the release blog post.
MSG MSG
if additional_words.any?
additional_message = <<-ADDITIONAL
Why are <#{additional_words.join(', ')}> in `#{constant_name}`?
If they are really required, update these specs to reflect that.
ADDITIONAL
message = [message, additional_message].join
end
message
end end
let(:all_routes) do let(:all_routes) do
...@@ -68,9 +81,26 @@ describe Gitlab::PathRegex, lib: true do ...@@ -68,9 +81,26 @@ describe Gitlab::PathRegex, lib: true do
let(:routes_not_starting_in_wildcard) { routes_without_format.select { |p| p !~ %r{^/[:*]} } } let(:routes_not_starting_in_wildcard) { routes_without_format.select { |p| p !~ %r{^/[:*]} } }
let(:top_level_words) do let(:top_level_words) do
routes_not_starting_in_wildcard.map do |route| words = routes_not_starting_in_wildcard.map do |route|
route.split('/')[1] route.split('/')[1]
end.compact.uniq end.compact.uniq
words += files_in_public
words + additional_top_level_words
end
let(:additional_top_level_words) do
# Required to keep the uploads safe, remove after
# https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12917 gets merged
['system']
end
let(:files_in_public) do
git = Gitlab.config.git.bin_path
`cd #{Rails.root} && #{git} ls-files public`
.split("\n")
.map { |entry| entry.gsub('public/', '') }
.uniq
end end
# All routes that start with a namespaced path, that have 1 or more # All routes that start with a namespaced path, that have 1 or more
...@@ -122,11 +152,13 @@ describe Gitlab::PathRegex, lib: true do ...@@ -122,11 +152,13 @@ describe Gitlab::PathRegex, lib: true do
it 'includes all the top level namespaces' do it 'includes all the top level namespaces' do
failure_block = lambda do failure_block = lambda do
missing_words = top_level_words - described_class::TOP_LEVEL_ROUTES missing_words = top_level_words - described_class::TOP_LEVEL_ROUTES
failure_message(missing_words, 'TOP_LEVEL_ROUTES', 'rename_root_paths') additional_words = described_class::TOP_LEVEL_ROUTES - top_level_words
failure_message('TOP_LEVEL_ROUTES', 'rename_root_paths',
missing_words: missing_words, additional_words: additional_words)
end end
expect(described_class::TOP_LEVEL_ROUTES) expect(described_class::TOP_LEVEL_ROUTES)
.to include(*top_level_words), failure_block .to contain_exactly(*top_level_words), failure_block
end end
end end
...@@ -134,7 +166,7 @@ describe Gitlab::PathRegex, lib: true do ...@@ -134,7 +166,7 @@ describe Gitlab::PathRegex, lib: true do
it "don't contain a second wildcard" do it "don't contain a second wildcard" do
failure_block = lambda do failure_block = lambda do
missing_words = paths_after_group_id - described_class::GROUP_ROUTES missing_words = paths_after_group_id - described_class::GROUP_ROUTES
failure_message(missing_words, 'GROUP_ROUTES', 'rename_child_paths') failure_message('GROUP_ROUTES', 'rename_child_paths', missing_words: missing_words)
end end
expect(described_class::GROUP_ROUTES) expect(described_class::GROUP_ROUTES)
...@@ -147,7 +179,7 @@ describe Gitlab::PathRegex, lib: true do ...@@ -147,7 +179,7 @@ describe Gitlab::PathRegex, lib: true do
aggregate_failures do aggregate_failures do
all_wildcard_paths.each do |path| all_wildcard_paths.each do |path|
expect(wildcards_include?(path)) expect(wildcards_include?(path))
.to be(true), failure_message(path, 'PROJECT_WILDCARD_ROUTES', 'rename_wildcard_paths') .to be(true), failure_message('PROJECT_WILDCARD_ROUTES', 'rename_wildcard_paths', missing_words: path)
end end
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