Merge branch 'security-2694-pipeline' into 'master'

[master] Removes `<br>` sent from backend on tooltips in jobs

See merge request gitlab/gitlabhq!2455

app/assets/javascripts/pipelines/components/graph/dropdown_job_component.vue

 <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}`;
     },
   },
 

app/assets/javascripts/pipelines/components/graph/job_component.vue

 <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"
     >
 

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
 

app/views/projects/jobs/_sidebar.html.haml

@@ -82,7 +82,7 @@
         - 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.dup)
-            = link_to(project_job_path(@project, build), data: { toggle: 'tooltip', html: 'true', title: tooltip, container: 'body' }) do
+            = 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)

lib/gitlab/ci/status/build/failed.rb

@@ -32,7 +32,7 @@ module Gitlab
           end
 
           def description
-            "<br> (#{failure_reason_message})"
+            "- (#{failure_reason_message})"
           end
 
           def failure_reason_message

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

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

spec/features/projects/pipelines/pipeline_spec.rb

@@ -126,7 +126,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
@@ -319,7 +319,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

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

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',
-    );
-  });
 });

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: 'status_success',
-            label: 'success',
-            tooltip: 'failed',
-          },
-        },
-      });
-
-      expect(
-        component.$el.querySelector('.js-job-component-tooltip').getAttribute('data-original-title'),
-      ).toEqual('&lt;img src=x onerror=alert(document.domain)&gt; - failed');
-    });
-  });
 });

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

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
 

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
 

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
 

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
 

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

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