Add documentation and specs for webhook URL rewriting

0d9ef34a · Sean McGivern · 7ff24772 · 0d9ef34a · 0d9ef34a · 0d9ef34a
Commit 0d9ef34a authored Jul 06, 2018 by Sean McGivern
7 changed files
--- a/changelogs/unreleased/satishperala-gitlab-ce-20720_webhooks_full_image_url.yml
+++ b/changelogs/unreleased/satishperala-gitlab-ce-20720_webhooks_full_image_url.yml
+---
+title: Include full image URL in webhooks for uploaded images
+merge_request: 18109
+author: Satish Perala
+type: changed
--- a/doc/user/project/integrations/webhooks.md
+++ b/doc/user/project/integrations/webhooks.md
@@ -6,6 +6,13 @@ Starting from GitLab 8.5:
 - the `project.ssh_url` key is deprecated in favor of the `project.git_ssh_url` key
 - the `project.http_url` key is deprecated in favor of the `project.git_http_url` key

+>**Note**
+Starting from GitLab 11.2:
+- The `description` field for issues, merge requests, comments, and wiki pages
+  is rewritten so that simple Markdown image references (like
+  `![](/uploads/...)`) have their target URL changed to an absolute URL. See
+  [image URL rewriting](#image-url-rewriting) for more details.
+
 Project webhooks allow you to trigger a URL if for example new code is pushed or
 a new issue is created. You can configure webhooks to listen for specific events
 like pushes, issues or merge requests. GitLab will send a POST request with data
@@ -1121,6 +1128,27 @@ X-Gitlab-Event: Build Hook
 }
 ```

+## Image URL rewriting
+
+From GitLab 11.2, simple image references are rewritten to use an absolute URL
+in webhooks. So if an image, merge request, comment, or wiki page has this in
+its description:
+
+```markdown
+![image](/uploads/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/image.png)
+```
+
+It will appear in the webhook body as the below (assuming that GitLab is
+installed at gitlab.example.com):
+
+```markdown
+![image](https://gitlab.example.com/uploads/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/image.png)
+```
+
+This will not rewrite URLs that already are pointing to HTTP, HTTPS, or
+protocol-relative URLs. It will also not rewrite image URLs using advanced
+Markdown features, like link labels.
+
 ## Testing webhooks

 You can trigger the webhook manually. Sample data from the project will be used.Sample data will take from the project.

--- a/lib/banzai/filter/blockquote_fence_filter.rb
+++ b/lib/banzai/filter/blockquote_fence_filter.rb
@@ -2,27 +2,7 @@ module Banzai
  module Filter
    class BlockquoteFenceFilter < HTML::Pipeline::TextFilter
      REGEX = %r{
-          (?<code>
-            # Code blocks:
-            # ```
-            # Anything, including `>>>` blocks which are ignored by this filter
-            # ```
-
-            ^```
-            .+?
-            \n```\ *$
-          )
-        |
-          (?<html>
-            # HTML block:
-            # <tag>
-            # Anything, including `>>>` blocks which are ignored by this filter
-            # </tag>
-
-            ^<[^>]+?>\ *\n
-            .+?
-            \n<\/[^>]+?>\ *$
-          )
+          #{::Gitlab::Regex.markdown_code_or_html_blocks}
        |
          (?:
            # Blockquote:

--- a/lib/gitlab/hook_data/base_builder.rb
+++ b/lib/gitlab/hook_data/base_builder.rb
@@ -3,17 +3,36 @@ module Gitlab
    class BaseBuilder
      attr_accessor :object

+      MARKDOWN_SIMPLE_IMAGE = %r{
+          #{::Gitlab::Regex.markdown_code_or_html_blocks}
+        |
+          (?<image>
+            !
+            \[(?<title>[^\n]*?)\]
+            \((?<url>(?!(https?://|//))[^\n]+?)\)
+          )
+      }mx.freeze
+
      def initialize(object)
        @object = object
      end

+      def self.absolute_image_urls(markdown_text)
+        return markdown_text unless markdown_text.present?
+
+        markdown_text.gsub(MARKDOWN_SIMPLE_IMAGE) do
+          if $~[:image]
+            "![#{$~[:title]}](#{Gitlab.config.gitlab.url}/#{$~[:url]})"
+          else
+            $~[0]
+          end
+        end
+      end
+
      private

      def absolute_image_urls(markdown_text)
-        return markdown_text unless markdown_text.present?
-
-        markdown_text.gsub(/!\[(.*?)\]\((.*?)\)/,
-                           "![\\1](#{Settings.gitlab.url}\\2)")
+        self.class.absolute_image_urls(markdown_text)
      end
    end
  end

--- a/lib/gitlab/hook_data/issuable_builder.rb
+++ b/lib/gitlab/hook_data/issuable_builder.rb
@@ -60,13 +60,6 @@ module Gitlab
          hash
        end
      end
-
-      def absolute_image_urls(markdown_text)
-        return markdown_text unless markdown_text.present?
-
-        markdown_text.gsub(/!\[(.*?)\]\((.*?)\)/,
-                           "![\\1](#{Settings.gitlab.url}\\2)")
-      end
    end
  end
 end
--- a/lib/gitlab/regex.rb
+++ b/lib/gitlab/regex.rb
@@ -73,5 +73,31 @@ module Gitlab
    def build_trace_section_regex
      @build_trace_section_regexp ||= /section_((?:start)|(?:end)):(\d+):([a-zA-Z0-9_.-]+)\r\033\[0K/.freeze
    end
+
+    def markdown_code_or_html_blocks
+      @markdown_code_or_html_blocks ||= %r{
+          (?<code>
+            # Code blocks:
+            # ```
+            # Anything, including `>>>` blocks which are ignored by this filter
+            # ```
+
+            ^```
+            .+?
+            \n```\ *$
+          )
+        |
+          (?<html>
+            # HTML block:
+            # <tag>
+            # Anything, including `>>>` blocks which are ignored by this filter
+            # </tag>
+
+            ^<[^>]+?>\ *\n
+            .+?
+            \n<\/[^>]+?>\ *$
+          )
+      }mx
+    end
  end
 end
--- a/spec/lib/gitlab/hook_data/base_builder_spec.rb
+++ b/spec/lib/gitlab/hook_data/base_builder_spec.rb
+require 'spec_helper'
+
+describe Gitlab::HookData::BaseBuilder do
+  describe '.absolute_image_urls' do
+    using RSpec::Parameterized::TableSyntax
+
+    where do
+      {
+        'relative image URL' => {
+          input: '![an image](foo.png)',
+          output: "![an image](#{Gitlab.config.gitlab.url}/foo.png)"
+        },
+        'HTTP URL' => {
+          input: '![an image](http://example.com/foo.png)',
+          output: '![an image](http://example.com/foo.png)'
+        },
+        'HTTPS URL' => {
+          input: '![an image](https://example.com/foo.png)',
+          output: '![an image](https://example.com/foo.png)'
+        },
+        'protocol-relative URL' => {
+          input: '![an image](//example.com/foo.png)',
+          output: '![an image](//example.com/foo.png)'
+        },
+        'URL reference by title' => {
+          input: "![foo]\n\n[foo]: foo.png",
+          output: "![foo]\n\n[foo]: foo.png"
+        },
+        'URL reference by label' => {
+          input: "![][foo]\n\n[foo]: foo.png",
+          output: "![][foo]\n\n[foo]: foo.png"
+        },
+        'in Markdown inline code block' => {
+          input: '`![an image](foo.png)`',
+          output: "`![an image](#{Gitlab.config.gitlab.url}/foo.png)`"
+        },
+        'in HTML tag on the same line' => {
+          input: '<p>![an image](foo.png)</p>',
+          output: "<p>![an image](#{Gitlab.config.gitlab.url}/foo.png)</p>"
+        },
+        'in Markdown multi-line code block' => {
+          input: "```\n![an image](foo.png)\n```",
+          output: "```\n![an image](foo.png)\n```"
+        },
+        'in HTML tag on different lines' => {
+          input: "<p>\n![an image](foo.png)\n</p>",
+          output: "<p>\n![an image](foo.png)\n</p>"
+        }
+      }
+    end
+
+    with_them do
+      it { expect(described_class.absolute_image_urls(input)).to eq(output) }
+    end
+  end
+end