Skip to content

Commit

Permalink
Validate lengths of string attributes
Browse files Browse the repository at this point in the history
When using the MySQL database, Rails automatically sets a limit of 255
characters for string columns. This changes ensures this limit is also
enforced in the validations. For several columns with a different limit,
that different limit is enforced instead.

For the PostgreSQL database, there is no limit in the database, but it
is useful to set a limit anyway to prevent absurdly large values from
being submitted.
  • Loading branch information
mvz committed Aug 14, 2022
1 parent 68599ec commit ca46da2
Show file tree
Hide file tree
Showing 20 changed files with 177 additions and 2 deletions.
1 change: 1 addition & 0 deletions publify_core/Manifest.txt
Expand Up @@ -160,6 +160,7 @@ app/models/article.rb
app/models/article/factory.rb
app/models/blog.rb
app/models/comment.rb
app/models/concerns/string_length_limit.rb
app/models/config_manager.rb
app/models/content.rb
app/models/content_base.rb
Expand Down
3 changes: 3 additions & 0 deletions publify_core/app/models/blog.rb
Expand Up @@ -9,6 +9,8 @@
#
class Blog < ApplicationRecord
include ConfigManager
include StringLengthLimit

include Rails.application.routes.url_helpers

has_many :contents
Expand Down Expand Up @@ -139,6 +141,7 @@ class Blog < ApplicationRecord

validate :permalink_has_identifier
# validates :base_url, presence: true
validates_default_string_length :base_url

# Find the Blog that matches a specific base URL. If no Blog object is found
# that matches, then grab the first blog. If *that* fails, then create a new
Expand Down
17 changes: 17 additions & 0 deletions publify_core/app/models/concerns/string_length_limit.rb
@@ -0,0 +1,17 @@
# frozen_string_literal: true

module StringLengthLimit
# Default string length limit for model attributes. When running on MySQL,
# this is equal to the default string length in the database as set by Rails.
STRING_LIMIT = 255

extend ActiveSupport::Concern

class_methods do
def validates_default_string_length(*names)
names.each do |name|
validates name, length: { maximum: STRING_LIMIT }
end
end
end
end
4 changes: 4 additions & 0 deletions publify_core/app/models/content.rb
Expand Up @@ -5,6 +5,7 @@

class Content < ApplicationRecord
include ContentBase
include StringLengthLimit

belongs_to :user, optional: true, touch: true
belongs_to :blog
Expand Down Expand Up @@ -38,6 +39,9 @@ class Content < ApplicationRecord

serialize :whiteboard

validates_default_string_length :title, :author, :permalink, :name,
:post_type, :text_filter_name

def author=(user)
if user.respond_to?(:login)
self[:author] = user.login
Expand Down
6 changes: 6 additions & 0 deletions publify_core/app/models/feedback.rb
Expand Up @@ -10,10 +10,16 @@ class Feedback < ApplicationRecord

include PublifyGuid
include ContentBase
include StringLengthLimit

validate :feedback_allowed, on: :create
validates :article, presence: true

validates_default_string_length :title, :author, :email, :url, :blog_name,
:user_agent, :text_filter_name

validates :ip, length: { maximum: 40 }

before_save :correct_url, :classify_content
before_create :create_guid

Expand Down
3 changes: 3 additions & 0 deletions publify_core/app/models/ping.rb
@@ -1,5 +1,8 @@
# frozen_string_literal: true

class Ping < ApplicationRecord
include StringLengthLimit

belongs_to :article
validates_default_string_length :url
end
4 changes: 4 additions & 0 deletions publify_core/app/models/post_type.rb
@@ -1,9 +1,13 @@
# frozen_string_literal: true

class PostType < ApplicationRecord
include StringLengthLimit

validates :name, uniqueness: true
validates :name, presence: true
validate :name_is_not_read
validates_default_string_length :name, :permalink, :description

before_save :sanitize_title

