Take a look at this code:
class Comment < ApplicationRecord validates_with Validators::CommentContentValidator end module Validators class CommentContentValidator def validate(comment) @comment = comment if contains_bad_language? @comment.errors.add(:comment, :bad_language) end if contains_unwanted_urls? @comment.errors.add(:comment, :bad_url) end end def contains_bad_language? # Imagine this takes a few seconds BadWords.find_each do |bad_word| break true if bad_word.has_a_bard_word?(@comment.message) end end def contains_bad_urls? @comment.message.match?(/.../) end end end A first glance it looks harmless right? And you might not see any issues for a long time.
But Rails actually initializes your Validators::CommentContentValidator at the time it loads the Comment model and the result is your instance variable @comment is not thread safe in your controllers.
If you get many requests saving comments at the same time you'll end up mixing up the @comment between requests.
E.g. imagine these two requests happens in the same split second:
request_a: Comment.create!(...) # contains a bad url request_b: Comment.create!(...) # is valid request_a: Calls Validators::CommentContentValidator#validate(comment) request_b: Calls Validators::CommentContentValidator#validate(comment) Request A starts out validating bad words, but before it's done request B's validation has started and now @comment in Validators::CommentContentValidator is overwritten by request B's Comment.
When request A gets to contains_bad_urls? it is now using the comment of request B and thereby making request A valid.
The solution
If you are using custom validators, then make sure to pass the variable along in all method calls:
module Validators class CommentContentValidator def validate(comment) if contains_bad_language?(comment) comment.errors.add(:comment, :bad_language) end if contains_unwanted_urls?(comment) comment.errors.add(:comment, :bad_url) end end def contains_bad_language?(comment) # Imagine this takes a few seconds BadWords.find_each do |bad_word| break true if bad_word.has_a_bard_word?(comment.message) end end def contains_bad_urls?(comment) comment.message.match?(/.../) end end end 
Top comments (0)