Commit 10bd8002 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'satishperala/gitlab-ce-20720_webhooks_full_image_url' into 'master'

Include full image URL in webhooks for uploaded images

Closes #20720

See merge request gitlab-org/gitlab-ce!18109
parents 05bef3c6 fc580935
......@@ -202,7 +202,7 @@ class Note < ActiveRecord::Base
end
def hook_attrs
attributes
Gitlab::HookData::NoteBuilder.new(self).build
end
def for_commit?
......
......@@ -60,7 +60,7 @@ class WikiPage
attr_accessor :attributes
def hook_attrs
attributes
Gitlab::HookData::WikiPageBuilder.new(self).build
end
def initialize(wiki, page = nil, persisted = false)
......
---
title: Include full image URL in webhooks for uploaded images
merge_request: 18109
author: Satish Perala
type: changed
......@@ -10,6 +10,13 @@ Starting from GitLab 8.5:
Starting from GitLab 11.1, the logs of web hooks are automatically removed after
one month.
>**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
......@@ -1125,6 +1132,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/$sha/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/$sha/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.
......
......@@ -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:
......
module Gitlab
module HookData
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
private
def absolute_image_urls(markdown_text)
return markdown_text unless markdown_text.present?
markdown_text.gsub(MARKDOWN_SIMPLE_IMAGE) do
if $~[:image]
url = $~[:url]
url = "/#{url}" unless url.start_with?('/')
"![#{$~[:title]}](#{Gitlab.config.gitlab.url}#{url})"
else
$~[0]
end
end
end
end
end
end
module Gitlab
module HookData
class IssuableBuilder
class IssuableBuilder < BaseBuilder
CHANGES_KEYS = %i[previous current].freeze
attr_accessor :issuable
def initialize(issuable)
@issuable = issuable
end
alias_method :issuable, :object
def build(user: nil, changes: {})
hook_data = {
......
module Gitlab
module HookData
class IssueBuilder
class IssueBuilder < BaseBuilder
SAFE_HOOK_ATTRIBUTES = %i[
assignee_id
author_id
......@@ -30,14 +30,11 @@ module Gitlab
total_time_spent
].freeze
attr_accessor :issue
def initialize(issue)
@issue = issue
end
alias_method :issue, :object
def build
attrs = {
description: absolute_image_urls(issue.description),
url: Gitlab::UrlBuilder.build(issue),
total_time_spent: issue.total_time_spent,
human_total_time_spent: issue.human_total_time_spent,
......
module Gitlab
module HookData
class MergeRequestBuilder
class MergeRequestBuilder < BaseBuilder
SAFE_HOOK_ATTRIBUTES = %i[
assignee_id
author_id
......@@ -35,14 +35,11 @@ module Gitlab
total_time_spent
].freeze
attr_accessor :merge_request
def initialize(merge_request)
@merge_request = merge_request
end
alias_method :merge_request, :object
def build
attrs = {
description: absolute_image_urls(merge_request.description),
url: Gitlab::UrlBuilder.build(merge_request),
source: merge_request.source_project.try(:hook_attrs),
target: merge_request.target_project.hook_attrs,
......
module Gitlab
module HookData
class NoteBuilder < BaseBuilder
SAFE_HOOK_ATTRIBUTES = %i[
attachment
author_id
change_position
commit_id
created_at
discussion_id
id
line_code
note
noteable_id
noteable_type
original_position
position
project_id
resolved_at
resolved_by_id
resolved_by_push
st_diff
system
type
updated_at
updated_by_id
].freeze
alias_method :note, :object
def build
note
.attributes
.with_indifferent_access
.slice(*SAFE_HOOK_ATTRIBUTES)
.merge(
description: absolute_image_urls(note.note),
url: Gitlab::UrlBuilder.build(note)
)
end
end
end
end
module Gitlab
module HookData
class WikiPageBuilder < BaseBuilder
alias_method :wiki_page, :object
def build
wiki_page
.attributes
.merge(
'content' => absolute_image_urls(wiki_page.content)
)
end
end
end
end
......@@ -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
require 'spec_helper'
describe Gitlab::HookData::BaseBuilder do
describe '#absolute_image_urls' do
let(:subclass) do
Class.new(described_class) do
public :absolute_image_urls
end
end
subject { subclass.new(nil) }
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(subject.absolute_image_urls(input)).to eq(output) }
end
end
end
......@@ -40,5 +40,14 @@ describe Gitlab::HookData::IssueBuilder do
expect(data).to include(:human_total_time_spent)
expect(data).to include(:assignee_ids)
end
context 'when the issue has an image in the description' do
let(:issue_with_description) { create(:issue, description: 'test![Issue_Image](/uploads/abc/Issue_Image.png)') }
let(:builder) { described_class.new(issue_with_description) }
it 'sets the image to use an absolute URL' do
expect(data[:description]).to eq("test![Issue_Image](#{Settings.gitlab.url}/uploads/abc/Issue_Image.png)")
end
end
end
end
......@@ -56,5 +56,14 @@ describe Gitlab::HookData::MergeRequestBuilder do
expect(data).to include(:human_time_estimate)
expect(data).to include(:human_total_time_spent)
end
context 'when the MR has an image in the description' do
let(:mr_with_description) { create(:merge_request, description: 'test![Issue_Image](/uploads/abc/Issue_Image.png)') }
let(:builder) { described_class.new(mr_with_description) }
it 'sets the image to use an absolute URL' do
expect(data[:description]).to eq("test![Issue_Image](#{Settings.gitlab.url}/uploads/abc/Issue_Image.png)")
end
end
end
end
......@@ -554,6 +554,16 @@ describe WikiPage do
end
end
describe '#hook_attrs' do
it 'adds absolute urls for images in the content' do
create_page("test page", "test![WikiPage_Image](/uploads/abc/WikiPage_Image.png)")
page = wiki.wiki.page(title: "test page")
wiki_page = described_class.new(wiki, page, true)
expect(wiki_page.hook_attrs['content']).to eq("test![WikiPage_Image](#{Settings.gitlab.url}/uploads/abc/WikiPage_Image.png)")
end
end
private
def remove_temp_repo(path)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment