Merge branch '41666-cannot-search-with-keyword-merge' into 'master'

Resolve "Cannot search with keyword "merge"" Closes #41666 See merge request gitlab-org/gitlab-ce!16462

Merge branch '41666-cannot-search-with-keyword-merge' into 'master'
Resolve "Cannot search with keyword "merge"" Closes #41666 See merge request gitlab-org/gitlab-ce!16462
a7d17262 · Rémy Coutable · 093856a7 · 91939161 · a7d17262 · a7d17262
--- a/app/helpers/blob_helper.rb
+++ b/app/helpers/blob_helper.rb
 module BlobHelper
  def highlight(blob_name, blob_content, repository: nil, plain: false)
+    plain ||= blob_content.length > Blob::MAXIMUM_TEXT_HIGHLIGHT_SIZE
    highlighted = Gitlab::Highlight.highlight(blob_name, blob_content, plain: plain, repository: repository)
+
    raw %(<pre class="code highlight"><code>#{highlighted}</code></pre>)
  end


--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -932,7 +932,7 @@ def search_files_by_content(query, ref)
    return [] if empty? || query.blank?

    offset = 2
-    args = %W(grep -i -I -n --before-context #{offset} --after-context #{offset} -E -e #{Regexp.escape(query)} #{ref || root_ref})
+    args = %W(grep -i -I -n -z --before-context #{offset} --after-context #{offset} -E -e #{Regexp.escape(query)} #{ref || root_ref})

    run_git(args).first.scrub.split(/^--$/)
  end

--- a/changelogs/unreleased/41666-cannot-search-with-keyword-merge-2.yml
+++ b/changelogs/unreleased/41666-cannot-search-with-keyword-merge-2.yml
+---
+title: Only highlight search results under the highlighting size limit
+merge_request: 16462
+author:
+type: performance
--- a/changelogs/unreleased/41666-cannot-search-with-keyword-merge.yml
+++ b/changelogs/unreleased/41666-cannot-search-with-keyword-merge.yml
+---
+title: Fix file search results when they match file contents with a number between
+  two colons
+merge_request: 16462
+author:
+type: fixed
--- a/lib/gitlab/project_search_results.rb
+++ b/lib/gitlab/project_search_results.rb
@@ -44,25 +44,20 @@ def self.parse_search_result(result)
      ref = nil
      filename = nil
      basename = nil
+      data = ""
      startline = 0

-      result.each_line.each_with_index do |line, index|
-        matches = line.match(/^(?<ref>[^:]*):(?<filename>.*):(?<startline>\d+):/)
-        if matches
+      result.strip.each_line.each_with_index do |line, index|
+        prefix ||= line.match(/^(?<ref>[^:]*):(?<filename>.*)\x00(?<startline>\d+)\x00/)&.tap do |matches|
          ref = matches[:ref]
          filename = matches[:filename]
          startline = matches[:startline]
          startline = startline.to_i - index
          extname = Regexp.escape(File.extname(filename))
          basename = filename.sub(/#{extname}$/, '')
-          break
        end
-      end
-
-      data = ""

-      result.each_line do |line|
-        data << line.sub(ref, '').sub(filename, '').sub(/^:-\d+-/, '').sub(/^::\d+:/, '')
+        data << line.sub(prefix.to_s, '')
      end

      FoundBlob.new(

--- a/spec/helpers/blob_helper_spec.rb
+++ b/spec/helpers/blob_helper_spec.rb
@@ -22,6 +22,13 @@ def test(input):
      expect(result).to eq(%[<pre class="code highlight"><code><span id="LC1" class="line" lang="">:type "assem"))</span></code></pre>])
    end

+    it 'returns plaintext for long blobs' do
+      stub_const('Blob::MAXIMUM_TEXT_HIGHLIGHT_SIZE', 1)
+      result = helper.highlight(blob_name, blob_content)
+
+      expect(result).to eq(%[<pre class="code highlight"><code><span id="LC1" class="line" lang="">(make-pathname :defaults name</span>\n<span id="LC2" class="line" lang="">:type "assem"))</span></code></pre>])
+    end
+
    it 'highlights single block' do
      expected = %Q[<pre class="code highlight"><code><span id="LC1" class="line" lang="common_lisp"><span class="p">(</span><span class="nb">make-pathname</span> <span class="ss">:defaults</span> <span class="nv">name</span></span>
 <span id="LC2" class="line" lang="common_lisp"><span class="ss">:type</span> <span class="s">"assem"</span><span class="p">))</span></span></code></pre>]

--- a/spec/lib/gitlab/project_search_results_spec.rb
+++ b/spec/lib/gitlab/project_search_results_spec.rb
@@ -70,15 +70,6 @@

      subject { described_class.parse_search_result(search_result) }

-      it 'can correctly parse filenames including ":"' do
-        special_char_result = "\nmaster:testdata/project::function1.yaml-1----\nmaster:testdata/project::function1.yaml:2:test: data1\n"
-
-        blob = described_class.parse_search_result(special_char_result)
-
-        expect(blob.ref).to eq('master')
-        expect(blob.filename).to eq('testdata/project::function1.yaml')
-      end
-
      it "returns a valid FoundBlob" do
        is_expected.to be_an Gitlab::SearchResults::FoundBlob
        expect(subject.id).to be_nil
@@ -90,8 +81,32 @@
        expect(subject.data.lines[2]).to eq("  - Feature: Replace teams with group membership\n")
      end

+      context 'when the matching filename contains a colon' do
+        let(:search_result) { "\nmaster:testdata/project::function1.yaml\x001\x00---\n" }
+
+        it 'returns a valid FoundBlob' do
+          expect(subject.filename).to eq('testdata/project::function1.yaml')
+          expect(subject.basename).to eq('testdata/project::function1')
+          expect(subject.ref).to eq('master')
+          expect(subject.startline).to eq(1)
+          expect(subject.data).to eq('---')
+        end
+      end
+
+      context 'when the matching content contains a number surrounded by colons' do
+        let(:search_result) { "\nmaster:testdata/foo.txt\x001\x00blah:9:blah" }
+
+        it 'returns a valid FoundBlob' do
+          expect(subject.filename).to eq('testdata/foo.txt')
+          expect(subject.basename).to eq('testdata/foo')
+          expect(subject.ref).to eq('master')
+          expect(subject.startline).to eq(1)
+          expect(subject.data).to eq('blah:9:blah')
+        end
+      end
+
      context "when filename has extension" do
-        let(:search_result) { "master:CONTRIBUTE.md:5:- [Contribute to GitLab](#contribute-to-gitlab)\n" }
+        let(:search_result) { "master:CONTRIBUTE.md\x005\x00- [Contribute to GitLab](#contribute-to-gitlab)\n" }

        it { expect(subject.path).to eq('CONTRIBUTE.md') }
        it { expect(subject.filename).to eq('CONTRIBUTE.md') }
@@ -99,7 +114,7 @@
      end

      context "when file under directory" do
-        let(:search_result) { "master:a/b/c.md:5:a b c\n" }
+        let(:search_result) { "master:a/b/c.md\x005\x00a b c\n" }

        it { expect(subject.path).to eq('a/b/c.md') }
        it { expect(subject.filename).to eq('a/b/c.md') }
@@ -144,7 +159,7 @@
    end

    it 'finds by content' do
-      expect(results).to include("master:Title.md:1:Content\n")
+      expect(results).to include("master:Title.md\x001\x00Content\n")
    end
  end


--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -657,7 +657,7 @@ def expect_to_raise_storage_error
      subject { results.first }

      it { is_expected.to be_an String }
-      it { expect(subject.lines[2]).to eq("master:CHANGELOG:190:  - Feature: Replace teams with group membership\n") }
+      it { expect(subject.lines[2]).to eq("master:CHANGELOG\x00190\x00  - Feature: Replace teams with group membership\n") }
    end
  end