diff options
| -rw-r--r-- | app/validators/namespace_validator.rb | 15 | ||||
| -rw-r--r-- | spec/validators/namespace_validator_spec.rb | 81 |
2 files changed, 69 insertions, 27 deletions
diff --git a/app/validators/namespace_validator.rb b/app/validators/namespace_validator.rb index 5a8b482d3db..4d99b09e98f 100644 --- a/app/validators/namespace_validator.rb +++ b/app/validators/namespace_validator.rb @@ -69,21 +69,10 @@ class NamespaceValidator < 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[tree commits wikis new edit create update logs_tree preview blob blame raw files create_dir find_file - artifacts graphs refs badges info git-upload-pack - git-receive-pack gitlab-lfs autocomplete_sources - templates avatar commit pages compare network snippets - services mattermost deploy_keys forks import merge_requests - branches merged_branches tags protected_branches variables - triggers pipelines environments cycle_analytics builds - hooks container_registry milestones labels issues - project_members group_links notes noteable boards todos - uploads runners runner_projects settings repository - transfer remove_fork archive unarchive housekeeping - toggle_star preview_markdown export remove_export - generate_new_export download_export activity - new_issue_address registry]) + artifacts graphs refs badges objects folders file]) STRICT_RESERVED = (TOP_LEVEL_ROUTES | WILDCARD_ROUTES) diff --git a/spec/validators/namespace_validator_spec.rb b/spec/validators/namespace_validator_spec.rb index 4b5dd59e048..7ddce74939d 100644 --- a/spec/validators/namespace_validator_spec.rb +++ b/spec/validators/namespace_validator_spec.rb @@ -2,27 +2,80 @@ require 'spec_helper' describe NamespaceValidator do let(:validator) { described_class.new(attributes: [:path]) } - describe 'RESERVED' do + + # Pass in a full path to remove the format segment: + # `/ci/lint(.:format)` -> `/ci/lint` + def without_format(path) + path.split('(', 2)[0] + end + + # Pass in a full path and get the last segment before a wildcard + # That's not a parameter + # `/*namespace_id/:project_id/builds/artifacts/*ref_name_and_path` + # -> 'artifacts' + def segment_before_last_wildcard(path) + path_segments = path.split('/').reject { |segment| segment.empty? } + last_wildcard_index = path_segments.rindex { |part| part.starts_with?('*') } + + index_of_non_param_segment = last_wildcard_index - 1 + part_before_wildcard = path_segments[index_of_non_param_segment] + while parameter?(part_before_wildcard) + index_of_non_param_segment -= 1 + part_before_wildcard = path_segments[index_of_non_param_segment] + end + + part_before_wildcard + end + + def parameter?(path_segment) + path_segment.starts_with?(':') || path_segment.starts_with?('*') + end + + let(:all_routes) do + Rails.application.routes.routes.routes. + map { |r| r.path.spec.to_s } + end + + let(:routes_without_format) { all_routes.map { |path| without_format(path) } } + + # Routes not starting with `/:` or `/*` + # all routes not starting with a param + let(:routes_not_starting_in_wildcard) { routes_without_format.select { |p| p !~ %r{^/[:*]} } } + + # All routes that start with a namespaced path, that have 1 or more + # path-segments before having another wildcard parameter. + # - Starting with paths: + # - `/*namespace_id/:project_id/` + # - `/*namespace_id/:id/` + # - Followed by one or more path-parts not starting with `:` or `/` + # - Followed by a path-part that includes a wildcard parameter `*` + # At the time of writing these routes match: http://rubular.com/r/QDxulzZlxZ + STARTING_WITH_NAMESPACE = /^\/\*namespace_id\/:(project_)?id/ + NON_PARAM_PARTS = /[^:*][a-z\-_\/]*/ + ANY_OTHER_PATH_PART = /[a-z\-_\/:]*/ + WILDCARD_SEGMENT = /\*/ + let(:namespaced_wildcard_routes) do + routes_without_format.select do |p| + p =~ %r{#{STARTING_WITH_NAMESPACE}\/#{NON_PARAM_PARTS}\/#{ANY_OTHER_PATH_PART}#{WILDCARD_SEGMENT}} + end + end + + describe 'TOP_LEVEL_ROUTES' do it 'includes all the top level namespaces' do - all_top_level_routes = Rails.application.routes.routes.routes. - map { |r| r.path.spec.to_s }. - select { |p| p !~ %r{^/[:*]} }. - map { |p| p.split('/')[1] }. - compact. - map { |p| p.split('(', 2)[0] }. - uniq + top_level_words = routes_not_starting_in_wildcard. + map { |p| p.split('/')[1] }. # Get the first part of the path + compact. + uniq - expect(described_class::RESERVED).to include(*all_top_level_routes) + expect(described_class::TOP_LEVEL_ROUTES).to include(*top_level_words) end end describe 'WILDCARD_ROUTES' do it 'includes all paths that can be used after a namespace/project path' do - all_wildcard_paths = Rails.application.routes.routes.routes. - map { |r| r.path.spec.to_s }. - select { |p| p =~ %r{^/\*namespace_id/:(project_)?id/[^:*]} }. - map { |p| p.split('/')[3].split('(', 2)[0] }. - uniq + all_wildcard_paths = namespaced_wildcard_routes.map do |path| + segment_before_last_wildcard(path) + end.uniq expect(described_class::WILDCARD_ROUTES).to include(*all_wildcard_paths) end |