From ef9eff8e7e1b38f48a354f90ecaeeb67da35b08c Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 14 Jun 2022 03:08:16 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../_git_lfs_limits.html.haml | 2 +- .../_import_export_limits.html.haml | 2 +- .../application_settings/_ip_limits.html.haml | 2 +- .../_issue_limits.html.haml | 2 +- .../_network_rate_limits.html.haml | 2 +- .../_note_limits.html.haml | 2 +- .../application_settings/_outbound.html.haml | 2 +- .../_performance.html.haml | 2 +- .../_pipeline_limits.html.haml | 2 +- .../_protected_paths.html.haml | 2 +- .../_search_limits.html.haml | 2 +- .../_users_api_limits.html.haml | 2 +- app/views/groups/runners/show.html.haml | 5 +- .../development/group_runner_view_ui.yml | 8 +++ ...20607140222_remove_invalid_integrations.rb | 2 +- doc/api/index.md | 1 + lib/gitlab/email/reply_parser.rb | 22 +++++- rubocop/cop/migration/migration_record.rb | 8 ++- spec/features/groups/group_runners_spec.rb | 40 ++++++----- .../service_desk_reply_illegal_utf8.eml | 2 +- spec/lib/gitlab/email/reply_parser_spec.rb | 67 +++++++++++++++++++ .../cop/migration/migration_record_spec.rb | 56 ++++++++-------- 22 files changed, 171 insertions(+), 64 deletions(-) create mode 100644 config/feature_flags/development/group_runner_view_ui.yml diff --git a/app/views/admin/application_settings/_git_lfs_limits.html.haml b/app/views/admin/application_settings/_git_lfs_limits.html.haml index b8970a5bcf1..7d47ca9a139 100644 --- a/app/views/admin/application_settings/_git_lfs_limits.html.haml +++ b/app/views/admin/application_settings/_git_lfs_limits.html.haml @@ -1,5 +1,5 @@ = gitlab_ui_form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-git-lfs-limits-settings'), html: { class: 'fieldset-form' } do |f| - = form_errors(@application_setting) + = form_errors(@application_setting, pajamas_alert: true) %fieldset %h5 diff --git a/app/views/admin/application_settings/_import_export_limits.html.haml b/app/views/admin/application_settings/_import_export_limits.html.haml index bc4a1577f90..4e774dd0a1e 100644 --- a/app/views/admin/application_settings/_import_export_limits.html.haml +++ b/app/views/admin/application_settings/_import_export_limits.html.haml @@ -1,5 +1,5 @@ = form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-import-export-limits-settings'), html: { class: 'fieldset-form' } do |f| - = form_errors(@application_setting) + = form_errors(@application_setting, pajamas_alert: true) %fieldset = html_escape(_("Set any rate limit to %{code_open}0%{code_close} to disable the limit.")) % { code_open: ''.html_safe, code_close: ''.html_safe } diff --git a/app/views/admin/application_settings/_ip_limits.html.haml b/app/views/admin/application_settings/_ip_limits.html.haml index 4362ae9cb9b..9a9038ef48e 100644 --- a/app/views/admin/application_settings/_ip_limits.html.haml +++ b/app/views/admin/application_settings/_ip_limits.html.haml @@ -1,5 +1,5 @@ = gitlab_ui_form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-ip-limits-settings'), html: { class: 'fieldset-form' } do |f| - = form_errors(@application_setting) + = form_errors(@application_setting, pajamas_alert: true) %fieldset = _("Rate limits can help reduce request volume (like from crawlers or abusive bots).") diff --git a/app/views/admin/application_settings/_issue_limits.html.haml b/app/views/admin/application_settings/_issue_limits.html.haml index 431e2a64c46..64aca50cbe9 100644 --- a/app/views/admin/application_settings/_issue_limits.html.haml +++ b/app/views/admin/application_settings/_issue_limits.html.haml @@ -1,5 +1,5 @@ = gitlab_ui_form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-issue-limits-settings'), html: { class: 'fieldset-form' } do |f| - = form_errors(@application_setting) + = form_errors(@application_setting, pajamas_alert: true) %fieldset .form-group diff --git a/app/views/admin/application_settings/_network_rate_limits.html.haml b/app/views/admin/application_settings/_network_rate_limits.html.haml index f1857a9749a..173e830c7da 100644 --- a/app/views/admin/application_settings/_network_rate_limits.html.haml +++ b/app/views/admin/application_settings/_network_rate_limits.html.haml @@ -1,5 +1,5 @@ = gitlab_ui_form_for @application_setting, url: network_admin_application_settings_path(anchor: anchor), html: { class: 'fieldset-form' } do |f| - = form_errors(@application_setting) + = form_errors(@application_setting, pajamas_alert: true) %fieldset = _("Rate limits can help reduce request volume (like from crawlers or abusive bots).") diff --git a/app/views/admin/application_settings/_note_limits.html.haml b/app/views/admin/application_settings/_note_limits.html.haml index 40760b3c45e..b783345b9df 100644 --- a/app/views/admin/application_settings/_note_limits.html.haml +++ b/app/views/admin/application_settings/_note_limits.html.haml @@ -1,5 +1,5 @@ = form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-note-limits-settings'), html: { class: 'fieldset-form' } do |f| - = form_errors(@application_setting) + = form_errors(@application_setting, pajamas_alert: true) %fieldset .form-group diff --git a/app/views/admin/application_settings/_outbound.html.haml b/app/views/admin/application_settings/_outbound.html.haml index 503e7d8afa6..2d91b777a0b 100644 --- a/app/views/admin/application_settings/_outbound.html.haml +++ b/app/views/admin/application_settings/_outbound.html.haml @@ -1,5 +1,5 @@ = gitlab_ui_form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-outbound-settings'), html: { class: 'fieldset-form' } do |f| - = form_errors(@application_setting) + = form_errors(@application_setting, pajamas_alert: true) %fieldset .form-group diff --git a/app/views/admin/application_settings/_performance.html.haml b/app/views/admin/application_settings/_performance.html.haml index e0ba8d93fbd..c87d166f8d9 100644 --- a/app/views/admin/application_settings/_performance.html.haml +++ b/app/views/admin/application_settings/_performance.html.haml @@ -1,5 +1,5 @@ = gitlab_ui_form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-performance-settings'), html: { class: 'fieldset-form' } do |f| - = form_errors(@application_setting) + = form_errors(@application_setting, pajamas_alert: true) %fieldset .form-group diff --git a/app/views/admin/application_settings/_pipeline_limits.html.haml b/app/views/admin/application_settings/_pipeline_limits.html.haml index e93823172db..3b33c41a924 100644 --- a/app/views/admin/application_settings/_pipeline_limits.html.haml +++ b/app/views/admin/application_settings/_pipeline_limits.html.haml @@ -1,5 +1,5 @@ = gitlab_ui_form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-pipeline-limits-settings'), html: { class: 'fieldset-form' } do |f| - = form_errors(@application_setting) + = form_errors(@application_setting, pajamas_alert: true) %fieldset .form-group diff --git a/app/views/admin/application_settings/_protected_paths.html.haml b/app/views/admin/application_settings/_protected_paths.html.haml index 1f3f67c71c7..00da0f59be4 100644 --- a/app/views/admin/application_settings/_protected_paths.html.haml +++ b/app/views/admin/application_settings/_protected_paths.html.haml @@ -1,5 +1,5 @@ = gitlab_ui_form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-protected-paths-settings'), html: { class: 'fieldset-form' } do |f| - = form_errors(@application_setting) + = form_errors(@application_setting, pajamas_alert: true) %fieldset .form-group diff --git a/app/views/admin/application_settings/_search_limits.html.haml b/app/views/admin/application_settings/_search_limits.html.haml index 945c9397f0d..93637b59d60 100644 --- a/app/views/admin/application_settings/_search_limits.html.haml +++ b/app/views/admin/application_settings/_search_limits.html.haml @@ -1,5 +1,5 @@ = form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-search-limits-settings'), html: { class: 'fieldset-form' } do |f| - = form_errors(@application_setting) + = form_errors(@application_setting, pajamas_alert: true) %fieldset .form-group diff --git a/app/views/admin/application_settings/_users_api_limits.html.haml b/app/views/admin/application_settings/_users_api_limits.html.haml index 3918c76b12c..f2edb81141d 100644 --- a/app/views/admin/application_settings/_users_api_limits.html.haml +++ b/app/views/admin/application_settings/_users_api_limits.html.haml @@ -1,5 +1,5 @@ = form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-users-api-limits-settings'), html: { class: 'fieldset-form' } do |f| - = form_errors(@application_setting) + = form_errors(@application_setting, pajamas_alert: true) %fieldset .form-group diff --git a/app/views/groups/runners/show.html.haml b/app/views/groups/runners/show.html.haml index b6c0c8a707f..5a9d2ca858e 100644 --- a/app/views/groups/runners/show.html.haml +++ b/app/views/groups/runners/show.html.haml @@ -1,3 +1,6 @@ - add_to_breadcrumbs _('Runners'), group_runners_path(@group) -= render 'shared/runners/runner_details', runner: @runner +- if Feature.enabled?(:group_runner_view_ui) + #js-group-runner-show{ data: {runner_id: @runner.id, runners_path: group_runners_path(@group)} } +- else + = render 'shared/runners/runner_details', runner: @runner diff --git a/config/feature_flags/development/group_runner_view_ui.yml b/config/feature_flags/development/group_runner_view_ui.yml new file mode 100644 index 00000000000..f3a9eb15f0f --- /dev/null +++ b/config/feature_flags/development/group_runner_view_ui.yml @@ -0,0 +1,8 @@ +--- +name: group_runner_view_ui +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/89638/ +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/364811 +milestone: '15.1' +type: development +group: group::runner +default_enabled: false diff --git a/db/post_migrate/20220607140222_remove_invalid_integrations.rb b/db/post_migrate/20220607140222_remove_invalid_integrations.rb index 6eb192fc7fe..49834d7b120 100644 --- a/db/post_migrate/20220607140222_remove_invalid_integrations.rb +++ b/db/post_migrate/20220607140222_remove_invalid_integrations.rb @@ -15,7 +15,7 @@ class RemoveInvalidIntegrations < Gitlab::Database::Migration[2.0] end # Isolated version of the Integration model - class Integration < ApplicationRecord + class Integration < MigrationRecord self.table_name = 'integrations' self.inheritance_column = :_type_disabled end diff --git a/doc/api/index.md b/doc/api/index.md index f7c835f2128..c29e43d3f8e 100644 --- a/doc/api/index.md +++ b/doc/api/index.md @@ -127,6 +127,7 @@ There are several ways you can authenticate with the GitLab API: - [OAuth2 tokens](#oauth2-tokens) - [Personal access tokens](../user/profile/personal_access_tokens.md) - [Project access tokens](../user/project/settings/project_access_tokens.md) +- [Group access tokens](../user/group/settings/group_access_tokens.md) - [Session cookie](#session-cookie) - [GitLab CI/CD job token](../ci/jobs/ci_job_token.md) **(Specific endpoints only)** diff --git a/lib/gitlab/email/reply_parser.rb b/lib/gitlab/email/reply_parser.rb index d39fa139abb..c2d645138d7 100644 --- a/lib/gitlab/email/reply_parser.rb +++ b/lib/gitlab/email/reply_parser.rb @@ -33,10 +33,10 @@ module Gitlab l.strip.empty? || (!allow_only_quotes && l.start_with?('>')) end - encoded_body = body.force_encoding(encoding).encode("UTF-8") + encoded_body = force_utf8(body.force_encoding(encoding)) return encoded_body unless @append_reply - [encoded_body, stripped_text.force_encoding(encoding).encode("UTF-8")] + [encoded_body, force_utf8(stripped_text.force_encoding(encoding))] end private @@ -70,13 +70,29 @@ module Gitlab return if object.nil? if object.charset - object.body.decoded.force_encoding(object.charset.gsub(/utf8/i, "UTF-8")).encode("UTF-8").to_s + # A part of a multi-part may have a different encoding. Its encoding + # is denoted in its header. For example: + # + # ``` + # ------=_Part_2192_32400445.1115745999735 + # Content-Type: text/plain; charset=ISO-8859-1 + # Content-Transfer-Encoding: 7bit + # + # Plain email. + # ``` + # So, we had to force its part to corresponding encoding before able + # to convert it to UTF-8 + force_utf8(object.body.decoded.force_encoding(object.charset.gsub(/utf8/i, "UTF-8"))) else object.body.to_s end rescue StandardError nil end + + def force_utf8(str) + Gitlab::EncodingHelper.encode_utf8(str).to_s + end end end end diff --git a/rubocop/cop/migration/migration_record.rb b/rubocop/cop/migration/migration_record.rb index bb81b3cbaf1..291644f10e3 100644 --- a/rubocop/cop/migration/migration_record.rb +++ b/rubocop/cop/migration/migration_record.rb @@ -11,7 +11,7 @@ module RuboCop ENFORCED_SINCE = 2022_04_26_00_00_00 MSG = <<~MSG - Don't inherit from ActiveRecord::Base but use MigrationRecord instead. + Don't inherit from ActiveRecord::Base or ApplicationRecord but use MigrationRecord instead. See https://docs.gitlab.com/ee/development/database/migrations_for_multiple_databases.html#example-usage-of-activerecord-classes. MSG @@ -19,9 +19,13 @@ module RuboCop (class _ (const (const _ :ActiveRecord) :Base) _) PATTERN + def_node_search :inherits_from_application_record?, <<~PATTERN + (class _ (const _ :ApplicationRecord) _) + PATTERN + def on_class(node) return unless relevant_migration?(node) - return unless inherits_from_active_record_base?(node) + return unless inherits_from_active_record_base?(node) || inherits_from_application_record?(node) add_offense(node, location: :expression) end diff --git a/spec/features/groups/group_runners_spec.rb b/spec/features/groups/group_runners_spec.rb index ec088f60b80..a60b8a60da0 100644 --- a/spec/features/groups/group_runners_spec.rb +++ b/spec/features/groups/group_runners_spec.rb @@ -149,31 +149,37 @@ RSpec.describe "Group Runners" do create(:ci_runner, :group, groups: [group], description: 'runner-foo', contacted_at: Time.zone.now) end - it 'user edits the runner to be protected' do - visit edit_group_runner_path(group, runner) - - expect(page.find_field('runner[access_level]')).not_to be_checked - - check 'runner_access_level' - click_button 'Save changes' - - expect(page).to have_content 'Protected Yes' - end - - context 'when a runner has a tag' do + context 'when group_runner_view_ui is disabled' do before do - runner.update!(tag_list: ['tag']) + stub_feature_flags(group_runner_view_ui: false) end - it 'user edits runner not to run untagged jobs' do + it 'user edits the runner to be protected' do visit edit_group_runner_path(group, runner) - expect(page.find_field('runner[run_untagged]')).to be_checked + expect(page.find_field('runner[access_level]')).not_to be_checked - uncheck 'runner_run_untagged' + check 'runner_access_level' click_button 'Save changes' - expect(page).to have_content 'Can run untagged jobs No' + expect(page).to have_content 'Protected Yes' + end + + context 'when a runner has a tag' do + before do + runner.update!(tag_list: ['tag']) + end + + it 'user edits runner not to run untagged jobs' do + visit edit_group_runner_path(group, runner) + + expect(page.find_field('runner[run_untagged]')).to be_checked + + uncheck 'runner_run_untagged' + click_button 'Save changes' + + expect(page).to have_content 'Can run untagged jobs No' + end end end end diff --git a/spec/fixtures/emails/service_desk_reply_illegal_utf8.eml b/spec/fixtures/emails/service_desk_reply_illegal_utf8.eml index c6e01cdddeb..8350348ae5f 100644 --- a/spec/fixtures/emails/service_desk_reply_illegal_utf8.eml +++ b/spec/fixtures/emails/service_desk_reply_illegal_utf8.eml @@ -11,7 +11,7 @@ In-Reply-To: Subject: The message subject! @all Mime-Version: 1.0 Content-Type: text/plain; - charset=ISO-8859-1 + charset=Shift_JIS Content-Transfer-Encoding: 7bit X-Sieve: CMU Sieve 2.2 X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu, diff --git a/spec/lib/gitlab/email/reply_parser_spec.rb b/spec/lib/gitlab/email/reply_parser_spec.rb index c0d177aff4d..c61d941406b 100644 --- a/spec/lib/gitlab/email/reply_parser_spec.rb +++ b/spec/lib/gitlab/email/reply_parser_spec.rb @@ -268,5 +268,72 @@ RSpec.describe Gitlab::Email::ReplyParser do expect(test_parse_body(fixture_file("emails/valid_new_issue_with_quote.eml"), { append_reply: true })) .to contain_exactly(body, reply) end + + context 'non-UTF-8 content' do + let(:charset) { '; charset=Shift_JIS' } + let(:raw_content) do + <<-BODY.strip_heredoc.chomp + From: Jake the Dog + To: incoming+email-test-project_id-issue-@appmail.adventuretime.ooo + Message-ID: + Subject: The message subject! @all + Content-Type: text/plain#{charset} + Content-Transfer-Encoding: 8bit + + こんにちは。 この世界は素晴らしいです。 + BODY + end + + # Strip encoding to simulate the case when Ruby fallback to ASCII-8bit + # when it meets an unknown encoding + let(:encoded_content) { raw_content.encode("Shift_JIS").bytes.pack("c*") } + + it "parses body under UTF-8 encoding" do + expect(test_parse_body(encoded_content)) + .to eq(<<-BODY.strip_heredoc.chomp) + こんにちは。 この世界は素晴らしいです。 + BODY + end + + # This test would raise an exception if encoding is not handled properly + # Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/364329 + context 'charset is absent and reply trimming is disabled' do + let(:charset) { '' } + + it "parses body under UTF-8 encoding" do + expect(test_parse_body(encoded_content, { trim_reply: false })) + .to eq(<<-BODY.strip_heredoc.chomp) + こんにちは。 この世界は素晴らしいです。 + BODY + end + end + + context 'multipart email' do + let(:raw_content) do + <<-BODY.strip_heredoc.chomp + From: Jake the Dog + To: incoming+email-test-project_id-issue-@appmail.adventuretime.ooo + Message-ID: + Subject: The message subject! @all + Content-Type: multipart/alternative; + boundary=Apple-Mail-B41C7F8E-3639-49B0-A5D5-440E125A7105 + Content-Transfer-Encoding: 7bbit + + --Apple-Mail-B41C7F8E-3639-49B0-A5D5-440E125A7105 + Content-Type: text/plain + Content-Transfer-Encodng: 7bit + + こんにちは。 この世界は素晴らしいです。 + BODY + end + + it "parses body under UTF-8 encoding" do + expect(test_parse_body(encoded_content, { trim_reply: false })) + .to eq(<<-BODY.strip_heredoc.chomp) + こんにちは。 この世界は素晴らしいです。 + BODY + end + end + end end end diff --git a/spec/rubocop/cop/migration/migration_record_spec.rb b/spec/rubocop/cop/migration/migration_record_spec.rb index bab0ca469df..bfe6228c421 100644 --- a/spec/rubocop/cop/migration/migration_record_spec.rb +++ b/spec/rubocop/cop/migration/migration_record_spec.rb @@ -6,53 +6,55 @@ require_relative '../../../../rubocop/cop/migration/migration_record' RSpec.describe RuboCop::Cop::Migration::MigrationRecord do subject(:cop) { described_class.new } - shared_examples 'a disabled cop' do + shared_examples 'a disabled cop' do |klass| it 'does not register any offenses' do expect_no_offenses(<<~SOURCE) class MyMigration < Gitlab::Database::Migration[2.0] - class Project < ActiveRecord::Base + class Project < #{klass} end end SOURCE end end - context 'outside of a migration' do - it_behaves_like 'a disabled cop' - end - - context 'in migration' do - before do - allow(cop).to receive(:in_migration?).and_return(true) + %w(ActiveRecord::Base ApplicationRecord).each do |klass| + context 'outside of a migration' do + it_behaves_like 'a disabled cop', klass end - context 'in an old migration' do + context 'in migration' do before do - allow(cop).to receive(:version).and_return(described_class::ENFORCED_SINCE - 5) + allow(cop).to receive(:in_migration?).and_return(true) end - it_behaves_like 'a disabled cop' - end + context 'in an old migration' do + before do + allow(cop).to receive(:version).and_return(described_class::ENFORCED_SINCE - 5) + end - context 'that is recent' do - before do - allow(cop).to receive(:version).and_return(described_class::ENFORCED_SINCE) + it_behaves_like 'a disabled cop', klass end - it 'adds an offense if inheriting from ActiveRecord::Base' do - expect_offense(<<~RUBY) - class Project < ActiveRecord::Base - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't inherit from ActiveRecord::Base but use MigrationRecord instead.[...] + context 'that is recent' do + before do + allow(cop).to receive(:version).and_return(described_class::ENFORCED_SINCE) + end + + it "adds an offense if inheriting from #{klass}" do + expect_offense(<<~RUBY) + class Project < #{klass} + ^^^^^^^^^^^^^^^^#{'^' * klass.length} Don't inherit from ActiveRecord::Base or ApplicationRecord but use MigrationRecord instead.[...] end - RUBY - end + RUBY + end - it 'adds an offense if inheriting from ::ActiveRecord::Base' do - expect_offense(<<~RUBY) - class Project < ::ActiveRecord::Base - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't inherit from ActiveRecord::Base but use MigrationRecord instead.[...] + it "adds an offense if inheriting from ::#{klass}" do + expect_offense(<<~RUBY) + class Project < ::#{klass} + ^^^^^^^^^^^^^^^^^^#{'^' * klass.length} Don't inherit from ActiveRecord::Base or ApplicationRecord but use MigrationRecord instead.[...] end - RUBY + RUBY + end end end end