chore: Add max length validation to text fields (#7073)

Introduces a default max length validation for all string and text fields to prevent processing large payloads.
This commit is contained in:
Sojan Jose
2023-05-12 22:12:21 +05:30
committed by GitHub
parent 198cd9b28d
commit 385eab6b96
6 changed files with 89 additions and 6 deletions

View File

@@ -34,7 +34,7 @@ class Account < ApplicationRecord
validates :name, presence: true
validates :auto_resolve_duration, numericality: { greater_than_or_equal_to: 1, less_than_or_equal_to: 999, allow_nil: true }
validates :name, length: { maximum: 255 }
validates :domain, length: { maximum: 100 }
has_many :account_users, dependent: :destroy_async
has_many :agent_bot_inboxes, dependent: :destroy_async

View File

@@ -2,6 +2,8 @@ class ApplicationRecord < ActiveRecord::Base
include Events::Types
self.abstract_class = true
before_validation :validates_column_content_length
# the models that exposed in email templates through liquid
DROPPABLES = %w[Account Channel Conversation Inbox User Message].freeze
@@ -14,6 +16,31 @@ class ApplicationRecord < ActiveRecord::Base
private
# Generic validation for all columns of type string and text
# Validates the length of the column to prevent DOS via large payloads
# if a custom length validation is already present, skip the validation
def validates_column_content_length
self.class.columns.each do |column|
check_and_validate_content_length(column) if column_of_type_string_or_text?(column)
end
end
def column_of_type_string_or_text?(column)
%i[string text].include?(column.type)
end
def check_and_validate_content_length(column)
length_validator = self.class.validators_on(column.name).find { |v| v.kind == :length }
validate_content_length(column) if length_validator.blank?
end
def validate_content_length(column)
max_length = column.type == :text ? 20_000 : 255
return if self[column.name].nil? || self[column.name].length <= max_length
errors.add(column.name.to_sym, "is too long (maximum is #{max_length} characters)")
end
def normalize_empty_string_to_nil(attrs = [])
attrs.each do |attr|
self[attr] = nil if self[attr].blank?

View File

@@ -36,7 +36,6 @@ class Contact < ApplicationRecord
validates :phone_number,
allow_blank: true, uniqueness: { scope: [:account_id] },
format: { with: /\+[1-9]\d{1,14}\z/, message: I18n.t('errors.contacts.phone_number.invalid') }
validates :name, length: { maximum: 255 }
belongs_to :account
has_many :conversations, dependent: :destroy_async

View File

@@ -23,6 +23,23 @@ RSpec.describe Account do
it { is_expected.to have_many(:categories).dependent(:destroy_async) }
it { is_expected.to have_many(:teams).dependent(:destroy_async) }
# This validation happens in ApplicationRecord
describe 'length validations' do
let(:account) { create(:account) }
it 'validates name length' do
account.name = 'a' * 256
account.valid?
expect(account.errors[:name]).to include('is too long (maximum is 255 characters)')
end
it 'validates domain length' do
account.domain = 'a' * 150
account.valid?
expect(account.errors[:domain]).to include('is too long (maximum is 100 characters)')
end
end
describe 'usage_limits' do
let(:account) { create(:account) }

View File

@@ -1,6 +1,11 @@
require 'rails_helper'
RSpec.describe Article, type: :model do
let!(:account) { create(:account) }
let(:user) { create(:user, account_ids: [account.id], role: :agent) }
let!(:portal_1) { create(:portal, account_id: account.id, config: { allowed_locales: %w[en es] }) }
let!(:category_1) { create(:category, slug: 'category_1', locale: 'en', portal_id: portal_1.id) }
context 'with validations' do
it { is_expected.to validate_presence_of(:account_id) }
it { is_expected.to validate_presence_of(:author_id) }
@@ -13,12 +18,30 @@ RSpec.describe Article, type: :model do
it { is_expected.to belong_to(:author) }
end
# This validation happens in ApplicationRecord
describe 'length validations' do
let(:article) do
create(:article, category_id: category_1.id, content: 'This is the content', description: 'this is the description',
slug: 'this-is-title', title: 'this is title',
portal_id: portal_1.id, author_id: user.id)
end
context 'when it validates content length' do
it 'valid when within limit' do
article.content = 'a' * 1000
expect(article.valid?).to be true
end
it 'invalid when crossed the limit' do
article.content = 'a' * 25_001
article.valid?
expect(article.errors[:content]).to include('is too long (maximum is 20000 characters)')
end
end
end
describe 'search' do
let!(:account) { create(:account) }
let(:user) { create(:user, account_ids: [account.id], role: :agent) }
let!(:portal_1) { create(:portal, account_id: account.id, config: { allowed_locales: %w[en es] }) }
let!(:portal_2) { create(:portal, account_id: account.id, config: { allowed_locales: %w[en es] }) }
let!(:category_1) { create(:category, slug: 'category_1', locale: 'en', portal_id: portal_1.id) }
let!(:category_2) { create(:category, slug: 'category_2', locale: 'es', portal_id: portal_1.id) }
let!(:category_3) { create(:category, slug: 'category_3', locale: 'es', portal_id: portal_2.id) }

View File

@@ -10,6 +10,23 @@ RSpec.describe Message, type: :model do
it { is_expected.to validate_presence_of(:account_id) }
end
describe 'length validations' do
let(:message) { create(:message) }
context 'when it validates name length' do
it 'valid when within limit' do
message.content = 'a' * 120_000
expect(message.valid?).to be true
end
it 'invalid when crossed the limit' do
message.content = 'a' * 150_001
message.valid?
expect(message.errors[:content]).to include('is too long (maximum is 150000 characters)')
end
end
end
describe 'concerns' do
it_behaves_like 'liqudable'
end