Merge pull request #2277 from vikhyat/fix-title-html

Fix HTML tags in topic titles

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>";
 });
 

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?

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