def name_is_not_read
Expand Down
4 changes: 4 additions & 0 deletions publify_core/app/models/redirect.rb
@@ -1,13 +1,17 @@
# frozen_string_literal: true

class Redirect < ApplicationRecord
include StringLengthLimit

belongs_to :content, optional: true, touch: true
belongs_to :blog

validates :from_path, uniqueness: true
validates :to_path, presence: true
validates :blog, presence: true

validates_default_string_length :from_path, :to_path

def full_to_path
path = to_path
# FIXME: Unify HTTP URI matchers
Expand Down
3 changes: 3 additions & 0 deletions publify_core/app/models/resource.rb
Expand Up @@ -4,9 +4,12 @@
require "carrierwave/orm/activerecord"

class Resource < ApplicationRecord
include StringLengthLimit
belongs_to :blog
belongs_to :content, optional: true

mount_uploader :upload, ResourceUploader
validates :upload, presence: true

validates_default_string_length :mime
end
3 changes: 3 additions & 0 deletions publify_core/app/models/tag.rb
@@ -1,12 +1,15 @@
# frozen_string_literal: true

class Tag < ApplicationRecord
include StringLengthLimit

belongs_to :blog
has_and_belongs_to_many :contents, order: "created_at DESC"

validates :name, uniqueness: { scope: :blog_id }
validates :blog, presence: true
validates :name, presence: true
validates_default_string_length :display_name

before_validation :ensure_naming_conventions

Expand Down
2 changes: 2 additions & 0 deletions publify_core/app/models/user.rb
Expand Up @@ -14,12 +14,14 @@ class User < ApplicationRecord
devise :database_authenticatable, :registerable,
:recoverable, :rememberable, :trackable, :validatable
include ConfigManager
include StringLengthLimit

before_validation :set_default_profile

validates :login, uniqueness: true
validates :email, :login, presence: true
validates :login, length: { in: 3..40 }
validates_default_string_length :email, :text_filter_name

belongs_to :resource, optional: true
has_many :notifications, foreign_key: "notify_user_id"
Expand Down
4 changes: 4 additions & 0 deletions publify_core/spec/models/blog_spec.rb
Expand Up @@ -113,6 +113,10 @@
describe "validations" do
let(:blog) { described_class.new }

it "requires base url to not be too long" do
expect(blog).to validate_length_of(:base_url).is_at_most(255)
end

it "requires blog name to not be too long" do
expect(blog).to validate_length_of(:blog_name).is_at_most(256)
end
Expand Down
28 changes: 28 additions & 0 deletions publify_core/spec/models/content_spec.rb
Expand Up @@ -144,4 +144,32 @@
it { expect(content.author_name).to eq(author.login) }
end
end

describe "validations" do
let(:content) { described_class.new }

it "requires title to not be too long" do
expect(content).to validate_length_of(:title).is_at_most(255)
end

it "requires author to not be too long" do
expect(content).to validate_length_of(:author).is_at_most(255)
end

it "requires permalink to not be too long" do
expect(content).to validate_length_of(:permalink).is_at_most(255)
end

it "requires name to not be too long" do
expect(content).to validate_length_of(:name).is_at_most(255)
end

it "requires post_type to not be too long" do
expect(content).to validate_length_of(:post_type).is_at_most(255)
end

it "requires text_filter_name to not be too long" do
expect(content).to validate_length_of(:text_filter_name).is_at_most(255)
end
end
end
36 changes: 36 additions & 0 deletions publify_core/spec/models/feedback_spec.rb
Expand Up @@ -178,4 +178,40 @@ def classify
assert !@comment.published?
end
end

describe "validations" do
let(:feedback) { described_class.new }

it "requires title to not be too long" do
expect(feedback).to validate_length_of(:title).is_at_most(255)
end

it "requires author to not be too long" do
expect(feedback).to validate_length_of(:author).is_at_most(255)
end

it "requires email to not be too long" do
expect(feedback).to validate_length_of(:email).is_at_most(255)
end

