Merge pull request #1200 from hennevogel/feature_real_markdown_comments

[webui] Sanitize HTML in comments

src/api/Gemfile

@@ -33,6 +33,8 @@ gem 'acts_as_list'
 gem 'xmlhash', '>=1.3.6'
 # to escape HTML (FIXME: do we still use this?)
 gem 'escape_utils'
+# to sanitize HTML/CSS
+gem 'sanitize'
 # as authorization system
 gem "pundit"
 #

src/api/Gemfile.lock

@@ -76,6 +76,7 @@ GEM
       thor (~> 0.19.1)
     crack (0.4.2)
       safe_yaml (~> 1.0.0)
+    crass (1.0.2)
     cssmin (1.0.3)
     daemons (1.2.2)
     dalli (2.7.4)
@@ -144,6 +145,8 @@ GEM
     netrc (0.10.3)
     nokogiri (1.6.6.2)
       mini_portile (~> 0.6.0)
+    nokogumbo (1.4.1)
+      nokogiri
     parser (2.2.2.6)
       ast (>= 1.1, < 3.0)
     pkg-config (1.1.6)
@@ -195,7 +198,7 @@ GEM
     raindrops (0.13.0)
     rake (10.4.2)
     rdoc (4.2.0)
-    redcarpet (3.3.2)
+    redcarpet (3.3.3)
     responders (2.1.0)
       railties (>= 4.2.0, < 5)
     rest-client (1.8.0)
@@ -214,6 +217,10 @@ GEM
     ruby_parser (3.7.0)
       sexp_processor (~> 4.1)
     safe_yaml (1.0.4)
+    sanitize (4.0.0)
+      crass (~> 1.0.2)
+      nokogiri (>= 1.4.4)
+      nokogumbo (= 1.4.1)
     sass (3.4.14)
     sass-rails (5.0.3)
       railties (>= 4.0.0, < 5.0)
@@ -331,6 +338,7 @@ DEPENDENCIES
   responders (~> 2.0)
   rubocop
   ruby-ldap
+  sanitize
   sass-rails (~> 5.0.1)
   simplecov
   single_test

src/api/app/helpers/comment_helper.rb

@@ -3,7 +3,11 @@
 module CommentHelper
   def comment_body(comment)
     # Initializes a Markdown parser, if needed
-    @md_parser ||= Redcarpet::Markdown.new(OBSApi::MarkdownRenderer, autolink: true)
+    @md_parser ||= Redcarpet::Markdown.new(OBSApi::MarkdownRenderer.new(no_styles: true),
+                                           autolink: true,
+                                           no_intra_emphasis: true,
+                                           fenced_code_blocks: true, disable_indented_code_blocks: true
+                                          )
     @md_parser.render(comment.to_s).html_safe
   end
 end

src/api/app/models/issue_tracker.rb

@@ -54,6 +54,10 @@ def get_html(text)
     text.gsub(Regexp.new(regex)) { show_url_for($1, true) }
   end
 
+  def get_markdown(text)
+    text.gsub(Regexp.new(regex)) { "[#{$&}](#{show_url_for($1, false)})" }
+  end
+
   def update_issues_bugzilla
     return unless self.enable_fetch
 

src/api/lib/obsapi/markdown_renderer.rb

@@ -7,19 +7,20 @@ def self.default_url_options
     end
 
     def preprocess(fulldoc)
-      # OBS requests
-      out = fulldoc.gsub(/(sr|req|request)#(\d+)/i) {|s| "<a href=\"#{request_show_url(id: $2)}\">#{s}</a>" }
-      # issues
+      # request#12345 links
+      fulldoc.gsub!(/(sr|req|request)#(\d+)/i) {|s| "[#{s}](#{request_show_url(id: Regexp.last_match(2))})" }
+      # @user links
+      fulldoc.gsub!(/([^\w]|^)@([-\w]+)([^\w]|$)/) \
+                   {"#{Regexp.last_match(1)}[@#{Regexp.last_match(2)}](#{user_show_url(Regexp.last_match(2))})#{Regexp.last_match(3)}" }
+      # bnc#12345 links
       IssueTracker.all.each do |t|
-        out = t.get_html(out)
+        fulldoc = t.get_markdown(fulldoc)
       end
-      # users
-      out.gsub!(/([^\w]|^)@([-\w]+)([^\w]|$)/) do
-        # We need to save $1,$2 and $3 since we are calling gsub again inside the block
-        s1, s2, s3 = $1, $2, $3
-        "#{s1}<a href=\"#{user_show_url(s2)}\">@#{s2.gsub('_', '\_')}</a>#{s3}"
-      end
-      out
+      # sanitize the HTML we get
+      Sanitize.fragment(fulldoc, Sanitize::Config.merge(Sanitize::Config::RESTRICTED,
+                                                        elements: Sanitize::Config::RESTRICTED[:elements] + ['pre'],
+                                                        remove_contents: true
+                                                       ))
     end
   end
 end

src/api/test/functional/webui/package_controller_test.rb

@@ -101,9 +101,9 @@ def test_succesful_comment_creation
       page.must_have_xpath '//a[contains(@href, "/request/show/23") and text()="sr#23"]'
       page.must_have_xpath '//a[@href="http://bugzilla.clutter-project.org/show_bug.cgi?id=24" and text()="bco#24"]'
       page.must_have_xpath '//a[@href="https://features.opensuse.org/25" and text()="fate#25"]'
-      page.must_have_xpath '//a[contains(@href, "user/show/_nobody_") and text()="@_nobody_"]'
-      page.must_have_xpath '//a[contains(@href, "user/show/a-dashed-user") and text()="@a-dashed-user"]'
-      page.must_have_xpath '//a[contains(@href, "user/show/Iggy") and text()="@Iggy"]'
+      page.must_have_link '@nobody'
+      page.must_have_link '@a-dashed-user'
+      page.must_have_link '@Iggy'
       page.must_have_xpath '//a[@href="http://link.com"]'
       page.must_have_xpath '//a[@href="https://anotherlink.com"]'
     end
@@ -118,7 +118,7 @@ def test_another_succesful_comment_creation
     within('div.thread_level_0') do
       page.must_have_xpath '//a[contains(@href, "/request/show/23") and text()="request#23"]'
       page.must_have_xpath '//a[contains(@href, "/request/show/23") and text()="Request#23"]'
-      page.must_have_xpath '//a[contains(@href, "user/show/Iggy") and text()="@Iggy"]'
+      page.must_have_link '@Iggy'
     end
   end