summaryrefslogtreecommitdiff
path: root/app
diff options
context:
space:
mode:
authorRobert Speicher <robert@gitlab.com>2017-05-24 15:08:05 +0000
committerRobert Speicher <robert@gitlab.com>2017-05-24 15:08:05 +0000
commit2a6227a9fca46ca5c982f1cb75754fb1c722b360 (patch)
treee1f1c3386336fdd24f6445134989161b3681c17b /app
parent03bd3081cafe249d9e8c73999411ce9999466c37 (diff)
parentb0498c176fa134761d899c9b369be12f1ca789c5 (diff)
downloadgitlab-ce-2a6227a9fca46ca5c982f1cb75754fb1c722b360.tar.gz
Merge branch 'dm-fix-routes' into 'master'
Fix ambiguous routing issues by teaching router about reserved words See merge request !11570
Diffstat (limited to 'app')
-rw-r--r--app/models/project.rb2
-rw-r--r--app/validators/dynamic_path_validator.rb203
2 files changed, 16 insertions, 189 deletions
diff --git a/app/models/project.rb b/app/models/project.rb
index 65745fd6d37..cfca0dcd2f2 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -205,7 +205,7 @@ class Project < ActiveRecord::Base
presence: true,
dynamic_path: true,
length: { maximum: 255 },
- format: { with: Gitlab::Regex.project_path_regex,
+ format: { with: Gitlab::Regex.project_path_format_regex,
message: Gitlab::Regex.project_path_regex_message },
uniqueness: { scope: :namespace_id }
diff --git a/app/validators/dynamic_path_validator.rb b/app/validators/dynamic_path_validator.rb
index d992b0c3725..8d4d7180baf 100644
--- a/app/validators/dynamic_path_validator.rb
+++ b/app/validators/dynamic_path_validator.rb
@@ -6,199 +6,26 @@
# Values are checked for formatting and exclusion from a list of reserved path
# names.
class DynamicPathValidator < ActiveModel::EachValidator
- # All routes that appear on the top level must be listed here.
- # This will make sure that groups cannot be created with these names
- # as these routes would be masked by the paths already in place.
- #
- # Example:
- # /api/api-project
- #
- # the path `api` shouldn't be allowed because it would be masked by `api/*`
- #
- TOP_LEVEL_ROUTES = %w[
- -
- .well-known
- abuse_reports
- admin
- all
- api
- assets
- autocomplete
- ci
- dashboard
- explore
- files
- groups
- health_check
- help
- hooks
- import
- invites
- issues
- jwt
- koding
- member
- merge_requests
- new
- notes
- notification_settings
- oauth
- profile
- projects
- public
- repository
- robots.txt
- s
- search
- sent_notifications
- services
- snippets
- teams
- u
- unicorn_test
- unsubscribes
- uploads
- users
- ].freeze
-
- # This list should contain all words following `/*namespace_id/:project_id` in
- # routes that contain a second wildcard.
- #
- # Example:
- # /*namespace_id/:project_id/badges/*ref/build
- #
- # If `badges` was allowed as a project/group name, we would not be able to access the
- # `badges` route for those projects:
- #
- # Consider a namespace with path `foo/bar` and a project called `badges`.
- # The route to the build badge would then be `/foo/bar/badges/badges/master/build.svg`
- #
- # When accessing this path the route would be matched to the `badges` path
- # with the following params:
- # - namespace_id: `foo`
- # - project_id: `bar`
- # - ref: `badges/master`
- #
- # Failing to find the project, this would result in a 404.
- #
- # By rejecting `badges` the router can _count_ on the fact that `badges` will
- # be preceded by the `namespace/project`.
- WILDCARD_ROUTES = %w[
- badges
- blame
- blob
- builds
- commits
- create
- create_dir
- edit
- environments/folders
- files
- find_file
- gitlab-lfs/objects
- info/lfs/objects
- new
- preview
- raw
- refs
- tree
- update
- wikis
- ].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
- # as the `/*id`.
- #
- # 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 = %w[
- activity
- analytics
- audit_events
- avatar
- edit
- group_members
- hooks
- issues
- labels
- ldap
- ldap_group_links
- merge_requests
- milestones
- notification_setting
- pipeline_quota
- projects
- subgroups
- ].freeze
-
- CHILD_ROUTES = (WILDCARD_ROUTES | GROUP_ROUTES).freeze
-
- def self.without_reserved_wildcard_paths_regex
- @without_reserved_wildcard_paths_regex ||= regex_excluding_child_paths(WILDCARD_ROUTES)
- end
-
- def self.without_reserved_child_paths_regex
- @without_reserved_child_paths_regex ||= regex_excluding_child_paths(CHILD_ROUTES)
- end
-
- # This is used to validate a full path.
- # It doesn't match paths
- # - 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)
- not_starting_in_reserved_word = %r{\A/?(?!(#{reserved_top_level_words})(/|\z))}
-
- reserved_child_level_words = Regexp.union(child_routes)
- not_containing_reserved_child = %r{(?!\S+/(#{reserved_child_level_words})(/|\z))}
-
- %r{#{not_starting_in_reserved_word}
- #{not_containing_reserved_child}
- #{Gitlab::Regex.full_namespace_regex}}x
- end
-
- def self.valid?(path)
- path =~ Gitlab::Regex.full_namespace_regex && !full_path_reserved?(path)
- end
-
- def self.full_path_reserved?(path)
- path = path.to_s.downcase
- _project_part, namespace_parts = path.reverse.split('/', 2).map(&:reverse)
-
- wildcard_reserved?(path) || child_reserved?(namespace_parts)
- end
-
- def self.child_reserved?(path)
- return false unless path
-
- path !~ without_reserved_child_paths_regex
- end
-
- def self.wildcard_reserved?(path)
- return false unless path
+ class << self
+ def valid_namespace_path?(path)
+ "#{path}/" =~ Gitlab::Regex.full_namespace_path_regex
+ end
- path !~ without_reserved_wildcard_paths_regex
+ def valid_project_path?(path)
+ "#{path}/" =~ Gitlab::Regex.full_project_path_regex
+ end
end
- delegate :full_path_reserved?,
- :child_reserved?,
- to: :class
-
- def path_reserved_for_record?(record, value)
+ def path_valid_for_record?(record, value)
full_path = record.respond_to?(:full_path) ? record.full_path : value
- # For group paths the entire path cannot contain a reserved child word
- # The path doesn't contain the last `_project_part` so we need to validate
- # 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)
+ return true unless full_path
+
+ case record
+ when Project
+ self.class.valid_project_path?(full_path)
else
- full_path_reserved?(full_path)
+ self.class.valid_namespace_path?(full_path)
end
end
@@ -208,7 +35,7 @@ class DynamicPathValidator < ActiveModel::EachValidator
return
end
- if path_reserved_for_record?(record, value)
+ unless path_valid_for_record?(record, value)
record.errors.add(attribute, "#{value} is a reserved name")
end
end