Merge branch '12905-support-design-management-links-when-adapting-links-in-markdown' into 'master'

Support Design Management links in Markdown

Closes #12905

See merge request gitlab-org/gitlab!15127

doc/user/markdown.md

@@ -407,6 +407,7 @@ GFM will recognize the following:
 | merge request                   | `!123`                     | `namespace/project!123`                 | `project!123`                  |
 | snippet                         | `$123`                     | `namespace/project$123`                 | `project$123`                  |
 | epic **(ULTIMATE)**             | `&123`                     | `group1/subgroup&123`                   |                                |
+| design **(PREMIUM)**            | `#123[file.jpg]` or `#123["file.png"]` | `group1/subgroup#123[file.png]`         | `project#123[file.png]`         |
 | label by ID                     | `~123`                     | `namespace/project~123`                 | `project~123`                  |
 | one-word label by name          | `~bug`                     | `namespace/project~bug`                 | `project~bug`                  |
 | multi-word label by name        | `~"feature request"`       | `namespace/project~"feature request"`   | `project~"feature request"`    |

doc/user/project/issues/design_management.md

@@ -37,6 +37,13 @@ Design Management requires that projects are using
 [hashed storage](../../../administration/repository_storage_types.html#hashed-storage)
 (the default storage type since v10.0).
 
+### Feature Flags
+
+- Reference Parsing
+
+  Designs support short references in Markdown, but this needs to be enabled by setting
+  the `:design_management_reference_filter_gfm_pipeline` feature flag.
+
 ## Limitations
 
 - Files uploaded must have a file extension of either `png`, `jpg`, `jpeg`, `gif`, `bmp`, `tiff` or `ico`.
@@ -137,3 +144,32 @@ Different discussions have different badge numbers:
 
 From GitLab 12.5 on, new annotations will be outputted to the issue activity,
 so that everyone involved can participate in the discussion.
+
+## References
+
+GitLab Flavored Markdown supports references to designs. The syntax for this is:
+
+  `#123[file.jpg]` - the issue reference, with the filename in square braces
+
+File names may contain a variety of odd characters, so two escaping mechanisms are supported:
+
+### Quoting
+
+File names may be quoted with double quotation marks, eg:
+
+  `#123["file.jpg"]`
+
+This is useful if, for instance, your filename has square braces in its name. In this scheme, all
+double quotation marks in the file name need to be escaped with backslashes, and backslashes need
+to be escaped likewise:
+
+  `#123["with with \"quote\" marks and a backslash \\.png"]`
+
+### Base64 Encoding
+
+In the case of file names that include HTML elements, you will need to escape these names to avoid
+them being processed as HTML literals. To do this, we support base64 encoding, eg.
+
+  The file `<a>.jpg` can be referenced as `#123[base64:PGE+LmpwZwo=]`
+
+Obviously we would advise against using such filenames.

ee/app/models/design_management/design.rb

@@ -26,6 +26,10 @@ module DesignManagement
 
     alias_attribute :title, :filename
 
+    # Pre-fetching scope to include the data necessary to construct a
+    # reference using `to_reference`.
+    scope :for_reference, -> { includes(issue: [{ project: [:route, :namespace] }]) }
+
     # Find designs visible at the given version
     #
     # @param version [nil, DesignManagement::Version]:
@@ -52,6 +56,7 @@ module DesignManagement
     end
 
     scope :with_filename, -> (filenames) { where(filename: filenames) }
+    scope :on_issue, ->(issue) { where(issue_id: issue) }
 
     # Scope called by our REST API to avoid N+1 problems
     scope :with_api_entity_associations, -> { preload(:issue) }
@@ -84,10 +89,68 @@ module DesignManagement
     #   #123[homescreen.png]
     #   other-project#72[sidebar.jpg]
     #   #38/designs[transition.gif]
+    #   #12["filename with [] in it.jpg"]
     def to_reference(from = nil, full: false)
       infix = full ? '/designs' : ''
+      totally_simple = %r{ \A #{self.class.simple_file_name} \z }x
+      safe_name = if totally_simple.match?(filename)
+                    filename
+                  elsif filename =~ /[<>]/
+                    %Q{base64:#{Base64.strict_encode64(filename)}}
+                  else
+                    escaped = filename.gsub(%r{[\\"]}) { |x| "\\#{x}" }
+                    %Q{"#{escaped}"}
+                  end
+
+      "#{issue.to_reference(from, full: full)}#{infix}[#{safe_name}]"
+    end
+
+    def self.reference_pattern
+      @reference_pattern ||= begin
+        # Filenames can be escaped with double quotes to name filenames
+        # that include square brackets, or other special characters
+        %r{
+          #{Issue.reference_pattern}
+          (\/designs)?
+          \[
+            (?<design> #{simple_file_name} | #{quoted_file_name} | #{base_64_encoded_name})
+          \]
+        }x
+      end
+    end
+
+    def self.simple_file_name
+      %r{
+          (?<simple_file_name>
+           ( \w | [_:,'-] | \. | \s )+
+           \.
+           \w+
+          )
+      }x
+    end
+
+    def self.base_64_encoded_name
+      %r{
+          base64:
+          (?<base_64_encoded_name>
+           [A-Za-z0-9+\n]+
+           =?
+          )
+      }x
+    end
+
+    def self.quoted_file_name
+      %r{
+          "
+          (?<escaped_filename>
+            (\\ \\ | \\ " | [^"\\])+
+          )
+          "
+      }x
+    end
 
-      "%s%s[%s]" % [issue.to_reference(from, full: full), infix, filename]
+    def self.link_reference_pattern
+      nil
     end
 
     def to_ability_name

ee/lib/banzai/filter/design_reference_filter.rb

+# frozen_string_literal: true
+
+module Banzai
+  module Filter
+    class DesignReferenceFilter < AbstractReferenceFilter
+      include Gitlab::Allowable
+
+      Identifier = Struct.new(:issue_iid, :filename, keyword_init: true)
+
+      # This filter must be enabled by setting the following flags:
+      #  - design_management
+      #  - design_management_reference_filter_gfm_pipeline
+      def call
+        return doc unless enabled?
+
+        super
+      end
+
+      def find_object(project, identifier)
+        records_per_parent[project][identifier]
+      end
+
+      def parent_records(project, identifiers)
+        return [] unless can_read_designs?(project)
+
+        iids      = identifiers.map(&:issue_iid).to_set
+        filenames = identifiers.map(&:filename).to_set
+        issues    = project.issues.where(iid: iids)
+        issue_map = issues.index_by(&:id)
+
+        designs(issues.to_a, filenames).select do |d|
+          issue = issue_map[d.issue_id]
+          # assign values we have already fetched
+          d.project = project
+          d.issue = issue
+          identifiers.include?(Identifier.new(filename: d.filename, issue_iid: issue.iid))
+        end
+      end
+
+      def relation_for_paths(paths)
+        super.includes(:route, :namespace, :group)
+      end
+
+      def parent_type
+        :project
+      end
+
+      # optimisation to reuse the parent_per_reference query information
+      def parent_from_ref(ref)
+        parent_per_reference[ref || current_parent_path]
+      end
+
+      def url_for_object(design, project)
+        path_options = { vueroute: design.filename }
+        Gitlab::Routing.url_helpers.designs_project_issue_path(project, design.issue, path_options)
+      end
+
+      def data_attributes_for(_text, _project, design, **_kwargs)
+        super.merge(issue: design.issue_id)
+      end
+
+      def self.object_class
+        ::DesignManagement::Design
+      end
+
+      def self.object_sym
+        :design
+      end
+
+      def self.parse_symbol(raw, match_data)
+        filename = parse_filename(raw, match_data)
+        Identifier.new(filename: filename, issue_iid: match_data[:issue].to_i)
+      end
+
+      def self.parse_filename(raw, match_data)
+        if efn = match_data[:escaped_filename]
+          efn.gsub(/(\\ \\ | \\ ")/x) { |x| x[1] }
+        elsif b64_name = match_data[:base_64_encoded_name]
+          Base64.decode64(b64_name)
+        elsif name = match_data[:simple_file_name]
+          name
+        else
+          raise "Unexpected name format: #{raw}"
+        end
+      end
+
+      def record_identifier(design)
+        Identifier.new(filename: design.filename, issue_iid: design.issue.iid)
+      end
+
+      private
+
+      def can_read_designs?(project)
+        DeclarativePolicy.user_scope { can?(current_user, :read_design, project) }
+      end
+
+      def designs(issues, filenames)
+        DesignManagement::Design.on_issue(issues).with_filename(filenames)
+      end
+
+      def enabled?
+        Feature.enabled?(:design_management_reference_filter_gfm_pipeline)
+      end
+    end
+  end
+end

ee/lib/ee/banzai/pipeline/gfm_pipeline.rb

@@ -10,6 +10,7 @@ module EE
           def reference_filters
             [
               ::Banzai::Filter::EpicReferenceFilter,
+              ::Banzai::Filter::DesignReferenceFilter,
               *super
             ]
           end

ee/spec/lib/banzai/filter/design_reference_filter_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Banzai::Filter::DesignReferenceFilter do
+  include FilterSpecHelper
+  include DesignManagementTestHelpers
+
+  # Persistent stuff we only want to create once
+  let_it_be(:issue)    { create(:issue) }
+  let_it_be(:issue_b)  { create(:issue, project: issue.project) }
+  let_it_be(:design_a) { create(:design, :with_versions, issue: issue) }
+  let_it_be(:design_b) { create(:design, :with_versions, issue: issue_b) }
+  let_it_be(:developer) { create(:user).tap { |u| issue.project.add_developer(u) } }
+
+  let_it_be(:project2)         { create(:project).tap { |p| p.add_developer(developer) } }
+  let_it_be(:issue2)           { create(:issue, project: project2) }
+  let_it_be(:x_project_design) { create(:design, :with_versions, issue: issue2) }
+
+  # Transitory stuff we can compute cheaply from other things
+  let(:design) { design_a }
+  let(:project) { issue.project }
+  let(:reference) { design.to_reference }
+  let(:input_text) { "Added #{reference}" }
+  let(:doc) { process_doc(input_text) }
+  let(:current_user) { developer }
+
+  def process_doc(text)
+    reference_filter(text, project: project, current_user: current_user)
+  end
+
+  def process(text)
+    process_doc(text).to_html
+  end
+
+  before do
+    enable_design_management
+  end
+
+  shared_examples 'a no-op filter' do
+    it 'does nothing' do
+      expect(process(input_text)).to eq(input_text)
+    end
+  end
+
+  describe '.call' do
+    it 'requires project context' do
+      expect { described_class.call('') }.to raise_error(ArgumentError, /:project/)
+    end
+  end
+
+  describe '#call' do
+    describe 'feature flags' do
+      context 'design management is not enabled' do
+        before do
+          enable_design_management(false, true)
+        end
+
+        it_behaves_like 'a no-op filter'
+      end
+
+      context 'design reference filter is not enabled' do
+        before do
+          enable_design_management(true, false)
+        end
+
+        it_behaves_like 'a no-op filter'
+      end
+    end
+  end
+
+  %w(pre code a style).each do |elem|
+    context "wrapped in a <#{elem}/>" do
+      let(:input_text) { "<#{elem}>Design #{design.to_reference}</#{elem}>" }
+
+      it_behaves_like 'a no-op filter'
+    end
+  end
+
+  describe '.parse_symbol' do
+    where(:filename) do
+      [
+        ['simple.png'],
+        ['SIMPLE.PNG'],
+        ['has spaces.png'],
+        ['has-hyphen.jpg'],
+        ['snake_case.svg'],
+        ['has ] right bracket.gif'],
+        [%q{has slashes \o/.png}],
+        [%q{has "quote" 'marks'.gif}],
+        [%q{<a href="has">html elements</a>.gif}]
+      ]
+    end
+
+    with_them do
+      where(:fullness) do
+        [
+          [true],
+          [false]
+        ]
+      end
+
+      with_them do
+        let(:design) { build(:design, issue: issue, filename: filename) }
+        let(:reference) { design.to_reference(full: fullness) }
+        let(:parsed) do
+          m = parse(reference)
+          described_class.parse_symbol(m[described_class.object_sym], m) if m
+        end
+
+        def parse(ref)
+          described_class.object_class.reference_pattern.match(ref)
+        end
+
+        it 'can parse the reference' do
+          expect(parsed).to have_attributes(
+            filename: filename,
+            issue_iid: issue.iid
+          )
+        end
+      end
+    end
+  end
+
+  describe '.object_sym' do
+    let(:subject) { described_class.object_sym }
+
+    it { is_expected.to eq(:design) }
+  end
+
+  describe '.object_class' do
+    let(:subject) { described_class.object_class }
+
+    it { is_expected.to eq(::DesignManagement::Design) }
+  end
+
+  describe '#data_attributes_for' do
+    let(:subject) { filter_instance.data_attributes_for(input_text, project, design) }
+
+    it do
+      is_expected.to include(issue: design.issue_id,
+                             original: input_text,
+                             project: project.id,
+                             design: design.id)
+    end
+  end
+
+  context 'a design with a quoted filename' do
+    let(:filename) { %q{A "very" good file.png} }
+    let(:design) { create(:design, :with_versions, issue: issue, filename: filename) }
+
+    it 'links to the design' do
+      expect(doc.css('a').first.attr('href'))
+        .to eq url_for_design(design)
+    end
+  end
+
+  context 'internal reference' do
+    it_behaves_like 'a reference containing an element node'
+
+    context "the reference is valid" do
+      context 'the user does not have permission to read this project' do
+        let(:current_user) { build_stubbed(:user) }
+
+        it_behaves_like 'a no-op filter'
+      end
+
+      context 'the user has permission' do
+        it 'produces a good link', :aggregate_failures do
+          link = doc.css('a').first
+
+          expect(link.attr('href')).to eq(url_for_design(design))
+          expect(link.attr('title')).to eq(design.filename)
+          expect(link.attr('class')).to eq('gfm gfm-design has-tooltip')
+          expect(link.attr('data-project')).to eq(project.id.to_s)
+          expect(link.attr('data-issue')).to eq(issue.id.to_s)
+          expect(link.attr('data-original')).to eq(reference)
+        end
+      end
+
+      context 'the filename needs to be escaped' do
+        let(:xss) do
+          <<~END
+            <script type="application/javascript">
+              alert('xss')
+            </script>
+          END
+        end
+
+        let(:filename) { %Q{#{xss}.png} }
+        let(:design) { create(:design, :with_versions, filename: filename, issue: issue) }
+
+        it 'leaves the text as is, but escapes the title', :aggregate_failures do
+          expect(doc.text).to eq(input_text)
+          expect(doc.css('a').first.attr('title')).to eq(design.filename)
+        end
+      end
+    end
+
+    context "the reference is to a non-existant design" do
+      let(:reference) { build(:design).to_reference }
+
+      it 'ignores it' do
+        expect(process(reference)).to eq(reference)
+      end
+    end
+  end
+
+  context 'cross-project / cross-namespace complete reference' do
+    let(:reference) { x_project_design.to_reference(project) }
+
+    it_behaves_like 'a reference containing an element node'
+
+    it 'links to a valid reference' do
+      expect(doc.css('a').first.attr('href'))
+        .to eq url_for_design(x_project_design)
+    end
+
+    context 'the current user does not have access to that project' do
+      let(:current_user) { build_stubbed(:user) }
+
+      it_behaves_like 'a no-op filter'
+    end
+
+    it 'link has valid text' do
+      expect(doc.css('a').first.text).to eql("#{project2.full_path}##{issue.iid}[#{x_project_design.filename}]")
+    end
+
+    it 'includes default classes' do
+      expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-design has-tooltip'
+    end
+
+    context 'the reference is invalid' do
+      let(:reference) { x_project_design.to_reference(project).gsub(/jpg/, 'gif') }
+
+      it_behaves_like 'a no-op filter'
+    end
+  end
+
+  describe 'performance' do
+    let!(:design_c) { create(:design, :with_versions, issue: issue) }
+    let!(:design_d) { create(:design, :with_versions, issue: issue_b) }
+    let!(:design_e) { create(:design, :with_versions, issue: create(:issue, project: x_project_design.project)) }
+
+    it 'is linear in the number of projects each design refers to' do
+      one_ref_per_project = <<~MD
+      Design #{design_a.to_reference}, #{x_project_design.to_reference(project)}
+      MD
+
+      multiple_references = <<~MD
+      Designs:
+       * #{design_a.to_reference}
+       * #{design_b.to_reference}
+       * #{design_c.to_reference}
+       * #{design_d.to_reference}
+       * #{x_project_design.to_reference(project)}
+       * #{design_e.to_reference(project)}
+       * #1[not a valid reference.gif]
+      MD
+
+      baseline = ActiveRecord::QueryRecorder.new { process(one_ref_per_project) }
+
+      # each project where the current_user has :read_design permission requires 5 queries:
+      #  * SELECT  "project_features".* FROM "project_features"
+      #     :in `feature_available?'*/
+      #  * SELECT MAX("project_authorizations"."access_level") ...
+      #     :in `block in max_member_access_for_user_ids'
+      #  * SELECT "issues".* FROM "issues" WHERE "issues"."project_id" = 1 AND ...
+      #      :in `parent_records'*/
+      #  * SELECT "_designs".* FROM "_designs"
+      #      WHERE "issue_id" IN (..) AND "filename" IN ('homescreen-1.jpg', ...)
+      #      :in `parent_records'*/
+      #  * SELECT  "routes".* FROM "routes" WHERE "routes"."source_id" = 1 AND "routes"."source_type" = 'Namespace' LIMIT 1
+      #      :in `full_path'*/
+      #  nb: routes is a polymorphic association, and cannot be optimised by the
+      #  use of `Project.includes(:route)
+      # and 3 if not:
+      #  * SELECT  "project_features".* FROM "project_features"
+      #     :in `feature_available?'*/
+      #  * SELECT MAX("project_authorizations"."access_level") ...
+      #     :in `block in max_member_access_for_user_ids'
+      #  * SELECT  "users".* FROM "users" WHERE "users"."id" = 5 LIMIT 1 :in `owner'*/
+      #  At this point we know that the current_user is not authorised.
+      #
+      # In addition there is a 1 query overhead for all the projects at the
+      # start. Currently, the baseline for 2 projects is `2 * 5 + 1 = 11` queries
+      #
+      expect { process(multiple_references) }.not_to exceed_query_limit(baseline.count)
+    end
+  end
+end

ee/spec/models/design_management/design_spec.rb

@@ -5,11 +5,11 @@ require 'spec_helper'
 describe DesignManagement::Design do
   include DesignManagementTestHelpers
 
-  set(:issue) { create(:issue) }
-  set(:design1) { create(:design, :with_versions, issue: issue, versions_count: 1) }
-  set(:design2) { create(:design, :with_versions, issue: issue, versions_count: 1) }
-  set(:design3) { create(:design, :with_versions, issue: issue, versions_count: 1) }
-  set(:deleted_design) { create(:design, :with_versions, deleted: true) }
+  let_it_be(:issue) { create(:issue) }
+  let_it_be(:design1) { create(:design, :with_versions, issue: issue, versions_count: 1) }
+  let_it_be(:design2) { create(:design, :with_versions, issue: issue, versions_count: 1) }
+  let_it_be(:design3) { create(:design, :with_versions, issue: issue, versions_count: 1) }
+  let_it_be(:deleted_design) { create(:design, :with_versions, deleted: true) }
 
   describe 'relations' do
     it { is_expected.to belong_to(:project) }
@@ -64,12 +64,13 @@ describe DesignManagement::Design do
   describe 'scopes' do
     describe '.visible_at_version' do
       let(:versions) { DesignManagement::Version.where(issue: issue).ordered }
+      let(:found) { described_class.visible_at_version(version) }
 
       context 'at oldest version' do
         let(:version) { versions.last }
 
         it 'finds the first design only' do
-          expect(described_class.visible_at_version(version)).to contain_exactly(design1)
+          expect(found).to contain_exactly(design1)
         end
       end
 
@@ -77,7 +78,7 @@ describe DesignManagement::Design do
         let(:version) { versions.second }
 
         it 'finds the first and second designs' do
-          expect(described_class.visible_at_version(version)).to contain_exactly(design1, design2)
+          expect(found).to contain_exactly(design1, design2)
         end
       end
 
@@ -85,7 +86,7 @@ describe DesignManagement::Design do
         let(:version) { versions.first }
 
         it 'finds designs' do
-          expect(described_class.visible_at_version(version)).to contain_exactly(design1, design2, design3)
+          expect(found).to contain_exactly(design1, design2, design3)
         end
       end
 
@@ -93,7 +94,7 @@ describe DesignManagement::Design do
         let(:version) { nil }
 
         it 'finds all undeleted designs' do
-          expect(described_class.visible_at_version(version)).to contain_exactly(design1, design2, design3)
+          expect(found).to contain_exactly(design1, design2, design3)
         end
       end
 
@@ -157,6 +158,18 @@ describe DesignManagement::Design do
       end
     end
 
+    describe '.on_issue' do
+      it 'returns correct designs when passed a single issue' do
+        expect(described_class.on_issue(issue)).to match_array(issue.designs)
+      end
+
+      it 'returns correct designs when passed an Array of issues' do
+        expect(
+          described_class.on_issue([issue, deleted_design.issue])
+        ).to contain_exactly(design1, design2, design3, deleted_design)
+      end
+    end
+
     describe '.current' do
       it 'returns just the undeleted designs' do
         delete_designs(design3)
@@ -370,4 +383,123 @@ describe DesignManagement::Design do
       subject.after_note_changed(build(:note, :system))
     end
   end
+
+  describe '.for_reference' do
+    let_it_be(:design_a) { create(:design) }
+    let_it_be(:design_b) { create(:design) }
+
+    it 'avoids extra queries when calling to_reference' do
+      designs = described_class.for_reference.where(id: [design_a.id, design_b.id]).to_a
+
+      expect { designs.map(&:to_reference) }.not_to exceed_query_limit(0)
+    end
+  end
+
+  describe '#to_reference' do
+    let(:namespace) { build(:namespace, path: 'sample-namespace') }
+    let(:project)   { build(:project, name: 'sample-project', namespace: namespace) }
+    let(:group)     { create(:group, name: 'Group', path: 'sample-group') }
+    let(:issue)     { build(:issue, iid: 1, project: project) }
+    let(:filename)  { 'homescreen.jpg' }
+    let(:design)    { build(:design, filename: filename, issue: issue, project: project) }
+
+    context 'when nil argument' do
+      let(:reference) { design.to_reference }
+
+      it 'uses the simple format' do
+        expect(reference).to eq "#1[homescreen.jpg]"
+      end
+
+      context 'when the filename contains spaces, hyphens, periods, single-quotes, underscores and colons' do
+        let(:filename) { %q{a complex filename: containing - _ : etc., but still 'simple'.gif} }
+
+        it 'uses the simple format' do
+          expect(reference).to eq "#1[#{filename}]"
+        end
+      end
+
+      context 'when the filename contains HTML angle brackets' do
+        let(:filename) { 'a <em>great</em> filename.jpg' }
+
+        it 'uses Base64 encoding' do
+          expect(reference).to eq "#1[base64:#{Base64.strict_encode64(filename)}]"
+        end
+      end
+
+      context 'when the filename contains quotation marks' do
+        let(:filename) { %q{a "great" filename.jpg} }
+
+        it 'uses enclosing quotes, with backslash encoding' do
+          expect(reference).to eq %q{#1["a \"great\" filename.jpg"]}
+        end
+      end
+
+      context 'when the filename contains square brackets' do
+        let(:filename) { %q{a [great] filename.jpg} }
+
+        it 'uses enclosing quotes' do
+          expect(reference).to eq %q{#1["a [great] filename.jpg"]}
+        end
+      end
+    end
+
+    context 'when full is true' do
+      it 'returns complete path to the issue' do
+        refs = [
+          design.to_reference(full: true),
+          design.to_reference(project, full: true),
+          design.to_reference(group, full: true)
+        ]
+
+        expect(refs).to all(eq 'sample-namespace/sample-project#1/designs[homescreen.jpg]')
+      end
+    end
+
+    context 'when full is false' do
+      it 'returns complete path to the issue' do
+        refs = [
+          design.to_reference(build(:project), full: false),
+          design.to_reference(group, full: false)
+        ]
+
+        expect(refs).to all(eq 'sample-namespace/sample-project#1[homescreen.jpg]')
+      end
+    end
+
+    context 'when same project argument' do
+      it 'returns bare reference' do
+        expect(design.to_reference(project)).to eq("#1[homescreen.jpg]")
+      end
+    end
+  end
+
+  describe 'reference_pattern' do
+    let(:match) { described_class.reference_pattern.match(ref) }
+    let(:ref) { design.to_reference }
+    let(:design) { build(:design, filename: filename) }
+
+    context 'simple_file_name' do
+      let(:filename) { 'simple-file-name.jpg' }
+
+      it 'matches :simple_file_name' do
+        expect(match[:simple_file_name]).to eq(filename)
+      end
+    end
+
+    context 'quoted_file_name' do
+      let(:filename) { 'simple "file" name.jpg' }
+
+      it 'matches :simple_file_name' do
+        expect(match[:escaped_filename].gsub(/\\"/, '"')).to eq(filename)
+      end
+    end
+
+    context 'Base64 name' do
+      let(:filename) { '<>.png' }
+
+      it 'matches base_64_encoded_name' do
+        expect(Base64.decode64(match[:base_64_encoded_name])).to eq(filename)
+      end
+    end
+  end
 end

ee/spec/support/helpers/design_management_test_helpers.rb

 # frozen_string_literal: true
 
 module DesignManagementTestHelpers
-  def enable_design_management(enabled = true)
+  def enable_design_management(enabled = true, ref_filter = true)
     stub_licensed_features(design_management: enabled)
     stub_lfs_setting(enabled: enabled)
+    stub_feature_flags(design_management_reference_filter_gfm_pipeline: ref_filter)
   end
 
   def delete_designs(*designs)
@@ -18,6 +19,11 @@ module DesignManagementTestHelpers
     act_on_designs(designs) { ::DesignManagement::Action.modification }
   end
 
+  def url_for_design(design)
+    path_options = { vueroute: design.filename }
+    Gitlab::Routing.url_helpers.designs_project_issue_path(design.project, design.issue, path_options)
+  end
+
   private
 
   def act_on_designs(designs, &block)

lib/banzai/filter/abstract_reference_filter.rb

@@ -43,15 +43,46 @@ module Banzai
       # Returns a String replaced with the return of the block.
       def self.references_in(text, pattern = object_class.reference_pattern)
         text.gsub(pattern) do |match|
-          symbol = $~[object_sym]
-          if object_class.reference_valid?(symbol)
-            yield match, symbol.to_i, $~[:project], $~[:namespace], $~
+          if ident = identifier($~)
+            yield match, ident, $~[:project], $~[:namespace], $~
           else
             match
           end
         end
       end
 
+      def self.identifier(match_data)
+        symbol = symbol_from_match(match_data)
+
+        parse_symbol(symbol, match_data) if object_class.reference_valid?(symbol)
+      end
+
+      def identifier(match_data)
+        self.class.identifier(match_data)
+      end
+
+      def self.symbol_from_match(match)
+        key = object_sym
+        match[key] if match.names.include?(key.to_s)
+      end
+
+      # Transform a symbol extracted from the text to a meaningful value
+      # In most cases these will be integers, so we call #to_i by default
+      #
+      # This method has the contract that if a string `ref` refers to a
+      # record `record`, then `parse_symbol(ref) == record_identifier(record)`.
+      def self.parse_symbol(symbol, match_data)
+        symbol.to_i
+      end
+
+      # We assume that most classes are identifying records by ID.
+      #
+      # This method has the contract that if a string `ref` refers to a
+      # record `record`, then `class.parse_symbol(ref) == record_identifier(record)`.
+      def record_identifier(record)
+        record.id
+      end
+
       def object_class
         self.class.object_class
       end
@@ -265,8 +296,10 @@ module Banzai
 
         @references_per[parent_type] ||= begin
           refs = Hash.new { |hash, key| hash[key] = Set.new }
-
-          regex = Regexp.union(object_class.reference_pattern, object_class.link_reference_pattern)
+          regex = [
+            object_class.reference_pattern,
+            object_class.link_reference_pattern
+          ].compact.reduce { |a, b| Regexp.union(a, b) }
 
           nodes.each do |node|
             node.to_html.scan(regex) do
@@ -276,8 +309,9 @@ module Banzai
                        full_group_path($~[:group])
                      end
 
-              symbol = $~[object_sym]
-              refs[path] << symbol if object_class.reference_valid?(symbol)
+              if ident = identifier($~)
+                refs[path] << ident
+              end
             end
           end
 

lib/banzai/filter/commit_reference_filter.rb

@@ -37,6 +37,11 @@ module Banzai
         end
       end
 
+      # The default behaviour is `#to_i` - we just pass the hash through.
+      def self.parse_symbol(sha_hash, _match)
+        sha_hash
+      end
+
       def url_for_object(commit, project)
         h = Gitlab::Routing.url_helpers
 
@@ -65,10 +70,6 @@ module Banzai
 
       private
 
-      def record_identifier(record)
-        record.id
-      end
-
       def parent_records(parent, ids)
         parent.commits_by(oids: ids.to_a)
       end

spec/lib/banzai/filter/abstract_reference_filter_spec.rb

@@ -3,30 +3,27 @@
 require 'spec_helper'
 
 describe Banzai::Filter::AbstractReferenceFilter do
-  let(:project) { create(:project) }
+  let_it_be(:project) { create(:project) }
+
+  let(:doc) { Nokogiri::HTML.fragment('') }
+  let(:filter) { described_class.new(doc, project: project) }
 
   describe '#references_per_parent' do
-    it 'returns a Hash containing references grouped per parent paths' do
-      doc = Nokogiri::HTML.fragment("#1 #{project.full_path}#2")
-      filter = described_class.new(doc, project: project)
+    let(:doc) { Nokogiri::HTML.fragment("#1 #{project.full_path}#2 #2") }
 
-      expect(filter).to receive(:object_class).exactly(4).times.and_return(Issue)
-      expect(filter).to receive(:object_sym).twice.and_return(:issue)
+    it 'returns a Hash containing references grouped per parent paths' do
+      expect(described_class).to receive(:object_class).exactly(6).times.and_return(Issue)
 
       refs = filter.references_per_parent
 
-      expect(refs).to be_an_instance_of(Hash)
-      expect(refs[project.full_path]).to eq(Set.new(%w[1 2]))
+      expect(refs).to match(a_hash_including(project.full_path => contain_exactly(1, 2)))
     end
   end
 
   describe '#parent_per_reference' do
     it 'returns a Hash containing projects grouped per parent paths' do
-      doc = Nokogiri::HTML.fragment('')
-      filter = described_class.new(doc, project: project)
-
       expect(filter).to receive(:references_per_parent)
-        .and_return({ project.full_path => Set.new(%w[1]) })
+        .and_return({ project.full_path => Set.new([1]) })
 
       expect(filter.parent_per_reference)
         .to eq({ project.full_path => project })
@@ -34,9 +31,6 @@ describe Banzai::Filter::AbstractReferenceFilter do
   end
 
   describe '#find_for_paths' do
-    let(:doc) { Nokogiri::HTML.fragment('') }
-    let(:filter) { described_class.new(doc, project: project) }
-
     context 'with RequestStore disabled' do
       it 'returns a list of Projects for a list of paths' do
         expect(filter.find_for_paths([project.full_path]))

spec/support/helpers/filter_spec_helper.rb

@@ -28,6 +28,17 @@ module FilterSpecHelper
     described_class.call(html, context)
   end
 
+  # Get an instance of the Filter class
+  #
+  # Use this for testing instance methods, but remember to test the result of
+  # the full pipeline by calling #call using the other methods in this helper.
+  def filter_instance
+    render_context = Banzai::RenderContext.new(project, current_user)
+    context = { project: project, current_user: current_user, render_context: render_context }
+
+    described_class.new(input_text, context)
+  end
+
   # Run text through HTML::Pipeline with the current filter and return the
   # result Hash
   #