Fix invalid artist URLs being allowed
The problem was that the Addressable parser does not catch all invalid URL cases, so some extra checks were added in. - hostname must contain a dot This accounts for URLs of the following type: http://http://something.com which has a hostname of http. The artist URL tests were also updated with cases which test all validation errors.
This commit is contained in:
@@ -120,9 +120,18 @@ class ArtistUrl < ApplicationRecord
|
||||
end
|
||||
end
|
||||
|
||||
def validate_scheme(uri)
|
||||
errors[:url] << "'#{uri}' must begin with http:// or https:// " unless uri.scheme.in?(%w[http https])
|
||||
end
|
||||
|
||||
def validate_hostname(uri)
|
||||
errors[:url] << "'#{uri}' has a hostname '#{uri.host}' that does not contain a dot" unless uri.host&.include?('.')
|
||||
end
|
||||
|
||||
def validate_url_format
|
||||
uri = Addressable::URI.parse(url)
|
||||
errors[:url] << "'#{uri}' must begin with http:// or https:// " if !uri.scheme.in?(%w[http https])
|
||||
validate_scheme(uri)
|
||||
validate_hostname(uri)
|
||||
rescue Addressable::URI::InvalidURIError => error
|
||||
errors[:url] << "'#{uri}' is malformed: #{error}"
|
||||
end
|
||||
|
||||
@@ -24,10 +24,18 @@ class ArtistUrlTest < ActiveSupport::TestCase
|
||||
end
|
||||
|
||||
should "disallow invalid urls" do
|
||||
url = FactoryBot.build(:artist_url, url: "www.example.com")
|
||||
urls = [
|
||||
FactoryBot.build(:artist_url, url: "www.example.com"),
|
||||
FactoryBot.build(:artist_url, url: ":www.example.com"),
|
||||
FactoryBot.build(:artist_url, url: "http://http://www.example.com"),
|
||||
]
|
||||
|
||||
assert_equal(false, url.valid?)
|
||||
assert_match(/must begin with http/, url.errors.full_messages.join)
|
||||
assert_equal(false, urls[0].valid?)
|
||||
assert_match(/must begin with http/, urls[0].errors.full_messages.join)
|
||||
assert_equal(false, urls[1].valid?)
|
||||
assert_match(/is malformed/, urls[1].errors.full_messages.join)
|
||||
assert_equal(false, urls[2].valid?)
|
||||
assert_match(/that does not contain a dot/, urls[2].errors.full_messages.join)
|
||||
end
|
||||
|
||||
should "always add a trailing slash when normalized" do
|
||||
|
||||
Reference in New Issue
Block a user