From 25aa7d1005de08145c70b3c86c05f40d2ce80835 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steve=20Fr=C3=A9cinaux?= Date: Fri, 6 Jan 2023 01:17:20 +0100 Subject: [PATCH 1/6] 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 --- app/models/user.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 9ab3be249..661de90ac 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -29,8 +29,8 @@ class User < ActiveRecord::Base has_many :taggings, -> { includes(:tag) }, dependent: :destroy has_many :tags, -> { distinct }, through: :taggings - validates_format_of :homesite, message: "L’adresse du site Web personnel n’est pas valide", with: URI::regexp(%w(http https)), allow_blank: true - validates :homesite, length: { maximum: 100, message: "L’adresse du site Web personnel est trop longue" } + validates :homesite, http_url: { protocols: ["http", "https"], message: "L’adresse du site Web personnel n’est pas valide" }, + length: { maximum: 100, message: "L’adresse du site Web personnel est trop longue" } validates :name, length: { maximum: 40, message: "Le nom affiché est trop long" } validates :jabber_id, length: { maximum: 32, message: "L’adresse XMPP est trop longue" } validates :signature, length: { maximum: 255, message: "La signature est trop longue" } From ab69ea5238b3fc7b8982116e28c66b2f869c1e9e Mon Sep 17 00:00:00 2001 From: Adrien Dorsaz Date: Sat, 7 Jan 2023 18:04:42 +0100 Subject: [PATCH 2/6] fix multiple sharp signs in URIs rename HttpUrlValidator as UriValidator --- app/models/bookmark.rb | 17 ++------- app/models/link.rb | 16 ++------ app/validators/http_url_validator.rb | 25 ------------ app/validators/uri_validator.rb | 57 ++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 52 deletions(-) delete mode 100644 app/validators/http_url_validator.rb create mode 100644 app/validators/uri_validator.rb diff --git a/app/models/bookmark.rb b/app/models/bookmark.rb index 75e47a37f..5de551d39 100644 --- a/app/models/bookmark.rb +++ b/app/models/bookmark.rb @@ -26,22 +26,11 @@ class Bookmark < Content validates :title, presence: { message: "Le titre est obligatoire" }, length: { maximum: 100, message: "Le titre est trop long" } validates :link, presence: { message: "Vous ne pouvez pas poster un lien vide" }, - http_url: { message: "Le lien n'est pas valide" }, + uri: { message: "Le lien n'est pas valide" }, length: { maximum: 255, message: "Le lien est trop long" } - def link=(raw) - raw.strip! - return write_attribute :url, nil if raw.blank? - uri = URI.parse(raw) - # Default to HTTP link if neither scheme nor host is found - if uri.scheme.blank? && uri.host.blank? - raw = "http://#{raw}" - uri = URI.parse(raw) - end - write_attribute :link, uri.to_s - # Let raw value if error when parsed, HttpUrlValidator will manage it - rescue URI::InvalidURIError - write_attribute :link, raw + before_validation do |bookmark| + bookmark.link = UriValidator.before_validation(bookmark.link); end def create_node(attrs={}) diff --git a/app/models/link.rb b/app/models/link.rb index 52fde9b42..9b34aeae4 100644 --- a/app/models/link.rb +++ b/app/models/link.rb @@ -26,22 +26,12 @@ class Link < ActiveRecord::Base validates :title, presence: { message: "Un lien doit obligatoirement avoir un titre" }, length: { maximum: 100, message: "Le titre est trop long" } - validates :url, http_url: { protocols: PROTOCOLS, message: "L'adresse n'est pas valide" }, + validates :url, uri: { protocols: PROTOCOLS, message: "L'adresse n'est pas valide" }, presence: { message: "Un lien doit obligatoirement avoir une adresse" }, length: { maximum: 255, message: "L’adresse est trop longue" } - def url=(raw) - raw.strip! - return write_attribute :url, nil if raw.blank? - uri = URI.parse(raw) - if uri.scheme.blank? && uri.host.blank? - raw = "http://#{raw}" - uri = URI.parse(raw) - end - write_attribute :url, uri.to_s - # Let raw value if error when parsed, HttpUrlValidator will manage it - rescue URI::InvalidURIError - write_attribute :url, raw + before_validation do |link| + link.url = UriValidator.before_validation(link.url); end ### Behaviour ### diff --git a/app/validators/http_url_validator.rb b/app/validators/http_url_validator.rb deleted file mode 100644 index d104a1a93..000000000 --- a/app/validators/http_url_validator.rb +++ /dev/null @@ -1,25 +0,0 @@ -# Validate if a value is a HTTP url -class HttpUrlValidator < ActiveModel::EachValidator - def validate_each(record, attribute, value) - if value.present? && not(valid?(value, options)) - record.errors.add attribute, (options[:message] || "n'est pas une URL HTTP valide") - end - end - - private - - def valid?(value, options) - # Valid links can be parsed by URI - uri = URI.parse(value) - # Authorize protocol - if options.has_key?(:protocols) - return options[:protocols].include?(uri.scheme) - end - # Links starting with "//MY_DOMAIN" are current scheme dependent and are valid - return true if uri.scheme.nil? && uri.host == MY_DOMAIN - # All other links are valid only if scheme and host exists - return uri.scheme.present? && uri.host.present? - rescue URI::InvalidURIError - false - end -end diff --git a/app/validators/uri_validator.rb b/app/validators/uri_validator.rb new file mode 100644 index 000000000..92f02671b --- /dev/null +++ b/app/validators/uri_validator.rb @@ -0,0 +1,57 @@ +# Validate if a value is a URI +class UriValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + if value.present? && not(valid?(value, options)) + record.errors.add attribute, (options[:message] || "n'est pas un lien valide") + end + end + + private + + def valid?(value, options) + # Valid links can be parsed by URI + uri = URI.parse(value) + + # Authorize only given protocol + if options.has_key?(:protocols) + return options[:protocols].include?(uri.scheme) + end + + # Links starting with "//MY_DOMAIN" are current scheme dependent and are valid + return true if uri.scheme.nil? && uri.host == MY_DOMAIN + + # All other links are valid only if scheme and host exists + return uri.scheme.present? && uri.host.present? + rescue URI::InvalidURIError + false + end + + def self.before_validation(raw, default_scheme='http://') + raw.strip! + return nil if raw.blank? + + # Automatically encodes sharp signs (#) found in URI fragment: + # RFC 3986 uses sharp to define URI fragment and requires other sharps + # to be percent encoded + fragments = raw.split("#") + if (fragments.length > 2) + raw = fragments[0] + '#' + fragments[1..-1].join('%23') + end + + uri = URI.parse(raw) + + # If user forgot to add scheme, add default + if default_scheme.present? && uri.scheme.blank? && uri.host.blank? + raw = "#{default_scheme}#{raw}" + uri = URI.parse(raw) + end + + return uri.to_s + + # Let raw value if error occured when we tried to parse it, because + # the HttpUrlValidator will manage it itself on validation + rescue URI::InvalidURIError + return raw + end + +end From f0a4417bd1aee07c3020797a557142c2a0424c03 Mon Sep 17 00:00:00 2001 From: Adrien Dorsaz Date: Sat, 28 Jan 2023 22:24:47 +0100 Subject: [PATCH 3/6] user: use the new URI validator instead of old HttpUrlValidator --- app/models/user.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 600b8f710..78484d593 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -30,15 +30,20 @@ class User < ActiveRecord::Base has_many :taggings, -> { includes(:tag) }, dependent: :destroy has_many :tags, -> { distinct }, through: :taggings - validates :homesite, http_url: { protocols: ["http", "https"], message: "L’adresse du site Web personnel n’est pas valide" }, + validates :homesite, uri: { protocols: ["http", "https"], message: "L’adresse du site Web personnel n’est pas valide" }, length: { maximum: 100, message: "L’adresse du site Web personnel est trop longue" } validates :name, length: { maximum: 40, message: "Le nom affiché est trop long" } validates :jabber_id, length: { maximum: 32, message: "L’adresse XMPP est trop longue" } - validates :mastodon_url, http_url: { protocols: ["https"], message: "L’adresse du compte Mastodon n’est pas valide" }, + validates :mastodon_url, uri: { protocols: ["https"], message: "L’adresse du compte Mastodon n’est pas valide" }, length: { maximum: 255, message: "L’adresse du compte Mastodon est trop longue" } validates :signature, length: { maximum: 255, message: "La signature est trop longue" } validates :custom_avatar_url, length: { maximum: 255, message: "L’adresse de l’avatar est trop longue" } + before_validation do |user| + user.homesite = UriValidator.before_validation(user.homesite) + user.mastodon_url = UriValidator.before_validation(user.mastodon_url) + end + def self.collective find_by(name: "Collectif") end From e6f4c64252fd59e6f9bf56065e7f5fc6786bafbd Mon Sep 17 00:00:00 2001 From: Adrien Dorsaz Date: Sat, 28 Jan 2023 22:32:41 +0100 Subject: [PATCH 4/6] remove comment reference to old HttpUrlValidator --- app/validators/uri_validator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/validators/uri_validator.rb b/app/validators/uri_validator.rb index 92f02671b..46c6d4f68 100644 --- a/app/validators/uri_validator.rb +++ b/app/validators/uri_validator.rb @@ -49,7 +49,7 @@ def self.before_validation(raw, default_scheme='http://') return uri.to_s # Let raw value if error occured when we tried to parse it, because - # the HttpUrlValidator will manage it itself on validation + # the UriValidator will manage it itself on validation rescue URI::InvalidURIError return raw end From 00c0b44281bfe522ecffc1678e888f2e39e546bf Mon Sep 17 00:00:00 2001 From: Adrien Dorsaz Date: Sat, 28 Jan 2023 23:30:41 +0100 Subject: [PATCH 5/6] 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. --- app/models/bookmark.rb | 4 ++++ app/models/link.rb | 4 ++++ app/models/user.rb | 5 +++++ app/validators/uri_validator.rb | 11 +++++++++++ 4 files changed, 24 insertions(+) diff --git a/app/models/bookmark.rb b/app/models/bookmark.rb index 5de551d39..05e1c5466 100644 --- a/app/models/bookmark.rb +++ b/app/models/bookmark.rb @@ -33,6 +33,10 @@ class Bookmark < Content bookmark.link = UriValidator.before_validation(bookmark.link); end + after_validation do |bookmark| + bookmark.link = UriValidator.after_validation(bookmark.link); + end + def create_node(attrs={}) attrs[:cc_licensed] = false super diff --git a/app/models/link.rb b/app/models/link.rb index 9b34aeae4..4f044e083 100644 --- a/app/models/link.rb +++ b/app/models/link.rb @@ -34,6 +34,10 @@ class Link < ActiveRecord::Base link.url = UriValidator.before_validation(link.url); end + after_validation do |link| + link.url = UriValidator.after_validation(link.url); + end + ### Behaviour ### def self.hit(id) diff --git a/app/models/user.rb b/app/models/user.rb index 78484d593..7a65c9d84 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -44,6 +44,11 @@ class User < ActiveRecord::Base user.mastodon_url = UriValidator.before_validation(user.mastodon_url) end + after_validation do |user| + user.homesite = UriValidator.after_validation(user.homesite) + user.mastodon_url = UriValidator.after_validation(user.mastodon_url) + end + def self.collective find_by(name: "Collectif") end diff --git a/app/validators/uri_validator.rb b/app/validators/uri_validator.rb index 46c6d4f68..65c1e1747 100644 --- a/app/validators/uri_validator.rb +++ b/app/validators/uri_validator.rb @@ -54,4 +54,15 @@ def self.before_validation(raw, default_scheme='http://') return raw end + def self.after_validation(raw) + # Decodes sharp signs (#) found in URI fragment to keep visual match with + # the user input + fragments = raw.split("#") + if (fragments.length == 2) + raw = fragments[0] + '#' + fragments[1].gsub('%23', '#') + end + + return raw + end + end From 51b2fd9bebd2e59fb0b8221d562a57d06b172aae Mon Sep 17 00:00:00 2001 From: Adrien Dorsaz Date: Sat, 28 Jan 2023 23:33:56 +0100 Subject: [PATCH 6/6] 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. --- app/controllers/bookmarks_controller.rb | 2 +- app/models/bookmark.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/bookmarks_controller.rb b/app/controllers/bookmarks_controller.rb index 91aa89629..43e3994f7 100644 --- a/app/controllers/bookmarks_controller.rb +++ b/app/controllers/bookmarks_controller.rb @@ -37,7 +37,7 @@ def create @bookmark.node = Node.new(user_id: current_user.id, cc_licensed: false) @bookmark.node.preview_tags = params[:tags] @bookmark.valid? - flash.now[:alert] = "Votre lien semble invalide. Le confimez‑vous ?" unless @bookmark.link =~ /\A#{URI::regexp(['http', 'https'])}\z/ + flash.now[:alert] = "Votre lien est invalide, veuillez le corriger." unless @bookmark.errors[:link].empty? render :new end end diff --git a/app/models/bookmark.rb b/app/models/bookmark.rb index 05e1c5466..e5fcff6db 100644 --- a/app/models/bookmark.rb +++ b/app/models/bookmark.rb @@ -26,7 +26,7 @@ class Bookmark < Content validates :title, presence: { message: "Le titre est obligatoire" }, length: { maximum: 100, message: "Le titre est trop long" } validates :link, presence: { message: "Vous ne pouvez pas poster un lien vide" }, - uri: { message: "Le lien n'est pas valide" }, + uri: { protocols: ["http", "https"], message: "Le lien n'est pas valide" }, length: { maximum: 255, message: "Le lien est trop long" } before_validation do |bookmark|