From 52c3179be94e857ecb0de2cfa8ecbd484a36d213 Mon Sep 17 00:00:00 2001
From: Marin Jankovski <marin@gitlab.com>
Date: Tue, 20 May 2014 15:29:14 +0200
Subject: [PATCH 1/3] Do not replace links inside code blocks, less code for
 the same amount of work.

---
 app/helpers/gitlab_markdown_helper.rb | 109 ++++++++++----------------
 lib/redcarpet/render/gitlab_html.rb   |  17 +---
 2 files changed, 43 insertions(+), 83 deletions(-)

diff --git a/app/helpers/gitlab_markdown_helper.rb b/app/helpers/gitlab_markdown_helper.rb
index 69425bc171d..2d226adb2a4 100644
--- a/app/helpers/gitlab_markdown_helper.rb
+++ b/app/helpers/gitlab_markdown_helper.rb
@@ -59,90 +59,59 @@ module GitlabMarkdownHelper
     end
   end
 
-  # text - whole text from a markdown file
-  # project_path_with_namespace - namespace/projectname, eg. gitlabhq/gitlabhq
-  # ref - name of the branch or reference, eg. stable
-  # requested_path - path of request, eg. doc/api/README.md, used in special case when path is pointing to the .md file were the original request is coming from
-  def create_relative_links(text, project, ref, requested_path)
-    @path_to_satellite = project.satellite.path
-    project_path_with_namespace = project.path_with_namespace
+  def create_relative_links(text)
     paths = extract_paths(text)
-    paths.each do |file_path|
-      original_file_path = extract(file_path)
-      new_path = rebuild_path(project_path_with_namespace, original_file_path, requested_path, ref)
-      if reference_path?(file_path)
-        # Replacing old string with a new one that contains updated path
-        # eg. [some document]: document.md will be replaced with [some document] /namespace/project/master/blob/document.md
-        text.gsub!(file_path, file_path.gsub(original_file_path, "/#{new_path}"))
-      else
-        # Replacing old string with a new one with brackets ]() to prevent replacing occurence of a word
-        # e.g. If we have a markdown like [test](test) this will replace ](test) and not the word test
-        text.gsub!("](#{file_path})", "](/#{new_path})")
-      end
+
+    paths.uniq.each do |file_path|
+      new_path = rebuild_path(file_path)
+      # Finds quoted path so we don't replace other mentions of the string
+      # eg. "doc/api" will be replaced and "/home/doc/api/text" won't
+      text.gsub!("\"#{file_path}\"", "\"/#{new_path}\"")
     end
+
     text
   end
 
-  def extract_paths(markdown_text)
-    all_markdown_paths = pick_out_paths(markdown_text)
-    paths = remove_empty(all_markdown_paths)
-    select_relative(paths)
+  def extract_paths(text)
+    links = substitute_links(text)
+    image_links = substitute_image_links(text)
+    links + image_links
   end
 
-  # Split the markdown text to each line and find all paths, this will match anything with - ]("some_text") and [some text]: file.md
-  def pick_out_paths(markdown_text)
-    inline_paths = markdown_text.split("\n").map { |text| text.scan(/\]\(([^(]+)\)/) }
-    reference_paths = markdown_text.split("\n").map { |text| text.scan(/\[.*\]:.*/) }
-    inline_paths + reference_paths
+  def substitute_links(text)
+    links = text.scan(/<a href=\"([^"]*)\">/)
+    relative_links = links.flatten.reject{ |link| link_to_ignore? link }
+    relative_links
   end
 
-  # Removes any empty result produced by not matching the regexp
-  def remove_empty(paths)
-    paths.reject{|l| l.empty? }.flatten
+  def substitute_image_links(text)
+    links = text.scan(/<img src=\"([^"]*)\"/)
+    relative_links = links.flatten.reject{ |link| link_to_ignore? link }
+    relative_links
   end
 
