Merge branch 'dm-reallow-project-path-ending-in-period' into 'master'
Reallow project paths ending in periods Closes #39695 See merge request gitlab-org/gitlab-ce!15190
This commit is contained in:
commit
ef2c5e321d
18 changed files with 223 additions and 159 deletions
|
@ -36,7 +36,7 @@ class Namespace < ActiveRecord::Base
|
|||
validates :path,
|
||||
presence: true,
|
||||
length: { maximum: 255 },
|
||||
dynamic_path: true
|
||||
namespace_path: true
|
||||
|
||||
validate :nesting_level_allowed
|
||||
|
||||
|
|
|
@ -240,10 +240,8 @@ class Project < ActiveRecord::Base
|
|||
message: Gitlab::Regex.project_name_regex_message }
|
||||
validates :path,
|
||||
presence: true,
|
||||
dynamic_path: true,
|
||||
project_path: true,
|
||||
length: { maximum: 255 },
|
||||
format: { with: Gitlab::PathRegex.project_path_format_regex,
|
||||
message: Gitlab::PathRegex.project_path_format_message },
|
||||
uniqueness: { scope: :namespace_id }
|
||||
|
||||
validates :namespace, presence: true
|
||||
|
|
|
@ -146,7 +146,7 @@ class User < ActiveRecord::Base
|
|||
presence: true,
|
||||
numericality: { greater_than_or_equal_to: 0, less_than_or_equal_to: Gitlab::Database::MAX_INT_VALUE }
|
||||
validates :username,
|
||||
dynamic_path: true,
|
||||
user_path: true,
|
||||
presence: true,
|
||||
uniqueness: { case_sensitive: false }
|
||||
|
||||
|
|
38
app/validators/abstract_path_validator.rb
Normal file
38
app/validators/abstract_path_validator.rb
Normal file
|
@ -0,0 +1,38 @@
|
|||
class AbstractPathValidator < ActiveModel::EachValidator
|
||||
extend Gitlab::EncodingHelper
|
||||
|
||||
def self.path_regex
|
||||
raise NotImplementedError
|
||||
end
|
||||
|
||||
def self.format_regex
|
||||
raise NotImplementedError
|
||||
end
|
||||
|
||||
def self.format_error_message
|
||||
raise NotImplementedError
|
||||
end
|
||||
|
||||
def self.full_path(record, value)
|
||||
value
|
||||
end
|
||||
|
||||
def self.valid_path?(path)
|
||||
encode!(path)
|
||||
"#{path}/" =~ path_regex
|
||||
end
|
||||
|
||||
def validate_each(record, attribute, value)
|
||||
unless value =~ self.class.format_regex
|
||||
record.errors.add(attribute, self.class.format_error_message)
|
||||
return
|
||||
end
|
||||
|
||||
full_path = self.class.full_path(record, value)
|
||||
return unless full_path
|
||||
|
||||
unless self.class.valid_path?(full_path)
|
||||
record.errors.add(attribute, "#{value} is a reserved name")
|
||||
end
|
||||
end
|
||||
end
|
|
@ -1,53 +0,0 @@
|
|||
# DynamicPathValidator
|
||||
#
|
||||
# Custom validator for GitLab path values.
|
||||
# These paths are assigned to `Namespace` (& `Group` as a subclass) & `Project`
|
||||
#
|
||||
# Values are checked for formatting and exclusion from a list of illegal path
|
||||
# names.
|
||||
class DynamicPathValidator < ActiveModel::EachValidator
|
||||
extend Gitlab::EncodingHelper
|
||||
|
||||
class << self
|
||||
def valid_user_path?(path)
|
||||
encode!(path)
|
||||
"#{path}/" =~ Gitlab::PathRegex.root_namespace_path_regex
|
||||
end
|
||||
|
||||
def valid_group_path?(path)
|
||||
encode!(path)
|
||||
"#{path}/" =~ Gitlab::PathRegex.full_namespace_path_regex
|
||||
end
|
||||
|
||||
def valid_project_path?(path)
|
||||
encode!(path)
|
||||
"#{path}/" =~ Gitlab::PathRegex.full_project_path_regex
|
||||
end
|
||||
end
|
||||
|
||||
def path_valid_for_record?(record, value)
|
||||
full_path = record.respond_to?(:build_full_path) ? record.build_full_path : value
|
||||
|
||||
return true unless full_path
|
||||
|
||||
case record
|
||||
when Project
|
||||
self.class.valid_project_path?(full_path)
|
||||
when Group
|
||||
self.class.valid_group_path?(full_path)
|
||||
else # User or non-Group Namespace
|
||||
self.class.valid_user_path?(full_path)
|
||||
end
|
||||
end
|
||||
|
||||
def validate_each(record, attribute, value)
|
||||
unless value =~ Gitlab::PathRegex.namespace_format_regex
|
||||
record.errors.add(attribute, Gitlab::PathRegex.namespace_format_message)
|
||||
return
|
||||
end
|
||||
|
||||
unless path_valid_for_record?(record, value)
|
||||
record.errors.add(attribute, "#{value} is a reserved name")
|
||||
end
|
||||
end
|
||||
end
|
19
app/validators/namespace_path_validator.rb
Normal file
19
app/validators/namespace_path_validator.rb
Normal file
|
@ -0,0 +1,19 @@
|
|||
class NamespacePathValidator < AbstractPathValidator
|
||||
extend Gitlab::EncodingHelper
|
||||
|
||||
def self.path_regex
|
||||
Gitlab::PathRegex.full_namespace_path_regex
|
||||
end
|
||||
|
||||
def self.format_regex
|
||||
Gitlab::PathRegex.namespace_format_regex
|
||||
end
|
||||
|
||||
def self.format_error_message
|
||||
Gitlab::PathRegex.namespace_format_message
|
||||
end
|
||||
|
||||
def self.full_path(record, value)
|
||||
record.build_full_path
|
||||
end
|
||||
end
|
19
app/validators/project_path_validator.rb
Normal file
19
app/validators/project_path_validator.rb
Normal file
|
@ -0,0 +1,19 @@
|
|||
class ProjectPathValidator < AbstractPathValidator
|
||||
extend Gitlab::EncodingHelper
|
||||
|
||||
def self.path_regex
|
||||
Gitlab::PathRegex.full_project_path_regex
|
||||
end
|
||||
|
||||
def self.format_regex
|
||||
Gitlab::PathRegex.project_path_format_regex
|
||||
end
|
||||
|
||||
def self.format_error_message
|
||||
Gitlab::PathRegex.project_path_format_message
|
||||
end
|
||||
|
||||
def self.full_path(record, value)
|
||||
record.build_full_path
|
||||
end
|
||||
end
|
15
app/validators/user_path_validator.rb
Normal file
15
app/validators/user_path_validator.rb
Normal file
|
@ -0,0 +1,15 @@
|
|||
class UserPathValidator < AbstractPathValidator
|
||||
extend Gitlab::EncodingHelper
|
||||
|
||||
def self.path_regex
|
||||
Gitlab::PathRegex.root_namespace_path_regex
|
||||
end
|
||||
|
||||
def self.format_regex
|
||||
Gitlab::PathRegex.namespace_format_regex
|
||||
end
|
||||
|
||||
def self.format_error_message
|
||||
Gitlab::PathRegex.namespace_format_message
|
||||
end
|
||||
end
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Reallow project paths ending in periods
|
||||
merge_request:
|
||||
author:
|
||||
type: fixed
|
|
@ -2,7 +2,7 @@ class GroupUrlConstrainer
|
|||
def matches?(request)
|
||||
full_path = request.params[:group_id] || request.params[:id]
|
||||
|
||||
return false unless DynamicPathValidator.valid_group_path?(full_path)
|
||||
return false unless NamespacePathValidator.valid_path?(full_path)
|
||||
|
||||
Group.find_by_full_path(full_path, follow_redirects: request.get?).present?
|
||||
end
|
||||
|
|
|
@ -4,7 +4,7 @@ class ProjectUrlConstrainer
|
|||
project_path = request.params[:project_id] || request.params[:id]
|
||||
full_path = [namespace_path, project_path].join('/')
|
||||
|
||||
return false unless DynamicPathValidator.valid_project_path?(full_path)
|
||||
return false unless ProjectPathValidator.valid_path?(full_path)
|
||||
|
||||
# We intentionally allow SELECT(*) here so result of this query can be used
|
||||
# as cache for further Project.find_by_full_path calls within request
|
||||
|
|
|
@ -2,7 +2,7 @@ class UserUrlConstrainer
|
|||
def matches?(request)
|
||||
full_path = request.params[:username]
|
||||
|
||||
return false unless DynamicPathValidator.valid_user_path?(full_path)
|
||||
return false unless UserPathValidator.valid_path?(full_path)
|
||||
|
||||
User.find_by_full_path(full_path, follow_redirects: request.get?).present?
|
||||
end
|
||||
|
|
|
@ -179,7 +179,7 @@ module Gitlab
|
|||
valid_username = ::Namespace.clean_path(username)
|
||||
|
||||
uniquify = Uniquify.new
|
||||
valid_username = uniquify.string(valid_username) { |s| !DynamicPathValidator.valid_user_path?(s) }
|
||||
valid_username = uniquify.string(valid_username) { |s| !UserPathValidator.valid_path?(s) }
|
||||
|
||||
name = auth_hash.name
|
||||
name = valid_username if name.strip.empty?
|
||||
|
|
|
@ -276,6 +276,12 @@ describe Project do
|
|||
|
||||
expect(project).to be_valid
|
||||
end
|
||||
|
||||
it 'allows a path ending in a period' do
|
||||
project = build(:project, path: 'foo.')
|
||||
|
||||
expect(project).to be_valid
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -1,97 +0,0 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe DynamicPathValidator do
|
||||
let(:validator) { described_class.new(attributes: [:path]) }
|
||||
|
||||
def expect_handles_invalid_utf8
|
||||
expect { yield('\255invalid') }.to be_falsey
|
||||
end
|
||||
|
||||
describe '.valid_user_path' do
|
||||
it 'handles invalid utf8' do
|
||||
expect(described_class.valid_user_path?("a\0weird\255path")).to be_falsey
|
||||
end
|
||||
end
|
||||
|
||||
describe '.valid_group_path' do
|
||||
it 'handles invalid utf8' do
|
||||
expect(described_class.valid_group_path?("a\0weird\255path")).to be_falsey
|
||||
end
|
||||
end
|
||||
|
||||
describe '.valid_project_path' do
|
||||
it 'handles invalid utf8' do
|
||||
expect(described_class.valid_project_path?("a\0weird\255path")).to be_falsey
|
||||
end
|
||||
end
|
||||
|
||||
describe '#path_valid_for_record?' do
|
||||
context 'for project' do
|
||||
it 'calls valid_project_path?' do
|
||||
project = build(:project, path: 'activity')
|
||||
|
||||
expect(described_class).to receive(:valid_project_path?).with(project.full_path).and_call_original
|
||||
|
||||
expect(validator.path_valid_for_record?(project, 'activity')).to be_truthy
|
||||
end
|
||||
end
|
||||
|
||||
context 'for group' do
|
||||
it 'calls valid_group_path?' do
|
||||
group = build(:group, :nested, path: 'activity')
|
||||
|
||||
expect(described_class).to receive(:valid_group_path?).with(group.full_path).and_call_original
|
||||
|
||||
expect(validator.path_valid_for_record?(group, 'activity')).to be_falsey
|
||||
end
|
||||
end
|
||||
|
||||
context 'for user' do
|
||||
it 'calls valid_user_path?' do
|
||||
user = build(:user, username: 'activity')
|
||||
|
||||
expect(described_class).to receive(:valid_user_path?).with(user.full_path).and_call_original
|
||||
|
||||
expect(validator.path_valid_for_record?(user, 'activity')).to be_truthy
|
||||
end
|
||||
end
|
||||
|
||||
context 'for user namespace' do
|
||||
it 'calls valid_user_path?' do
|
||||
user = create(:user, username: 'activity')
|
||||
namespace = user.namespace
|
||||
|
||||
expect(described_class).to receive(:valid_user_path?).with(namespace.full_path).and_call_original
|
||||
|
||||
expect(validator.path_valid_for_record?(namespace, 'activity')).to be_truthy
|
||||
end
|
||||
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::PathRegex.namespace_format_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
|
||||
|
||||
it 'updating to an invalid path is not allowed' do
|
||||
project = create(:project)
|
||||
project.path = 'update'
|
||||
|
||||
validator.validate_each(project, :path, 'update')
|
||||
|
||||
expect(project.errors[:path]).to include('update is a reserved name')
|
||||
end
|
||||
end
|
||||
end
|
38
spec/validators/namespace_path_validator_spec.rb
Normal file
38
spec/validators/namespace_path_validator_spec.rb
Normal file
|
@ -0,0 +1,38 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe NamespacePathValidator do
|
||||
let(:validator) { described_class.new(attributes: [:path]) }
|
||||
|
||||
describe '.valid_path?' do
|
||||
it 'handles invalid utf8' do
|
||||
expect(described_class.valid_path?("a\0weird\255path")).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::PathRegex.namespace_format_message)
|
||||
end
|
||||
|
||||
it 'adds a message when the path is reserved when creating' do
|
||||
group = build(:group, path: 'help')
|
||||
|
||||
validator.validate_each(group, :path, 'help')
|
||||
|
||||
expect(group.errors[:path]).to include('help is a reserved name')
|
||||
end
|
||||
|
||||
it 'adds a message when the path is reserved when updating' do
|
||||
group = create(:group)
|
||||
group.path = 'help'
|
||||
|
||||
validator.validate_each(group, :path, 'help')
|
||||
|
||||
expect(group.errors[:path]).to include('help is a reserved name')
|
||||
end
|
||||
end
|
||||
end
|
38
spec/validators/project_path_validator_spec.rb
Normal file
38
spec/validators/project_path_validator_spec.rb
Normal file
|
@ -0,0 +1,38 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe ProjectPathValidator do
|
||||
let(:validator) { described_class.new(attributes: [:path]) }
|
||||
|
||||
describe '.valid_path?' do
|
||||
it 'handles invalid utf8' do
|
||||
expect(described_class.valid_path?("a\0weird\255path")).to be_falsey
|
||||
end
|
||||
end
|
||||
|
||||
describe '#validates_each' do
|
||||
it 'adds a message when the path is not in the correct format' do
|
||||
project = build(:project)
|
||||
|
||||
validator.validate_each(project, :path, "Path with spaces, and comma's!")
|
||||
|
||||
expect(project.errors[:path]).to include(Gitlab::PathRegex.project_path_format_message)
|
||||
end
|
||||
|
||||
it 'adds a message when the path is reserved when creating' do
|
||||
project = build(:project, path: 'blob')
|
||||
|
||||
validator.validate_each(project, :path, 'blob')
|
||||
|
||||
expect(project.errors[:path]).to include('blob is a reserved name')
|
||||
end
|
||||
|
||||
it 'adds a message when the path is reserved when updating' do
|
||||
project = create(:project)
|
||||
project.path = 'blob'
|
||||
|
||||
validator.validate_each(project, :path, 'blob')
|
||||
|
||||
expect(project.errors[:path]).to include('blob is a reserved name')
|
||||
end
|
||||
end
|
||||
end
|
38
spec/validators/user_path_validator_spec.rb
Normal file
38
spec/validators/user_path_validator_spec.rb
Normal file
|
@ -0,0 +1,38 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe UserPathValidator do
|
||||
let(:validator) { described_class.new(attributes: [:username]) }
|
||||
|
||||
describe '.valid_path?' do
|
||||
it 'handles invalid utf8' do
|
||||
expect(described_class.valid_path?("a\0weird\255path")).to be_falsey
|
||||
end
|
||||
end
|
||||
|
||||
describe '#validates_each' do
|
||||
it 'adds a message when the path is not in the correct format' do
|
||||
user = build(:user)
|
||||
|
||||
validator.validate_each(user, :username, "Path with spaces, and comma's!")
|
||||
|
||||
expect(user.errors[:username]).to include(Gitlab::PathRegex.namespace_format_message)
|
||||
end
|
||||
|
||||
it 'adds a message when the path is reserved when creating' do
|
||||
user = build(:user, username: 'help')
|
||||
|
||||
validator.validate_each(user, :username, 'help')
|
||||
|
||||
expect(user.errors[:username]).to include('help is a reserved name')
|
||||
end
|
||||
|
||||
it 'adds a message when the path is reserved when updating' do
|
||||
user = create(:user)
|
||||
user.username = 'help'
|
||||
|
||||
validator.validate_each(user, :username, 'help')
|
||||
|
||||
expect(user.errors[:username]).to include('help is a reserved name')
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in a new issue