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
This commit is contained in:
Vikhyat Korrapati 2014-04-18 10:18:38 +05:30
parent b3ed8b6a32
commit 25a080a8e1
3 changed files with 16 additions and 13 deletions
app
assets/javascripts/discourse/helpers
models
spec/models

View file

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

View file

@ -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?

View file

@ -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