diff options
author | Stan Hu <stanhu@gmail.com> | 2019-08-28 08:48:24 +0300 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2019-08-28 10:18:33 +0300 |
commit | 29ce13e9992c296fbb2c4ad2706f53e491143d3e (patch) | |
tree | adb1e4f4885c4d205c5d2161bf12b8795a56ce17 | |
parent | 2ad1621c7f7bb7b749f2f4f8d89d84a0f2fbc9f7 (diff) |
Fix moving issues API failing when text includes commit URLs
When a issue is moved from one project to another, all associated
Markdown text is rewritten in the context of the new project. If the
note contained a link to a commit URL, `CommitRewriter#rewrite` would
fail because `Commit#link_reference_pattern` would match `nil` `commit`
values in the HTML generated from the Markdown. These `nil` values were
passed along to `Project#commits_by` because `Commit#reference_valid?`
was always returning `true`.
To prevent this issue from happening, we tighten up the check for
`Commit#reference_valid?` to look for valid SHA values.
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/66666
-rw-r--r-- | app/models/commit.rb | 7 | ||||
-rw-r--r-- | changelogs/unreleased/sh-fix-issue-move-api.yml | 5 | ||||
-rw-r--r-- | spec/lib/gitlab/gfm/reference_rewriter_spec.rb | 17 | ||||
-rw-r--r-- | spec/models/commit_spec.rb | 18 |
4 files changed, 46 insertions, 1 deletions
diff --git a/app/models/commit.rb b/app/models/commit.rb index 0889ce7e287..1470b50f396 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -35,6 +35,7 @@ class Commit MIN_SHA_LENGTH = Gitlab::Git::Commit::MIN_SHA_LENGTH COMMIT_SHA_PATTERN = /\h{#{MIN_SHA_LENGTH},40}/.freeze + EXACT_COMMIT_SHA_PATTERN = /\A#{COMMIT_SHA_PATTERN}\z/.freeze # Used by GFM to match and present link extensions on node texts and hrefs. LINK_EXTENSION_PATTERN = /(patch)/.freeze @@ -90,7 +91,7 @@ class Commit end def valid_hash?(key) - !!(/\A#{COMMIT_SHA_PATTERN}\z/ =~ key) + !!(EXACT_COMMIT_SHA_PATTERN =~ key) end def lazy(project, oid) @@ -139,6 +140,10 @@ class Commit '@' end + def self.reference_valid?(reference) + !!(reference =~ EXACT_COMMIT_SHA_PATTERN) + end + # Pattern used to extract commit references from text # # This pattern supports cross-project references. diff --git a/changelogs/unreleased/sh-fix-issue-move-api.yml b/changelogs/unreleased/sh-fix-issue-move-api.yml new file mode 100644 index 00000000000..e98d04b02d9 --- /dev/null +++ b/changelogs/unreleased/sh-fix-issue-move-api.yml @@ -0,0 +1,5 @@ +--- +title: Fix moving issues API failing when text includes commit URLs +merge_request: 32317 +author: +type: fixed diff --git a/spec/lib/gitlab/gfm/reference_rewriter_spec.rb b/spec/lib/gitlab/gfm/reference_rewriter_spec.rb index 4d2f08f95fc..790b0428d19 100644 --- a/spec/lib/gitlab/gfm/reference_rewriter_spec.rb +++ b/spec/lib/gitlab/gfm/reference_rewriter_spec.rb @@ -102,6 +102,23 @@ describe Gitlab::Gfm::ReferenceRewriter do end end + context 'with a commit' do + let(:old_project) { create(:project, :repository, name: 'old-project', group: group) } + let(:commit) { old_project.commit } + + context 'reference to an absolute URL to a commit' do + let(:text) { Gitlab::UrlBuilder.build(commit) } + + it { is_expected.to eq(text) } + end + + context 'reference to a commit' do + let(:text) { commit.id } + + it { is_expected.to eq("#{old_project_ref}@#{text}") } + end + end + context 'reference contains project milestone' do let!(:milestone) do create(:milestone, title: '9.0', project: old_project) diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 7b35c2ffd36..5ef824b9950 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -192,6 +192,24 @@ describe Commit do end end + describe '.reference_valid?' do + using RSpec::Parameterized::TableSyntax + + where(:ref, :result) do + '1234567' | true + '123456' | false + '1' | false + '0' * 40 | true + 'c1acaa58bbcbc3eafe538cb8274ba387047b69f8' | true + 'H1acaa58bbcbc3eafe538cb8274ba387047b69f8' | false + nil | false + end + + with_them do + it { expect(described_class.reference_valid?(ref)).to eq(result) } + end + end + describe '#reference_link_text' do let(:project) { create(:project, :repository, path: 'sample-project') } |