From 05c220619fc2e36f3731512f281d09611ad42619 Mon Sep 17 00:00:00 2001 From: rhymes Date: Fri, 16 Apr 2021 20:20:11 +0200 Subject: [PATCH] [15 min fix] Enforce an upper limit for articles.body_markdown (#13418) * Enforce a length limit for articles.body_markdown * Fix spec assertion * Add bytesize validation to body_markdown --- app/models/article.rb | 4 +++- app/validators/bytesize_validator.rb | 35 ++++++++++++++++++++++++++++ spec/models/article_spec.rb | 19 +++++++++++++-- 3 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 app/validators/bytesize_validator.rb diff --git a/app/models/article.rb b/app/models/article.rb index c156e90bc..565772369 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -54,7 +54,9 @@ class Article < ApplicationRecord inverse_of: :commentable, class_name: "Comment" - validates :body_markdown, length: { minimum: 0, allow_nil: false }, uniqueness: { scope: %i[user_id title] } + validates :body_markdown, bytesize: { maximum: 800.kilobytes, too_long: "is too long." } + validates :body_markdown, length: { minimum: 0, allow_nil: false } + validates :body_markdown, uniqueness: { scope: %i[user_id title] } validates :boost_states, presence: true validates :cached_tag_list, length: { maximum: 126 } validates :canonical_url, uniqueness: { allow_nil: true } diff --git a/app/validators/bytesize_validator.rb b/app/validators/bytesize_validator.rb new file mode 100644 index 000000000..533ac83ff --- /dev/null +++ b/app/validators/bytesize_validator.rb @@ -0,0 +1,35 @@ +# Adapted from https://github.com/rails/rails/issues/19570#issuecomment-348366536 +# and https://github.com/rails/rails/blob/v6.1.3.1/activemodel/lib/active_model/validations/length.rb + +# Currently only supports `:maximum` +class BytesizeValidator < ActiveModel::EachValidator + MESSAGES = { maximum: :too_long }.freeze + CHECKS = { maximum: :<= }.freeze + RESERVED_OPTIONS = %i[maximum too_long].freeze + + ERROR_MISSING_OPTIONS_MESSAGE = "Specify the :maximum option.".freeze + ERROR_NON_NEGATIVE_MESSAGE = ":maximum must be a non-negative Integer".freeze + + def check_validity! + raise ArgumentError, ERROR_MESSAGE unless options.key?(:maximum) + + maximum = options[:maximum] + raise ArgumentError, ERROR_NON_NEGATIVE_MESSAGE unless maximum.is_a?(Integer) && maximum >= 0 + end + + def validate_each(record, attribute, value) + key = :maximum + value_bytesize = value.respond_to?(:bytesize) ? value.bytesize : value.to_s.bytesize + errors_options = options.except(*RESERVED_OPTIONS) + + check_value = options[key] + return if value_bytesize.public_send(CHECKS[key], check_value) + + errors_options[:count] = check_value + + default_message = options[MESSAGES[key]] + errors_options[:message] ||= default_message if default_message + + record.errors.add(attribute, MESSAGES[key], **errors_options) + end +end diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index 9f0d4e301..95f04a804 100644 --- a/spec/models/article_spec.rb +++ b/spec/models/article_spec.rb @@ -32,6 +32,7 @@ RSpec.describe Article, type: :model do it { is_expected.to have_many(:tags) } it { is_expected.to have_many(:user_subscriptions).dependent(:nullify) } + it { is_expected.to validate_length_of(:body_markdown).is_at_least(0) } it { is_expected.to validate_length_of(:cached_tag_list).is_at_most(126) } it { is_expected.to validate_length_of(:title).is_at_most(128) } @@ -81,11 +82,25 @@ RSpec.describe Article, type: :model do end describe "#body_markdown" do - it "is unique scoped for user_id and title" do + it "is unique scoped for user_id and title", :aggregate_failures do art2 = build(:article, body_markdown: article.body_markdown, user: article.user, title: article.title) expect(art2).not_to be_valid - expect(art2.errors.full_messages.to_sentence).to match("markdown has already been taken") + expect(art2.errors_as_sentence).to match("markdown has already been taken") + end + + # using https://unicode-table.com/en/11A15/ multibyte char + it "is valid if its bytesize is less than 800 kilobytes" do + article.body_markdown = "𑨕" * 204_800 # 4 bytes x 204800 = 800 kilobytes + + expect(article).to be_valid + end + + it "is not valid if its bytesize exceeds 800 kilobytes" do + article.body_markdown = "𑨕" * 204_801 + + expect(article).not_to be_valid + expect(article.errors_as_sentence).to match("too long") end end