it "requires url to not be too long" do
expect(feedback).to validate_length_of(:url).is_at_most(255)
end

it "requires ip to not be too long" do
expect(feedback).to validate_length_of(:ip).is_at_most(40)
end

it "requires blog_name to not be too long" do
expect(feedback).to validate_length_of(:blog_name).is_at_most(255)
end

it "requires user_agent to not be too long" do
expect(feedback).to validate_length_of(:user_agent).is_at_most(255)
end

it "requires text_filter_name to not be too long" do
expect(feedback).to validate_length_of(:text_filter_name).is_at_most(255)
end
end
end
7 changes: 7 additions & 0 deletions publify_core/spec/models/ping_spec.rb
Expand Up @@ -3,4 +3,11 @@
require "rails_helper"

describe Ping, type: :model do
describe "validations" do
let(:ping) { described_class.new }

it "requires url to not be too long" do
expect(ping).to validate_length_of(:url).is_at_most(255)
end
end
end
16 changes: 16 additions & 0 deletions publify_core/spec/models/post_type_spec.rb
Expand Up @@ -29,4 +29,20 @@
expect(test_type).not_to be_valid
expect(test_type.errors[:name]).to eq(["has already been taken"])
end

describe "validations" do
let(:post_type) { described_class.new }

it "requires name to not be too long" do
expect(post_type).to validate_length_of(:name).is_at_most(255)
end

it "requires permalink to not be too long" do
expect(post_type).to validate_length_of(:permalink).is_at_most(255)
end

it "requires description to not be too long" do
expect(post_type).to validate_length_of(:description).is_at_most(255)
end
end
end
12 changes: 12 additions & 0 deletions publify_core/spec/models/redirect_spec.rb
Expand Up @@ -20,4 +20,16 @@
expect(redirect.from_url).to eq "#{blog.base_url}/right/here"
end
end

describe "validations" do
let(:redirect) { described_class.new }

it "requires from_path to not be too long" do
expect(redirect).to validate_length_of(:from_path).is_at_most(255)
end

it "requires to_path to not be too long" do
expect(redirect).to validate_length_of(:to_path).is_at_most(255)
end
end
end
8 changes: 8 additions & 0 deletions publify_core/spec/models/resource_spec.rb
Expand Up @@ -33,4 +33,12 @@
expect(img_resource.upload_url(:thumb)).to eq "/files/resource/1/thumb_testfile.png"
end
end

describe "validations" do
let(:resource) { described_class.new }

it "requires mime to not be too long" do
expect(resource).to validate_length_of(:mime).is_at_most(255)
end
end
end
10 changes: 8 additions & 2 deletions publify_core/spec/models/tag_spec.rb
Expand Up @@ -12,9 +12,15 @@
expect(test_tag.errors[:name]).to eq(["has already been taken"])
end

describe "validation" do
describe "validations" do
let(:tag) { described_class.new }

it "requires name to be present" do
expect(blog.tags.build(name: "")).not_to be_valid
expect(tag).to validate_presence_of(:name)
end

it "requires display_name to not be too long" do
expect(tag).to validate_length_of(:display_name).is_at_most(255)
end
end

Expand Down
8 changes: 8 additions & 0 deletions publify_core/spec/models/user_spec.rb
Expand Up @@ -45,6 +45,10 @@
describe "validations" do
let(:user) { described_class.new }

it "requires email to not be too long" do
expect(user).to validate_length_of(:email).is_at_most(255)
end

it "requires first name to not be too long" do
expect(user).to validate_length_of(:firstname).is_at_most(256)
end
Expand Down Expand Up @@ -73,6 +77,10 @@
expect(user).to validate_presence_of(:login)
end

it "requires text_filter_name to not be too long" do
expect(user).to validate_length_of(:text_filter_name).is_at_most(255)
end

it "does not allow duplicate logins when updating a user" do
create :user, login: "foo"
bar = create :user, login: "bar"
Expand Down

0 comments on commit ca46da2

Please sign in to comment.