diff options
author | José Iván Vargas López <jvargas@gitlab.com> | 2018-08-24 16:45:55 +0000 |
---|---|---|
committer | Jose Vargas <jvargas@gitlab.com> | 2018-08-24 14:47:14 -0500 |
commit | a70266767b1f565a98eece14e51d309f1b09081c (patch) | |
tree | 807c9051e3a85880b725f9dc6abd02a3920e100e | |
parent | 18ebc1882f3f5e4562756a42a8b0ebf751e5ed44 (diff) | |
download | gitlab-ce-a70266767b1f565a98eece14e51d309f1b09081c.tar.gz |
Merge branch 'security-2694-pipeline-11-1' into 'security-11-1'
[11.1] Removes <br> sent from backend on tooltips in jobs
See merge request gitlab/gitlabhq!2457
18 files changed, 28 insertions, 60 deletions
diff --git a/app/assets/javascripts/pipelines/components/graph/dropdown_job_component.vue b/app/assets/javascripts/pipelines/components/graph/dropdown_job_component.vue index 14518f86dc7..c32dc83da8e 100644 --- a/app/assets/javascripts/pipelines/components/graph/dropdown_job_component.vue +++ b/app/assets/javascripts/pipelines/components/graph/dropdown_job_component.vue @@ -1,6 +1,5 @@ <script> import $ from 'jquery'; -import _ from 'underscore'; import JobNameComponent from './job_name_component.vue'; import JobComponent from './job_component.vue'; import tooltip from '../../../vue_shared/directives/tooltip'; @@ -47,7 +46,7 @@ export default { computed: { tooltipText() { - return _.escape(`${this.job.name} - ${this.job.status.label}`); + return `${this.job.name} - ${this.job.status.label}`; }, }, diff --git a/app/assets/javascripts/pipelines/components/graph/job_component.vue b/app/assets/javascripts/pipelines/components/graph/job_component.vue index 84a3d58b770..2e34ac561dc 100644 --- a/app/assets/javascripts/pipelines/components/graph/job_component.vue +++ b/app/assets/javascripts/pipelines/components/graph/job_component.vue @@ -1,5 +1,4 @@ <script> -import _ from 'underscore'; import ActionComponent from './action_component.vue'; import JobNameComponent from './job_name_component.vue'; import tooltip from '../../../vue_shared/directives/tooltip'; @@ -62,7 +61,7 @@ export default { const textBuilder = []; if (this.job.name) { - textBuilder.push(_.escape(this.job.name)); + textBuilder.push(this.job.name); } if (this.job.name && this.status.tooltip) { @@ -106,7 +105,6 @@ export default { :class="cssClassJobName" :data-boundary="tooltipBoundary" data-container="body" - data-html="true" class="js-pipeline-graph-job-link" > @@ -122,7 +120,6 @@ export default { :title="tooltipText" :class="cssClassJobName" class="js-job-component-tooltip non-details-job-component" - data-html="true" data-container="body" > diff --git a/app/views/ci/status/_dropdown_graph_badge.html.haml b/app/views/ci/status/_dropdown_graph_badge.html.haml index 8b0463db000..9de9143e8b1 100644 --- a/app/views/ci/status/_dropdown_graph_badge.html.haml +++ b/app/views/ci/status/_dropdown_graph_badge.html.haml @@ -6,12 +6,12 @@ - tooltip = "#{subject.name} - #{status.status_tooltip}" - if status.has_details? - = link_to status.details_path, class: 'mini-pipeline-graph-dropdown-item', data: { toggle: 'tooltip', title: tooltip, html: 'true', container: 'body' } do + = link_to status.details_path, class: 'mini-pipeline-graph-dropdown-item', data: { toggle: 'tooltip', title: tooltip, container: 'body' } do %span{ class: klass }= sprite_icon(status.icon) %span.ci-build-text= subject.name - else - .menu-item.mini-pipeline-graph-dropdown-item{ data: { toggle: 'tooltip', html: 'true', title: tooltip, container: 'body' } } + .menu-item.mini-pipeline-graph-dropdown-item{ data: { toggle: 'tooltip', title: tooltip, container: 'body' } } %span{ class: klass }= sprite_icon(status.icon) %span.ci-build-text= subject.name diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml index 64c441492bc..74f88486738 100644 --- a/app/views/projects/jobs/_sidebar.html.haml +++ b/app/views/projects/jobs/_sidebar.html.haml @@ -86,8 +86,8 @@ - HasStatus::ORDERED_STATUSES.each do |build_status| - builds.select{|build| build.status == build_status}.each do |build| .build-job{ class: sidebar_build_class(build, @build), data: { stage: build.stage } } - - tooltip = sanitize(build.tooltip_message) - = link_to(project_job_path(@project, build), data: { toggle: 'tooltip', html: 'true', title: tooltip, container: 'body' }) do + - tooltip = sanitize(build.tooltip_message.dup) + = link_to(project_job_path(@project, build), data: { toggle: 'tooltip', title: tooltip, container: 'body' }) do = sprite_icon('arrow-right', size:16, css_class: 'icon-arrow-right') %span{ class: "ci-status-icon-#{build.status}" } = ci_icon_for_status(build.status) diff --git a/lib/gitlab/ci/status/build/failed.rb b/lib/gitlab/ci/status/build/failed.rb index 155f4fc1343..bbdf1b4496d 100644 --- a/lib/gitlab/ci/status/build/failed.rb +++ b/lib/gitlab/ci/status/build/failed.rb @@ -31,7 +31,7 @@ module Gitlab end def description - "<br> (#{REASONS[subject.failure_reason]})" + "- (#{REASONS[subject.failure_reason]})" end end end diff --git a/spec/features/projects/jobs/user_browses_job_spec.rb b/spec/features/projects/jobs/user_browses_job_spec.rb index ce0b38b7239..2b4739efa7d 100644 --- a/spec/features/projects/jobs/user_browses_job_spec.rb +++ b/spec/features/projects/jobs/user_browses_job_spec.rb @@ -40,7 +40,7 @@ describe 'User browses a job', :js do it 'displays the failure reason' do within('.builds-container') do build_link = first('.build-job > a') - expect(build_link['data-title']).to eq('test - failed <br> (unknown failure)') + expect(build_link['data-title']).to eq('test - failed - (unknown failure)') end end end @@ -51,7 +51,7 @@ describe 'User browses a job', :js do it 'displays the failure reason and retried label' do within('.builds-container') do build_link = first('.build-job > a') - expect(build_link['data-title']).to eq('test - failed <br> (unknown failure) (retried)') + expect(build_link['data-title']).to eq('test - failed - (unknown failure) (retried)') end end end diff --git a/spec/features/projects/jobs/user_browses_jobs_spec.rb b/spec/features/projects/jobs/user_browses_jobs_spec.rb index 786ec327b92..6b3fc4077d5 100644 --- a/spec/features/projects/jobs/user_browses_jobs_spec.rb +++ b/spec/features/projects/jobs/user_browses_jobs_spec.rb @@ -36,7 +36,7 @@ describe 'User browses jobs' do it 'displays a tooltip with the failure reason' do page.within('.ci-table') do failed_job_link = page.find('.ci-failed') - expect(failed_job_link[:title]).to eq('Failed <br> (unknown failure)') + expect(failed_job_link[:title]).to eq('Failed - (unknown failure)') end end end diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index ecc7cf84138..fde6b426d68 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -120,7 +120,7 @@ describe 'Pipeline', :js do it 'should include the failure reason' do page.within('#ci-badge-test') do build_link = page.find('.js-pipeline-graph-job-link') - expect(build_link['data-original-title']).to eq('test - failed <br> (unknown failure)') + expect(build_link['data-original-title']).to eq('test - failed - (unknown failure)') end end end @@ -313,7 +313,7 @@ describe 'Pipeline', :js do it 'displays a tooltip with the failure reason' do page.within('.ci-table') do failed_job_link = page.find('.ci-failed') - expect(failed_job_link[:title]).to eq('Failed <br> (unknown failure)') + expect(failed_job_link[:title]).to eq('Failed - (unknown failure)') end end end diff --git a/spec/features/projects/pipelines/pipelines_spec.rb b/spec/features/projects/pipelines/pipelines_spec.rb index 9c165b17704..6695b755335 100644 --- a/spec/features/projects/pipelines/pipelines_spec.rb +++ b/spec/features/projects/pipelines/pipelines_spec.rb @@ -406,7 +406,7 @@ describe 'Pipelines', :js do within('.js-builds-dropdown-list') do build_element = page.find('.mini-pipeline-graph-dropdown-item') - expect(build_element['data-original-title']).to eq('build - failed <br> (unknown failure)') + expect(build_element['data-original-title']).to eq('build - failed - (unknown failure)') end end end diff --git a/spec/javascripts/pipelines/graph/dropdown_job_component_spec.js b/spec/javascripts/pipelines/graph/dropdown_job_component_spec.js index 608a0d4be67..66a1e0ed34c 100644 --- a/spec/javascripts/pipelines/graph/dropdown_job_component_spec.js +++ b/spec/javascripts/pipelines/graph/dropdown_job_component_spec.js @@ -82,12 +82,4 @@ describe('dropdown job component', () => { it('renders dropdown with jobs', () => { expect(vm.$el.querySelectorAll('.scrollable-menu>ul>li').length).toEqual(mock.jobs.length); }); - - it('escapes tooltip title', () => { - expect( - vm.$el.querySelector('.js-pipeline-graph-job-link').getAttribute('data-original-title'), - ).toEqual( - '<img src=x onerror=alert(document.domain)> - passed', - ); - }); }); diff --git a/spec/javascripts/pipelines/graph/job_component_spec.js b/spec/javascripts/pipelines/graph/job_component_spec.js index e71da9946ee..c6f9906ed3b 100644 --- a/spec/javascripts/pipelines/graph/job_component_spec.js +++ b/spec/javascripts/pipelines/graph/job_component_spec.js @@ -161,24 +161,4 @@ describe('pipeline graph job component', () => { expect(component.$el.querySelector(tooltipBoundary)).toBeNull(); }); }); - - describe('tooltipText', () => { - it('escapes job name', () => { - component = mountComponent(JobComponent, { - job: { - id: 4259, - name: '<img src=x onerror=alert(document.domain)>', - status: { - icon: 'icon_status_success', - label: 'success', - tooltip: 'failed', - }, - }, - }); - - expect( - component.$el.querySelector('.js-job-component-tooltip').getAttribute('data-original-title'), - ).toEqual('<img src=x onerror=alert(document.domain)> - failed'); - }); - }); }); diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb index d53a7d468e3..8b92088902b 100644 --- a/spec/lib/gitlab/ci/status/build/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb @@ -88,7 +88,7 @@ describe Gitlab::Ci::Status::Build::Factory do expect(status.icon).to eq 'status_failed' expect(status.favicon).to eq 'favicon_status_failed' expect(status.label).to eq 'failed' - expect(status.status_tooltip).to eq 'failed <br> (unknown failure)' + expect(status.status_tooltip).to eq 'failed - (unknown failure)' expect(status).to have_details expect(status).to have_action end diff --git a/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb b/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb index bfaa508785e..af03d5a1308 100644 --- a/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb +++ b/spec/lib/gitlab/ci/status/build/failed_allowed_spec.rb @@ -76,7 +76,7 @@ describe Gitlab::Ci::Status::Build::FailedAllowed do let(:status) { described_class.new(build_status) } it 'does override badge_tooltip' do - expect(status.badge_tooltip).to eq('failed <br> (unknown failure)') + expect(status.badge_tooltip).to eq('failed - (unknown failure)') end end @@ -87,7 +87,7 @@ describe Gitlab::Ci::Status::Build::FailedAllowed do let(:status) { described_class.new(build_status) } it 'does override status_tooltip' do - expect(status.status_tooltip).to eq 'failed <br> (unknown failure) (allowed to fail)' + expect(status.status_tooltip).to eq 'failed - (unknown failure) (allowed to fail)' end end diff --git a/spec/lib/gitlab/ci/status/build/failed_spec.rb b/spec/lib/gitlab/ci/status/build/failed_spec.rb index cadb424ea2c..3e4b8d22d21 100644 --- a/spec/lib/gitlab/ci/status/build/failed_spec.rb +++ b/spec/lib/gitlab/ci/status/build/failed_spec.rb @@ -52,7 +52,7 @@ describe Gitlab::Ci::Status::Build::Failed do let(:status) { Gitlab::Ci::Status::Failed.new(build, user) } it 'does override badge_tooltip' do - expect(subject.badge_tooltip).to eq 'failed <br> (script failure)' + expect(subject.badge_tooltip).to eq 'failed - (script failure)' end end @@ -61,7 +61,7 @@ describe Gitlab::Ci::Status::Build::Failed do let(:status) { Gitlab::Ci::Status::Failed.new(build, user) } it 'does override status_tooltip' do - expect(subject.status_tooltip).to eq 'failed <br> (script failure)' + expect(subject.status_tooltip).to eq 'failed - (script failure)' end end diff --git a/spec/lib/gitlab/ci/status/build/retried_spec.rb b/spec/lib/gitlab/ci/status/build/retried_spec.rb index ee9acaf1c21..76c2fb01e3f 100644 --- a/spec/lib/gitlab/ci/status/build/retried_spec.rb +++ b/spec/lib/gitlab/ci/status/build/retried_spec.rb @@ -66,7 +66,7 @@ describe Gitlab::Ci::Status::Build::Retried do let(:status) { Gitlab::Ci::Status::Build::Failed.new(failed_status) } it 'does override status_tooltip' do - expect(subject.status_tooltip).to eq 'failed <br> (unknown failure) (retried)' + expect(subject.status_tooltip).to eq 'failed - (unknown failure) (retried)' end end diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb index 6dfaa3b72f7..547d95e0908 100644 --- a/spec/presenters/ci/build_presenter_spec.rb +++ b/spec/presenters/ci/build_presenter_spec.rb @@ -78,7 +78,7 @@ describe Ci::BuildPresenter do it 'returns the reason of failure' do status_title = presenter.status_title - expect(status_title).to eq('Failed <br> (unknown failure)') + expect(status_title).to eq('Failed - (unknown failure)') end end @@ -89,7 +89,7 @@ describe Ci::BuildPresenter do status_title = presenter.status_title expect(status_title).not_to include('(retried)') - expect(status_title).to eq('Failed <br> (unknown failure)') + expect(status_title).to eq('Failed - (unknown failure)') end end @@ -99,7 +99,7 @@ describe Ci::BuildPresenter do it 'returns the reason of failure' do status_title = presenter.status_title - expect(status_title).to eq('Failed <br> (unknown failure)') + expect(status_title).to eq('Failed - (unknown failure)') end end @@ -173,7 +173,7 @@ describe Ci::BuildPresenter do it 'returns the reason of failure' do tooltip = subject.tooltip_message - expect(tooltip).to eq("#{build.name} - failed <br> (script failure)") + expect(tooltip).to eq("#{build.name} - failed - (script failure)") end end @@ -183,7 +183,7 @@ describe Ci::BuildPresenter do it 'should include the reason of failure and the retried title' do tooltip = subject.tooltip_message - expect(tooltip).to eq("#{build.name} - failed <br> (script failure) (retried)") + expect(tooltip).to eq("#{build.name} - failed - (script failure) (retried)") end end @@ -193,7 +193,7 @@ describe Ci::BuildPresenter do it 'should include the reason of failure' do tooltip = subject.tooltip_message - expect(tooltip).to eq("#{build.name} - failed <br> (script failure) (allowed to fail)") + expect(tooltip).to eq("#{build.name} - failed - (script failure) (allowed to fail)") end end diff --git a/spec/serializers/build_serializer_spec.rb b/spec/serializers/build_serializer_spec.rb index 52459cd369d..302ef147eb2 100644 --- a/spec/serializers/build_serializer_spec.rb +++ b/spec/serializers/build_serializer_spec.rb @@ -37,7 +37,7 @@ describe BuildSerializer do it 'serializes only status' do expect(subject[:text]).to eq(status.text) expect(subject[:label]).to eq('failed') - expect(subject[:tooltip]).to eq('failed <br> (unknown failure)') + expect(subject[:tooltip]).to eq('failed - (unknown failure)') expect(subject[:icon]).to eq(status.icon) expect(subject[:favicon]).to match_asset_path("/assets/ci_favicons/#{status.favicon}.png") end diff --git a/spec/serializers/job_entity_spec.rb b/spec/serializers/job_entity_spec.rb index a5581a34517..8e1ca3f308d 100644 --- a/spec/serializers/job_entity_spec.rb +++ b/spec/serializers/job_entity_spec.rb @@ -142,7 +142,7 @@ describe JobEntity do end it 'should indicate the failure reason on tooltip' do - expect(subject[:status][:tooltip]).to eq('failed <br> (API failure)') + expect(subject[:status][:tooltip]).to eq('failed - (API failure)') end it 'should include a callout message with a verbose output' do @@ -166,7 +166,7 @@ describe JobEntity do end it 'should indicate the failure reason on tooltip' do - expect(subject[:status][:tooltip]).to eq('failed <br> (API failure) (allowed to fail)') + expect(subject[:status][:tooltip]).to eq('failed - (API failure) (allowed to fail)') end it 'should include a callout message with a verbose output' do |