From 25a080a8e172cca0d66f9ed0cbecf6575590a6bc Mon Sep 17 00:00:00 2001
From: Vikhyat Korrapati <vikhyatk@gmail.com>
Date: Fri, 18 Apr 2014 10:18:38 +0530
Subject: [PATCH] Fix HTML tags in topic titles.

We no longer sanitize titles before saving to the database since it
would cause problems like HTML entities showing up when you try to edit
a topic title. It isn't even really necessary since we only render
fancy_title directly and never title.

The escaping logic used here is the same that is used both in lodash and
onebox. See:

1. https://github.com/discourse/onebox/pull/190/files
2. https://github.com/lodash/lodash/blob/2.4.1/dist/lodash.compat.js#L6194
---
 .../discourse/helpers/application_helpers.js    |  2 +-
 app/models/topic.rb                             | 17 ++++++++++-------
 spec/models/topic_spec.rb                       | 10 +++++-----
 3 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/app/assets/javascripts/discourse/helpers/application_helpers.js b/app/assets/javascripts/discourse/helpers/application_helpers.js
index aaa2cada8e9..328472f5ffe 100644
--- a/app/assets/javascripts/discourse/helpers/application_helpers.js
+++ b/app/assets/javascripts/discourse/helpers/application_helpers.js
@@ -61,7 +61,7 @@ Handlebars.registerHelper('shorten', function(property, options) {
 **/
 Handlebars.registerHelper('topicLink', function(property, options) {
   var topic = Ember.Handlebars.get(this, property, options),
-      title = topic.get('fancy_title') || topic.get('title');
+      title = topic.get('fancy_title');
   return "<a href='" + topic.get('lastUnreadUrl') + "' class='title'>" + title + "</a>";
 });
 
diff --git a/app/models/topic.rb b/app/models/topic.rb
index e145e41da13..9515b947ee0 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -65,7 +65,6 @@ class Topic < ActiveRecord::Base
 
 
   before_validation do
-    self.sanitize_title
     self.title = TextCleaner.clean_title(TextSentinel.title_sentinel(title).text) if errors[:title].empty?
   end
 
@@ -242,17 +241,21 @@ class Topic < ActiveRecord::Base
   end
 
   def fancy_title
-    return title unless SiteSetting.title_fancy_entities?
+    sanitized_title = title.gsub(/['&\"<>]/, {
+      "'" => '&#39;',
+      '&' => '&amp;',
+      '"' => '&quot;',
+      '<' => '&lt;',
+      '>' => '&gt;',
+    })
+
+    return sanitized_title unless SiteSetting.title_fancy_entities?
 
     # We don't always have to require this, if fancy is disabled
     # see: http://meta.discourse.org/t/pattern-for-defer-loading-gems-and-profiling-with-perftools-rb/4629
     require 'redcarpet' unless defined? Redcarpet
 
-    Redcarpet::Render::SmartyPants.render(title)
-  end
-
-  def sanitize_title
-    self.title = sanitize(title.to_s, tags: [], attributes: []).strip.presence
+    Redcarpet::Render::SmartyPants.render(sanitized_title)
   end
 
   def new_version_required?
diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb
index d2426eac8ea..39032c7796b 100644
--- a/spec/models/topic_spec.rb
+++ b/spec/models/topic_spec.rb
@@ -121,15 +121,15 @@ describe Topic do
     let(:topic_script) { build_topic_with_title("Topic with <script>alert('title')</script> script in its title" ) }
 
     it "escapes script contents" do
-      topic_script.title.should == "Topic with script in its title"
+      topic_script.fancy_title.should == "Topic with &lt;script&gt;alert(&lsquo;title&rsquo;)&lt;/script&gt; script in its title"
     end
 
     it "escapes bold contents" do
-      topic_bold.title.should == "Topic with bold text in its title"
+      topic_bold.fancy_title.should == "Topic with &lt;b&gt;bold&lt;/b&gt; text in its title"
     end
 
     it "escapes image contents" do
-      topic_image.title.should == "Topic with image in its title"
+      topic_image.fancy_title.should == "Topic with &lt;img src=&lsquo;something&rsquo;&gt; image in its title"
     end
 
   end
@@ -142,8 +142,8 @@ describe Topic do
         SiteSetting.stubs(:title_fancy_entities).returns(false)
       end
 
-      it "doesn't change the title to add entities" do
-        topic.fancy_title.should == topic.title
+      it "doesn't add entities to the title" do
+        topic.fancy_title.should == "&quot;this topic&quot; -- has ``fancy stuff&#39;&#39;"
       end
     end