Skip to content

Commit 8c74b4b

Browse files
TrimOumph
authored andcommitted
fix URI validation, and btw multiple sharp signs in URIs
Fix validating the URL of user home sites Home site validation used the URL regexp, but didn't anchor it. Because of this it was possible to add text before or after the URL and have it considered valid. For instance, this was considered a valid homesite value: "lorem ipsum https://example.org/#foo#bar dolor sit amet" It is possible to fix it by anchoring the regexp, but IMO it is wiser to use the same URL validator used at other places in the site. Hence this commit replaces this validation with the `http_url` validator used for news links and bookmarks. https://linuxfr.org/suivi/impossible-de-mettre-un-lien-vers-un-salon-matrix-dans-les-liens-d-une-depeche#comment-1911550 user: use the new URI validator instead of old HttpUrlValidator remove comment reference to old HttpUrlValidator adds after_validation method to UriValidator This is used to revert the workaround applied on fragment part of the URI to accept multiple sharp sign in the URI. This decoding of sharp sign in the fragment part allows to keep visual match with the user input. bookmark controllers use the bookmark model validator to validate link So the bookmark model is the only leader the validation of the link. Thus the protocol validation is moved to the model too.
1 parent cf0797e commit 8c74b4b

6 files changed

Lines changed: 96 additions & 56 deletions

File tree

app/controllers/bookmarks_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ def create
3939
@bookmark.node.preview_tags = params[:tags]
4040
end
4141
@bookmark.valid?
42-
flash.now[:alert] = "Votre lien semble invalide. Le confimez‑vous ?" unless @bookmark.link =~ /\A#{URI::regexp(['http', 'https'])}\z/
42+
flash.now[:alert] = "Votre lien est invalide, veuillez le corriger." unless @bookmark.errors[:link].empty?
4343
render :new
4444
end
4545
end

app/models/bookmark.rb

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,24 +27,17 @@ class Bookmark < Content
2727
length: { maximum: 160, message: "Le titre est trop long" },
2828
uniqueness: { message: "Un lien avec le même titre a déjà été proposé" }
2929
validates :link, presence: { message: "Vous ne pouvez pas poster un lien vide" },
30-
http_url: { message: "Le lien n'est pas valide" },
30+
uri: { protocols: ["http", "https"], message: "Le lien n'est pas valide" },
3131
length: { maximum: 255, message: "Le lien est trop long" },
3232
uniqueness: { message: "Cette adresse de lien a déjà été proposée" }
3333
validates :lang, inclusion: { in: Lang.valid_codes, allow_nil: false, message: "La langue du lien doit être définie" }
3434

35-
def link=(raw)
36-
raw.strip!
37-
return write_attribute :link, nil if raw.blank?
38-
uri = URI.parse(raw)
39-
# Default to HTTP link if neither scheme nor host is found
40-
if uri.scheme.blank? && uri.host.blank?
41-
raw = "http://#{raw}"
42-
uri = URI.parse(raw)
43-
end
44-
write_attribute :link, uri.to_s
45-
# Let raw value if error when parsed, HttpUrlValidator will manage it
46-
rescue URI::InvalidURIError
47-
write_attribute :link, raw
35+
before_validation do |bookmark|
36+
bookmark.link = UriValidator.before_validation(bookmark.link);
37+
end
38+
39+
after_validation do |bookmark|
40+
bookmark.link = UriValidator.after_validation(bookmark.link);
4841
end
4942

5043
def title=(raw)

app/models/link.rb

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,23 +26,17 @@ class Link < ActiveRecord::Base
2626

2727
validates :title, presence: { message: "Un lien doit obligatoirement avoir un titre" },
2828
length: { maximum: 100, message: "Le titre du lien est trop long" }
29-
validates :url, http_url: { protocols: PROTOCOLS, message: "L'adresse n'est pas valide" },
29+
validates :url, uri: { protocols: PROTOCOLS, message: "L'adresse n'est pas valide" },
3030
presence: { message: "Un lien doit obligatoirement avoir une adresse" },
3131
length: { maximum: 255, message: "L’adresse est trop longue" }
3232
validates :lang, inclusion: { in: Lang.valid_codes, allow_nil: false, message: "La langue du lien doit être définie" }
3333

34-
def url=(raw)
35-
raw.strip!
36-
return write_attribute :url, nil if raw.blank?
37-
uri = URI.parse(raw)
38-
if uri.scheme.blank? && uri.host.blank?
39-
raw = "http://#{raw}"
40-
uri = URI.parse(raw)
41-
end
42-
write_attribute :url, uri.to_s
43-
# Let raw value if error when parsed, HttpUrlValidator will manage it
44-
rescue URI::InvalidURIError
45-
write_attribute :url, raw
34+
before_validation do |link|
35+
link.url = UriValidator.before_validation(link.url);
36+
end
37+
38+
after_validation do |link|
39+
link.url = UriValidator.after_validation(link.url);
4640
end
4741

