diff --git a/app/models/article.rb b/app/models/article.rb index 1cea1c313..170790af6 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -32,6 +32,7 @@ class Article < ApplicationRecord MAX_USER_MENTION_LIVE_AT = Time.utc(2021, 4, 7).freeze UNIQUE_URL_ERROR = "has already been taken. " \ "Email #{ForemInstance.email} for further details.".freeze + PROHIBITED_UNICODE_CHARACTERS_REGEX = /[\u202a-\u202e]/ # BIDI embedding controls has_one :discussion_lock, dependent: :delete @@ -81,7 +82,6 @@ class Article < ApplicationRecord validates :slug, presence: { if: :published? }, format: /\A[0-9a-z\-_]*\z/ validates :slug, uniqueness: { scope: :user_id } validates :title, presence: true, length: { maximum: 128 } - validates :user_id, presence: true validates :user_subscriptions_count, presence: true validates :video, url: { allow_blank: true, schemes: %w[https http] } validates :video_closed_caption_track_url, url: { allow_blank: true, schemes: ["https"] } @@ -101,6 +101,7 @@ class Article < ApplicationRecord validate :validate_co_authors_exist, unless: -> { co_author_ids.blank? } before_validation :evaluate_markdown, :create_slug + before_validation :remove_prohibited_unicode_characters before_save :update_cached_user before_save :set_all_dates @@ -847,4 +848,10 @@ class Article < ApplicationRecord ::Articles::EnrichImageAttributesWorker.perform_async(id) end + + def remove_prohibited_unicode_characters + return unless title&.match?(PROHIBITED_UNICODE_CHARACTERS_REGEX) + + self.title = title.gsub(PROHIBITED_UNICODE_CHARACTERS_REGEX, "") + end end diff --git a/app/models/tag.rb b/app/models/tag.rb index 7fe324c6b..967cd7b7f 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -77,7 +77,7 @@ class Tag < ActsAsTaggableOn::Tag # [:alnum:] is not used here because it supports diacritical characters. # If we decide to allow diacritics in the future, we should replace the # following regex with [:alnum:]. - errors.add(:name, "contains non-alphanumeric characters") unless name.match?(/\A[[:alnum:]]+\z/i) + errors.add(:name, I18n.t("errors.messages.contains_prohibited_characters")) unless name.match?(/\A[[:alnum:]]+\z/i) end def errors_as_sentence diff --git a/config/locales/en.yml b/config/locales/en.yml index 2f2486504..3d5fbb08f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -140,6 +140,7 @@ en: bad_uri: is an invalid url blank: can't be blank confirmation: doesn't match %{attribute} + contains_prohibited_characters: contains non-alphanumeric or prohibited unicode characters empty: can't be empty equal_to: must be equal to %{count} even: must be even diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 287ef18c2..372a61661 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -157,6 +157,7 @@ fr: blank: ne peut pas être vide confirmation: ne correspond pas à %{attribute} confirmation_period_expired: doit être confirmé dans %{period}, veuillez en demander un nouveau + contains_prohibited_characters: contient des caractères unicode non alphanumériques ou interdits empty: ne peut pas être vide equal_to: doit être égal à %{count} even: doit être pair diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index d9bea1e29..87c351a94 100644 --- a/spec/models/article_spec.rb +++ b/spec/models/article_spec.rb @@ -47,7 +47,6 @@ RSpec.describe Article, type: :model do it { is_expected.to validate_presence_of(:reactions_count) } it { is_expected.to validate_presence_of(:user_subscriptions_count) } it { is_expected.to validate_presence_of(:title) } - it { is_expected.to validate_presence_of(:user_id) } it { is_expected.to validate_uniqueness_of(:slug).scoped_to(:user_id) } @@ -233,6 +232,34 @@ RSpec.describe Article, type: :model do end end + describe "title validation" do + it "produces a proper title" do + test_article = build(:article, title: "An Article Title") + + test_article.validate + + expect(test_article.title).to eq("An Article Title") + end + + it "sanitizes the title" do + test_article = build(:article, title: "\u202dThis starts with BIDI override") + + test_article.validate + + expect(test_article.title).not_to match(/\u202d/) + expect(test_article.title).to eq("This starts with BIDI override") + end + + it "rejects empty titles after sanitizing" do + test_article = build(:article, title: "\u202a\u202b\u202c\u202d\u202e") + + test_article.validate + + expect(test_article).not_to be_valid + expect(test_article.errors_as_sentence).to match("Title can't be blank") + end + end + describe "tag validation" do let(:article) { build(:article, user: user) } diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index bcc14a811..81acf6ad0 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -86,6 +86,11 @@ RSpec.describe Tag, type: :model do end end + it "fails validation if name is a prohibited (whitespace) unicode character" do + tag.name = "U+202D" + expect(tag).not_to be_valid + end + describe "alias_for" do it "passes validation if the alias refers to an existing tag" do tag = create(:tag)