-  # If a path is a reference style link we need to omit ]:
-  def extract(path)
-    path.split("]: ").last
-  end
-
-  # Reject any path that contains ignored protocol
-  # eg. reject "https://gitlab.org} but accept "doc/api/README.md"
-  def select_relative(paths)
-    paths.reject{|path| ignored_protocols.map{|protocol| path.include?(protocol)}.any?}
-  end
-
-  # Check whether a path is a reference-style link
-  def reference_path?(path)
-    path.include?("]: ")
+  def link_to_ignore?(link)
+    ignored_protocols.map{ |protocol| link.include?(protocol) }.any?
   end
 
   def ignored_protocols
     ["http://","https://", "ftp://", "mailto:"]
   end
 
-  def rebuild_path(path_with_namespace, path, requested_path, ref)
+  def rebuild_path(path)
     path.gsub!(/(#.*)/, "")
     id = $1 || ""
-    file_path = relative_file_path(path, requested_path)
+    file_path = relative_file_path(path)
     [
-      path_with_namespace,
-      path_with_ref(file_path, ref),
+      Gitlab.config.gitlab.relative_url_root,
+      @project.path_with_namespace,
+      path_with_ref(file_path),
       file_path
-    ].compact.join("/").gsub(/\/*$/, '') + id
+    ].compact.join("/").gsub(/^\/*|\/*$/, '') + id
   end
 
-  # Checks if the path exists in the repo
-  # eg. checks if doc/README.md exists, if not then link to blob
-  def path_with_ref(path, ref)
-    if file_exists?(path)
-      "#{local_path(path)}/#{correct_ref(ref)}"
-    else
-      "blob/#{correct_ref(ref)}"
-    end
-  end
-
-  def relative_file_path(path, requested_path)
+  def relative_file_path(path)
+    requested_path = @path
     nested_path = build_nested_path(path, requested_path)
     return nested_path if file_exists?(nested_path)
     path
@@ -166,6 +135,16 @@ module GitlabMarkdownHelper
     end
   end
 
+  # Checks if the path exists in the repo
+  # eg. checks if doc/README.md exists, if not then link to blob
+  def path_with_ref(path)
+    if file_exists?(path)
+      "#{local_path(path)}/#{correct_ref}"
+    else
+      "blob/#{correct_ref}"
+    end
+  end
+
   def file_exists?(path)
     return false if path.nil?
     return @repository.blob_at(current_sha, path).present? || @repository.tree(current_sha, path).entries.any?
@@ -179,10 +158,6 @@ module GitlabMarkdownHelper
     return "blob"
   end
 
-  def current_ref
-    @commit.nil? ? "master" : @commit.id
-  end
-
   def current_sha
     if @commit
       @commit.id
@@ -192,7 +167,7 @@ module GitlabMarkdownHelper
   end
 
   # We will assume that if no ref exists we can point to master
-  def correct_ref(ref)
-    ref ? ref : "master"
+  def correct_ref
+    @ref ? @ref : "master"
   end
 end
diff --git a/lib/redcarpet/render/gitlab_html.rb b/lib/redcarpet/render/gitlab_html.rb
index 7d9428ff27d..29bf42e7626 100644
--- a/lib/redcarpet/render/gitlab_html.rb
+++ b/lib/redcarpet/render/gitlab_html.rb
@@ -45,23 +45,8 @@ class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML
     end
   end
 
-  def preprocess(full_document)
-    if is_wiki?
-      full_document
-    elsif @project
-      h.create_relative_links(full_document, @project, @ref, @request_path)
-    else
-      full_document
-    end
-  end
-
   def postprocess(full_document)
+    full_document = h.create_relative_links(full_document)
     h.gfm(full_document)
   end
-
-  def is_wiki?
-    if @template.instance_variable_get("@project_wiki")
-      @template.instance_variable_get("@page")
-    end
-  end
 end

From 603e04bc21b07b8b6f70e972383ea2f29d0bfce0 Mon Sep 17 00:00:00 2001
From: Marin Jankovski <marin@gitlab.com>
Date: Tue, 20 May 2014 15:58:30 +0200
Subject: [PATCH 2/3] Make sure slashes are handled properly.

---
 app/helpers/gitlab_markdown_helper.rb | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/app/helpers/gitlab_markdown_helper.rb b/app/helpers/gitlab_markdown_helper.rb
index 2d226adb2a4..b8891d801aa 100644
--- a/app/helpers/gitlab_markdown_helper.rb
+++ b/app/helpers/gitlab_markdown_helper.rb
@@ -102,6 +102,8 @@ module GitlabMarkdownHelper
     path.gsub!(/(#.*)/, "")
     id = $1 || ""
     file_path = relative_file_path(path)
+    file_path = sanitize_slashes(file_path)
+
     [
       Gitlab.config.gitlab.relative_url_root,
       @project.path_with_namespace,
@@ -110,6 +112,12 @@ module GitlabMarkdownHelper
     ].compact.join("/").gsub(/^\/*|\/*$/, '') + id
   end
 
+  def sanitize_slashes(path)
+    path[0] = "" if path.start_with?("/")
+    path.chop if path.end_with?("/")
+    path
+  end
+
   def relative_file_path(path)
     requested_path = @path
     nested_path = build_nested_path(path, requested_path)

From 281643a1bfd6f4da88a278bfce20133c80105de5 Mon Sep 17 00:00:00 2001
From: Marin Jankovski <marin@gitlab.com>
Date: Fri, 23 May 2014 12:58:34 +0200
Subject: [PATCH 3/3] Add feature spec.

---
 features/project/issues/issues.feature |  6 ++++++
 features/steps/project/issues.rb       | 12 ++++++++++++
 lib/redcarpet/render/gitlab_html.rb    |  6 +++---
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/features/project/issues/issues.feature b/features/project/issues/issues.feature
index c5311544efa..191e8dcbe7f 100644
--- a/features/project/issues/issues.feature
+++ b/features/project/issues/issues.feature
@@ -68,6 +68,12 @@ Feature: Project Issues
     And I leave a comment with a header containing "Comment with a header"
     Then The comment with the header should not have an ID
 
+  @javascript
+  Scenario: Blocks inside comments should not build relative links
+    Given I visit issue page "Release 0.4"
+    And I leave a comment with code block
+    Then The code block should be unchanged
+
   Scenario: Issues on empty project
     Given empty project "Empty Project"
     When I visit empty project page
diff --git a/features/steps/project/issues.rb b/features/steps/project/issues.rb
index d1f3ba25a21..d0b4aa6e080 100644
--- a/features/steps/project/issues.rb
+++ b/features/steps/project/issues.rb
@@ -163,4 +163,16 @@ class ProjectIssues < Spinach::FeatureSteps
     project = Project.find_by(name: 'Empty Project')
     visit project_issues_path(project)
   end
+
+  step 'I leave a comment with code block' do
+    within(".js-main-target-form") do
+      fill_in "note[note]", with: "```\nCommand [1]: /usr/local/bin/git , see [text](doc/text)\n```"
+      click_button "Add Comment"
+      sleep 0.05
+    end
+  end
+
+  step 'The code block should be unchanged' do
+    page.should have_content("```\nCommand [1]: /usr/local/bin/git , see [text](doc/text)\n```")
+  end
 end
diff --git a/lib/redcarpet/render/gitlab_html.rb b/lib/redcarpet/render/gitlab_html.rb
index 29bf42e7626..bb225f1acd8 100644
--- a/lib/redcarpet/render/gitlab_html.rb
+++ b/lib/redcarpet/render/gitlab_html.rb
@@ -6,8 +6,6 @@ class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML
   def initialize(template, options = {})
     @template = template
     @project = @template.instance_variable_get("@project")
-    @ref = @template.instance_variable_get("@ref")
-    @request_path = @template.instance_variable_get("@path")
     @options = options.dup
     super options
   end
@@ -46,7 +44,9 @@ class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML
   end
 
   def postprocess(full_document)
-    full_document = h.create_relative_links(full_document)
+    unless @template.instance_variable_get("@project_wiki") || @project.nil?
+      full_document = h.create_relative_links(full_document)
+    end
     h.gfm(full_document)
   end
 end