Commit a10925e1 authored by Douwe Maan's avatar Douwe Maan

Reallow project paths ending in periods

parent 97b80fef
......@@ -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 }
......
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
# 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
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
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
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
---
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
......
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
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
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
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
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