Download as
Browse Code »

Commit 1607e1bb3805eaca9bc44349202b0d6ec6010fe3

Authored by Antonio Terceiro
Committed by Joenio Costa
1 parent 520af0a8
Exists in staging and in 42 other branches all_pending_tasks_api, api-articles-period, api_roles, caching-rails4, captcha_serpro_plugin, comments_permissions, content-manager-hostspot, elasticsearch, elasticsearch_api, elasticsearch_categories, elasticsearch_filter, elasticsearch_sort, elasticsearch_to_merge, elasticsearch_view, environment-exposes-api, export-comment-api, export-comment-paragraph, export_data, external_followers, federation-webfinger, federation_followers, federation_followers_backend, federation_oauth_provider, federation_webfinger, fix_event_date_issue, fix_notification_email, fix_string_downcase_and_upcase, follower_permition, json_cookie_serializer, login-captcha, master, master_profile_followers, oauth_external_login, oauth_login, private-scraps, private-scraps-rebase, production, production-vendorized, profile_api_improvements, tasks_keep_filter_params, user_mention, webfinger_server

Fix TextileArticle 

  * extract textile to HTML conversion in its own method, which sanitizes
    the generated HTML before returning it.
  * use textile for both the body and the lead.
  * add tests to make sure the textile markup is not being broken.
Showing 2 changed files with 61 additions and 1 deletions   Show diff stats
app/models/textile_article.rb
  Diff comments   View file @1607e1b
... ... @@ -9,11 +9,26 @@ class TextileArticle < TextArticle
9 9 end
10 10  
11 11 def to_html(options ={})
12   - RedCloth.new(self.body|| '').to_html
  12 + convert_to_html(body)
  13 + end
  14 +
  15 + def lead
  16 + if abstract.blank?
  17 + super
  18 + else
  19 + convert_to_html(abstract)
  20 + end
13 21 end
14 22  
15 23 def notifiable?
16 24 true
17 25 end
18 26  
  27 + protected
  28 +
  29 + def convert_to_html(textile)
  30 + @@sanitizer ||= HTML::WhiteListSanitizer.new
  31 + @@sanitizer.sanitize(RedCloth.new(textile|| '').to_html)
  32 + end
  33 +
19 34 end
... ...
test/unit/textile_article_test.rb
  Diff comments   View file @1607e1b
... ... @@ -145,4 +145,49 @@ class TextileArticleTest < Test::Unit::TestCase
145 145 assert_equal false, a.advertise?
146 146 assert_equal false, a.is_trackable?
147 147 end
  148 +
  149 + should 'generate proper HTML for links' do
  150 + assert_tag_in_string build_article('"Noosfero":http://noosfero.org/').to_html, :tag => 'a', :attributes => { :href => 'http://noosfero.org/' }
  151 + end
  152 +
  153 + should 'generate proper HTML for > symbols' do
  154 + assert_match /^sqlite>$/, build_article(' sqlite>').to_html
  155 + end
  156 +
  157 + should 'not mess up with textile markup' do
  158 + assert_equal ' sqlite> stuff', build_article(' sqlite> stuff').body
  159 + noosfero_cool = '"Noosfero":http://noosfero.org/ is a very cool project'
  160 + assert_equal noosfero_cool, build_article(noosfero_cool).body
  161 + end
  162 +
  163 + should 'not allow arbitrary HTML' do
  164 + assert_not_equal '<script>alert(1)</script>', build_article('<script>alert(1)</script>').to_html
  165 + end
  166 +
  167 + should 'not allow Javascript on links' do
  168 + assert_no_tag_in_string build_article('<a href="javascript: alert(\'BOOM\')" onclick="javascript: alert(\'BOOM\')"></a>').to_html, :tag => 'a', :attributes => { :href => /./, :onclick => /./ }
  169 + end
  170 +
  171 + should 'allow harmless HTML' do
  172 + code = "<pre><code> code example\n</code></pre>"
  173 + assert_equal code, build_article(code).body
  174 + assert_equal code, build_article(code).to_html
  175 + end
  176 +
  177 + should 'use Textile markup for lead as well' do
  178 + assert_tag_in_string build_article(nil, :abstract => '"Noosfero":http://noosfero.org/').lead, :tag => 'a', :attributes => { :href => 'http://noosfero.org/' }
  179 + end
  180 +
  181 + should 'not allow arbitrary HTML in the lead' do
  182 + assert_not_equal '<script>alert(1)</script>', build_article(nil, :abstract => '<script>alert(1)</script>').lead
  183 + end
  184 +
  185 + protected
  186 +
  187 + def build_article(input = nil, options = {})
  188 + article = TextileArticle.new({:body => input}.merge(options))
  189 + article.valid? # trigger the xss terminate thingy
  190 + article
  191 + end
  192 +
148 193 end
... ...