Commit 5b893d60 authored by James Lopez's avatar James Lopez

few changes based on feedback

parent 0ca27574
......@@ -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
......
......@@ -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
......
......@@ -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
......@@ -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) }
......
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
......
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