diff --git a/app/logical/visible_string_validator.rb b/app/logical/visible_string_validator.rb new file mode 100644 index 000000000..0716f588b --- /dev/null +++ b/app/logical/visible_string_validator.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +# A custom validator for ensuring a string isn't blank. Like the `presence` validator, but checks for certain invisible +# Unicode characters such as zero-width spaces too. +# +# @example +# validates :body, visible_string: true +# +# @see https://invisible-characters.com/ +# @see https://guides.rubyonrails.org/active_record_validations.html#presence +# @see https://guides.rubyonrails.org/active_record_validations.html#custom-validators +class VisibleStringValidator < ActiveModel::EachValidator + def validate_each(record, attr, string) + return if options[:allow_empty] && string.empty? + + if string.nil? || string.invisible? + record.errors.add(attr, "can't be blank") + end + end +end diff --git a/app/models/ban.rb b/app/models/ban.rb index bc5747476..16c3fd61c 100644 --- a/app/models/ban.rb +++ b/app/models/ban.rb @@ -14,7 +14,7 @@ class Ban < ApplicationRecord validates :duration, presence: true validates :duration, inclusion: { in: [1.day, 3.days, 1.week, 1.month, 3.months, 6.months, 1.year, 100.years], message: "%{value} is not a valid ban duration" }, if: :duration_changed? - validates :reason, presence: true + validates :reason, visible_string: true validate :user, :validate_user_is_bannable, on: :create scope :unexpired, -> { where("bans.created_at + bans.duration > ?", Time.zone.now) } diff --git a/app/models/bulk_update_request.rb b/app/models/bulk_update_request.rb index 61d6cca9d..91f740531 100644 --- a/app/models/bulk_update_request.rb +++ b/app/models/bulk_update_request.rb @@ -10,9 +10,9 @@ class BulkUpdateRequest < ApplicationRecord belongs_to :forum_post, optional: true belongs_to :approver, optional: true, class_name: "User" - validates :reason, presence: true, on: :create - validates :script, presence: true - validates :title, presence: true, if: ->(rec) { rec.forum_topic_id.blank? } + validates :reason, visible_string: true, on: :create + validates :script, visible_string: true + validates :title, visible_string: true, if: ->(rec) { rec.forum_topic_id.blank? } validates :forum_topic, presence: true, if: ->(rec) { rec.forum_topic_id.present? } validates :status, inclusion: { in: STATUSES } validate :validate_script, if: :script_changed? diff --git a/app/models/comment.rb b/app/models/comment.rb index 4eeb6d16e..19fdbaa58 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -13,7 +13,7 @@ class Comment < ApplicationRecord has_many :active_votes, -> { active }, class_name: "CommentVote" has_many :mod_actions, as: :subject, dependent: :destroy - validates :body, presence: true, length: { maximum: 15_000 }, if: :body_changed? + validates :body, visible_string: true, length: { maximum: 15_000 }, if: :body_changed? before_create :autoreport_spam before_save :handle_reports_on_deletion diff --git a/app/models/dmail.rb b/app/models/dmail.rb index 1b6cbf788..2bfde03a7 100644 --- a/app/models/dmail.rb +++ b/app/models/dmail.rb @@ -4,8 +4,8 @@ class Dmail < ApplicationRecord attr_accessor :creator_ip_addr, :disable_email_notifications validate :validate_sender_is_not_limited, on: :create - validates :title, presence: true, length: { maximum: 200 }, if: :title_changed? - validates :body, presence: true, length: { maximum: 50_000 }, if: :body_changed? + validates :title, visible_string: true, length: { maximum: 200 }, if: :title_changed? + validates :body, visible_string: true, length: { maximum: 50_000 }, if: :body_changed? belongs_to :owner, :class_name => "User" belongs_to :to, :class_name => "User" diff --git a/app/models/favorite_group.rb b/app/models/favorite_group.rb index 9d744b102..4e80c5fbe 100644 --- a/app/models/favorite_group.rb +++ b/app/models/favorite_group.rb @@ -5,7 +5,7 @@ class FavoriteGroup < ApplicationRecord before_validation :normalize_name - validates :name, presence: true + validates :name, visible_string: true validates :name, uniqueness: { case_sensitive: false, scope: :creator_id } validate :validate_name, if: :name_changed? validate :creator_can_create_favorite_groups, :on => :create diff --git a/app/models/forum_post.rb b/app/models/forum_post.rb index cb5e6ec72..fbc4652f2 100644 --- a/app/models/forum_post.rb +++ b/app/models/forum_post.rb @@ -16,7 +16,7 @@ class ForumPost < ApplicationRecord has_one :tag_implication has_one :bulk_update_request - validates :body, presence: true, length: { maximum: 200_000 }, if: :body_changed? + validates :body, visible_string: true, length: { maximum: 200_000 }, if: :body_changed? validate :validate_deletion_of_original_post validate :validate_undeletion_of_post diff --git a/app/models/forum_topic.rb b/app/models/forum_topic.rb index fd8af88b2..a53d4e054 100644 --- a/app/models/forum_topic.rb +++ b/app/models/forum_topic.rb @@ -27,7 +27,7 @@ class ForumTopic < ApplicationRecord has_many :tag_implications has_many :mod_actions, as: :subject, dependent: :destroy - validates :title, presence: true, length: { maximum: 200 }, if: :title_changed? + validates :title, visible_string: true, length: { maximum: 200 }, if: :title_changed? validates :category_id, inclusion: { in: CATEGORIES.keys } validates :min_level, inclusion: { in: MIN_LEVELS.values } diff --git a/app/models/ip_ban.rb b/app/models/ip_ban.rb index 8440ea345..60fc911c1 100644 --- a/app/models/ip_ban.rb +++ b/app/models/ip_ban.rb @@ -7,7 +7,7 @@ class IpBan < ApplicationRecord has_many :mod_actions, as: :subject, dependent: :destroy validate :validate_ip_addr - validates :reason, presence: true + validates :reason, visible_string: true after_save :create_mod_action diff --git a/app/models/moderation_report.rb b/app/models/moderation_report.rb index 60e4d4f92..d6e192202 100644 --- a/app/models/moderation_report.rb +++ b/app/models/moderation_report.rb @@ -10,7 +10,7 @@ class ModerationReport < ApplicationRecord has_many :mod_actions, as: :subject, dependent: :destroy before_validation(on: :create) { model.lock! } - validates :reason, presence: true + validates :reason, visible_string: true validates :model_type, inclusion: { in: MODEL_TYPES } validates :creator, uniqueness: { scope: [:model_type, :model_id], message: "have already reported this message." }, on: :create diff --git a/app/models/note.rb b/app/models/note.rb index 3bde8579f..bd0625f85 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -11,7 +11,7 @@ class Note < ApplicationRecord validates :y, presence: true validates :width, presence: true validates :height, presence: true - validates :body, presence: true + validates :body, visible_string: true validate :note_within_image after_save :update_post after_save :create_version diff --git a/app/models/pool.rb b/app/models/pool.rb index 0a66afd2d..6250c08e7 100644 --- a/app/models/pool.rb +++ b/app/models/pool.rb @@ -6,7 +6,7 @@ class Pool < ApplicationRecord array_attribute :post_ids, parse: /\d+/, cast: :to_i - validates :name, uniqueness: { case_sensitive: false }, if: :name_changed? + validates :name, visible_string: true, uniqueness: { case_sensitive: false }, if: :name_changed? validate :validate_name, if: :name_changed? validates :category, inclusion: { in: %w[series collection] } validate :updater_can_edit_deleted diff --git a/app/models/post_appeal.rb b/app/models/post_appeal.rb index de0c5612d..437781f88 100644 --- a/app/models/post_appeal.rb +++ b/app/models/post_appeal.rb @@ -4,7 +4,7 @@ class PostAppeal < ApplicationRecord belongs_to :creator, :class_name => "User" belongs_to :post - validates :reason, length: { maximum: 140 } + validates :reason, visible_string: { allow_empty: true }, length: { maximum: 140 } validate :validate_post_is_appealable, on: :create validate :validate_creator_is_not_limited, on: :create validates :creator, uniqueness: { scope: :post, message: "have already appealed this post" }, on: :create diff --git a/app/models/post_flag.rb b/app/models/post_flag.rb index 10ad3bc2c..011a6687f 100644 --- a/app/models/post_flag.rb +++ b/app/models/post_flag.rb @@ -10,7 +10,7 @@ class PostFlag < ApplicationRecord belongs_to :post before_validation { post.lock! } - validates :reason, presence: true, length: { in: 1..140 } + validates :reason, visible_string: true, length: { in: 1..140 } validate :validate_creator_is_not_limited, on: :create validate :validate_post, on: :create validates :creator_id, uniqueness: { scope: :post_id, on: :create, unless: :is_deletion, message: "have already flagged this post" } diff --git a/app/models/saved_search.rb b/app/models/saved_search.rb index c4d547f64..da50b3ce9 100644 --- a/app/models/saved_search.rb +++ b/app/models/saved_search.rb @@ -11,7 +11,7 @@ class SavedSearch < ApplicationRecord normalize :query, :normalize_query normalize :labels, :normalize_labels - validates :query, presence: true + validates :query, visible_string: true validate :validate_count, on: :create scope :labeled, ->(label) { where_array_includes_any_lower(:labels, [normalize_label(label)]) } diff --git a/app/models/user_feedback.rb b/app/models/user_feedback.rb index 17c3611e6..3930d08ce 100644 --- a/app/models/user_feedback.rb +++ b/app/models/user_feedback.rb @@ -7,7 +7,7 @@ class UserFeedback < ApplicationRecord belongs_to :user belongs_to :creator, class_name: "User" - validates :body, presence: true + validates :body, visible_string: true validates :category, presence: true, inclusion: { in: %w[positive negative neutral] } after_create :create_dmail, unless: :disable_dmail_notification after_update :create_mod_action diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index 240824ccc..d6a92a510 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -14,7 +14,7 @@ class WikiPage < ApplicationRecord array_attribute :other_names # XXX must come after `normalize :other_names` validates :title, tag_name: true, presence: true, uniqueness: true, if: :title_changed? - validates :body, presence: true, unless: -> { is_deleted? || other_names.present? } + validates :body, visible_string: true, unless: -> { is_deleted? || other_names.present? } validate :validate_rename validate :validate_other_names diff --git a/config/initializers/core_extensions.rb b/config/initializers/core_extensions.rb index 1dbf8c990..19272c542 100644 --- a/config/initializers/core_extensions.rb +++ b/config/initializers/core_extensions.rb @@ -5,6 +5,22 @@ require "danbooru" module Danbooru module Extensions module String + # https://invisible-characters.com + # https://character.construction/blanks + # https://www.unicode.org/review/pr-5.html (5.22 Default Ignorable Code Points) + # https://en.wikipedia.org/wiki/Whitespace_character + # + # [[:space:]] = https://codepoints.net/search?gc[]=Z (Space_Separator | Line_Separator | Paragraph_Separator | U+0009 | U+000A | U+000B | U+000C | U+000D | U+0085) + # \p{di} = https://codepoints.net/search?DI=1 (Default_Ignorable_Code_Point) + # \u2800 = https://codepoints.net/U+2800 (BRAILLE PATTERN BLANK) + INVISIBLE_REGEX = /\A[[:space:]\p{di}\u2800]*\z/ + + # Returns true if the string consists entirely of invisible characters. Like `#blank?`, but includes control + # characters and certain other invisible Unicode characters that aren't classified as spaces. + def invisible? + match?(INVISIBLE_REGEX) + end + def to_escaped_for_sql_like string = self.gsub(/%|_|\*|\\\*|\\\\|\\/) do |str| case str diff --git a/test/unit/comment_test.rb b/test/unit/comment_test.rb index 5eb39ad2b..8c2b63c4d 100644 --- a/test/unit/comment_test.rb +++ b/test/unit/comment_test.rb @@ -202,8 +202,11 @@ class CommentTest < ActiveSupport::TestCase end context "during validation" do - subject { FactoryBot.build(:comment) } + subject { build(:comment) } + + should_not allow_value("").for(:body) should_not allow_value(" ").for(:body) + should_not allow_value("\u200B").for(:body) end end end diff --git a/test/unit/dmail_test.rb b/test/unit/dmail_test.rb index 5f25bff0f..6269021df 100644 --- a/test/unit/dmail_test.rb +++ b/test/unit/dmail_test.rb @@ -149,8 +149,12 @@ class DmailTest < ActiveSupport::TestCase context "during validation" do subject { FactoryBot.build(:dmail) } + should_not allow_value("").for(:title) should_not allow_value(" ").for(:title) + should_not allow_value("\u200B").for(:title) + should_not allow_value("").for(:body) should_not allow_value(" ").for(:body) + should_not allow_value("\u200B").for(:body) should_not allow_value(nil).for(:to) should_not allow_value(nil).for(:from) should_not allow_value(nil).for(:owner) diff --git a/test/unit/favorite_group_test.rb b/test/unit/favorite_group_test.rb index a5f9e8d89..26699157f 100644 --- a/test/unit/favorite_group_test.rb +++ b/test/unit/favorite_group_test.rb @@ -68,5 +68,8 @@ class FavoriteGroupTest < ActiveSupport::TestCase should_not allow_value("_").for(:name) should_not allow_value("any").for(:name) should_not allow_value("none").for(:name) + should_not allow_value("").for(:name) + should_not allow_value(" ").for(:name) + should_not allow_value("\u200B").for(:name) end end diff --git a/test/unit/forum_post_test.rb b/test/unit/forum_post_test.rb index b29b646a4..e082d0441 100644 --- a/test/unit/forum_post_test.rb +++ b/test/unit/forum_post_test.rb @@ -165,5 +165,13 @@ class ForumPostTest < ActiveSupport::TestCase assert_equal(@second_user.id, @post.updater_id) end end + + context "during validation" do + subject { build(:forum_post) } + + should_not allow_value("").for(:body) + should_not allow_value(" ").for(:body) + should_not allow_value("\u200B").for(:body) + end end end diff --git a/test/unit/forum_topic_test.rb b/test/unit/forum_topic_test.rb index 207fa4f75..3d3bd0597 100644 --- a/test/unit/forum_topic_test.rb +++ b/test/unit/forum_topic_test.rb @@ -82,5 +82,13 @@ class ForumTopicTest < ActiveSupport::TestCase end end end + + context "during validation" do + subject { build(:forum_topic) } + + should_not allow_value("").for(:title) + should_not allow_value(" ").for(:title) + should_not allow_value("\u200B").for(:title) + end end end diff --git a/test/unit/note_test.rb b/test/unit/note_test.rb index 6129156f1..194a5a79c 100644 --- a/test/unit/note_test.rb +++ b/test/unit/note_test.rb @@ -145,5 +145,11 @@ class NoteTest < ActiveSupport::TestCase end end end + + context "when validating notes" do + should_not allow_value("").for(:body) + should_not allow_value(" ").for(:body) + should_not allow_value("\u200B").for(:body) + end end end diff --git a/test/unit/pool_test.rb b/test/unit/pool_test.rb index 8e2ba5f32..bd5d29cc8 100644 --- a/test/unit/pool_test.rb +++ b/test/unit/pool_test.rb @@ -244,9 +244,17 @@ class PoolTest < ActiveSupport::TestCase end context "when validating names" do - ["foo,bar", "foo*bar", "123", "___", " ", "any", "none", "series", "collection"].each do |bad_name| - should_not allow_value(bad_name).for(:name) - end + should_not allow_value("foo,bar").for(:name) + should_not allow_value("foo*bar").for(:name) + should_not allow_value("123").for(:name) + should_not allow_value("any").for(:name) + should_not allow_value("none").for(:name) + should_not allow_value("series").for(:name) + should_not allow_value("collection").for(:name) + should_not allow_value("___").for(:name) + should_not allow_value(" ").for(:name) + should_not allow_value("\u200B").for(:name) + should_not allow_value("").for(:name) end end diff --git a/test/unit/post_appeal_test.rb b/test/unit/post_appeal_test.rb index e69f97ab8..1b52d3d36 100644 --- a/test/unit/post_appeal_test.rb +++ b/test/unit/post_appeal_test.rb @@ -52,6 +52,15 @@ class PostAppealTest < ActiveSupport::TestCase end end end + + context "validation" do + subject { build(:post_appeal) } + + should allow_value("").for(:reason) + + should_not allow_value(" ").for(:reason) + should_not allow_value("\u200B").for(:reason) + end end end end diff --git a/test/unit/post_flag_test.rb b/test/unit/post_flag_test.rb index dde365a72..98db81a6d 100644 --- a/test/unit/post_flag_test.rb +++ b/test/unit/post_flag_test.rb @@ -114,5 +114,13 @@ class PostFlagTest < ActiveSupport::TestCase end end end + + context "during validation" do + subject { build(:post_flag) } + + should_not allow_value("").for(:reason) + should_not allow_value(" ").for(:reason) + should_not allow_value("\u200B").for(:reason) + end end end diff --git a/test/unit/string_test.rb b/test/unit/string_test.rb index 1691d4ec2..c178c8633 100644 --- a/test/unit/string_test.rb +++ b/test/unit/string_test.rb @@ -41,4 +41,26 @@ class StringTest < ActiveSupport::TestCase assert_equal("foo\r\nbar", "foo\u2029bar".normalize_whitespace) end end + + context "String#invisible?" do + should "work" do + assert_equal(true, "".invisible?) + assert_equal(true, " ".invisible?) + assert_equal(true, "\v\t\f\r\n".invisible?) + assert_equal(true, "\u00A0\u00AD\u034F\u061C\u115F\u1160\u17B4\u17B5\u180B\u180C\u180D\u180E".invisible?) + assert_equal(true, "\u200B\u200C\u200D\u200E\u200F\u2028\u2029\u2060\u206F\u2800\u3000\u3164".invisible?) + assert_equal(true, "\uFE00\uFE0F\uFEFF\uFFA0".invisible?) + assert_equal(true, "\u{E0001}\u{E007F}".invisible?) + + assert_equal(false, "foo".invisible?) + assert_equal(false, "\u0000".invisible?) # https://codepoints.net/U+0000 (NULL) + assert_equal(false, "\u0001".invisible?) # https://codepoints.net/U+0001 (START OF HEADING) + assert_equal(false, "\uE000".invisible?) # https://codepoints.net/U+E000 (PRIVATE USE CHARACTER) + assert_equal(false, "\uFDD0".invisible?) # https://codepoints.net/U+FDD0 (NONCHARACTER) + assert_equal(false, "\uFFF9".invisible?) # https://codepoints.net/U+FFF9 (INTERLINEAR ANNOTATION ANCHOR) + assert_equal(false, "\uFFFF".invisible?) # https://codepoints.net/U+FFFF (NONCHARACTER) + assert_equal(false, "\u{1D159}".invisible?) # https://codepoints.net/U+1D159 (MUSICAL SYMBOL NULL NOTEHEAD) + assert_equal(false, "\u{1D455}".invisible?) # https://codepoints.net/U+1D455 () + end + end end diff --git a/test/unit/user_feedback_test.rb b/test/unit/user_feedback_test.rb index 5f20faa66..034af6e7c 100644 --- a/test/unit/user_feedback_test.rb +++ b/test/unit/user_feedback_test.rb @@ -17,5 +17,11 @@ class UserFeedbackTest < ActiveSupport::TestCase assert_equal(dmail, user.dmails.last.body) end end + + context "on validation" do + should_not allow_value("").for(:body) + should_not allow_value(" ").for(:body) + should_not allow_value("\u200B").for(:body) + end end end diff --git a/test/unit/wiki_page_test.rb b/test/unit/wiki_page_test.rb index 4a754b886..ce2c3a8d4 100644 --- a/test/unit/wiki_page_test.rb +++ b/test/unit/wiki_page_test.rb @@ -114,6 +114,8 @@ class WikiPageTest < ActiveSupport::TestCase should normalize_attribute(:title).from(" Foo___ Bar ").to("foo_bar") should_not allow_value("").for(:title).on(:create) + should_not allow_value(" ").for(:title).on(:create) + should_not allow_value("\u200B").for(:title).on(:create) should_not allow_value("___").for(:title).on(:create) should_not allow_value("-foo").for(:title).on(:create) should_not allow_value("/foo").for(:title).on(:create) @@ -126,6 +128,12 @@ class WikiPageTest < ActiveSupport::TestCase should_not allow_value("X"*171).for(:title).on(:create) end + context "during body validation" do + should_not allow_value("").for(:body) + should_not allow_value(" ").for(:body) + should_not allow_value("\u200B").for(:body) + end + context "with other names" do should "not allow artist wikis to have other names" do tag = create(:artist_tag)