diff options
25 files changed, 210 insertions, 66 deletions
diff --git a/app/controllers/projects/pages_domains_controller.rb b/app/controllers/projects/pages_domains_controller.rb index cdf6f5ce828..cccf8fe4358 100644 --- a/app/controllers/projects/pages_domains_controller.rb +++ b/app/controllers/projects/pages_domains_controller.rb @@ -7,6 +7,8 @@ class Projects::PagesDomainsController < Projects::ApplicationController before_action :authorize_update_pages! before_action :domain, except: [:new, :create] + helper_method :domain_presenter + def show end @@ -27,7 +29,7 @@ class Projects::PagesDomainsController < Projects::ApplicationController end def retry_auto_ssl - PagesDomains::RetryAcmeOrderService.new(@domain.pages_domain).execute + PagesDomains::RetryAcmeOrderService.new(@domain).execute redirect_to project_pages_domain_path(@project, @domain) end @@ -88,6 +90,10 @@ class Projects::PagesDomainsController < Projects::ApplicationController end def domain - @domain ||= @project.pages_domains.find_by_domain!(params[:id].to_s).present(current_user: current_user) + @domain ||= @project.pages_domains.find_by_domain!(params[:id].to_s) + end + + def domain_presenter + @domain_presenter ||= domain.present(current_user: current_user) end end diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb index 517c22e2334..20aab58243a 100644 --- a/app/uploaders/file_uploader.rb +++ b/app/uploaders/file_uploader.rb @@ -15,7 +15,7 @@ class FileUploader < GitlabUploader prepend ObjectStorage::Extension::RecordsUploads MARKDOWN_PATTERN = %r{\!?\[.*?\]\(/uploads/(?<secret>[0-9a-f]{32})/(?<file>.*?)\)}.freeze - DYNAMIC_PATH_PATTERN = %r{.*/(?<secret>\h{10,32})/(?<identifier>.*)}.freeze + DYNAMIC_PATH_PATTERN = %r{.*(?<secret>\b(\h{10}|\h{32}))\/(?<identifier>.*)}.freeze VALID_SECRET_PATTERN = %r{\A\h{10,32}\z}.freeze InvalidSecret = Class.new(StandardError) diff --git a/app/views/projects/pages_domains/_certificate.html.haml b/app/views/projects/pages_domains/_certificate.html.haml index e95841f2867..11c7e4a950b 100644 --- a/app/views/projects/pages_domains/_certificate.html.haml +++ b/app/views/projects/pages_domains/_certificate.html.haml @@ -1,7 +1,7 @@ - auto_ssl_available = ::Gitlab::LetsEncrypt.enabled? -- auto_ssl_enabled = @domain.auto_ssl_enabled? +- auto_ssl_enabled = domain_presenter.auto_ssl_enabled? - auto_ssl_available_and_enabled = auto_ssl_available && auto_ssl_enabled -- has_user_defined_certificate = @domain.certificate && @domain.certificate_user_provided? +- has_user_defined_certificate = domain_presenter.certificate && domain_presenter.certificate_user_provided? - if auto_ssl_available .form-group.border-section @@ -36,9 +36,9 @@ = _('Certificate') .d-flex.justify-content-between.align-items-center.p-3 %span - = @domain.pages_domain.subject || _('missing') + = domain_presenter.pages_domain.subject || _('missing') = link_to _('Remove'), - clean_certificate_project_pages_domain_path(@project, @domain), + clean_certificate_project_pages_domain_path(@project, domain_presenter), data: { confirm: _('Are you sure?') }, class: 'btn btn-remove btn-sm', method: :delete diff --git a/app/views/projects/pages_domains/_dns.html.haml b/app/views/projects/pages_domains/_dns.html.haml index e4e590f0a98..8e2a9c3bab4 100644 --- a/app/views/projects/pages_domains/_dns.html.haml +++ b/app/views/projects/pages_domains/_dns.html.haml @@ -1,5 +1,5 @@ - verification_enabled = Gitlab::CurrentSettings.pages_domain_verification_enabled? -- dns_record = "#{@domain.domain} CNAME #{@domain.project.pages_subdomain}.#{Settings.pages.host}." +- dns_record = "#{domain_presenter.domain} CNAME #{domain_presenter.project.pages_subdomain}.#{Settings.pages.host}." .form-group.border-section .row @@ -13,17 +13,17 @@ %p.form-text.text-muted = _("To access this domain create a new DNS record") - if verification_enabled - - verification_record = "#{@domain.verification_domain} TXT #{@domain.keyed_verification_code}" + - verification_record = "#{domain_presenter.verification_domain} TXT #{domain_presenter.keyed_verification_code}" .form-group.border-section .row .col-sm-2 = _("Verification status") .col-sm-10 .status-badge - - text, status = @domain.unverified? ? [_('Unverified'), 'badge-danger'] : [_('Verified'), 'badge-success'] + - text, status = domain_presenter.unverified? ? [_('Unverified'), 'badge-danger'] : [_('Verified'), 'badge-success'] .badge{ class: status } = text - = link_to sprite_icon("redo"), verify_project_pages_domain_path(@project, @domain), method: :post, class: "btn has-tooltip", title: _("Retry verification") + = link_to sprite_icon("redo"), verify_project_pages_domain_path(@project, domain_presenter), method: :post, class: "btn has-tooltip", title: _("Retry verification") .input-group = text_field_tag :domain_verification, verification_record, class: "monospace js-select-on-focus form-control", readonly: true .input-group-append diff --git a/app/views/projects/pages_domains/_form.html.haml b/app/views/projects/pages_domains/_form.html.haml index e06dab9be06..9e9f60a6f09 100644 --- a/app/views/projects/pages_domains/_form.html.haml +++ b/app/views/projects/pages_domains/_form.html.haml @@ -1,15 +1,15 @@ -- if @domain.errors.any? +- if domain_presenter.errors.any? .alert.alert-danger - - @domain.errors.full_messages.each do |msg| + - domain_presenter.errors.full_messages.each do |msg| = msg .form-group.border-section .row - - if @domain.persisted? + - if domain_presenter.persisted? .col-sm-2 = _("Domain") .col-sm-10 - = external_link(@domain.url, @domain.url) + = external_link(domain_presenter.url, domain_presenter.url) - else .col-sm-2 = f.label :domain, _("Domain") @@ -17,7 +17,7 @@ .input-group = f.text_field :domain, required: true, autocomplete: "off", class: "form-control" -- if @domain.persisted? +- if domain_presenter.persisted? = render 'dns' - if Gitlab.config.pages.external_https diff --git a/app/views/projects/pages_domains/_lets_encrypt_callout.html.haml b/app/views/projects/pages_domains/_lets_encrypt_callout.html.haml index f2de42b218c..a86637c36b3 100644 --- a/app/views/projects/pages_domains/_lets_encrypt_callout.html.haml +++ b/app/views/projects/pages_domains/_lets_encrypt_callout.html.haml @@ -1,6 +1,6 @@ -- if @domain.enabled? - - if @domain.auto_ssl_enabled - - if @domain.show_auto_ssl_failed_warning? +- if domain_presenter.enabled? + - if domain_presenter.auto_ssl_enabled + - if domain_presenter.show_auto_ssl_failed_warning? .form-group.border-section.js-shown-if-auto-ssl{ class: ("d-none" unless auto_ssl_available_and_enabled) } .row .col-sm-10.offset-sm-2 @@ -9,8 +9,8 @@ = icon('warning', class: 'mr-2') = _("Something went wrong while obtaining the Let's Encrypt certificate.") .row.mx-0.mt-3 - = link_to s_('GitLabPagesDomains|Retry'), retry_auto_ssl_project_pages_domain_path(@project, @domain), class: "btn btn-sm btn-grouped btn-warning", method: :post - - elsif !@domain.certificate_gitlab_provided? + = link_to s_('GitLabPagesDomains|Retry'), retry_auto_ssl_project_pages_domain_path(@project, domain_presenter), class: "btn btn-sm btn-grouped btn-warning", method: :post + - elsif !domain_presenter.certificate_gitlab_provided? .form-group.border-section.js-shown-if-auto-ssl{ class: ("d-none" unless auto_ssl_available_and_enabled) } .row .col-sm-10.offset-sm-2 diff --git a/app/views/projects/pages_domains/new.html.haml b/app/views/projects/pages_domains/new.html.haml index 3210bfe9231..f5dc3ccc60e 100644 --- a/app/views/projects/pages_domains/new.html.haml +++ b/app/views/projects/pages_domains/new.html.haml @@ -4,7 +4,7 @@ = _("New Pages Domain") = render 'projects/pages_domains/helper_text' %div - = form_for [@project.namespace.becomes(Namespace), @project, @domain], html: { class: 'fieldset-form' } do |f| + = form_for [@project.namespace.becomes(Namespace), @project, domain_presenter], html: { class: 'fieldset-form' } do |f| = render 'form', { f: f } .form-actions = f.submit _('Create New Domain'), class: "btn btn-success" diff --git a/app/views/projects/pages_domains/show.html.haml b/app/views/projects/pages_domains/show.html.haml index a08be65d7e4..e1be7335a3f 100644 --- a/app/views/projects/pages_domains/show.html.haml +++ b/app/views/projects/pages_domains/show.html.haml @@ -1,10 +1,10 @@ - add_to_breadcrumbs _("Pages"), project_pages_path(@project) -- breadcrumb_title @domain.domain -- page_title @domain.domain +- breadcrumb_title domain_presenter.domain +- page_title domain_presenter.domain - verification_enabled = Gitlab::CurrentSettings.pages_domain_verification_enabled? -- if verification_enabled && @domain.unverified? +- if verification_enabled && domain_presenter.unverified? = content_for :flash_message do .alert.alert-warning .container-fluid.container-limited @@ -14,7 +14,7 @@ = _('Pages Domain') = render 'projects/pages_domains/helper_text' %div - = form_for [@project.namespace.becomes(Namespace), @project, @domain], html: { class: 'fieldset-form' } do |f| + = form_for [@project.namespace.becomes(Namespace), @project, domain_presenter], html: { class: 'fieldset-form' } do |f| = render 'form', { f: f } .form-actions.d-flex.justify-content-between = f.submit _('Save Changes'), class: "btn btn-success" diff --git a/changelogs/unreleased/216728-actionview-template-error-undefined-method-pages_domain-for-pagesd.yml b/changelogs/unreleased/216728-actionview-template-error-undefined-method-pages_domain-for-pagesd.yml new file mode 100644 index 00000000000..d34e092c9ee --- /dev/null +++ b/changelogs/unreleased/216728-actionview-template-error-undefined-method-pages_domain-for-pagesd.yml @@ -0,0 +1,5 @@ +--- +title: Fix 500 on creating an invalid domains and verification +merge_request: 31190 +author: +type: fixed diff --git a/changelogs/unreleased/216851-graphql-externallypaginatedarrayconnection-can-return-incorrect-nu.yml b/changelogs/unreleased/216851-graphql-externallypaginatedarrayconnection-can-return-incorrect-nu.yml new file mode 100644 index 00000000000..dd36d52f1c4 --- /dev/null +++ b/changelogs/unreleased/216851-graphql-externallypaginatedarrayconnection-can-return-incorrect-nu.yml @@ -0,0 +1,5 @@ +--- +title: Fix incorrect number of errors returned when querying sentry errors +merge_request: 31252 +author: +type: fixed diff --git a/changelogs/unreleased/216970-add-instance-to-service-if-missing.yml b/changelogs/unreleased/216970-add-instance-to-service-if-missing.yml new file mode 100644 index 00000000000..d8fcbb8f587 --- /dev/null +++ b/changelogs/unreleased/216970-add-instance-to-service-if-missing.yml @@ -0,0 +1,5 @@ +--- +title: Add instance column to services table if it's missing +merge_request: 31631 +author: +type: fixed diff --git a/changelogs/unreleased/217602-file-uploads-on-local-storage-with-nil-secret-in-the-db-are-broken.yml b/changelogs/unreleased/217602-file-uploads-on-local-storage-with-nil-secret-in-the-db-are-broken.yml new file mode 100644 index 00000000000..48e585b59e8 --- /dev/null +++ b/changelogs/unreleased/217602-file-uploads-on-local-storage-with-nil-secret-in-the-db-are-broken.yml @@ -0,0 +1,5 @@ +--- +title: Fix incorrect regex used in FileUploader#extract_dynamic_path +merge_request: 32271 +author: +type: fixed diff --git a/changelogs/unreleased/cat-duplicate-ci-pipelines-index-215790.yml b/changelogs/unreleased/cat-duplicate-ci-pipelines-index-215790.yml new file mode 100644 index 00000000000..a21cafe5e14 --- /dev/null +++ b/changelogs/unreleased/cat-duplicate-ci-pipelines-index-215790.yml @@ -0,0 +1,5 @@ +--- +title: Fix duplicate index removal on ci_pipelines.project_id +merge_request: 31043 +author: +type: fixed diff --git a/db/migrate/20191229140154_drop_index_ci_pipelines_on_project_id.rb b/db/migrate/20191229140154_drop_index_ci_pipelines_on_project_id.rb index dbfe3758cda..9e78457b007 100644 --- a/db/migrate/20191229140154_drop_index_ci_pipelines_on_project_id.rb +++ b/db/migrate/20191229140154_drop_index_ci_pipelines_on_project_id.rb @@ -8,10 +8,13 @@ class DropIndexCiPipelinesOnProjectId < ActiveRecord::Migration[5.2] disable_ddl_transaction! def up - remove_concurrent_index :ci_pipelines, :project_id + remove_concurrent_index_by_name :ci_pipelines, 'index_ci_pipelines_on_project_id' + + # extra (duplicate) index that already existed on some installs + remove_concurrent_index_by_name :ci_pipelines, 'ci_pipelines_project_id_idx' end def down - add_concurrent_index :ci_pipelines, :project_id + add_concurrent_index :ci_pipelines, :project_id, name: 'index_ci_pipelines_on_project_id' end end diff --git a/db/post_migrate/20200511162057_add_missing_instance_to_servicess.rb b/db/post_migrate/20200511162057_add_missing_instance_to_servicess.rb new file mode 100644 index 00000000000..efaef085e8c --- /dev/null +++ b/db/post_migrate/20200511162057_add_missing_instance_to_servicess.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class AddMissingInstanceToServicess < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + # This is a corrective migration to keep the instance column. + # Upgrade from 12.7 to 12.9 removes the instance column as it was first added + # in the normal migration and then removed in the post migration. + # + # 12.8 removed the instance column in a post deployment migration https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24885 + # 12.9 added the instance column in a normal migration https://gitlab.com/gitlab-org/gitlab/-/merge_requests/25714 + # + # rubocop:disable Migration/AddColumnWithDefault + # rubocop:disable Migration/UpdateLargeTable + def up + unless column_exists?(:services, :instance) + add_column_with_default(:services, :instance, :boolean, default: false) + end + end + # rubocop:enable Migration/AddColumnWithDefault + # rubocop:enable Migration/UpdateLargeTable + + def down + # Does not apply + end +end diff --git a/db/post_migrate/20200511162115_add_missing_index_to_service_unique_instance_per_type.rb b/db/post_migrate/20200511162115_add_missing_index_to_service_unique_instance_per_type.rb new file mode 100644 index 00000000000..c9e0193f5d2 --- /dev/null +++ b/db/post_migrate/20200511162115_add_missing_index_to_service_unique_instance_per_type.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class AddMissingIndexToServiceUniqueInstancePerType < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + # This is a corrective migration to keep the index on instance column. + # Upgrade from 12.7 to 12.9 removes the instance column as it was first added + # in the normal migration and then removed in the post migration. + # + # 12.8 removed the instance column in a post deployment migration https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24885 + # 12.9 added the instance column in a normal migration https://gitlab.com/gitlab-org/gitlab/-/merge_requests/25714 + def up + unless index_exists_by_name?(:services, 'index_services_on_type_and_instance') + add_concurrent_index(:services, [:type, :instance], unique: true, where: 'instance IS TRUE') + end + end + + def down + # Does not apply + end +end diff --git a/db/structure.sql b/db/structure.sql index 3fec80b1aba..1f0f401165e 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13214,5 +13214,7 @@ COPY "schema_migrations" (version) FROM STDIN; 20200416111111 20200416120128 20200416120354 +20200511162057 +20200511162115 \. diff --git a/doc/user/project/issues/index.md b/doc/user/project/issues/index.md index 55e9967a370..0f9295e1afd 100644 --- a/doc/user/project/issues/index.md +++ b/doc/user/project/issues/index.md @@ -82,7 +82,7 @@ must be set. While you can view and manage the full details of an issue on the [issue page](#issue-page), you can also work with multiple issues at a time using the [Issues List](#issues-list), -[Issue Boards](#issue-boards), Issue references, and [Epics](#epics-ultimate)**(ULTIMATE)**. +[Issue Boards](#issue-boards), Issue references, and [Epics](#epics-premium)**(PREMIUM)**. Key actions for Issues include: @@ -132,7 +132,7 @@ With [Design Management](design_management.md), you can upload design assets to issues and view them all together to easily share and collaborate with your team. -### Epics **(ULTIMATE)** +### Epics **(PREMIUM)** [Epics](../../group/epics/index.md) let you manage your portfolio of projects more efficiently and with less effort by tracking groups of issues that share a theme, across diff --git a/doc/user/project/issues/issue_data_and_actions.md b/doc/user/project/issues/issue_data_and_actions.md index 06524f785ab..65e21566d5d 100644 --- a/doc/user/project/issues/issue_data_and_actions.md +++ b/doc/user/project/issues/issue_data_and_actions.md @@ -16,7 +16,7 @@ You can find all the information for that issue on one screen. - **2.** [To Do](#to-do) - **3.** [Assignee](#assignee) - **3.1.** [Multiple Assignees **(STARTER)**](#multiple-assignees-starter) -- **4.** [Epic **(ULTIMATE)**](#epic-ultimate) +- **4.** [Epic **(PREMIUM)**](#epic-premium) - **5.** [Milestone](#milestone) - **6.** [Time tracking](#time-tracking) - **7.** [Due date](#due-date) @@ -100,7 +100,7 @@ to track in large teams where there is shared ownership of an issue. In [GitLab Starter](https://about.gitlab.com/pricing/), you can [assign multiple people](multiple_assignees_for_issues.md) to an issue. -### Epic **(ULTIMATE)** +### Epic **(PREMIUM)** You can assign issues to an [Epic](../../group/epics/index.md), which allows better management of groups of related issues. diff --git a/lib/gitlab/graphql/pagination/externally_paginated_array_connection.rb b/lib/gitlab/graphql/pagination/externally_paginated_array_connection.rb index 1f01dd07571..12e047420bf 100644 --- a/lib/gitlab/graphql/pagination/externally_paginated_array_connection.rb +++ b/lib/gitlab/graphql/pagination/externally_paginated_array_connection.rb @@ -23,6 +23,20 @@ module Gitlab alias_method :has_next_page, :next_page? alias_method :has_previous_page, :previous_page? + + private + + def load_nodes + @nodes ||= begin + # As the pagination happens externally we just grab all the nodes + limited_nodes = items + + limited_nodes = limited_nodes.first(first) if first + limited_nodes = limited_nodes.last(last) if last + + limited_nodes + end + end end end end diff --git a/spec/controllers/projects/pages_domains_controller_spec.rb b/spec/controllers/projects/pages_domains_controller_spec.rb index c78c5fe2886..40a6f77f0d6 100644 --- a/spec/controllers/projects/pages_domains_controller_spec.rb +++ b/spec/controllers/projects/pages_domains_controller_spec.rb @@ -148,16 +148,10 @@ describe Projects::PagesDomainsController do describe 'POST verify' do let(:params) { request_params.merge(id: pages_domain.domain) } - def stub_service - service = double(:service) - - expect(VerifyPagesDomainService).to receive(:new) { service } - - service - end - it 'handles verification success' do - expect(stub_service).to receive(:execute).and_return(status: :success) + expect_next_instance_of(VerifyPagesDomainService, pages_domain) do |service| + expect(service).to receive(:execute).and_return(status: :success) + end post :verify, params: params @@ -166,7 +160,9 @@ describe Projects::PagesDomainsController do end it 'handles verification failure' do - expect(stub_service).to receive(:execute).and_return(status: :failed) + expect_next_instance_of(VerifyPagesDomainService, pages_domain) do |service| + expect(service).to receive(:execute).and_return(status: :failed) + end post :verify, params: params diff --git a/spec/features/projects/pages_spec.rb b/spec/features/projects/pages_spec.rb index f4f70e7efbc..faa2b3c9424 100644 --- a/spec/features/projects/pages_spec.rb +++ b/spec/features/projects/pages_spec.rb @@ -158,6 +158,17 @@ shared_examples 'pages settings editing' do expect(page).to have_content('my.test.domain.com') end + it 'shows validation error if domain is duplicated' do + project.pages_domains.create!(domain: 'my.test.domain.com') + + visit new_project_pages_domain_path(project) + + fill_in 'Domain', with: 'my.test.domain.com' + click_button 'Create New Domain' + + expect(page).to have_content('Domain has already been taken') + end + describe 'with dns verification enabled' do before do stub_application_setting(pages_domain_verification_enabled: true) diff --git a/spec/lib/gitlab/graphql/pagination/externally_paginated_array_connection_spec.rb b/spec/lib/gitlab/graphql/pagination/externally_paginated_array_connection_spec.rb index 85a5b1dacc7..11cf14523c2 100644 --- a/spec/lib/gitlab/graphql/pagination/externally_paginated_array_connection_spec.rb +++ b/spec/lib/gitlab/graphql/pagination/externally_paginated_array_connection_spec.rb @@ -19,6 +19,20 @@ describe Gitlab::Graphql::Pagination::ExternallyPaginatedArrayConnection do it_behaves_like 'connection with paged nodes' do let(:paged_nodes_size) { values.size } end + + context 'when after or before is specified, they are ignored' do + # after and before are not used to filter the array, as they + # were already used to directly fetch the external array + it_behaves_like 'connection with paged nodes' do + let(:arguments) { { after: next_cursor } } + let(:paged_nodes_size) { values.size } + end + + it_behaves_like 'connection with paged nodes' do + let(:arguments) { { before: prev_cursor } } + let(:paged_nodes_size) { values.size } + end + end end describe '#start_cursor' do diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb index 5fd64da6328..629c84778b9 100644 --- a/spec/uploaders/file_uploader_spec.rb +++ b/spec/uploaders/file_uploader_spec.rb @@ -145,39 +145,57 @@ describe FileUploader do end describe '.extract_dynamic_path' do - context 'with a 32-byte hexadecimal secret in the path' do - let(:secret) { SecureRandom.hex } - let(:path) { "export/4b227777d4dd1fc61c6f884f48641d02b4d121d3fd328cb08b5531fcacdabf8a/test/uploads/#{secret}/dummy.txt" } + shared_examples 'a valid secret' do |root_path| + context 'with a 32-byte hexadecimal secret' do + let(:secret) { SecureRandom.hex } + let(:path) { File.join(*[root_path, secret, 'dummy.txt'].compact) } - it 'extracts the secret' do - expect(described_class.extract_dynamic_path(path)[:secret]).to eq(secret) - end + it 'extracts the secret' do + expect(described_class.extract_dynamic_path(path)[:secret]).to eq(secret) + end - it 'extracts the identifier' do - expect(described_class.extract_dynamic_path(path)[:identifier]).to eq('dummy.txt') + it 'extracts the identifier' do + expect(described_class.extract_dynamic_path(path)[:identifier]).to eq('dummy.txt') + end end - end - context 'with a 10-byte hexadecimal secret in the path' do - let(:secret) { SecureRandom.hex(10) } - let(:path) { "export/4b227777d4dd1fc61c6f884f48641d02b4d121d3fd328cb08b5531fcacdabf8a/test/uploads/#{secret}/dummy.txt" } + context 'with a 10-byte hexadecimal secret' do + let(:secret) { SecureRandom.hex[0, 10] } + let(:path) { File.join(*[root_path, secret, 'dummy.txt'].compact) } + + it 'extracts the secret' do + expect(described_class.extract_dynamic_path(path)[:secret]).to eq(secret) + end - it 'extracts the secret' do - expect(described_class.extract_dynamic_path(path)[:secret]).to eq(secret) + it 'extracts the identifier' do + expect(described_class.extract_dynamic_path(path)[:identifier]).to eq('dummy.txt') + end end - it 'extracts the identifier' do - expect(described_class.extract_dynamic_path(path)[:identifier]).to eq('dummy.txt') + context 'with an invalid secret' do + let(:secret) { 'foo' } + let(:path) { File.join(*[root_path, secret, 'dummy.txt'].compact) } + + it 'returns nil' do + expect(described_class.extract_dynamic_path(path)).to be_nil + end end end - context 'with an invalid secret in the path' do - let(:secret) { 'foo' } - let(:path) { "export/4b227777d4dd1fc61c6f884f48641d02b4d121d3fd328cb08b5531fcacdabf8a/test/uploads/#{secret}/dummy.txt" } + context 'with an absolute path without a slash in the beginning' do + it_behaves_like 'a valid secret', 'export/4b227777d4dd1fc61c6f884f48641d02b4d121d3fd328cb08b5531fcacdabf8a/test/uploads' + end - it 'returns nil' do - expect(described_class.extract_dynamic_path(path)).to be_nil - end + context 'with an absolute path with a slash in the beginning' do + it_behaves_like 'a valid secret', '/export/4b227777d4dd1fc61c6f884f48641d02b4d121d3fd328cb08b5531fcacdabf8a/test/uploads' + end + + context 'with an relative path without a slash in the beginning' do + it_behaves_like 'a valid secret', nil + end + + context 'with an relative path with a slash in the beginning' do + it_behaves_like 'a valid secret', '/' end end @@ -202,7 +220,7 @@ describe FileUploader do end context "10-byte hexadecimal" do - let(:secret) { SecureRandom.hex(10) } + let(:secret) { SecureRandom.hex[0, 10] } it "returns the secret" do expect(uploader.secret).to eq(secret) diff --git a/spec/views/projects/pages_domains/show.html.haml_spec.rb b/spec/views/projects/pages_domains/show.html.haml_spec.rb index 7d502e74d10..2de82a63560 100644 --- a/spec/views/projects/pages_domains/show.html.haml_spec.rb +++ b/spec/views/projects/pages_domains/show.html.haml_spec.rb @@ -7,7 +7,7 @@ describe 'projects/pages_domains/show' do before do assign(:project, project) - assign(:domain, domain.present) + allow(view).to receive(:domain_presenter).and_return(domain.present) stub_pages_setting(external_https: true) end |