From 6af69f7e776939da7b4726561d9609671f91c9c4 Mon Sep 17 00:00:00 2001 From: Jeremy Banks Date: Fri, 15 Feb 2013 20:58:33 -0500 Subject: [PATCH 1/2] Do not strip leading and trailing whitespace from raw posts. --- app/models/post.rb | 8 ++----- app/models/stripped_length_validator.rb | 14 +++++++++++ lib/text_sentinel.rb | 9 +++---- spec/components/text_sentinel_spec.rb | 32 +++++++++++++++++++------ 4 files changed, 46 insertions(+), 17 deletions(-) create mode 100644 app/models/stripped_length_validator.rb diff --git a/app/models/post.rb b/app/models/post.rb index 520d2ce743d..566348e36ba 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -31,7 +31,7 @@ class Post < ActiveRecord::Base has_many :post_actions validates_presence_of :raw, :user_id, :topic_id - validates :raw, length: {in: SiteSetting.min_post_length..SiteSetting.max_post_length} + validates :raw, stripped_length: {in: SiteSetting.min_post_length..SiteSetting.max_post_length} validate :raw_quality validate :max_mention_validator validate :max_images_validator @@ -59,10 +59,6 @@ class Post < ActiveRecord::Base TopicUser.auto_track(self.user_id, self.topic_id, TopicUser::NotificationReasons::CREATED_POST) end - before_validation do - self.raw.strip! if self.raw.present? - end - def raw_quality sentinel = TextSentinel.new(self.raw, min_entropy: SiteSetting.body_min_entropy) @@ -214,7 +210,7 @@ class Post < ActiveRecord::Base # We only filter quotes when there is exactly 1 return cooked unless (quote_count == 1) - parent_raw = parent_post.raw.sub(/\[quote.+\/quote\]/m, '').strip + parent_raw = parent_post.raw.sub(/\[quote.+\/quote\]/m, '') if raw[parent_raw] or (parent_raw.size < SHORT_POST_CHARS) return cooked.sub(/\/m, '') diff --git a/app/models/stripped_length_validator.rb b/app/models/stripped_length_validator.rb new file mode 100644 index 00000000000..2342eab71c7 --- /dev/null +++ b/app/models/stripped_length_validator.rb @@ -0,0 +1,14 @@ +class StrippedLengthValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + unless value.nil? + stripped_length = value.strip.length + range = options[:in] + record.errors.add attribute, (options[:message] || "is too short (minimum is #{range.begin}).") unless + stripped_length >= range.begin + record.errors.add attribute, (options[:message] || "is too long (maximum is #{range.end}).") unless + stripped_length <= range.end + else + record.errors.add attribute, (options[:message] || "is required.") + end + end +end diff --git a/lib/text_sentinel.rb b/lib/text_sentinel.rb index 61515929ad5..295e4054b7a 100644 --- a/lib/text_sentinel.rb +++ b/lib/text_sentinel.rb @@ -20,8 +20,8 @@ class TextSentinel @opts = opts || {} if @text.present? - @text.strip! @text.gsub!(/ +/m, ' ') if @opts[:remove_interior_spaces] + @text.strip! if @opts[:strip] end end @@ -29,19 +29,20 @@ class TextSentinel TextSentinel.new(text, min_entropy: SiteSetting.title_min_entropy, max_word_length: SiteSetting.max_word_length, - remove_interior_spaces: true) + remove_interior_spaces: true, + strip: true) end # Entropy is a number of how many unique characters the string needs. def entropy return 0 if @text.blank? - @entropy ||= @text.each_char.to_a.uniq.size + @entropy ||= @text.strip.each_char.to_a.uniq.size end def valid? # Blank strings are not valid - return false if @text.blank? + return false if @text.blank? || @text.strip.blank? # Entropy check if required return false if @opts[:min_entropy].present? and (entropy < @opts[:min_entropy]) diff --git a/spec/components/text_sentinel_spec.rb b/spec/components/text_sentinel_spec.rb index 2b50aef8384..946b3ca7fd7 100644 --- a/spec/components/text_sentinel_spec.rb +++ b/spec/components/text_sentinel_spec.rb @@ -38,10 +38,6 @@ describe TextSentinel do context "cleaning up" do - it "strips leading or trailing whitespace" do - TextSentinel.new(" \t test \t ").text.should == "test" - end - it "allows utf-8 chars" do TextSentinel.new("йȝîûηыეமிᚉ⠛").text.should == "йȝîûηыეமிᚉ⠛" end @@ -49,15 +45,37 @@ describe TextSentinel do context "interior spaces" do let(:spacey_string) { "hello there's weird spaces here." } + let(:unspacey_string) { "hello there's weird spaces here." } it "ignores intra spaces by default" do TextSentinel.new(spacey_string).text.should == spacey_string end it "fixes intra spaces when enabled" do - TextSentinel.new(spacey_string, remove_interior_spaces: true).text.should == "hello there's weird spaces here." - end + TextSentinel.new(spacey_string, remove_interior_spaces: true).text.should == unspacey_string + end + it "fixes intra spaces in titles" do + TextSentinel.title_sentinel(spacey_string).text.should == unspacey_string + end + + end + + context "stripping whitespace" do + let(:spacey_string) { " \t test \t " } + let(:unspacey_string) { "test" } + + it "does not strip leading and trailing whitespace by default" do + TextSentinel.new(spacey_string).text.should == spacey_string + end + + it "strips leading and trailing whitespace when enabled" do + TextSentinel.new(spacey_string, strip: true).text.should == unspacey_string + end + + it "strips leading and trailing whitespace in titles" do + TextSentinel.title_sentinel(spacey_string).text.should == unspacey_string + end end end @@ -100,4 +118,4 @@ describe TextSentinel do end -end \ No newline at end of file +end From d42782b31868687fbf7e1caec8fe3dbbdb9bd493 Mon Sep 17 00:00:00 2001 From: Jeremy Banks Date: Mon, 18 Feb 2013 21:57:46 -0500 Subject: [PATCH 2/2] Using LengthValidator's localized messages for StrippedLengthValidator. --- app/models/stripped_length_validator.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/stripped_length_validator.rb b/app/models/stripped_length_validator.rb index 2342eab71c7..b84ff0b0090 100644 --- a/app/models/stripped_length_validator.rb +++ b/app/models/stripped_length_validator.rb @@ -3,12 +3,12 @@ class StrippedLengthValidator < ActiveModel::EachValidator unless value.nil? stripped_length = value.strip.length range = options[:in] - record.errors.add attribute, (options[:message] || "is too short (minimum is #{range.begin}).") unless + record.errors.add attribute, (options[:message] || I18n.t('errors.messages.too_short', count: range.begin)) unless stripped_length >= range.begin - record.errors.add attribute, (options[:message] || "is too long (maximum is #{range.end}).") unless + record.errors.add attribute, (options[:message] || I18n.t('errors.messages.too_long', count: range.end)) unless stripped_length <= range.end else - record.errors.add attribute, (options[:message] || "is required.") + record.errors.add attribute, (options[:message] || I18n.t('errors.messages.blank')) end end end