sanitize comment attributes when they enter the db, not when they're …

…displayed

git-svn-id: http://svn.techno-weenie.net/projects/mephisto/trunk@2822 567b1171-46fb-0310-a4c9-b4bef9110e78

app/drops/comment_drop.rb

@@ -20,7 +20,7 @@ def url
   end
 
   def author_link
-    @source.author_url.blank? ? "<span>#{CGI::escapeHTML(@source.author)}</span>" : %Q{<a href="#{CGI::escapeHTML author_url}">#{CGI::escapeHTML @source.author}</a>}
+    @source.author_url.blank? ? "<span>#{@source.author}</span>" : %Q{<a href="#{author_url}">#{@source.author}</a>}
   end
 
   def presentation_class

app/models/comment.rb

@@ -7,11 +7,12 @@ class Comment < Content
   before_validation :clean_up_author_url
   after_validation_on_create  :snag_article_attributes
   before_create  :check_comment_expiration
+  before_create  :sanitize_attributes
   before_save    :update_counter_cache
   before_destroy :decrement_counter_cache
   belongs_to :article
   has_one :event, :dependent => :destroy
-  attr_protected :approved
+  attr_accessible :article, :article_id, :user_id, :user, :excerpt, :body, :author, :author_url, :author_email, :author_ip, :updater_id, :updater, :comment_age, :user_agent, :referrer
 
   def self.find_all_by_section(section, options = {})
     find :all, options.update(:conditions => ['contents.approved = ? and assigned_sections.section_id = ?', true, section.id], 
@@ -67,9 +68,15 @@ def mark_as_ham(site, request)
   end
 
   protected
+    def sanitize_attributes
+      [:author, :author_url, :author_email, :author_ip, :user_agent, :referrer].each do |a|
+        self.send("#{a}=", CGI::escapeHTML(self.send(a).to_s))
+      end
+    end
+
     def snag_article_attributes
       self.filter ||= article.site.filter
-      self.attributes = { :site => article.site, :title => article.title, :published_at => article.published_at, :permalink => article.permalink }
+      [:site, :title, :published_at, :permalink].each { |a| self.send("#{a}=", article.send(a)) }
     end
 
     def check_comment_expiration

db/migrate/071_filter_current_comments.rb

@@ -0,0 +1,14 @@
+class FilterCurrentComments < ActiveRecord::Migration
+  def self.up
+    transaction do
+      Comment.find(:all).each do |c|
+        Comment.update_all ['author = ?, author_url = ?, author_email = ?, author_ip = ?, user_agent = ?, referrer = ?', 
+          CGI::escapeHTML(c.author), CGI::escapeHTML(c.author_url), CGI::escapeHTML(c.author_email), CGI::escapeHTML(c.author_ip),
+          CGI::escapeHTML(c.user_agent), CGI::escapeHTML(referrer)], ['id = ?', c.id]
+      end
+    end
+  end
+
+  def self.down
+  end
+end

db/schema.rb

@@ -2,7 +2,7 @@
 # migrations feature of ActiveRecord to incrementally modify your database, and
 # then regenerate this schema definition.
 
-ActiveRecord::Schema.define(:version => 70) do
+ActiveRecord::Schema.define(:version => 71) do
 
   create_table "assets", :force => true do |t|
     t.column "content_type",     :string

test/unit/comment_drop_test.rb

@@ -36,6 +36,7 @@ def test_should_return_correct_author_link
     assert_equal %Q{<a href="https://abc">rico</a>}, @comment.author_link
     @comment.source.author     = '<strong>rico</strong>'
     @comment.source.author_url = '<strong>https://abc</strong>'
+    @comment.source.send(:sanitize_attributes)
     assert_equal %Q{<a href="http://&lt;strong&gt;https://abc&lt;/strong&gt;">&lt;strong&gt;rico&lt;/strong&gt;</a>}, @comment.author_link
   end
 

test/unit/comment_test.rb

@@ -29,7 +29,7 @@ def test_should_pass_filter_down_from_article_site
   def test_should_allow_set_filter_on_comment
     old_times = contents(:welcome).comments.collect &:updated_at
     comment = contents(:welcome).comments.create :body => 'test comment', :author => 'bob', :author_ip => '127.0.0.1', :filter => 'markdown_filter'
-    assert_equal 'markdown_filter', comment.filter
+    comment.filter = 'markdown_filter'
     assert_valid comment
     assert_equal old_times, contents(:welcome).comments(true).collect(&:updated_at)
   end