Merge branch 'fix/url_validator_' into 'master'

EE: Align UrlValidator to validate_url gem implementation.

See merge request gitlab-org/gitlab-ee!10693

app/models/application_setting.rb

@@ -48,17 +48,17 @@ class ApplicationSetting < ApplicationRecord
 
   validates :home_page_url,
             allow_blank: true,
-            url: true,
+            addressable_url: true,
             if: :home_page_url_column_exists?
 
   validates :help_page_support_url,
             allow_blank: true,
-            url: true,
+            addressable_url: true,
             if: :help_page_support_url_column_exists?
 
   validates :after_sign_out_path,
             allow_blank: true,
-            url: true
+            addressable_url: true
 
   validates :admin_notification_email,
             devise_email: true,
@@ -218,7 +218,7 @@ class ApplicationSetting < ApplicationRecord
             if: :external_authorization_service_enabled
 
   validates :external_authorization_service_url,
-            url: true, allow_blank: true,
+            addressable_url: true, allow_blank: true,
             if: :external_authorization_service_enabled
 
   validates :external_authorization_service_timeout,

app/models/badge.rb

@@ -22,7 +22,7 @@ class Badge < ApplicationRecord
 
   scope :order_created_at_asc, -> { reorder(created_at: :asc) }
 
-  validates :link_url, :image_url, url: { protocols: %w(http https) }
+  validates :link_url, :image_url, addressable_url: true
   validates :type, presence: true
 
   def rendered_link_url(project = nil)

app/models/ci/build_runner_session.rb

@@ -13,7 +13,7 @@ module Ci
     belongs_to :build, class_name: 'Ci::Build', inverse_of: :runner_session
 
     validates :build, presence: true
-    validates :url, url: { protocols: %w(https) }
+    validates :url, addressable_url: { schemes: %w(https) }
 
     def terminal_specification
       wss_url = Gitlab::UrlHelpers.as_wss(self.url)

app/models/environment.rb

@@ -35,7 +35,7 @@ class Environment < ApplicationRecord
   validates :external_url,
             length: { maximum: 255 },
             allow_nil: true,
-            url: true
+            addressable_url: true
 
   delegate :stop_action, :manual_actions, to: :last_deployment, allow_nil: true
 

app/models/error_tracking/project_error_tracking_setting.rb

@@ -22,7 +22,7 @@ module ErrorTracking
 
     belongs_to :project
 
-    validates :api_url, length: { maximum: 255 }, public_url: true, url: { enforce_sanitization: true, ascii_only: true }, allow_nil: true
+    validates :api_url, length: { maximum: 255 }, public_url: { enforce_sanitization: true, ascii_only: true }, allow_nil: true
 
     validates :api_url, presence: { message: 'is a required field' }, if: :enabled
 

app/models/generic_commit_status.rb

@@ -3,7 +3,7 @@
 class GenericCommitStatus < CommitStatus
   before_validation :set_default_values
 
-  validates :target_url, url: true,
+  validates :target_url, addressable_url: true,
                          length: { maximum: 255 },
                          allow_nil: true
 

app/models/project.rb

@@ -327,7 +327,7 @@ class Project < ApplicationRecord
 
   validates :namespace, presence: true
   validates :name, uniqueness: { scope: :namespace_id }
