Commit 2a7b1367 authored by Douglas Barbosa Alexandre's avatar Douglas Barbosa Alexandre

Merge branch 'fix-url-validator' into 'master'

Allow UrlValidator to work with attr_encrypted

See merge request gitlab-org/gitlab-ce!21776
parents be7328de b73f3ce5
...@@ -41,12 +41,13 @@ class UrlValidator < ActiveModel::EachValidator ...@@ -41,12 +41,13 @@ class UrlValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value) def validate_each(record, attribute, value)
@record = record @record = record
if value.present? unless value.present?
value.strip!
else
record.errors.add(attribute, 'must be a valid URL') record.errors.add(attribute, 'must be a valid URL')
return
end end
value = strip_value!(record, attribute, value)
Gitlab::UrlBlocker.validate!(value, blocker_args) Gitlab::UrlBlocker.validate!(value, blocker_args)
rescue Gitlab::UrlBlocker::BlockedUrlError => e rescue Gitlab::UrlBlocker::BlockedUrlError => e
record.errors.add(attribute, "is blocked: #{e.message}") record.errors.add(attribute, "is blocked: #{e.message}")
...@@ -54,6 +55,13 @@ class UrlValidator < ActiveModel::EachValidator ...@@ -54,6 +55,13 @@ class UrlValidator < ActiveModel::EachValidator
private private
def strip_value!(record, attribute, value)
new_value = value.strip
return value if new_value == value
record.public_send("#{attribute}=", new_value) # rubocop:disable GitlabSecurity/PublicSend
end
def default_options def default_options
# By default the validator doesn't block any url based on the ip address # By default the validator doesn't block any url based on the ip address
{ {
......
...@@ -24,6 +24,21 @@ describe UrlValidator do ...@@ -24,6 +24,21 @@ describe UrlValidator do
expect(badge.errors.empty?).to be true expect(badge.errors.empty?).to be true
end end
it 'strips urls' do
badge.link_url = "\n\r\n\nhttps://127.0.0.1\r\n\r\n\n\n\n"
# It's unusual for a validator to modify its arguments. Some extensions,
# such as attr_encrypted, freeze the string to signal that modifications
# will not be persisted, so freeze this string to ensure the scheme is
# compatible with them.
badge.link_url.freeze
subject
expect(badge.errors).to be_empty
expect(badge.link_url).to eq('https://127.0.0.1')
end
end end
context 'when allow_localhost is set to false' do context 'when allow_localhost is set to false' do
......
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