diff options
| -rw-r--r-- | app/validators/url_validator.rb | 14 | ||||
| -rw-r--r-- | spec/validators/url_validator_spec.rb | 15 |
2 files changed, 26 insertions, 3 deletions
diff --git a/app/validators/url_validator.rb b/app/validators/url_validator.rb index faaf1283078..216acf79cbd 100644 --- a/app/validators/url_validator.rb +++ b/app/validators/url_validator.rb @@ -41,12 +41,13 @@ class UrlValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) @record = record - if value.present? - value.strip! - else + unless value.present? record.errors.add(attribute, 'must be a valid URL') + return end + value = strip_value!(record, attribute, value) + Gitlab::UrlBlocker.validate!(value, blocker_args) rescue Gitlab::UrlBlocker::BlockedUrlError => e record.errors.add(attribute, "is blocked: #{e.message}") @@ -54,6 +55,13 @@ class UrlValidator < ActiveModel::EachValidator 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 # By default the validator doesn't block any url based on the ip address { diff --git a/spec/validators/url_validator_spec.rb b/spec/validators/url_validator_spec.rb index 93fe013d11c..ab6100509a6 100644 --- a/spec/validators/url_validator_spec.rb +++ b/spec/validators/url_validator_spec.rb @@ -24,6 +24,21 @@ describe UrlValidator do expect(badge.errors.empty?).to be true 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 context 'when allow_localhost is set to false' do |