Fix #5365: Don't allow whitespace-only text submission.
Fix bug where it was possible to submit blank text in various text fields. Caused by `String#blank?` not considering certain Unicode characters as blank. `blank?` is defined as `match?(/\A[[:space:]]*\z/)`, where `[[:space:]]` matches ASCII spaces (space, tab, newline, etc) and Unicode characters in the Space category ([1]). However, there are other space-like characters not in the Space category. This includes U+200B (Zero-Width Space), and many more. It turns out the "Default ignorable code points" [2][3] are what we're after. These are the set of 400 or so formatting and control characters that are invisible when displayed. Note that there are other control characters that aren't invisible when rendered, instead they're shown with a placeholder glyph. These include the ASCII C0 and C1 control codes [4], certain Unicode control characters [5], and unassigned, reserved, and private use codepoints. There is one outlier: the Braille pattern blank (U+2800) [6]. This character is visually blank, but is not considered to be a space or an ignorable code point. [1]: https://codepoints.net/search?gc[]=Z [2]: https://codepoints.net/search?DI=1 [3]: https://www.unicode.org/review/pr-5.html [4]: https://codepoints.net/search?gc[]=Cc [5]: https://codepoints.net/search?gc[]=Cf [6]: https://codepoints.net/U+2800 [7]: https://en.wikipedia.org/wiki/Whitespace_character [8]: https://character.construction/blanks [9]: https://invisible-characters.com
This commit is contained in:
20
app/logical/visible_string_validator.rb
Normal file
20
app/logical/visible_string_validator.rb
Normal file
@@ -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
|
||||
@@ -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) }
|
||||
|
||||
@@ -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?
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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 }
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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" }
|
||||
|
||||
@@ -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)]) }
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 (<reserved>)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user