Permalink
Browse files

Adding a no/do follow policy on the whole blog. Admin can now chose b…

…etween nofollow and dofollow in the comments and comments author URL.

For now this is a bit "raw" and may need some improvement:
1- allow admin to chose after how many comments a visitor is dofollowed, for example 3, but this would have an impact on performances: each time a comment is rendered, Typo would do a select count(*) based on the author email address.
2- improve comment moderation to have an email sent even though the comment was not published, or any other way to moderate comments. I need to think about it without breaking / recoding our state machine.

Oh, and this indeed come with a bunch of tests that shows I haven't broken anything.
  • Loading branch information...
fdv committed Dec 23, 2010
1 parent af5181c commit a156ddd9d47e3159850f3ab016ab51d4945dc056
View
@@ -63,6 +63,7 @@ class Blog < ActiveRecord::Base
setting :index_tags, :boolean, true
setting :admin_display_elements, :integer, 10
setting :google_verification, :string, ''
+ setting :nofollowify, :boolean, true
validate :permalink_has_identifier
@@ -75,6 +75,14 @@
<%= text_area(:setting, :robots, :rows => 10, :class => 'medium')%>
</div>
</div>
+ <div class='setting clear'>
+ <label class="float" for="setting_nofollowify"><%= _("Use nofollow in comments")%></label>
+ <div class='input_text'>
+ <%= check_box(:setting, :nofollowify)%>
+ <span><%= _("Maybe you want to moderate feedbacks when turning this off") %></span>
+ </div>
+ </div>
+
</div>
<% unless robot_writable? -%>
@@ -1,7 +1,7 @@
<div class="comment" id="comment-<%= @comment.id %>">
<div class="author">
<%= content_tag(:div, gravatar_tag(@comment.email)) if this_blog.use_gravatar and @comment.email %>
- <cite><%= (@comment.url.blank?) ? h(@comment.author) : link_to(h(@comment.author), @comment.url) %></cite>
+ <cite><%= (@comment.url.blank?) ? h(@comment.author) : link_to(h(@comment.author), @comment.url).nofollowify %></cite>
<abbr title="<%= @comment.created_at.xmlschema %>"><%= distance_of_time_in_words @comment.article.published_at, @comment.created_at %> later:</abbr>
</div>
<div class="content">
View
@@ -10,7 +10,8 @@ def to_url
# A quick and dirty fix to add 'nofollow' to any urls in a string.
# Decidedly unsafe, but will have to do for now.
def nofollowify
- self.gsub(/<a(.*?)>/i, '<a\1 rel="nofollow">')
+ return self.gsub(/<a(.*?)>/i, '<a\1 rel="nofollow">') if Blog.default.nofollowify
+ self
end
# Strips any html markup from a string
@@ -116,3 +116,121 @@ def comment_options
{ :body => %{foo@example.com} }
end
end
+
+shared_examples_for "CommentSanitizationWithDofollow" do
+ before do
+ @article = mock_model(Article, :created_at => Time.now, :published_at => Time.now)
+ Article.stub!(:find).and_return(@article)
+ this_blog.use_gravatar = false
+ this_blog.lang = 'en_US'
+ this_blog.nofollowify = false
+
+ prepare_comment
+
+ @comment.stub!(:id).and_return(1)
+ assign(:comment, @comment)
+ end
+
+ def prepare_comment
+ Comment.with_options(:body => 'test foo <script>do_evil();</script>',
+ :author => 'Bob', :article => @article,
+ :created_at => Time.now) do |klass|
+ @comment = klass.new(comment_options)
+ end
+ end
+
+ ['', 'markdown', 'textile', 'smartypants', 'markdown smartypants'].each do |value|
+ it "Should sanitize content rendered with the #{value} textfilter" do
+ this_blog.comment_text_filter = value
+
+ render :file => 'comments/show'
+ rendered.should have_selector('.content')
+ rendered.should have_selector('.author')
+
+ rendered.should_not have_selector('.content script')
+ rendered.should_not have_selector(".content a[rel=nofollow]")
+ # No links with javascript
+ rendered.should_not have_selector(".content a[onclick]")
+ rendered.should_not have_selector(".content a[href^=\"javascript:\"]")
+
+ rendered.should_not have_selector('.author script')
+ rendered.should_not have_selector(".author a[rel=nofollow]")
+ # No links with javascript
+ rendered.should_not have_selector(".author a[onclick]")
+ rendered.should_not have_selector(".author a[href^=\"javascript:\"]")
+ end
+ end
+end
+
+describe "First dodgy comment with dofollow" do
+ it_should_behave_like "CommentSanitizationWithDofollow"
+
+ def comment_options
+ { :body => 'test foo <script>do_evil();</script>' }
+ end
+end
+
+describe "Second dodgy comment with dofollow" do
+ it_should_behave_like "CommentSanitizationWithDofollow"
+
+ def comment_options
+ { :body => 'link to [spammy goodness](http://spammer.example.com)'}
+ end
+end
+
+describe "Dodgy comment #3 with dofollow" do
+ it_should_behave_like "CommentSanitizationWithDofollow"
+
+ def comment_options
+ { :body => 'link to <a href="spammer.com">spammy goodness</a>'}
+ end
+end
+
+describe "Extra Dodgy comment with dofollow" do
+ it_should_behave_like "CommentSanitizationWithDofollow"
+
+ def comment_options
+ { :body => '<a href="http://spam.org">spam</a>',
+ :author => '<a href="http://spamme.com>spamme</a>',
+ :email => '<a href="http://itsallspam.com/">its all spam</a>' }
+ end
+end
+
+describe "XSS1 with dofollow" do
+ it_should_behave_like "CommentSanitizationWithDofollow"
+
+ def comment_options
+ { :body => %{Have you ever <script lang="javascript">alert("foo");</script> been hacked?} }
+ end
+end
+
+describe "XSS2 with dofollow" do
+ it_should_behave_like "CommentSanitizationWithDofollow"
+ def comment_options
+ { :body => %{<a href="#" onclick="javascript">bad link</a>}}
+ end
+end
+
+describe "XSS2 with dofollow" do
+ it_should_behave_like "CommentSanitizationWithDofollow"
+
+ def comment_options
+ { :body => %{<a href="javascript:bad">bad link</a>}}
+ end
+end
+
+describe "Comment with bare http URL with dofollow" do
+ it_should_behave_like "CommentSanitizationWithDofollow"
+
+ def comment_options
+ { :body => %{http://www.example.com} }
+ end
+end
+
+describe "Comment with bare email address with dofollow" do
+ it_should_behave_like "CommentSanitizationWithDofollow"
+
+ def comment_options
+ { :body => %{foo@example.com} }
+ end
+end
View
@@ -22,4 +22,5 @@ default:
"sp_article_auto_close" => 300,
"link_to_author" => false,
"comment_text_filter" => "markdown",
+ "nofollowify" => true,
"permalink_format" => "/%year%/%month%/%day%/%title%"}.to_yaml.inspect %>
@@ -1,7 +1,7 @@
<li class="comment" id="comment-<%= comment.id %>">
<div class="author">
<%= content_tag(:div, gravatar_tag(comment.email)) if this_blog.use_gravatar and comment.email %>
- <strong><cite><%= (comment.url.blank?) ? h(comment.author) : link_to(h(comment.author), comment.url) %></cite></strong><br />
+ <strong><cite><%= (comment.url.blank?) ? h(comment.author) : link_to(h(comment.author), comment.url).nofollowify %></cite></strong><br />
<abbr title="<%= comment.created_at.xmlschema %>"><%= distance_of_time_in_words comment.article.published_at, comment.created_at %> later:</abbr>
</div>
<div class="content">
@@ -1,7 +1,7 @@
<li class="comment" id="comment-<%= comment.id %>">
<div class="author">
<%= content_tag(:div, gravatar_tag(comment.email)) if this_blog.use_gravatar and comment.email %>
- <cite><%= (comment.url.blank?) ? h(comment.author) : link_to(h(comment.author), comment.url) %></cite>
+ <cite><%= (comment.url.blank?) ? h(comment.author) : link_to(h(comment.author), comment.url).nofollowify %></cite>
<abbr title="<%= comment.created_at.xmlschema %>"><%= distance_of_time_in_words comment.article.published_at, comment.created_at %> <%= _("later")%>:</abbr>
</div>
<div class="content">
@@ -1,7 +1,7 @@
<li class="comment <%= 'fdv' if comment.user_id? %>" id="comment-<%= comment.id %>">
<%= gravatar_tag(comment.email) if this_blog.use_gravatar and comment.email %>
<h4>
- <%= _("By") %> <%= (comment.url.blank?) ? h(comment.author) : link_to(h(comment.author), comment.url, :rel => "nofollow") %>
+ <%= _("By") %> <%= (comment.url.blank?) ? h(comment.author) : link_to(h(comment.author), comment.url).nofollowify %>
<%= distance_of_time_in_words comment.article.published_at, comment.created_at %> <%= _("later:")%>
</h4>
<br class='clear' />
@@ -1,7 +1,7 @@
<li class="comment" id="comment-<%= comment.id %>">
<div class="author">
<%= content_tag(:div, gravatar_tag(comment.email)) if this_blog.use_gravatar and comment.email %>
- <cite><%= (comment.url.blank?) ? h(comment.author) : link_to(h(comment.author), comment.url) %></cite>
+ <cite><%= (comment.url.blank?) ? h(comment.author) : link_to(h(comment.author), comment.url).nofollowify %></cite>
<abbr title="<%= comment.created_at.xmlschema %>"><%= distance_of_time_in_words comment.article.published_at, comment.created_at %> <%= _("later")%>:</abbr>
</div>
<div class="commentContent">

0 comments on commit a156ddd

Please sign in to comment.