diff options
| -rw-r--r-- | CHANGELOG | 4 | ||||
| -rw-r--r-- | app/models/project.rb | 4 | ||||
| -rw-r--r-- | app/validators/addressable_url_validator.rb | 13 | ||||
| -rw-r--r-- | db/migrate/20160620110927_fix_no_validatable_import_url.rb | 6 | ||||
| -rw-r--r-- | lib/gitlab/url_sanitizer.rb | 10 |
5 files changed, 20 insertions, 17 deletions
diff --git a/CHANGELOG b/CHANGELOG index 9f76b8fa2d9..118811cdda5 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -14,6 +14,7 @@ v 8.10.0 (unreleased) - Check for conflicts with existing Project's wiki path when creating a new project. - Add API endpoint for a group issues !4520 (mahcsig) - Allow [ci skip] to be in any case and allow [skip ci]. !4785 (simon_w) + - Set import_url validation to be more strict v 8.9.3 (unreleased) - Fix encrypted data backwards compatibility after upgrading attr_encrypted gem @@ -66,9 +67,6 @@ v 8.9.1 - Add SMTP as default delivery method to match gitlab-org/omnibus-gitlab!826. !4915 - Remove duplicate 'New Page' button on edit wiki page -v 8.9.1 (unreleased) - - Set import_url validation to be more strict - v 8.9.0 - Fix builds API response not including commit data - Fix error when CI job variables key specified but not defined diff --git a/app/models/project.rb b/app/models/project.rb index 2b1b25ab9d2..89ce61b95ec 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -445,11 +445,11 @@ class Project < ActiveRecord::Base end def import_url=(value) + return super(value) unless Gitlab::UrlSanitizer.valid?(value) + import_url = Gitlab::UrlSanitizer.new(value) create_or_update_import_data(credentials: import_url.credentials) super(import_url.sanitized_url) - rescue Addressable::URI::InvalidURIError - errors.add(:import_url, 'must be a valid URL.') end def import_url diff --git a/app/validators/addressable_url_validator.rb b/app/validators/addressable_url_validator.rb index 634a15aea01..c97acf7da95 100644 --- a/app/validators/addressable_url_validator.rb +++ b/app/validators/addressable_url_validator.rb @@ -18,6 +18,9 @@ # end # class AddressableUrlValidator < ActiveModel::EachValidator + + DEFAULT_OPTIONS = { protocols: %w(http https ssh git) } + def validate_each(record, attribute, value) unless valid_url?(value) record.errors.add(attribute, "must be a valid URL") @@ -29,15 +32,9 @@ class AddressableUrlValidator < ActiveModel::EachValidator def valid_url?(value) return false unless value - value.strip! - valid_protocol?(value) && valid_uri?(value) end - def default_options - @default_options ||= { protocols: %w(http https ssh git) } - end - def valid_uri?(value) Addressable::URI.parse(value).is_a?(Addressable::URI) rescue Addressable::URI::InvalidURIError @@ -45,7 +42,7 @@ class AddressableUrlValidator < ActiveModel::EachValidator end def valid_protocol?(value) - options = default_options.merge(self.options) - !!(value =~ /\A#{URI.regexp(options[:protocols])}\z/) + options = DEFAULT_OPTIONS.merge(self.options) + value =~ /\A#{URI.regexp(options[:protocols])}\z/ end end diff --git a/db/migrate/20160620110927_fix_no_validatable_import_url.rb b/db/migrate/20160620110927_fix_no_validatable_import_url.rb index 9cb84faaec1..e111691ea3c 100644 --- a/db/migrate/20160620110927_fix_no_validatable_import_url.rb +++ b/db/migrate/20160620110927_fix_no_validatable_import_url.rb @@ -38,8 +38,6 @@ class FixNoValidatableImportUrl < ActiveRecord::Migration def valid_url?(value) return false unless value - value.strip! - valid_uri?(value) && valid_protocol?(value) rescue Addressable::URI::InvalidURIError false @@ -50,11 +48,13 @@ class FixNoValidatableImportUrl < ActiveRecord::Migration end def valid_protocol?(value) - !!(value =~ /\A#{URI.regexp(%w(http https ssh git))}\z/) + value =~ /\A#{URI.regexp(%w(http https ssh git))}\z/ end end def up + return unless defined?(Addressable::URI::InvalidURIError) + say('Cleaning up invalid import URLs... This may take a few minutes if we have a large number of imported projects.') invalid_import_url_project_ids.each { |project_id| cleanup_import_url(project_id) } diff --git a/lib/gitlab/url_sanitizer.rb b/lib/gitlab/url_sanitizer.rb index 7d02fe3c971..2eb6085a3ca 100644 --- a/lib/gitlab/url_sanitizer.rb +++ b/lib/gitlab/url_sanitizer.rb @@ -1,5 +1,9 @@ module Gitlab class UrlSanitizer + + attr_reader :valid + alias_method :valid?, :valid + def self.sanitize(content) regexp = URI::Parser.new.make_regexp(['http', 'https', 'ssh', 'git']) @@ -7,8 +11,12 @@ module Gitlab end def initialize(url, credentials: nil) - @url = Addressable::URI.parse(url) + @valid = true + @url = Addressable::URI.parse(url.strip) @credentials = credentials + rescue Addressable::URI::InvalidURIError + @valid = false + raise end def sanitized_url |