summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2019-08-27 22:48:24 -0700
committerStan Hu <stanhu@gmail.com>2019-08-28 00:18:33 -0700
commit29ce13e9992c296fbb2c4ad2706f53e491143d3e (patch)
treeadb1e4f4885c4d205c5d2161bf12b8795a56ce17
parent2ad1621c7f7bb7b749f2f4f8d89d84a0f2fbc9f7 (diff)
downloadgitlab-ce-29ce13e9992c296fbb2c4ad2706f53e491143d3e.tar.gz
Fix moving issues API failing when text includes commit URLssh-fix-issue-move-api
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.rb7
-rw-r--r--changelogs/unreleased/sh-fix-issue-move-api.yml5
-rw-r--r--spec/lib/gitlab/gfm/reference_rewriter_spec.rb17
-rw-r--r--spec/models/commit_spec.rb18
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') }