-  validates :import_url, public_url: { protocols: ->(project) { project.persisted? ? VALID_MIRROR_PROTOCOLS : VALID_IMPORT_PROTOCOLS },
+  validates :import_url, public_url: { schemes: ->(project) { project.persisted? ? VALID_MIRROR_PROTOCOLS : VALID_IMPORT_PROTOCOLS },
                                        ports: ->(project) { project.persisted? ? VALID_MIRROR_PORTS : VALID_IMPORT_PORTS },
                                        enforce_user: true }, if: [:external_import?, :import_url_changed?]
   validates :star_count, numericality: { greater_than_or_equal_to: 0 }

app/models/releases/link.rb

@@ -6,7 +6,7 @@ module Releases
 
     belongs_to :release
 
-    validates :url, presence: true, url: { protocols: %w(http https ftp) }, uniqueness: { scope: :release }
+    validates :url, presence: true, addressable_url: { schemes: %w(http https ftp) }, uniqueness: { scope: :release }
     validates :name, presence: true, uniqueness: { scope: :release }
 
     scope :sorted, -> { order(created_at: :desc) }

app/models/remote_mirror.rb

@@ -17,7 +17,7 @@ class RemoteMirror < ApplicationRecord
 
   belongs_to :project, inverse_of: :remote_mirrors
 
-  validates :url, presence: true, public_url: { protocols: %w(ssh git http https), allow_blank: true, enforce_user: true }
+  validates :url, presence: true, public_url: { schemes: %w(ssh git http https), allow_blank: true, enforce_user: true }
 
   before_save :set_new_remote_name, if: :mirror_url_changed?
 

app/validators/url_validator.rb → app/validators/addressable_url_validator.rb

 # frozen_string_literal: true
 
-# UrlValidator
+# AddressableUrlValidator
 #
-# Custom validator for URLs.
+# Custom validator for URLs. This is a stricter version of UrlValidator - it also checks
+# for using the right protocol, but it actually parses the URL checking for any syntax errors.
+# The regex is also different from `URI` as we use `Addressable::URI` here.
 #
-# By default, only URLs for the HTTP(S) protocols will be considered valid.
-# Provide a `:protocols` option to configure accepted protocols.
+# By default, only URLs for the HTTP(S) schemes will be considered valid.
+# Provide a `:schemes` option to configure accepted schemes.
 #
 # Example:
 #
 #   class User < ActiveRecord::Base
-#     validates :personal_url, url: true
+#     validates :personal_url, addressable_url: true
 #
-#     validates :ftp_url, url: { protocols: %w(ftp) }
+#     validates :ftp_url, addressable_url: { schemes: %w(ftp) }
 #
-#     validates :git_url, url: { protocols: %w(http https ssh git) }
+#     validates :git_url, addressable_url: { schemes: %w(http https ssh git) }
 #   end
 #
 # This validator can also block urls pointing to localhost or the local network to
 # protect against Server-side Request Forgery (SSRF), or check for the right port.
 #
-# The available options are:
-# - protocols: Allowed protocols. Default: http and https
-# - allow_localhost: Allow urls pointing to localhost. Default: true
-# - allow_local_network: Allow urls pointing to private network addresses. Default: true
-# - ports: Allowed ports. Default: all.
-# - enforce_user: Validate user format. Default: false
-# - enforce_sanitization: Validate that there are no html/css/js tags. Default: false
+# Configuration options:
+# * <tt>message</tt> - A custom error message (default is: "must be a valid URL").
+# * <tt>schemes</tt> - Array of URI schemes. Default: +['http', 'https']+
+# * <tt>allow_localhost</tt> - Allow urls pointing to +localhost+. Default: +true+
+# * <tt>allow_local_network</tt> - Allow urls pointing to private network addresses. Default: +true+
+# * <tt>allow_blank</tt> - Allow urls to be +blank+. Default: +false+
+# * <tt>allow_nil</tt> - Allow urls to be +nil+. Default: +false+
+# * <tt>ports</tt> - Allowed ports. Default: +all+.
+# * <tt>enforce_user</tt> - Validate user format. Default: +false+
+# * <tt>enforce_sanitization</tt> - Validate that there are no html/css/js tags. Default: +false+
 #
 # Example:
 #   class User < ActiveRecord::Base
-#     validates :personal_url, url: { allow_localhost: false, allow_local_network: false}
+#     validates :personal_url, addressable_url: { allow_localhost: false, allow_local_network: false}
 #
-#     validates :web_url, url: { ports: [80, 443] }
+#     validates :web_url, addressable_url: { ports: [80, 443] }
 #   end
-class UrlValidator < ActiveModel::EachValidator
-  DEFAULT_PROTOCOLS = %w(http https).freeze
-
+class AddressableUrlValidator < ActiveModel::EachValidator
   attr_reader :record
 
+  BLOCKER_VALIDATE_OPTIONS = {
+    schemes: %w(http https),
+    ports: [],
+    allow_localhost: true,
+    allow_local_network: true,
+    ascii_only: false,
+    enforce_user: false,
+    enforce_sanitization: false
+  }.freeze
+
+  DEFAULT_OPTIONS = BLOCKER_VALIDATE_OPTIONS.merge({
+    message: 'must be a valid URL'
+  }).freeze
+
+  def initialize(options)
+    options.reverse_merge!(DEFAULT_OPTIONS)
+
+    super(options)
+  end
+
   def validate_each(record, attribute, value)
     @record = record
 
     unless value.present?
-      record.errors.add(attribute, 'must be a valid URL')
+      record.errors.add(attribute, options.fetch(:message))
       return
     end
 
@@ -63,36 +86,21 @@ class UrlValidator < ActiveModel::EachValidator
     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
-    {
-      protocols: DEFAULT_PROTOCOLS,
-      ports: [],
-      allow_localhost: true,
-      allow_local_network: true,
-      ascii_only: false,
-      enforce_user: false,
-      enforce_sanitization: false
-    }
-  end
-
   def current_options
-    options = self.options.map do |option, value|
+    options.map do |option, value|
       [option, value.is_a?(Proc) ? value.call(record) : value]
     end.to_h
-
-    default_options.merge(options)
   end
 
   def blocker_args
-    current_options.slice(*default_options.keys).tap do |args|
-      if allow_setting_local_requests?
+    current_options.slice(*BLOCKER_VALIDATE_OPTIONS.keys).tap do |args|
+      if self.class.allow_setting_local_requests?
         args[:allow_localhost] = args[:allow_local_network] = true
       end
     end
   end
 
-  def allow_setting_local_requests?
+  def self.allow_setting_local_requests?
     # We cannot use Gitlab::CurrentSettings as ApplicationSetting itself
     # uses UrlValidator to validate urls. This ends up in a cycle
     # when Gitlab::CurrentSettings creates an ApplicationSetting which then

app/validators/public_url_validator.rb

@@ -2,7 +2,7 @@
 
 # PublicUrlValidator
 #
-# Custom validator for URLs. This validator works like UrlValidator but
+# Custom validator for URLs. This validator works like AddressableUrlValidator but
 # it blocks by default urls pointing to localhost or the local network.
 #
 # This validator accepts the same params UrlValidator does.
@@ -12,17 +12,20 @@
 #   class User < ActiveRecord::Base
 #     validates :personal_url, public_url: true
 #
-#     validates :ftp_url, public_url: { protocols: %w(ftp) }
+#     validates :ftp_url, public_url: { schemes: %w(ftp) }
 #
 #     validates :git_url, public_url: { allow_localhost: true, allow_local_network: true}
 #   end
 #
-class PublicUrlValidator < UrlValidator
-  private
+class PublicUrlValidator < AddressableUrlValidator
+  DEFAULT_OPTIONS = {
+    allow_localhost: false,
+    allow_local_network: false
+  }.freeze
 
-  def default_options
-    # By default block all urls pointing to localhost or the local network
-    super.merge(allow_localhost: false,
-                allow_local_network: false)
+  def initialize(options)
+    options.reverse_merge!(DEFAULT_OPTIONS)
+
+    super(options)
   end
 end

changelogs/unreleased/24985-align-urlvalidator-to-validate_url-gem-implementation.yml

+---
+title: "Align UrlValidator to validate_url gem implementation"
+merge_request: 27194
+author: Horatiu Eugen Vlad
+type: fixed

ee/app/models/geo_node.rb

@@ -18,8 +18,8 @@ class GeoNode < ApplicationRecord
   default_values url: ->(record) { record.class.current_node_url },
                  primary: false
 
-  validates :url, presence: true, uniqueness: { case_sensitive: false }, url: true
-  validates :internal_url, url: true, allow_blank: true, allow_nil: true
+  validates :url, presence: true, uniqueness: { case_sensitive: false }, addressable_url: true
+  validates :internal_url, addressable_url: true, allow_blank: true, allow_nil: true
 
   validates :primary, uniqueness: { message: 'node already exists' }, if: :primary
   validates :enabled, if: :primary, acceptance: { message: 'Geo primary node cannot be disabled' }

ee/app/models/hooks/group_hook.rb

@@ -21,5 +21,5 @@ class GroupHook < ProjectHook
   belongs_to :group
 
   clear_validators!
-  validates :url, presence: true, url: true
+  validates :url, presence: true, addressable_url: true
 end

ee/app/models/project_services/github_service.rb

@@ -10,7 +10,7 @@ class GithubService < Service
   delegate :api_url, :owner, :repository_name, to: :remote_project
 
   validates :token, presence: true, if: :activated?
-  validates :repository_url, url: true, allow_blank: true
+  validates :repository_url, addressable_url: true, allow_blank: true
 
   default_value_for :pipeline_events, true
 

ee/app/models/project_services/jenkins_service.rb

@@ -5,7 +5,7 @@ class JenkinsService < CiService
 
   before_update :reset_password
 
-  validates :jenkins_url, presence: true, url: true, if: :activated?
+  validates :jenkins_url, presence: true, addressable_url: true, if: :activated?
   validates :project_name, presence: true, if: :activated?
   validates :username, presence: true, if: ->(service) { service.activated? && service.password_touched? && service.password.present? }
 

ee/app/models/saml_provider.rb

@@ -7,7 +7,7 @@ class SamlProvider < ApplicationRecord
   has_many :identities
 
   validates :group, presence: true, top_level_group: true
-  validates :sso_url, presence: true, url: { protocols: %w(https), ascii_only: true }
+  validates :sso_url, presence: true, addressable_url: { schemes: %w(https), ascii_only: true }
   validates :certificate_fingerprint, presence: true, certificate_fingerprint: true
 
   after_initialize :set_defaults, if: :new_record?

lib/gitlab/import_export/after_export_strategies/web_upload_strategy.rb

@@ -8,7 +8,7 @@ module Gitlab
         POST_METHOD = 'POST'.freeze
         INVALID_HTTP_METHOD = 'invalid. Only PUT and POST methods allowed.'.freeze
 
-        validates :url, url: true
+        validates :url, addressable_url: true
 
         validate do
           unless [PUT_METHOD, POST_METHOD].include?(http_method.upcase)

lib/gitlab/url_blocker.rb

@@ -8,7 +8,7 @@ module Gitlab
     BlockedUrlError = Class.new(StandardError)
 
     class << self
-      def validate!(url, ports: [], protocols: [], allow_localhost: false, allow_local_network: true, ascii_only: false, enforce_user: false, enforce_sanitization: false)
+      def validate!(url, ports: [], schemes: [], allow_localhost: false, allow_local_network: true, ascii_only: false, enforce_user: false, enforce_sanitization: false)
         return true if url.nil?
 
         # Param url can be a string, URI or Addressable::URI
@@ -20,7 +20,7 @@ module Gitlab
         return true if internal?(uri)
 
         port = get_port(uri)
-        validate_protocol!(uri.scheme, protocols)
+        validate_scheme!(uri.scheme, schemes)
         validate_port!(port, ports) if ports.any?
         validate_user!(uri.user) if enforce_user
         validate_hostname!(uri.hostname)
@@ -85,9 +85,9 @@ module Gitlab
         raise BlockedUrlError, "Only allowed ports are #{ports.join(', ')}, and any over 1024"
       end
 
-      def validate_protocol!(protocol, protocols)
-        if protocol.blank? || (protocols.any? && !protocols.include?(protocol))
-          raise BlockedUrlError, "Only allowed protocols are #{protocols.join(', ')}"
+      def validate_scheme!(scheme, schemes)
+        if scheme.blank? || (schemes.any? && !schemes.include?(scheme))
+          raise BlockedUrlError, "Only allowed schemes are #{schemes.join(', ')}"
         end
       end
 

spec/controllers/projects/mirrors_controller_spec.rb

@@ -79,7 +79,7 @@ describe Projects::MirrorsController do
         do_put(project, remote_mirrors_attributes: remote_mirror_attributes)
 
         expect(response).to redirect_to(project_settings_repository_path(project, anchor: 'js-push-remote-settings'))
-        expect(flash[:alert]).to match(/Only allowed protocols are/)
+        expect(flash[:alert]).to match(/Only allowed schemes are/)
       end
 
       it 'does not create a RemoteMirror object' do

spec/lib/gitlab/url_blocker_spec.rb

@@ -23,10 +23,10 @@ describe Gitlab::UrlBlocker do
       expect(described_class.blocked_url?('https://gitlab.com:25/foo/foo.git', ports: ports)).to be true
     end
 
-    it 'returns true for bad protocol' do
-      expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git', protocols: ['https'])).to be false
+    it 'returns true for bad scheme' do
+      expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git', schemes: ['https'])).to be false
       expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git')).to be false
-      expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git', protocols: ['http'])).to be true
+      expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git', schemes: ['http'])).to be true
     end
 
     it 'returns true for bad protocol on configured web/SSH host and ports' do

spec/requests/api/commit_statuses_spec.rb

@@ -306,7 +306,22 @@ describe API::CommitStatuses do
         it 'responds with bad request status and validation errors' do
           expect(response).to have_gitlab_http_status(400)
           expect(json_response['message']['target_url'])
-            .to include 'is blocked: Only allowed protocols are http, https'
+            .to include 'is blocked: Only allowed schemes are http, https'
+        end
+      end
+
+      context 'when target URL is an unsupported scheme' do
+        before do
+          post api(post_url, developer), params: {
+                                           state: 'pending',
+                                           target_url: 'git://example.com'
+                                         }
+        end
+
+        it 'responds with bad request status and validation errors' do
+          expect(response).to have_gitlab_http_status(400)
+          expect(json_response['message']['target_url'])
+              .to include 'is blocked: Only allowed schemes are http, https'
         end
       end
     end

spec/support/shared_examples/url_validator_examples.rb

-RSpec.shared_examples 'url validator examples' do |protocols|
+RSpec.shared_examples 'url validator examples' do |schemes|
   let(:validator) { described_class.new(attributes: [:link_url], **options) }
   let!(:badge) { build(:badge, link_url: 'http://www.example.com') }
 
-  subject { validator.validate_each(badge, :link_url, badge.link_url) }
+  subject { validator.validate(badge) }
 
-  describe '#validates_each' do
+  describe '#validate' do
     context 'with no options' do
       let(:options) { {} }
 
-      it "allows #{protocols.join(',')} protocols by default" do
-        expect(validator.send(:default_options)[:protocols]).to eq protocols
+      it "allows #{schemes.join(',')} schemes by default" do
+        expect(validator.options[:schemes]).to eq schemes
       end
 
       it 'checks that the url structure is valid' do
@@ -17,25 +17,25 @@ RSpec.shared_examples 'url validator examples' do |protocols|
 
         subject
 
-        expect(badge.errors.empty?).to be false
+        expect(badge.errors).to be_present
       end
     end
 
-    context 'with protocols' do
-      let(:options) { { protocols: %w[http] } }
+    context 'with schemes' do
+      let(:options) { { schemes: %w(http) } }
 
-      it 'allows urls with the defined protocols' do
+      it 'allows urls with the defined schemes' do
         subject
 
-        expect(badge.errors.empty?).to be true
+        expect(badge.errors).to be_empty
       end
 
-      it 'add error if the url protocol does not match the selected ones' do
+      it 'add error if the url scheme does not match the selected ones' do
         badge.link_url = 'https://www.example.com'
 
         subject
 
-        expect(badge.errors.empty?).to be false
+        expect(badge.errors).to be_present
       end
     end
   end

spec/validators/url_validator_spec.rb → spec/validators/addressable_url_validator_spec.rb

@@ -2,11 +2,11 @@
 
 require 'spec_helper'
 
-describe UrlValidator do
+describe AddressableUrlValidator do
   let!(:badge) { build(:badge, link_url: 'http://www.example.com') }
-  subject { validator.validate_each(badge, :link_url, badge.link_url) }
+  subject { validator.validate(badge) }
 
-  include_examples 'url validator examples', described_class::DEFAULT_PROTOCOLS
+  include_examples 'url validator examples', described_class::DEFAULT_OPTIONS[:schemes]
 
   describe 'validations' do
     include_context 'invalid urls'
@@ -14,13 +14,13 @@ describe UrlValidator do
     let(:validator) { described_class.new(attributes: [:link_url]) }
 
     it 'returns error when url is nil' do
-      expect(validator.validate_each(badge, :link_url, nil)).to be_nil
-      expect(badge.errors.first[1]).to eq 'must be a valid URL'
+      expect(validator.validate_each(badge, :link_url, nil)).to be_falsey
+      expect(badge.errors.first[1]).to eq validator.options.fetch(:message)
     end
 
     it 'returns error when url is empty' do
-      expect(validator.validate_each(badge, :link_url, '')).to be_nil
-      expect(badge.errors.first[1]).to eq 'must be a valid URL'
+      expect(validator.validate_each(badge, :link_url, '')).to be_falsey
+      expect(badge.errors.first[1]).to eq validator.options.fetch(:message)
     end
 
     it 'does not allow urls with CR or LF characters' do
@@ -30,6 +30,17 @@ describe UrlValidator do
         end
       end
     end
+
+    it 'provides all arguments to UrlBlock validate' do
+      expect(Gitlab::UrlBlocker)
+          .to receive(:validate!)
+                  .with(badge.link_url, described_class::BLOCKER_VALIDATE_OPTIONS)
+                  .and_return(true)
+
+      subject
+
+      expect(badge.errors).to be_empty
+    end
   end
 
   context 'by default' do
@@ -40,7 +51,7 @@ describe UrlValidator do
 
       subject
 
-      expect(badge.errors.empty?).to be true
+      expect(badge.errors).to be_empty
     end
 
     it 'does not block urls pointing to the local network' do
@@ -48,7 +59,23 @@ describe UrlValidator do
 
       subject
 
-      expect(badge.errors.empty?).to be true
+      expect(badge.errors).to be_empty
+    end
+
+    it 'does block nil urls' do
+      badge.link_url = nil
+
+      subject
+
+      expect(badge.errors).to be_present
+    end
+
+    it 'does block blank urls' do
+      badge.link_url = '\n\r \n'
+
+      subject
+
+      expect(badge.errors).to be_present
     end
 
     it 'strips urls' do
@@ -67,6 +94,40 @@ describe UrlValidator do
     end
   end
 
+  context 'when message is set' do
+    let(:message) { 'is blocked: test message' }
+    let(:validator) { described_class.new(attributes: [:link_url], allow_nil: false, message: message) }
+
+    it 'does block nil url with provided error message' do
+      expect(validator.validate_each(badge, :link_url, nil)).to be_falsey
+      expect(badge.errors.first[1]).to eq message
+    end
+  end
+
+  context 'when allow_nil is set to true' do
+    let(:validator) { described_class.new(attributes: [:link_url], allow_nil: true) }
+
+    it 'does not block nil urls' do
+      badge.link_url = nil
+
+      subject
+
+      expect(badge.errors).to be_empty
+    end
+  end
+
+  context 'when allow_blank is set to true' do
+    let(:validator) { described_class.new(attributes: [:link_url], allow_blank: true) }
+
+    it 'does not block blank urls' do
+      badge.link_url = "\n\r \n"
+
+      subject
+
+      expect(badge.errors).to be_empty
+    end
+  end
+
   context 'when allow_localhost is set to false' do
     let(:validator) { described_class.new(attributes: [:link_url], allow_localhost: false) }
 
@@ -75,7 +136,21 @@ describe UrlValidator do
 
       subject
 
-      expect(badge.errors.empty?).to be false
+      expect(badge.errors).to be_present
+    end
+
+    context 'when allow_setting_local_requests is set to true' do
+      it 'does not block urls pointing to localhost' do
+        expect(described_class)
+          .to receive(:allow_setting_local_requests?)
+            .and_return(true)
+
+        badge.link_url = 'https://127.0.0.1'
+
+        subject
+
+        expect(badge.errors).to be_empty
+      end
     end
   end
 
@@ -87,7 +162,21 @@ describe UrlValidator do
 
       subject
 
-      expect(badge.errors.empty?).to be false
+      expect(badge.errors).to be_present
+    end
+
+    context 'when allow_setting_local_requests is set to true' do
+      it 'does not block urls pointing to local network' do
+        expect(described_class)
+          .to receive(:allow_setting_local_requests?)
+            .and_return(true)
+
+        badge.link_url = 'https://192.168.1.1'
+
+        subject
+
+        expect(badge.errors).to be_empty
+      end
     end
   end
 
@@ -100,7 +189,7 @@ describe UrlValidator do
       it 'does not block any port' do
         subject
 
-        expect(badge.errors.empty?).to be true
+        expect(badge.errors).to be_empty
       end
     end
 
@@ -110,7 +199,7 @@ describe UrlValidator do
       it 'blocks urls with a different port' do
         subject
 
-        expect(badge.errors.empty?).to be false
+        expect(badge.errors).to be_present
       end
     end
   end
@@ -127,7 +216,7 @@ describe UrlValidator do
 
         subject
 
-        expect(badge.errors.empty?).to be false
+        expect(badge.errors).to be_present
       end
     end
 
@@ -139,7 +228,7 @@ describe UrlValidator do
 
         subject
 
-        expect(badge.errors.empty?).to be true
+        expect(badge.errors).to be_empty
       end
     end
   end
@@ -156,7 +245,7 @@ describe UrlValidator do
 
         subject
 
-        expect(badge.errors.empty?).to be false
+        expect(badge.errors).to be_present
       end
     end
 
@@ -168,7 +257,7 @@ describe UrlValidator do
 
         subject
 
-        expect(badge.errors.empty?).to be true
+        expect(badge.errors).to be_empty
       end
     end
   end
@@ -191,7 +280,7 @@ describe UrlValidator do
 
         subject
 
-        expect(badge.errors.empty?).to be false
+        expect(badge.errors).to be_present
       end
 
       it 'prevents unsafe internal urls' do
@@ -199,7 +288,7 @@ describe UrlValidator do
 
         subject
 
-        expect(badge.errors.empty?).to be false
+        expect(badge.errors).to be_present
       end
 
       it 'allows safe urls' do
@@ -207,7 +296,7 @@ describe UrlValidator do
 
         subject
 
-        expect(badge.errors.empty?).to be true
+        expect(badge.errors).to be_empty
       end
     end
 
@@ -219,7 +308,7 @@ describe UrlValidator do
 
         subject
 
-        expect(badge.errors.empty?).to be true
+        expect(badge.errors).to be_empty
       end
     end
   end

spec/validators/public_url_validator_spec.rb

 require 'spec_helper'
 
 describe PublicUrlValidator do
-  include_examples 'url validator examples', described_class::DEFAULT_PROTOCOLS
+  include_examples 'url validator examples', AddressableUrlValidator::DEFAULT_OPTIONS[:schemes]
 
   context 'by default' do
     let(:validator) { described_class.new(attributes: [:link_url]) }
     let!(:badge) { build(:badge, link_url: 'http://www.example.com') }
 
-    subject { validator.validate_each(badge, :link_url, badge.link_url) }
+    subject { validator.validate(badge) }
 
     it 'blocks urls pointing to localhost' do
       badge.link_url = 'https://127.0.0.1'
 
       subject
 
-      expect(badge.errors.empty?).to be false
+      expect(badge.errors).to be_present
     end
 
     it 'blocks urls pointing to the local network' do
@@ -22,7 +22,7 @@ describe PublicUrlValidator do
 
       subject
 
-      expect(badge.errors.empty?).to be false
+      expect(badge.errors).to be_present
     end
   end
 end