diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-01-31 21:10:00 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-01-31 21:10:00 +0300 |
commit | 22dde36e800253350e5fa1d902f191a7f64bc6e9 (patch) | |
tree | 3b53aee41daed5efa0674f9ee2da83e17ef4e676 /spec | |
parent | 6f18a8d0b00eae84d262dff137fddd9639f3c52a (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
24 files changed, 539 insertions, 102 deletions
diff --git a/spec/features/issues/user_bulk_edits_issues_spec.rb b/spec/features/issues/user_bulk_edits_issues_spec.rb index fc48bc4baf9..5696bde4069 100644 --- a/spec/features/issues/user_bulk_edits_issues_spec.rb +++ b/spec/features/issues/user_bulk_edits_issues_spec.rb @@ -46,7 +46,7 @@ RSpec.describe 'Multiple issue updating from issues#index', :js, feature_categor click_button 'Edit issues' check 'Select all' click_update_assignee_button - click_link user.username + click_button user.username click_update_issues_button @@ -64,7 +64,7 @@ RSpec.describe 'Multiple issue updating from issues#index', :js, feature_categor click_button 'Edit issues' check 'Select all' click_update_assignee_button - click_link 'Unassigned' + click_button 'Unassigned' click_update_issues_button expect(find('.issue:first-of-type')).not_to have_link "Assigned to #{user.name}" diff --git a/spec/features/merge_requests/user_mass_updates_spec.rb b/spec/features/merge_requests/user_mass_updates_spec.rb index 5a9054ece48..b0be76d386a 100644 --- a/spec/features/merge_requests/user_mass_updates_spec.rb +++ b/spec/features/merge_requests/user_mass_updates_spec.rb @@ -121,7 +121,7 @@ RSpec.describe 'Merge requests > User mass updates', :js, feature_category: :cod within 'aside[aria-label="Bulk update"]' do click_button 'Select assignee' wait_for_requests - click_link text + click_button text end click_update_merge_requests_button end diff --git a/spec/fixtures/packages/helm/corrupted_chart.tgz b/spec/fixtures/packages/helm/corrupted_chart.tgz Binary files differnew file mode 100644 index 00000000000..b2ac93b271e --- /dev/null +++ b/spec/fixtures/packages/helm/corrupted_chart.tgz diff --git a/spec/fixtures/safe_zip/invalid-unexpected-large.zip b/spec/fixtures/safe_zip/invalid-unexpected-large.zip Binary files differnew file mode 100644 index 00000000000..3005da8c779 --- /dev/null +++ b/spec/fixtures/safe_zip/invalid-unexpected-large.zip diff --git a/spec/fixtures/safe_zip/valid-symlinks-first.zip b/spec/fixtures/safe_zip/valid-symlinks-first.zip Binary files differindex f5952ef71c9..1d7ecfd7bed 100644 --- a/spec/fixtures/safe_zip/valid-symlinks-first.zip +++ b/spec/fixtures/safe_zip/valid-symlinks-first.zip diff --git a/spec/frontend/frequent_items/components/frequent_items_list_item_spec.js b/spec/frontend/frequent_items/components/frequent_items_list_item_spec.js index 4f2badf869d..bbc27a621ea 100644 --- a/spec/frontend/frequent_items/components/frequent_items_list_item_spec.js +++ b/spec/frontend/frequent_items/components/frequent_items_list_item_spec.js @@ -154,7 +154,7 @@ describe('FrequentItemsListItemComponent', () => { link.vm.$emit('click'); expect(trackingSpy).toHaveBeenCalledWith(undefined, 'click_link', { - label: 'projects_dropdown_frequent_items_list_item_git_lab_community_edition', + label: 'projects_dropdown_frequent_items_list_item', }); }); }); diff --git a/spec/frontend/super_sidebar/components/help_center_spec.js b/spec/frontend/super_sidebar/components/help_center_spec.js new file mode 100644 index 00000000000..f1db755a711 --- /dev/null +++ b/spec/frontend/super_sidebar/components/help_center_spec.js @@ -0,0 +1,87 @@ +import { GlDisclosureDropdown } from '@gitlab/ui'; +import { within } from '@testing-library/dom'; +import toggleWhatsNewDrawer from '~/whats_new'; +import { mountExtended } from 'helpers/vue_test_utils_helper'; +import HelpCenter from '~/super_sidebar/components/help_center.vue'; +import { helpPagePath } from '~/helpers/help_page_helper'; +import { PROMO_URL } from 'jh_else_ce/lib/utils/url_utility'; +import { sidebarData } from '../mock_data'; + +jest.mock('~/whats_new'); + +describe('HelpCenter component', () => { + let wrapper; + + const findDropdown = () => wrapper.findComponent(GlDisclosureDropdown); + const withinComponent = () => within(wrapper.element); + const findButton = (name) => withinComponent().getByRole('button', { name }); + + const createWrapper = () => { + wrapper = mountExtended(HelpCenter, { + propsData: { sidebarData }, + }); + }; + + describe('default', () => { + beforeEach(() => { + createWrapper(); + }); + + it('renders menu items', () => { + expect(findDropdown().props('items')[0].items).toEqual([ + { text: HelpCenter.i18n.help, href: helpPagePath() }, + { text: HelpCenter.i18n.support, href: sidebarData.support_path }, + { text: HelpCenter.i18n.docs, href: 'https://docs.gitlab.com' }, + { text: HelpCenter.i18n.plans, href: `${PROMO_URL}/pricing` }, + { text: HelpCenter.i18n.forum, href: 'https://forum.gitlab.com/' }, + { + text: HelpCenter.i18n.contribute, + href: helpPagePath('', { anchor: 'contributing-to-gitlab' }), + }, + { text: HelpCenter.i18n.feedback, href: 'https://about.gitlab.com/submit-feedback' }, + ]); + + expect(findDropdown().props('items')[1].items).toEqual([ + expect.objectContaining({ text: HelpCenter.i18n.shortcuts }), + expect.objectContaining({ text: HelpCenter.i18n.whatsnew }), + ]); + }); + + describe('showKeyboardShortcuts', () => { + beforeEach(() => { + jest.spyOn(wrapper.vm.$refs.dropdown, 'close'); + window.toggleShortcutsHelp = jest.fn(); + findButton('Keyboard shortcuts').click(); + }); + + it('closes the dropdown', () => { + expect(wrapper.vm.$refs.dropdown.close).toHaveBeenCalled(); + }); + + it('shows the keyboard shortcuts modal', () => { + expect(window.toggleShortcutsHelp).toHaveBeenCalled(); + }); + }); + + describe('showWhatsNew', () => { + beforeEach(() => { + jest.spyOn(wrapper.vm.$refs.dropdown, 'close'); + findButton("What's new").click(); + }); + + it('closes the dropdown', () => { + expect(wrapper.vm.$refs.dropdown.close).toHaveBeenCalled(); + }); + + it('shows the "What\'s new" slideout', () => { + expect(toggleWhatsNewDrawer).toHaveBeenCalledWith(expect.any(Object)); + }); + + it('shows the existing "What\'s new" slideout instance on subsequent clicks', () => { + findButton("What's new").click(); + expect(toggleWhatsNewDrawer).toHaveBeenCalledTimes(2); + expect(toggleWhatsNewDrawer).toHaveBeenLastCalledWith(); + }); + }); + }); +}); diff --git a/spec/frontend/super_sidebar/components/super_sidebar_spec.js b/spec/frontend/super_sidebar/components/super_sidebar_spec.js index 86ba1c1ea45..45fc30c08f0 100644 --- a/spec/frontend/super_sidebar/components/super_sidebar_spec.js +++ b/spec/frontend/super_sidebar/components/super_sidebar_spec.js @@ -1,5 +1,6 @@ import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import SuperSidebar from '~/super_sidebar/components/super_sidebar.vue'; +import HelpCenter from '~/super_sidebar/components/help_center.vue'; import UserBar from '~/super_sidebar/components/user_bar.vue'; import { sidebarData } from '../mock_data'; @@ -7,6 +8,7 @@ describe('SuperSidebar component', () => { let wrapper; const findUserBar = () => wrapper.findComponent(UserBar); + const findHelpCenter = () => wrapper.findComponent(HelpCenter); const createWrapper = (props = {}) => { wrapper = shallowMountExtended(SuperSidebar, { @@ -25,5 +27,9 @@ describe('SuperSidebar component', () => { it('renders UserBar with sidebarData', () => { expect(findUserBar().props('sidebarData')).toBe(sidebarData); }); + + it('renders HelpCenter with sidebarData', () => { + expect(findHelpCenter().props('sidebarData')).toBe(sidebarData); + }); }); }); diff --git a/spec/frontend/super_sidebar/mock_data.js b/spec/frontend/super_sidebar/mock_data.js index bdbc25e49f0..379e4c2bffb 100644 --- a/spec/frontend/super_sidebar/mock_data.js +++ b/spec/frontend/super_sidebar/mock_data.js @@ -48,4 +48,6 @@ export const sidebarData = { todos_pending_count: 3, issues_dashboard_path: 'path/to/issues', create_new_menu_groups: createNewMenuGroups, + support_path: '/support', + display_whats_new: true, }; diff --git a/spec/frontend/vue_shared/components/user_select_spec.js b/spec/frontend/vue_shared/components/user_select_spec.js index 874796f653a..b0e9584a15b 100644 --- a/spec/frontend/vue_shared/components/user_select_spec.js +++ b/spec/frontend/vue_shared/components/user_select_spec.js @@ -285,6 +285,20 @@ describe('User select dropdown', () => { expect(wrapper.emitted('input')).toEqual([[[]]]); }); + it('hides the dropdown after clicking on `Unassigned`', async () => { + createComponent({ + props: { + value: [assignee], + }, + }); + wrapper.vm.$refs.dropdown.hide = jest.fn(); + await waitForPromises(); + + findUnassignLink().trigger('click'); + + expect(wrapper.vm.$refs.dropdown.hide).toHaveBeenCalledTimes(1); + }); + it('emits an empty array after unselecting the only selected assignee', async () => { createComponent({ props: { diff --git a/spec/helpers/sidebars_helper_spec.rb b/spec/helpers/sidebars_helper_spec.rb index fcaff83ada4..ecbc1597bdf 100644 --- a/spec/helpers/sidebars_helper_spec.rb +++ b/spec/helpers/sidebars_helper_spec.rb @@ -66,7 +66,9 @@ RSpec.describe SidebarsHelper do assigned_open_issues_count: 1, assigned_open_merge_requests_count: 2, todos_pending_count: 3, - issues_dashboard_path: issues_dashboard_path(assignee_username: user.username) + issues_dashboard_path: issues_dashboard_path(assignee_username: user.username), + support_path: helper.support_url, + display_whats_new: helper.display_whats_new? }) end diff --git a/spec/lib/api/entities/draft_note_spec.rb b/spec/lib/api/entities/draft_note_spec.rb new file mode 100644 index 00000000000..59555319bb1 --- /dev/null +++ b/spec/lib/api/entities/draft_note_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Entities::DraftNote, feature_category: :code_review_workflow do + let_it_be(:entity) { create(:draft_note, :on_discussion) } + let_it_be(:json) { entity.as_json } + + it 'exposes correct attributes' do + expect(json["id"]).to eq entity.id + expect(json["author_id"]).to eq entity.author_id + expect(json["merge_request_id"]).to eq entity.merge_request_id + expect(json["resolve_discussion"]).to eq entity.resolve_discussion + expect(json["discussion_id"]).to eq entity.discussion_id + expect(json["note"]).to eq entity.note + expect(json["position"].transform_keys(&:to_sym)).to eq entity.position.to_h + end +end diff --git a/spec/lib/gitlab/ci/artifact_file_reader_spec.rb b/spec/lib/gitlab/ci/artifact_file_reader_spec.rb index e982f0eb015..813dc15e79f 100644 --- a/spec/lib/gitlab/ci/artifact_file_reader_spec.rb +++ b/spec/lib/gitlab/ci/artifact_file_reader_spec.rb @@ -10,71 +10,117 @@ RSpec.describe Gitlab::Ci::ArtifactFileReader do subject { described_class.new(job).read(path) } context 'when job has artifacts and metadata' do - let!(:artifacts) { create(:ci_job_artifact, :archive, job: job) } - let!(:metadata) { create(:ci_job_artifact, :metadata, job: job) } + shared_examples 'extracting job artifact archive' do + it 'returns the content at the path' do + is_expected.to be_present + expect(YAML.safe_load(subject).keys).to contain_exactly('rspec', 'time', 'custom') + end - it 'returns the content at the path' do - is_expected.to be_present - expect(YAML.safe_load(subject).keys).to contain_exactly('rspec', 'time', 'custom') - end + context 'when path does not exist' do + let(:path) { 'file/does/not/exist.txt' } + let(:expected_error) do + "Path `#{path}` does not exist inside the `#{job.name}` artifacts archive!" + end - context 'when path does not exist' do - let(:path) { 'file/does/not/exist.txt' } - let(:expected_error) do - "Path `#{path}` does not exist inside the `#{job.name}` artifacts archive!" + it 'raises an error' do + expect { subject }.to raise_error(described_class::Error, expected_error) + end end - it 'raises an error' do - expect { subject }.to raise_error(described_class::Error, expected_error) + context 'when path points to a directory' do + let(:path) { 'other_artifacts_0.1.2' } + let(:expected_error) do + "Path `#{path}` was expected to be a file but it was a directory!" + end + + it 'raises an error' do + expect { subject }.to raise_error(described_class::Error, expected_error) + end end - end - context 'when path points to a directory' do - let(:path) { 'other_artifacts_0.1.2' } - let(:expected_error) do - "Path `#{path}` was expected to be a file but it was a directory!" + context 'when path is nested' do + # path exists in ci_build_artifacts.zip + let(:path) { 'other_artifacts_0.1.2/doc_sample.txt' } + + it 'returns the content at the nested path' do + is_expected.to be_present + end end - it 'raises an error' do - expect { subject }.to raise_error(described_class::Error, expected_error) + context 'when artifact archive size is greater than the limit' do + let(:expected_error) do + "Artifacts archive for job `#{job.name}` is too large: max 1 KB" + end + + before do + stub_const("#{described_class}::MAX_ARCHIVE_SIZE", 1.kilobyte) + end + + it 'raises an error' do + expect { subject }.to raise_error(described_class::Error, expected_error) + end end - end - context 'when path is nested' do - # path exists in ci_build_artifacts.zip - let(:path) { 'other_artifacts_0.1.2/doc_sample.txt' } + context 'when metadata entry shows size greater than the limit' do + let(:expected_error) do + "Artifacts archive for job `#{job.name}` is too large: max 5 MB" + end - it 'returns the content at the nested path' do - is_expected.to be_present + before do + expect_next_instance_of(Gitlab::Ci::Build::Artifacts::Metadata::Entry) do |entry| + expect(entry).to receive(:total_size).and_return(10.megabytes) + end + end + + it 'raises an error' do + expect { subject }.to raise_error(described_class::Error, expected_error) + end end end - context 'when artifact archive size is greater than the limit' do - let(:expected_error) do - "Artifacts archive for job `#{job.name}` is too large: max 1 KB" - end + context 'when job artifact is on local storage' do + let!(:artifacts) { create(:ci_job_artifact, :archive, job: job) } + let!(:metadata) { create(:ci_job_artifact, :metadata, job: job) } + + it_behaves_like 'extracting job artifact archive' + end + context 'when job artifact is on remote storage' do before do - stub_const("#{described_class}::MAX_ARCHIVE_SIZE", 1.kilobyte) + stub_artifacts_object_storage + stub_request(:get, %r{https://artifacts.+ci_build_artifacts\.zip}) + .to_return( + status: 200, + body: File.open(Rails.root.join('spec/fixtures/ci_build_artifacts.zip')), + headers: {} + ) + stub_request(:get, %r{https://artifacts.+ci_build_artifacts_metadata}) + .to_return( + status: 200, + body: File.open(Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz')), + headers: {} + ) end - it 'raises an error' do - expect { subject }.to raise_error(described_class::Error, expected_error) - end + let!(:artifacts) { create(:ci_job_artifact, :archive, :remote_store, job: job) } + let!(:metadata) { create(:ci_job_artifact, :metadata, :remote_store, job: job) } + + it_behaves_like 'extracting job artifact archive' end - context 'when metadata entry shows size greater than the limit' do - let(:expected_error) do - "Artifacts archive for job `#{job.name}` is too large: max 5 MB" - end + context 'when extracting job artifact raises entry size error' do + let!(:artifacts) { create(:ci_job_artifact, :archive, job: job) } + let!(:metadata) { create(:ci_job_artifact, :metadata, job: job) } before do - expect_next_instance_of(Gitlab::Ci::Build::Artifacts::Metadata::Entry) do |entry| - expect(entry).to receive(:total_size).and_return(10.megabytes) + allow_next_instance_of(SafeZip::Extract, anything) do |extractor| + allow(extractor).to receive(:extract).and_raise(SafeZip::Extract::EntrySizeError) end end it 'raises an error' do + expected_error = "Path `#{path}` has invalid size in the zip!" + expect { subject }.to raise_error(described_class::Error, expected_error) end end diff --git a/spec/lib/safe_zip/entry_spec.rb b/spec/lib/safe_zip/entry_spec.rb index 9929b8073a0..8d49e2bcece 100644 --- a/spec/lib/safe_zip/entry_spec.rb +++ b/spec/lib/safe_zip/entry_spec.rb @@ -5,12 +5,13 @@ require 'spec_helper' RSpec.describe SafeZip::Entry do let(:target_path) { Dir.mktmpdir('safe-zip') } let(:directories) { %w(public folder/with/subfolder) } - let(:params) { SafeZip::ExtractParams.new(directories: directories, to: target_path) } + let(:files) { %w(public/index.html public/assets/image.png) } + let(:params) { SafeZip::ExtractParams.new(directories: directories, files: files, to: target_path) } let(:entry) { described_class.new(zip_archive, zip_entry, params) } let(:entry_name) { 'public/folder/index.html' } let(:entry_path_dir) { File.join(target_path, File.dirname(entry_name)) } - let(:entry_path) { File.join(target_path, entry_name) } + let(:entry_path) { File.join(File.realpath(target_path), entry_name) } let(:zip_archive) { double } let(:zip_entry) do @@ -28,7 +29,7 @@ RSpec.describe SafeZip::Entry do describe '#path_dir' do subject { entry.path_dir } - it { is_expected.to eq(target_path + '/public/folder') } + it { is_expected.to eq(File.realpath(target_path) + '/public/folder') } end describe '#exist?' do @@ -51,6 +52,9 @@ RSpec.describe SafeZip::Entry do subject { entry.extract } context 'when entry does not match the filtered directories' do + let(:directories) { %w(public folder/with/subfolder) } + let(:files) { [] } + using RSpec::Parameterized::TableSyntax where(:entry_name) do @@ -70,7 +74,30 @@ RSpec.describe SafeZip::Entry do end end - context 'when entry does exist' do + context 'when entry does not match the filtered files' do + let(:directories) { [] } + let(:files) { %w(public/index.html public/assets/image.png) } + + using RSpec::Parameterized::TableSyntax + + where(:entry_name) do + [ + 'assets/folder/index.html', + 'public/../folder/index.html', + 'public/../../../../../index.html', + '../../../../../public/index.html', + '/etc/passwd' + ] + end + + with_them do + it 'does not extract file' do + is_expected.to be_falsey + end + end + end + + context 'when there is an existing extracted entry' do before do create_entry end diff --git a/spec/lib/safe_zip/extract_params_spec.rb b/spec/lib/safe_zip/extract_params_spec.rb index 880c4358663..0ebfb7430c5 100644 --- a/spec/lib/safe_zip/extract_params_spec.rb +++ b/spec/lib/safe_zip/extract_params_spec.rb @@ -4,8 +4,10 @@ require 'spec_helper' RSpec.describe SafeZip::ExtractParams do let(:target_path) { Dir.mktmpdir("safe-zip") } - let(:params) { described_class.new(directories: directories, to: target_path) } + let(:real_target_path) { File.realpath(target_path) } + let(:params) { described_class.new(directories: directories, files: files, to: target_path) } let(:directories) { %w(public folder/with/subfolder) } + let(:files) { %w(public/index.html public/assets/image.png) } after do FileUtils.remove_entry_secure(target_path) @@ -14,13 +16,13 @@ RSpec.describe SafeZip::ExtractParams do describe '#extract_path' do subject { params.extract_path } - it { is_expected.to eq(target_path) } + it { is_expected.to eq(real_target_path) } end describe '#matching_target_directory' do using RSpec::Parameterized::TableSyntax - subject { params.matching_target_directory(target_path + path) } + subject { params.matching_target_directory(real_target_path + path) } where(:path, :result) do '/public/index.html' | '/public/' @@ -30,7 +32,7 @@ RSpec.describe SafeZip::ExtractParams do end with_them do - it { is_expected.to eq(result ? target_path + result : nil) } + it { is_expected.to eq(result ? real_target_path + result : nil) } end end @@ -38,7 +40,7 @@ RSpec.describe SafeZip::ExtractParams do subject { params.target_directories } it 'starts with target_path' do - is_expected.to all(start_with(target_path + '/')) + is_expected.to all(start_with(real_target_path + '/')) end it 'ends with / for all paths' do @@ -53,4 +55,27 @@ RSpec.describe SafeZip::ExtractParams do is_expected.to all(end_with('/*')) end end + + describe '#matching_target_file' do + using RSpec::Parameterized::TableSyntax + + subject { params.matching_target_file(real_target_path + path) } + + where(:path, :result) do + '/public/index.html' | true + '/non/existing/path' | false + '/public/' | false + '/folder/with/index.html' | false + end + + with_them do + it { is_expected.to eq(result) } + end + end + + context 'when directories and files are empty' do + it 'is invalid' do + expect { described_class.new(to: target_path) }.to raise_error(ArgumentError, /directories or files are required/) + end + end end diff --git a/spec/lib/safe_zip/extract_spec.rb b/spec/lib/safe_zip/extract_spec.rb index 443430b267d..c727475e271 100644 --- a/spec/lib/safe_zip/extract_spec.rb +++ b/spec/lib/safe_zip/extract_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe SafeZip::Extract do let(:target_path) { Dir.mktmpdir('safe-zip') } let(:directories) { %w(public) } + let(:files) { %w(public/index.html) } let(:object) { described_class.new(archive) } let(:archive) { Rails.root.join('spec', 'fixtures', 'safe_zip', archive_name) } @@ -13,20 +14,36 @@ RSpec.describe SafeZip::Extract do end describe '#extract' do - subject { object.extract(directories: directories, to: target_path) } + subject { object.extract(directories: directories, files: files, to: target_path) } shared_examples 'extracts archive' do - it 'does extract archive' do - subject + context 'when specifying directories' do + subject { object.extract(directories: directories, to: target_path) } - expect(File.exist?(File.join(target_path, 'public', 'index.html'))).to eq(true) - expect(File.exist?(File.join(target_path, 'source'))).to eq(false) + it 'does extract archive' do + subject + + expect(File.exist?(File.join(target_path, 'public', 'index.html'))).to eq(true) + expect(File.exist?(File.join(target_path, 'public', 'assets', 'image.png'))).to eq(true) + expect(File.exist?(File.join(target_path, 'source'))).to eq(false) + end + end + + context 'when specifying files' do + subject { object.extract(files: files, to: target_path) } + + it 'does extract archive' do + subject + + expect(File.exist?(File.join(target_path, 'public', 'index.html'))).to eq(true) + expect(File.exist?(File.join(target_path, 'public', 'assets', 'image.png'))).to eq(false) + end end end shared_examples 'fails to extract archive' do it 'does not extract archive' do - expect { subject }.to raise_error(SafeZip::Extract::Error) + expect { subject }.to raise_error(SafeZip::Extract::Error, including(error_message)) end end @@ -38,9 +55,18 @@ RSpec.describe SafeZip::Extract do end end - %w(invalid-symlink-does-not-exist.zip invalid-symlinks-outside.zip).each do |name| - context "when using #{name} archive" do + context 'when zip files are invalid' do + using RSpec::Parameterized::TableSyntax + + where(:name, :message) do + 'invalid-symlink-does-not-exist.zip' | 'does not exist' + 'invalid-symlinks-outside.zip' | 'Symlink cannot be created' + 'invalid-unexpected-large.zip' | 'larger when inflated' + end + + with_them do let(:archive_name) { name } + let(:error_message) { message } it_behaves_like 'fails to extract archive' end @@ -49,6 +75,19 @@ RSpec.describe SafeZip::Extract do context 'when no matching directories are found' do let(:archive_name) { 'valid-simple.zip' } let(:directories) { %w(non/existing) } + let(:error_message) { 'No entries extracted' } + + subject { object.extract(directories: directories, to: target_path) } + + it_behaves_like 'fails to extract archive' + end + + context 'when no matching files are found' do + let(:archive_name) { 'valid-simple.zip' } + let(:files) { %w(non/existing) } + let(:error_message) { 'No entries extracted' } + + subject { object.extract(files: files, to: target_path) } it_behaves_like 'fails to extract archive' end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index e553e34ab51..206b3ae61cf 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -65,7 +65,6 @@ RSpec.describe Issuable do it { is_expected.to validate_presence_of(:author) } it { is_expected.to validate_presence_of(:title) } it { is_expected.to validate_length_of(:title).is_at_most(described_class::TITLE_LENGTH_MAX) } - it { is_expected.to validate_length_of(:description).is_at_most(described_class::DESCRIPTION_LENGTH_MAX).on(:create) } it_behaves_like 'validates description length with custom validation' do before do diff --git a/spec/models/concerns/sanitizable_spec.rb b/spec/models/concerns/sanitizable_spec.rb index 4a1d463d666..be7169f8dca 100644 --- a/spec/models/concerns/sanitizable_spec.rb +++ b/spec/models/concerns/sanitizable_spec.rb @@ -75,7 +75,58 @@ RSpec.describe Sanitizable do it 'is not valid', :aggregate_failures do expect(record).not_to be_valid - expect(record.errors.full_messages).to include('Name cannot contain escaped HTML entities') + expect(record.errors.full_messages).to contain_exactly( + 'Name cannot contain escaped HTML entities', + 'Description cannot contain escaped HTML entities' + ) + end + end + + context 'when input contains double-escaped data' do + let_it_be(:input) do + '%2526lt%253Bscript%2526gt%253Balert%25281%2529%2526lt%253B%252Fscript%2526gt%253B' + end + + it_behaves_like 'noop' + + it 'is not valid', :aggregate_failures do + expect(record).not_to be_valid + expect(record.errors.full_messages).to contain_exactly( + 'Name cannot contain escaped components', + 'Description cannot contain escaped components' + ) + end + end + + context 'when input contains a path traversal attempt' do + let_it_be(:input) { 'main../../../../../../api/v4/projects/1/import_project_members/2' } + + it_behaves_like 'noop' + + it 'is not valid', :aggregate_failures do + expect(record).not_to be_valid + expect(record.errors.full_messages).to contain_exactly( + 'Name cannot contain a path traversal component', + 'Description cannot contain a path traversal component' + ) + end + end + + context 'when input contains both path traversal attempt and pre-escaped entities' do + let_it_be(:input) do + 'main../../../../../../api/v4/projects/1/import_project_members/2<script>alert(1)</script>' + end + + it_behaves_like 'noop' + + it 'is not valid', :aggregate_failures do + expect(record).not_to be_valid + expect(record.errors.full_messages).to contain_exactly( + 'Name cannot contain a path traversal component', + 'Name cannot contain escaped HTML entities', + 'Description cannot contain a path traversal component', + 'Description cannot contain escaped HTML entities' + ) end end end diff --git a/spec/models/namespace_setting_spec.rb b/spec/models/namespace_setting_spec.rb index 0bf6fdf4fa0..15b80749aa2 100644 --- a/spec/models/namespace_setting_spec.rb +++ b/spec/models/namespace_setting_spec.rb @@ -18,7 +18,7 @@ RSpec.describe NamespaceSetting, feature_category: :subgroups, type: :model do describe "#default_branch_name_content" do let_it_be(:group) { create(:group) } - let(:namespace_settings) { group.namespace_settings } + subject(:namespace_settings) { group.namespace_settings } shared_examples "doesn't return an error" do it "doesn't return an error" do @@ -28,6 +28,10 @@ RSpec.describe NamespaceSetting, feature_category: :subgroups, type: :model do end context "when not set" do + before do + namespace_settings.default_branch_name = nil + end + it_behaves_like "doesn't return an error" end diff --git a/spec/requests/api/draft_notes_spec.rb b/spec/requests/api/draft_notes_spec.rb new file mode 100644 index 00000000000..fad93fff839 --- /dev/null +++ b/spec/requests/api/draft_notes_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::DraftNotes, feature_category: :code_review_workflow do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :public) } + let_it_be(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) } + + let_it_be(:merge_request_note) { create(:note, noteable: merge_request, project: project, author: user) } + let_it_be(:draft_note_by_current_user) { create(:draft_note, merge_request: merge_request, author: user) } + let_it_be(:draft_note_by_random_user) { create(:draft_note, merge_request: merge_request) } + + before do + project.add_developer(user) + end + + describe "Get a list of merge request draft notes" do + it "returns 200 OK status" do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/draft_notes", user) + + expect(response).to have_gitlab_http_status(:ok) + end + + it "returns only draft notes authored by the current user" do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/draft_notes", user) + + draft_note_ids = json_response.pluck("id") + + expect(draft_note_ids).to include(draft_note_by_current_user.id) + expect(draft_note_ids).not_to include(draft_note_by_random_user.id) + expect(draft_note_ids).not_to include(merge_request_note.id) + end + end +end diff --git a/spec/scripts/trigger-build_spec.rb b/spec/scripts/trigger-build_spec.rb index 760b9bda541..78cc57b6c91 100644 --- a/spec/scripts/trigger-build_spec.rb +++ b/spec/scripts/trigger-build_spec.rb @@ -16,7 +16,7 @@ RSpec.describe Trigger, feature_category: :tooling do 'CI_COMMIT_SHA' => 'ci_commit_sha', 'CI_MERGE_REQUEST_PROJECT_ID' => 'ci_merge_request_project_id', 'CI_MERGE_REQUEST_IID' => 'ci_merge_request_iid', - 'GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN' => 'bot-token', + 'PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE' => 'bot-token', 'CI_JOB_TOKEN' => 'job-token', 'GITLAB_USER_NAME' => 'gitlab_user_name', 'GITLAB_USER_LOGIN' => 'gitlab_user_login', @@ -26,7 +26,7 @@ RSpec.describe Trigger, feature_category: :tooling do end let(:com_api_endpoint) { 'https://gitlab.com/api/v4' } - let(:com_api_token) { env['GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN'] } + let(:com_api_token) { env['PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE'] } let(:com_gitlab_client) { double('com_gitlab_client') } let(:downstream_gitlab_client_endpoint) { com_api_endpoint } @@ -114,25 +114,6 @@ RSpec.describe Trigger, feature_category: :tooling do subject.invoke! end - - context 'with downstream_job_name: "foo"' do - let(:downstream_job) { Struct.new(:id, :name).new(42, 'foo') } - let(:paginated_resources) { Struct.new(:auto_paginate).new([downstream_job]) } - - before do - stub_env('CI_COMMIT_REF_NAME', "#{ref}-ee") - end - - it 'fetches the downstream job' do - expect_run_trigger_with_params - expect(downstream_gitlab_client).to receive(:pipeline_jobs) - .with(downstream_project_path, stubbed_pipeline.id).and_return(paginated_resources) - expect(Trigger::Job).to receive(:new) - .with(downstream_project_path, downstream_job.id, downstream_gitlab_client) - - subject.invoke!(downstream_job_name: 'foo') - end - end end end diff --git a/spec/services/packages/helm/extract_file_metadata_service_spec.rb b/spec/services/packages/helm/extract_file_metadata_service_spec.rb index 273f679b736..f4c61c12344 100644 --- a/spec/services/packages/helm/extract_file_metadata_service_spec.rb +++ b/spec/services/packages/helm/extract_file_metadata_service_spec.rb @@ -54,4 +54,17 @@ RSpec.describe Packages::Helm::ExtractFileMetadataService do it { expect { subject }.to raise_error(described_class::ExtractionError, 'Error while parsing Chart.yaml: (<unknown>): did not find expected node content while parsing a flow node at line 2 column 1') } end + + context 'with a corrupted Chart.yaml of incorrect size' do + let(:helm_fixture_path) { expand_fixture_path('packages/helm/corrupted_chart.tgz') } + let(:expected_error_message) { 'Chart.yaml too big' } + + before do + allow(Zlib::GzipReader).to receive(:new).and_return(Zlib::GzipReader.new(File.open(helm_fixture_path))) + end + + it 'raises an error with the expected message' do + expect { subject }.to raise_error(::Packages::Helm::ExtractFileMetadataService::ExtractionError, expected_error_message) + end + end end diff --git a/spec/support/shared_examples/models/concerns/issuable_shared_examples.rb b/spec/support/shared_examples/models/concerns/issuable_shared_examples.rb index 3a407088997..f49ec906382 100644 --- a/spec/support/shared_examples/models/concerns/issuable_shared_examples.rb +++ b/spec/support/shared_examples/models/concerns/issuable_shared_examples.rb @@ -10,40 +10,111 @@ RSpec.shared_examples 'matches_cross_reference_regex? fails fast' do end RSpec.shared_examples 'validates description length with custom validation' do - let(:issuable) { build(:issue, description: 'x' * (::Issuable::DESCRIPTION_LENGTH_MAX + 1)) } - let(:context) { :update } + let(:invalid_description) { 'x' * (::Issuable::DESCRIPTION_LENGTH_MAX + 1) } + let(:valid_description) { 'short description' } + let(:issuable) { build(:issue, description: description) } + + let(:error_message) do + format( + _('is too long (%{size}). The maximum size is %{max_size}.'), + size: ActiveSupport::NumberHelper.number_to_human_size(invalid_description.bytesize), + max_size: ActiveSupport::NumberHelper.number_to_human_size(::Issuable::DESCRIPTION_LENGTH_MAX) + ) + end - subject { issuable.validate(context) } + subject(:validate) { issuable.validate(context) } context 'when Issuable is a new record' do - it 'validates the maximum description length' do - subject - expect(issuable.errors[:description]).to eq(["is too long (maximum is #{::Issuable::DESCRIPTION_LENGTH_MAX} characters)"]) - end + let(:context) { :create } + + context 'when description exceeds the maximum size' do + let(:description) { invalid_description } - context 'on create' do - let(:context) { :create } + it 'adds a description too long error' do + validate - it 'does not validate the maximum description length' do - allow(issuable).to receive(:description_max_length_for_new_records_is_valid).and_call_original + expect(issuable.errors[:description]).to contain_exactly(error_message) + end + end - subject + context 'when description is within the allowed limits' do + let(:description) { valid_description } - expect(issuable).not_to have_received(:description_max_length_for_new_records_is_valid) + it 'does not add a validation error' do + validate + + expect(issuable.errors).not_to have_key(:description) end end end context 'when Issuable is an existing record' do + let(:context) { :update } + before do allow(issuable).to receive(:expire_etag_cache) # to skip the expire_etag_cache callback + issuable.description = existing_description issuable.save!(validate: false) + issuable.description = description + end + + context 'when record already had a valid description' do + let(:existing_description) { 'small difference so it triggers description_changed?' } + + context 'when new description exceeds the maximum size' do + let(:description) { invalid_description } + + it 'adds a description too long error' do + validate + + expect(issuable.errors[:description]).to contain_exactly(error_message) + end + end + + context 'when new description is within the allowed limits' do + let(:description) { valid_description } + + it 'does not add a validation error' do + validate + + expect(issuable.errors).not_to have_key(:description) + end + end end - it 'does not validate the maximum description length' do - subject - expect(issuable.errors).not_to have_key(:description) + context 'when record existed with an invalid description' do + let(:existing_description) { "#{invalid_description} small difference so it triggers description_changed?" } + + context 'when description is not changed' do + let(:description) { existing_description } + + it 'does not add a validation error' do + validate + + expect(issuable.errors).not_to have_key(:description) + end + end + + context 'when new description exceeds the maximum size' do + let(:description) { invalid_description } + + it 'allows updating descriptions that already existed above the limit' do + validate + + expect(issuable.errors).not_to have_key(:description) + end + end + + context 'when new description is within the allowed limits' do + let(:description) { valid_description } + + it 'does not add a validation error' do + validate + + expect(issuable.errors).not_to have_key(:description) + end + end end end end diff --git a/spec/support/shared_examples/models/concerns/sanitizable_shared_examples.rb b/spec/support/shared_examples/models/concerns/sanitizable_shared_examples.rb index aedbfe4deb3..9bfa4ace05c 100644 --- a/spec/support/shared_examples/models/concerns/sanitizable_shared_examples.rb +++ b/spec/support/shared_examples/models/concerns/sanitizable_shared_examples.rb @@ -32,8 +32,25 @@ RSpec.shared_examples 'sanitizable' do |factory, fields| subject { build(factory, attributes) } it 'is not valid', :aggregate_failures do + error = 'cannot contain escaped HTML entities' + + expect(subject).not_to be_valid + expect(subject.errors.details[field].flat_map(&:values)).to contain_exactly(error) + end + end + + context 'when it contains a path component' do + let_it_be(:input) do + 'main../../../../../../api/v4/projects/1/import_project_members/2' + end + + subject { build(factory, attributes) } + + it 'is not valid', :aggregate_failures do + error = 'cannot contain a path traversal component' + expect(subject).not_to be_valid - expect(subject.errors.details[field].flat_map(&:values)).to include('cannot contain escaped HTML entities') + expect(subject.errors.details[field].flat_map(&:values)).to contain_exactly(error) end end end |