Merge branch 'fix/cross-project-labels-project-reference' into 'master'

Fix cross-project label ref used with invalid project

When cross-project label reference was used with invalid project reference, `nil` was assigned to project variable which later caused an exception.

Closes #15168

See merge request !3811

CHANGELOG

@@ -4,6 +4,7 @@ v 8.8.0 (unreleased)
 
 v 8.7.0 (unreleased)
   - The number of InfluxDB points stored per UDP packet can now be configured
+  - Fix error when cross-project label reference used with non-existent project
   - Transactions for /internal/allowed now have an "action" tag set
   - Method instrumentation now uses Module#prepend instead of aliasing methods
   - Repository.clean_old_archives is now instrumented

lib/banzai/filter/label_reference_filter.rb

@@ -18,9 +18,7 @@ module Banzai
 
       def references_in(text, pattern = Label.reference_pattern)
         text.gsub(pattern) do |match|
-          project = project_from_ref($~[:project])
-          params = label_params($~[:label_id].to_i, $~[:label_name])
-          label = project.labels.find_by(params)
+          label = find_label($~[:project], $~[:label_id], $~[:label_name])
 
           if label
             yield match, label.id, $~[:project], $~
@@ -30,6 +28,29 @@ module Banzai
         end
       end
 
+      def find_label(project_ref, label_id, label_name)
+        project = project_from_ref(project_ref)
+        return unless project
+
+        label_params = label_params(label_id, label_name)
+        project.labels.find_by(label_params)
+      end
+
+      # Parameters to pass to `Label.find_by` based on the given arguments
+      #
+      # id   - Integer ID to pass. If present, returns {id: id}
+      # name - String name to pass. If `id` is absent, finds by name without
+      #        surrounding quotes.
+      #
+      # Returns a Hash.
+      def label_params(id, name)
+        if name
+          { name: name.tr('"', '') }
+        else
+          { id: id.to_i }
+        end
+      end
+
       def url_for_object(label, project)
         h = Gitlab::Routing.url_helpers
         h.namespace_project_issues_url(project.namespace, project, label_name: label.name,
@@ -43,21 +64,6 @@ module Banzai
           LabelsHelper.render_colored_cross_project_label(object)
         end
       end
-
-      # Parameters to pass to `Label.find_by` based on the given arguments
-      #
-      # id   - Integer ID to pass. If present, returns {id: id}
-      # name - String name to pass. If `id` is absent, finds by name without
-      #        surrounding quotes.
-      #
-      # Returns a Hash.
-      def label_params(id, name)
-        if name
-          { name: name.tr('"', '') }
-        else
-          { id: id }
-        end
-      end
     end
   end
 end

spec/lib/banzai/filter/label_reference_filter_spec.rb

@@ -178,27 +178,37 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do
   end
 
   describe 'cross project label references' do
-    let(:another_project)  { create(:empty_project, :public) }
-    let(:project_name) { another_project.name_with_namespace }
-    let(:label) { create(:label, project: another_project, color: '#00ff00') }
-    let(:reference) { label.to_reference(project) }
+    context 'valid project referenced' do
+      let(:another_project)  { create(:empty_project, :public) }
+      let(:project_name) { another_project.name_with_namespace }
+      let(:label) { create(:label, project: another_project, color: '#00ff00') }
+      let(:reference) { label.to_reference(project) }
 
-    let!(:result) { reference_filter("See #{reference}") }
+      let!(:result) { reference_filter("See #{reference}") }
 
-    it 'points to referenced project issues page' do
-      expect(result.css('a').first.attr('href'))
-        .to eq urls.namespace_project_issues_url(another_project.namespace,
-                                                 another_project,
-                                                 label_name: label.name)
+      it 'points to referenced project issues page' do
+        expect(result.css('a').first.attr('href'))
+          .to eq urls.namespace_project_issues_url(another_project.namespace,
+                                                   another_project,
+                                                   label_name: label.name)
+      end
+
+      it 'has valid color' do
+        expect(result.css('a span').first.attr('style'))
+          .to match /background-color: #00ff00/
+      end
+
+      it 'contains cross project content' do
+        expect(result.css('a').first.text).to eq "#{label.name} in #{project_name}"
+      end
     end
 
-    it 'has valid color' do
-      expect(result.css('a span').first.attr('style'))
-        .to match /background-color: #00ff00/
-    end
+    context 'project that does not exist referenced' do
+      let(:result) { reference_filter('aaa/bbb~ccc') }
 
-    it 'contains cross project content' do
-      expect(result.css('a').first.text).to eq "#{label.name} in #{project_name}"
+      it 'does not link reference' do
+        expect(result.to_html).to eq 'aaa/bbb~ccc'
+      end
     end
   end
 end