Fix parsing of ref-like Urls in links and images in GFM

Fixes #2166

Fix parsing of ref-like Urls in links and images in GFM
Fixes #2166
Riyad Preukschas
1 parent 16b54178
Showing 3 changed files with 36 additions and 7 deletions Show diff stats
lib/gitlab/markdown.rb
lib/redcarpet/render/gitlab_html.rb
spec/helpers/gitlab_markdown_helper_spec.rb
@@ -45,12 +45,11 @@ module Gitlab
       # Extract pre blocks so they are not altered
       # from http://github.github.com/github-flavored-markdown/
-      extractions = {}
-      text.gsub!(%r{<pre>.*?</pre>|<code>.*?</code>}m) do |match|
-        md5 = Digest::MD5.hexdigest(match)
-        extractions[md5] = match
-        "{gfm-extraction-#{md5}}"
-      end
+      text.gsub!(%r{<pre>.*?</pre>|<code>.*?</code>}m) { |match| extract_piece(match) }
+      # Extract links with probably parsable hrefs
+      text.gsub!(%r{<a.*?>.*?</a>}m) { |match| extract_piece(match) }
+      # Extract images with probably parsable src
+      text.gsub!(%r{<img.*?>}m) { |match| extract_piece(match) }
       # TODO: add popups with additional information
@@ -58,7 +57,7 @@ module Gitlab
       # Insert pre block extractions
       text.gsub!(/\{gfm-extraction-(\h{32})\}/) do
-        extractions[$1]
+        insert_piece($1)
       end
       sanitize text.html_safe, attributes: ActionView::Base.sanitized_allowed_attributes + %w(id class)
@@ -66,6 +65,18 @@ module Gitlab
     private
+    def extract_piece(text)
+      @extractions ||= {}
+
+      md5 = Digest::MD5.hexdigest(text)
+      @extractions[md5] = text
+      "{gfm-extraction-#{md5}}"
+    end
+
+    def insert_piece(id)
+      @extractions[id]
+    end
+
     # Private: Parses text for references and emoji
     #
     # text - Text to parse
@@ -27,6 +27,10 @@ class Redcarpet::Render::GitlabHTML &lt; Redcarpet::Render::HTML
     HTML
   end
+  def link(link, title, content)
+    h.link_to_gfm(content, link, title: title)
+  end
+
   def postprocess(full_document)
     h.gfm(full_document)
   end
 require "spec_helper"
 describe GitlabMarkdownHelper do
+  include ApplicationHelper
+
   let!(:project) { create(:project) }
   let(:user)          { create(:user, username: 'gfm') }
@@ -340,6 +342,18 @@ describe GitlabMarkdownHelper do
       markdown("\nDon't use `$#{snippet.id}` here.\n").should == "<p>Don&#39;t use <code>$#{snippet.id}</code> here.</p>\n"
     end
+    it "should leave ref-like autolinks untouched" do
+      markdown("look at http://example.tld/#!#{merge_request.id}").should == "<p>look at <a href=\"http://example.tld/#!#{merge_request.id}\">http://example.tld/#!#{merge_request.id}</a></p>\n"
+    end
+
+    it "should leave ref-like href of 'manual' links untouched" do
+      markdown("why not [inspect !#{merge_request.id}](http://example.tld/#!#{merge_request.id})").should == "<p>why not <a href=\"http://example.tld/#!#{merge_request.id}\">inspect </a><a href=\"http://test.host/project60/merge_requests/#{merge_request.id}\" class=\"gfm gfm-merge_request \" title=\"Merge Request: #{merge_request.title}\">!#{merge_request.id}</a><a href=\"http://example.tld/#!#{merge_request.id}\"></a></p>\n"
+    end
+
+    it "should leave ref-like src of images untouched" do
+      markdown("screen shot: ![some image](http://example.tld/#!#{merge_request.id})").should == "<p>screen shot: <img src=\"http://example.tld/#!#{merge_request.id}\" alt=\"some image\"></p>\n"
+    end
+
     it "should generate absolute urls for refs" do
       markdown("##{issue.id}").should include(project_issue_url(project, issue))
     end
1	require "spec_helper"	1	require "spec_helper"
2		2
3	describe GitlabMarkdownHelper do	3	describe GitlabMarkdownHelper do
		4	+ include ApplicationHelper
		5	+
4	let!(:project) { create(:project) }	6	let!(:project) { create(:project) }
5		7
6	let(:user) { create(:user, username: 'gfm') }	8	let(:user) { create(:user, username: 'gfm') }
	@@ -340,6 +342,18 @@ describe GitlabMarkdownHelper do		@@ -340,6 +342,18 @@ describe GitlabMarkdownHelper do
340	markdown("\nDon't use `$#{snippet.id}` here.\n").should == "<p>Don't use <code>$#{snippet.id}</code> here.</p>\n"	342	markdown("\nDon't use `$#{snippet.id}` here.\n").should == "<p>Don't use <code>$#{snippet.id}</code> here.</p>\n"
341	end	343	end
342		344
		345	+ it "should leave ref-like autolinks untouched" do
		346	+ markdown("look at http://example.tld/#!#{merge_request.id}").should == "<p>look at <a href=\"http://example.tld/#!#{merge_request.id}\">http://example.tld/#!#{merge_request.id}</a></p>\n"
		347	+ end
		348	+
		349	+ it "should leave ref-like href of 'manual' links untouched" do
		350	+ markdown("why not [inspect !#{merge_request.id}](http://example.tld/#!#{merge_request.id})").should == "<p>why not <a href=\"http://example.tld/#!#{merge_request.id}\">inspect </a><a href=\"http://test.host/project60/merge_requests/#{merge_request.id}\" class=\"gfm gfm-merge_request \" title=\"Merge Request: #{merge_request.title}\">!#{merge_request.id}</a><a href=\"http://example.tld/#!#{merge_request.id}\"></a></p>\n"
		351	+ end
		352	+
		353	+ it "should leave ref-like src of images untouched" do
		354	+ markdown("screen shot: ![some image](http://example.tld/#!#{merge_request.id})").should == "<p>screen shot: <img src=\"http://example.tld/#!#{merge_request.id}\" alt=\"some image\"></p>\n"
		355	+ end
		356	+
343	it "should generate absolute urls for refs" do	357	it "should generate absolute urls for refs" do
344	markdown("##{issue.id}").should include(project_issue_url(project, issue))	358	markdown("##{issue.id}").should include(project_issue_url(project, issue))
345	end	359	end