Add latest changes from gitlab-org/gitlab@master

GITLAB_PAGES_VERSION

-1.15.0
+1.16.0

app/models/ci/job_artifact.rb

@@ -42,6 +42,7 @@ module Ci
       metrics: :gzip,
       metrics_referee: :gzip,
       network_referee: :gzip,
+      lsif: :gzip,
 
       # All these file formats use `raw` as we need to store them uncompressed
       # for Frontend to fetch the files and do analysis
@@ -53,8 +54,7 @@ module Ci
       dast: :raw,
       license_management: :raw,
       license_scanning: :raw,
-      performance: :raw,
-      lsif: :raw
+      performance: :raw
     }.freeze
 
     TYPE_AND_FORMAT_PAIRS = INTERNAL_TYPES.merge(REPORT_TYPES).freeze

app/models/issue.rb

@@ -128,12 +128,12 @@ class Issue < ApplicationRecord
   def self.reference_pattern
     @reference_pattern ||= %r{
       (#{Project.reference_pattern})?
-      #{Regexp.escape(reference_prefix)}(?<issue>\d+)
+      #{Regexp.escape(reference_prefix)}#{Gitlab::Regex.issue}
     }x
   end
 
   def self.link_reference_pattern
-    @link_reference_pattern ||= super("issues", /(?<issue>\d+)/)
+    @link_reference_pattern ||= super("issues", Gitlab::Regex.issue)
   end
 
   def self.reference_valid?(reference)

app/models/merge_request.rb

@@ -48,6 +48,7 @@ class MergeRequest < ApplicationRecord
   # 1. There are arguments - in which case we might be trying to force-reload.
   # 2. This association is already loaded.
   # 3. The latest diff does not exist.
+  # 4. It doesn't have any merge_request_diffs - it returns an empty MergeRequestDiff
   #
   # The second one in particular is important - MergeRequestDiff#merge_request
   # is the inverse of MergeRequest#merge_request_diff, which means it may not be
@@ -56,7 +57,7 @@ class MergeRequest < ApplicationRecord
   def merge_request_diff
     fallback = latest_merge_request_diff unless association(:merge_request_diff).loaded?
 
-    fallback || super
+    fallback || super || MergeRequestDiff.new(merge_request_id: id)
   end
 
   belongs_to :head_pipeline, foreign_key: "head_pipeline_id", class_name: "Ci::Pipeline"
@@ -404,7 +405,7 @@ class MergeRequest < ApplicationRecord
   end
 
   def commits(limit: nil)
-    return merge_request_diff.commits(limit: limit) if persisted?
+    return merge_request_diff.commits(limit: limit) if merge_request_diff.persisted?
 
     commits_arr = if compare_commits
                     reversed_commits = compare_commits.reverse
@@ -421,7 +422,7 @@ class MergeRequest < ApplicationRecord
   end
 
   def commits_count
-    if persisted?
+    if merge_request_diff.persisted?
       merge_request_diff.commits_count
     elsif compare_commits
       compare_commits.size
@@ -431,7 +432,7 @@ class MergeRequest < ApplicationRecord
   end
 
   def commit_shas(limit: nil)
-    return merge_request_diff.commit_shas(limit: limit) if persisted?
+    return merge_request_diff.commit_shas(limit: limit) if merge_request_diff.persisted?
 
     shas =
       if compare_commits
@@ -492,11 +493,11 @@ class MergeRequest < ApplicationRecord
   end
 
   def first_commit
-    merge_request_diff ? merge_request_diff.first_commit : compare_commits.first
+    compare_commits.present? ? compare_commits.first : merge_request_diff.first_commit
   end
 
   def raw_diffs(*args)
-    merge_request_diff ? merge_request_diff.raw_diffs(*args) : compare.raw_diffs(*args)
+    compare.present? ? compare.raw_diffs(*args) : merge_request_diff.raw_diffs(*args)
   end
 
   def diffs(diff_options = {})
@@ -557,7 +558,7 @@ class MergeRequest < ApplicationRecord
   end
 
   def diff_base_commit
-    if persisted?
+    if merge_request_diff.persisted?
       merge_request_diff.base_commit
     else
       branch_merge_base_commit
@@ -565,7 +566,7 @@ class MergeRequest < ApplicationRecord
   end
 
   def diff_start_commit
-    if persisted?
+    if merge_request_diff.persisted?
       merge_request_diff.start_commit
     else
       target_branch_head
@@ -573,7 +574,7 @@ class MergeRequest < ApplicationRecord
   end
 
   def diff_head_commit
-    if persisted?
+    if merge_request_diff.persisted?
       merge_request_diff.head_commit
     else
       source_branch_head
@@ -581,7 +582,7 @@ class MergeRequest < ApplicationRecord
   end
 
   def diff_start_sha
-    if persisted?
+    if merge_request_diff.persisted?
       merge_request_diff.start_commit_sha
     else
       target_branch_head.try(:sha)
@@ -589,7 +590,7 @@ class MergeRequest < ApplicationRecord
   end
 
   def diff_base_sha
-    if persisted?
+    if merge_request_diff.persisted?
       merge_request_diff.base_commit_sha
     else
       branch_merge_base_commit.try(:sha)
@@ -597,7 +598,7 @@ class MergeRequest < ApplicationRecord
   end
 
   def diff_head_sha
-    if persisted?
+    if merge_request_diff.persisted?
       merge_request_diff.head_commit_sha
     else
       source_branch_head.try(:sha)
@@ -758,7 +759,7 @@ class MergeRequest < ApplicationRecord
   end
 
   def ensure_merge_request_diff
-    merge_request_diff || create_merge_request_diff
+    merge_request_diff.persisted? || create_merge_request_diff
   end
 
   def create_merge_request_diff
@@ -1005,7 +1006,7 @@ class MergeRequest < ApplicationRecord
   def closes_issues(current_user = self.author)
     if target_branch == project.default_branch
       messages = [title, description]
-      messages.concat(commits.map(&:safe_message)) if merge_request_diff
+      messages.concat(commits.map(&:safe_message)) if merge_request_diff.persisted?
 
       Gitlab::ClosingIssueExtractor.new(project, current_user)
         .closed_by_message(messages.join("\n"))
@@ -1421,7 +1422,7 @@ class MergeRequest < ApplicationRecord
   end
 
   def has_commits?
-    merge_request_diff && commits_count.to_i > 0
+    merge_request_diff.persisted? && commits_count.to_i > 0
   end
 
   def has_no_commits?

app/models/project_services/issue_tracker_service.rb

@@ -19,9 +19,9 @@ class IssueTrackerService < Service
   # overridden patterns. See ReferenceRegexes.external_pattern
   def self.reference_pattern(only_long: false)
     if only_long
-      /(\b[A-Z][A-Z0-9_]*-)(?<issue>\d+)/
+      /(\b[A-Z][A-Z0-9_]*-)#{Gitlab::Regex.issue}/
     else
-      /(\b[A-Z][A-Z0-9_]*-|#{Issue.reference_prefix})(?<issue>\d+)/
+      /(\b[A-Z][A-Z0-9_]*-|#{Issue.reference_prefix})#{Gitlab::Regex.issue}/
     end
   end
 

app/models/project_services/youtrack_service.rb

@@ -8,7 +8,7 @@ class YoutrackService < IssueTrackerService
     if only_long
       /(?<issue>\b[A-Za-z][A-Za-z0-9_]*-\d+\b)/
     else
-      /(?<issue>\b[A-Za-z][A-Za-z0-9_]*-\d+\b)|(#{Issue.reference_prefix}(?<issue>\d+))/
+      /(?<issue>\b[A-Za-z][A-Za-z0-9_]*-\d+\b)|(#{Issue.reference_prefix}#{Gitlab::Regex.issue})/
     end
   end
 

changelogs/unreleased/feat-drop-signatures-email-replies.yml

+---
+title: Drop signatures in email replies
+merge_request: 25389
+author: Diego Louzán
+type: changed

changelogs/unreleased/pages-1-16-release.yml

+---
+title: Upgrade pages to 1.16.0
+merge_request: 25238
+author:
+type: added

doc/development/testing_guide/flaky_tests.md

@@ -87,6 +87,7 @@ For instance `RETRIES=1 bin/rspec ...` would retry the failing examples once.
   - [Dropdowns rendering upward or downward due to window size and scroll position](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/17660)
   - [Lazy loaded images can cause Capybara to misclick](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/18713)
 - [Triggering JS events before the event handlers are set up](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/18742)
+- [Wait for the image to be lazy-loaded when asserting on a Markdown image's src attribute](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/25408)
 
 #### Capybara viewport size related issues
 

lib/gitlab/email/attachment_uploader.rb

@@ -12,7 +12,7 @@ module Gitlab
       def execute(upload_parent:, uploader_class:)
         attachments = []
 
-        message.attachments.each do |attachment|
+        filter_signature_attachments(message).each do |attachment|
           tmp = Tempfile.new("gitlab-email-attachment")
           begin
             File.open(tmp.path, "w+b") { |f| f.write attachment.body.decoded }
@@ -32,6 +32,22 @@ module Gitlab
 
         attachments
       end
+
+      private
+
+      # If this is a signed message (e.g. S/MIME or PGP), remove the signature
+      # from the uploaded attachments
+      def filter_signature_attachments(message)
+        attachments = message.attachments
+
+        if message.content_type&.starts_with?('multipart/signed')
+          signature_protocol = message.content_type_parameters[:protocol]
+
+          attachments.delete_if { |attachment| attachment.content_type.starts_with?(signature_protocol) } if signature_protocol.present?
+        end
+
+        attachments
+      end
     end
   end
 end

lib/gitlab/regex.rb

@@ -112,7 +112,7 @@ module Gitlab
     # Based on Jira's project key format
     # https://confluence.atlassian.com/adminjiraserver073/changing-the-project-key-format-861253229.html
     def jira_issue_key_regex
-      @jira_issue_key_regex ||= /[A-Z][A-Z_0-9]+-\d+/
+      @jira_issue_key_regex ||= /[A-Z][A-Z_0-9]+-\d+\b/
     end
 
     def jira_transition_id_regex
@@ -144,6 +144,10 @@ module Gitlab
     def utc_date_regex
       @utc_date_regex ||= /\A[0-9]{4}-[0-9]{2}-[0-9]{2}\z/.freeze
     end
+
+    def issue
+      @issue ||= /(?<issue>\d+\b)/
+    end
   end
 end
 

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -77,6 +77,18 @@ describe Projects::MergeRequestsController do
         end
       end
 
+      context 'when diff is missing' do
+        render_views
+
+        it 'renders merge request page' do
+          merge_request.merge_request_diff.destroy
+
+          go(format: :html)
+
+          expect(response).to be_successful
+        end
+      end
+
       it "renders merge request page" do
         expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original
 

spec/factories/ci/job_artifacts.rb

@@ -141,11 +141,11 @@ FactoryBot.define do
 
     trait :lsif do
       file_type { :lsif }
-      file_format { :raw }
+      file_format { :gzip }
 
       after(:build) do |artifact, evaluator|
         artifact.file = fixture_file_upload(
-          Rails.root.join('spec/fixtures/lsif.json.gz'), 'application/octet-stream')
+          Rails.root.join('spec/fixtures/lsif.json.gz'), 'application/x-gzip')
       end
     end
 

spec/features/snippets/user_creates_snippet_spec.rb

@@ -53,7 +53,7 @@ describe 'User creates snippet', :js do
     page.within('#new_personal_snippet .md-preview-holder') do
       expect(page).to have_content('My Snippet')
 
-      link = find('a.no-attachment-icon img[alt="banana_sample"]')['src']
+      link = find('a.no-attachment-icon img.js-lazy-loaded[alt="banana_sample"]')['src']
       expect(link).to match(%r{/uploads/-/system/user/#{user.id}/\h{32}/banana_sample\.gif\z})
 
       # Adds a cache buster for checking if the image exists as Selenium is now handling the cached regquests
@@ -73,7 +73,7 @@ describe 'User creates snippet', :js do
     click_button('Create snippet')
     wait_for_requests
 
-    link = find('a.no-attachment-icon img[alt="banana_sample"]')['src']
+    link = find('a.no-attachment-icon img.js-lazy-loaded[alt="banana_sample"]')['src']
     expect(link).to match(%r{/uploads/-/system/personal_snippet/#{Snippet.last.id}/\h{32}/banana_sample\.gif\z})
 
     reqs = inspect_requests { visit("#{link}?ran=#{SecureRandom.base64(20)}") }
@@ -98,7 +98,7 @@ describe 'User creates snippet', :js do
       expect(page).to have_selector('strong')
     end
     expect(page).to have_content('Hello World!')
-    link = find('a.no-attachment-icon img[alt="banana_sample"]')['src']
+    link = find('a.no-attachment-icon img.js-lazy-loaded[alt="banana_sample"]')['src']
     expect(link).to match(%r{/uploads/-/system/personal_snippet/#{Snippet.last.id}/\h{32}/banana_sample\.gif\z})
 
     reqs = inspect_requests { visit("#{link}?ran=#{SecureRandom.base64(20)}") }

spec/fixtures/emails/.gitattributes

+# Do not mangle line endings or signature will be invalid
+valid_reply_signed_smime.eml eol=crlf
\ No newline at end of file

spec/lib/gitlab/email/attachment_uploader_spec.rb

@@ -16,5 +16,20 @@ describe Gitlab::Email::AttachmentUploader do
       expect(link[:alt]).to eq("bricks")
       expect(link[:url]).to include("bricks.png")
     end
+
+    context 'with a signed message' do
+      let(:message_raw) { fixture_file("emails/valid_reply_signed_smime.eml") }
+
+      it 'uploads all attachments except the signature' do
+        links = described_class.new(message).execute(upload_parent: project, uploader_class: FileUploader)
+
+        expect(links).not_to include(a_hash_including(alt: 'smime.p7s'))
+
+        image_link = links.first
+        expect(image_link).not_to be_nil
+        expect(image_link[:alt]).to eq('gitlab_logo')
+        expect(image_link[:url]).to include('gitlab_logo.png')
+      end
+    end
   end
 end

spec/lib/gitlab/email/handler_spec.rb

@@ -3,17 +3,23 @@
 require 'spec_helper'
 
 describe Gitlab::Email::Handler do
+  let(:email) { Mail.new { body 'email' } }
+
   describe '.for' do
     it 'picks issue handler if there is no merge request prefix' do
-      expect(described_class.for('email', 'project+key')).to be_an_instance_of(Gitlab::Email::Handler::CreateIssueHandler)
+      expect(described_class.for(email, 'project+key')).to be_an_instance_of(Gitlab::Email::Handler::CreateIssueHandler)
     end
 
     it 'picks merge request handler if there is merge request key' do
-      expect(described_class.for('email', 'project+merge-request+key')).to be_an_instance_of(Gitlab::Email::Handler::CreateMergeRequestHandler)
+      expect(described_class.for(email, 'project+merge-request+key')).to be_an_instance_of(Gitlab::Email::Handler::CreateMergeRequestHandler)
     end
 
     it 'returns nil if no handler is found' do
-      expect(described_class.for('email', '')).to be_nil
+      expect(described_class.for(email, '')).to be_nil
+    end
+
+    it 'returns nil if provided email is nil' do
+      expect(described_class.for(nil, '')).to be_nil
     end
   end
 
@@ -25,7 +31,7 @@ describe Gitlab::Email::Handler do
 
     it 'picks each handler at least once' do
       matched_handlers = addresses.map do |address|
-        described_class.for('email', address).class
+        described_class.for(email, address).class
       end
 
       expect(matched_handlers.uniq).to match_array(ce_handlers)
@@ -34,7 +40,7 @@ describe Gitlab::Email::Handler do
     it 'can pick exactly one handler for each address' do
       addresses.each do |address|
         matched_handlers = ce_handlers.select do |handler|
-          handler.new('email', address).can_handle?
+          handler.new(email, address).can_handle?
         end
 
         expect(matched_handlers.count).to eq(1), "#{address} matches #{matched_handlers.count} handlers: #{matched_handlers}"

spec/lib/gitlab/import_export/merge_request_parser_spec.rb

@@ -52,10 +52,10 @@ describe Gitlab::ImportExport::MergeRequestParser do
     context 'when the diff is invalid' do
       let(:merge_request_diff) { build(:merge_request_diff, merge_request: merge_request, base_commit_sha: 'foobar') }
 
-      it 'sets the diff to nil' do
+      it 'sets the diff to empty diff' do
         expect(merge_request_diff).to be_invalid
         expect(merge_request_diff.merge_request).to eq merge_request
-        expect(parsed_merge_request.merge_request_diff).to be_nil
+        expect(parsed_merge_request.merge_request_diff).to be_empty
       end
     end
   end

spec/models/merge_request_spec.rb

@@ -662,13 +662,12 @@ describe MergeRequest do
   end
 
   describe '#raw_diffs' do
-    let(:merge_request) { build(:merge_request) }
     let(:options) { { paths: ['a/b', 'b/a', 'c/*'] } }
 
     context 'when there are MR diffs' do
-      it 'delegates to the MR diffs' do
-        merge_request.merge_request_diff = MergeRequestDiff.new
+      let(:merge_request) { create(:merge_request, :with_diffs) }
 
+      it 'delegates to the MR diffs' do
         expect(merge_request.merge_request_diff).to receive(:raw_diffs).with(options)
 
         merge_request.raw_diffs(options)
@@ -676,6 +675,8 @@ describe MergeRequest do
     end
 
     context 'when there are no MR diffs' do
+      let(:merge_request) { build(:merge_request) }
+
       it 'delegates to the compare object' do
         merge_request.compare = double(:compare)
 

spec/models/project_services/youtrack_service_spec.rb

@@ -38,8 +38,8 @@ describe YoutrackService do
       expect(described_class.reference_pattern.match('YT-123')[:issue]).to eq('YT-123')
     end
 
-    it 'does not allow issue number to be followed by a letter' do
-      expect(described_class.reference_pattern.match('YT-123A')).to eq(nil)
+    it 'allows lowercase project key on the reference' do
+      expect(described_class.reference_pattern.match('yt-123')[:issue]).to eq('yt-123')
     end
   end
 

spec/support/shared_examples/models/issue_tracker_service_shared_examples.rb

@@ -21,4 +21,8 @@ RSpec.shared_examples 'allows project key on reference pattern' do |url_attr|
     expect(described_class.reference_pattern.match('3EXT_EXT-1234')).to eq nil
     expect(described_class.reference_pattern.match('EXT_EXT-1234')[0]).to eq 'EXT_EXT-1234'
   end
+
+  it 'does not allow issue number to finish with a letter' do
+    expect(described_class.reference_pattern.match('EXT-123A')).to eq(nil)
+  end
 end