diff options
author | Sean McGivern <sean@gitlab.com> | 2018-10-15 13:42:15 +0300 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2018-10-16 12:54:49 +0300 |
commit | 0bcfd0adb303f0fcaac40b703deda49f3dd683f4 (patch) | |
tree | dc4f44b2019a47e577bf75a7f2964d14a6655c1d /spec/lib/gitlab/hook_data | |
parent | c7fd95584eb2a595d84a5693a14b4b46c2885251 (diff) |
Fix image webhook rewriting for uploads
This rewrote URLs to be absolute URLs. However, for uploads (the most
common case), we actually need them to point to not just the GitLab
instance, but the project they're from. Thankfully, we can normally get
that information from the object we're building the hook for.
Diffstat (limited to 'spec/lib/gitlab/hook_data')
-rw-r--r-- | spec/lib/gitlab/hook_data/base_builder_spec.rb | 129 | ||||
-rw-r--r-- | spec/lib/gitlab/hook_data/issue_builder_spec.rb | 5 | ||||
-rw-r--r-- | spec/lib/gitlab/hook_data/merge_request_builder_spec.rb | 7 |
3 files changed, 92 insertions, 49 deletions
diff --git a/spec/lib/gitlab/hook_data/base_builder_spec.rb b/spec/lib/gitlab/hook_data/base_builder_spec.rb index a921dd766c3..e3c5ee3b905 100644 --- a/spec/lib/gitlab/hook_data/base_builder_spec.rb +++ b/spec/lib/gitlab/hook_data/base_builder_spec.rb @@ -8,57 +8,94 @@ describe Gitlab::HookData::BaseBuilder do 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>" + context 'with an upload prefix specified' do + let(:project_with_path) { double(full_path: 'baz/bar') } + let(:object_with_project) { double(project: project_with_path) } + subject { subclass.new(object_with_project) } + + where do + { + 'relative image URL' => { + input: '![an image](foo.png)', + output: "![an image](#{Gitlab.config.gitlab.url}/foo.png)" + }, + 'absolute upload URL' => { + input: '![an image](/uploads/foo.png)', + output: "![an image](#{Gitlab.config.gitlab.url}/baz/bar/uploads/foo.png)" + }, + 'absolute non-upload URL' => { + input: '![an image](/downloads/foo.png)', + output: "![an image](#{Gitlab.config.gitlab.url}/downloads/foo.png)" + } } - } + end + + with_them do + it { expect(subject.absolute_image_urls(input)).to eq(output) } + end end - with_them do - it { expect(subject.absolute_image_urls(input)).to eq(output) } + context 'without an upload prefix specified' do + subject { subclass.new(nil) } + + where do + { + 'relative image URL' => { + input: '![an image](foo.png)', + output: "![an image](#{Gitlab.config.gitlab.url}/foo.png)" + }, + 'absolute upload URL' => { + input: '![an image](/uploads/foo.png)', + output: "![an image](#{Gitlab.config.gitlab.url}/uploads/foo.png)" + }, + 'absolute non-upload URL' => { + input: '![an image](/downloads/foo.png)', + output: "![an image](#{Gitlab.config.gitlab.url}/downloads/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 end diff --git a/spec/lib/gitlab/hook_data/issue_builder_spec.rb b/spec/lib/gitlab/hook_data/issue_builder_spec.rb index 60093474f8a..f066c0e3813 100644 --- a/spec/lib/gitlab/hook_data/issue_builder_spec.rb +++ b/spec/lib/gitlab/hook_data/issue_builder_spec.rb @@ -46,7 +46,10 @@ describe Gitlab::HookData::IssueBuilder do 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)") + expected_path = "#{issue_with_description.project.path_with_namespace}/uploads/abc/Issue_Image.png" + + expect(data[:description]) + .to eq("test![Issue_Image](#{Settings.gitlab.url}/#{expected_path})") end end end diff --git a/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb b/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb index dd586af6118..9ce697adbba 100644 --- a/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb +++ b/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb @@ -58,11 +58,14 @@ describe Gitlab::HookData::MergeRequestBuilder do 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(:mr_with_description) { create(:merge_request, description: 'test![MR_Image](/uploads/abc/MR_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)") + expected_path = "#{mr_with_description.project.path_with_namespace}/uploads/abc/MR_Image.png" + + expect(data[:description]) + .to eq("test![MR_Image](#{Settings.gitlab.url}/#{expected_path})") end end end |