4842
def title=(raw)

app/models/user.rb

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,25 @@ class User < ActiveRecord::Base
3030
has_many :taggings, -> { includes(:tag) }, dependent: :destroy
3131
has_many :tags, -> { distinct }, through: :taggings
3232

33-
validates_format_of :homesite, message: "L’adresse du site web personnel n’est pas valide", with: URI::regexp(%w(http https)), allow_blank: true
34-
validates :homesite, length: { maximum: 100, message: "L’adresse du site web personnel est trop longue" }
33+
validates :homesite, uri: { protocols: ["http", "https"], message: "L’adresse du site Web personnel n’est pas valide" },
34+
length: { maximum: 100, message: "L’adresse du site Web personnel est trop longue" }
3535
validates :name, length: { maximum: 40, message: "Le nom affiché est trop long" }
3636
validates :jabber_id, length: { maximum: 32, message: "L’adresse XMPP est trop longue" }
37-
validates :mastodon_url, http_url: { protocols: ["https"], message: "L’adresse du compte Mastodon n’est pas valide" },
37+
validates :mastodon_url, uri: { protocols: ["https"], message: "L’adresse du compte Mastodon n’est pas valide" },
3838
length: { maximum: 255, message: "L’adresse du compte Mastodon est trop longue" }
3939
validates :signature, length: { maximum: 255, message: "La signature est trop longue" }
4040
validates :custom_avatar_url, length: { maximum: 255, message: "L’adresse de l’avatar est trop longue" }
4141

42+
before_validation do |user|
43+
user.homesite = UriValidator.before_validation(user.homesite)
44+
user.mastodon_url = UriValidator.before_validation(user.mastodon_url)
45+
end
46+
47+
after_validation do |user|
48+
user.homesite = UriValidator.after_validation(user.homesite)
49+
user.mastodon_url = UriValidator.after_validation(user.mastodon_url)
50+
end
51+
4252
def self.collective
4353
find_by(name: "Collectif")
4454
end

app/validators/http_url_validator.rb

Lines changed: 0 additions & 25 deletions
This file was deleted.

app/validators/uri_validator.rb

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# Validate if a value is a URI
2+
class UriValidator < ActiveModel::EachValidator
3+
def validate_each(record, attribute, value)
4+
if value.present? && not(valid?(value, options))
5+
record.errors.add attribute, (options[:message] || "n'est pas un lien valide")
6+
end
7+
end
8+
9+
private
10+
11+
def valid?(value, options)
12+
# Valid links can be parsed by URI
13+
uri = URI.parse(value)
14+
15+
# Authorize only given protocol
16+
if options.has_key?(:protocols)
17+
return options[:protocols].include?(uri.scheme)
18+
end
19+
20+
# Links starting with "//MY_DOMAIN" are current scheme dependent and are valid
21+
return true if uri.scheme.nil? && uri.host == MY_DOMAIN
22+
23+
# All other links are valid only if scheme and host exists
24+
return uri.scheme.present? && uri.host.present?
25+
rescue URI::InvalidURIError
26+
false
27+
end
28+
29+
def self.before_validation(raw, default_scheme='http://')
30+
raw.strip!
31+
return nil if raw.blank?
32+
33+
# Automatically encodes sharp signs (#) found in URI fragment:
34+
# RFC 3986 uses sharp to define URI fragment and requires other sharps
35+
# to be percent encoded
36+
fragments = raw.split("#")
37+
if (fragments.length > 2)
38+
raw = fragments[0] + '#' + fragments[1..-1].join('%23')
39+
end
40+
41+
uri = URI.parse(raw)
42+
43+
# If user forgot to add scheme, add default
44+
if default_scheme.present? && uri.scheme.blank? && uri.host.blank?
45+
raw = "#{default_scheme}#{raw}"
46+
uri = URI.parse(raw)
47+
end
48+
49+
return uri.to_s
50+
51+
# Let raw value if error occured when we tried to parse it, because
52+
# the UriValidator will manage it itself on validation
53+
rescue URI::InvalidURIError
54+
return raw
55+
end
56+
57+
def self.after_validation(raw)
58+
# Decodes sharp signs (#) found in URI fragment to keep visual match with
59+
# the user input
60+
fragments = raw.split("#")
61+
if (fragments.length == 2)
62+
raw = fragments[0] + '#' + fragments[1].gsub('%23', '#')
63+
end
64+
65+
return raw
66+
end
67+
68+
end

0 commit comments

Comments
 (0)