diff --git a/lib/pact_broker/api/renderers/html_pact_renderer.rb b/lib/pact_broker/api/renderers/html_pact_renderer.rb index 86ef66bb8..bdde3e3ad 100644 --- a/lib/pact_broker/api/renderers/html_pact_renderer.rb +++ b/lib/pact_broker/api/renderers/html_pact_renderer.rb @@ -4,6 +4,7 @@ require 'pact/doc/markdown/consumer_contract_renderer' require 'pact_broker/api/pact_broker_urls' require 'pact_broker/logging' +require 'rack' module PactBroker module Api @@ -58,8 +59,8 @@ def pact_metadata #{badge_list_item} #{badge_markdown_item}
Repository URL: <% -repository_link = "#{escape_html(repository_url)}" +repository_link = "#{repository_url}" %> <%= Sanitize.fragment(repository_link, Sanitize::Config::BASIC) %> @@ -105,7 +105,7 @@ var relationshipPath = function(relationship, nodeLocations, pacticipants) { var latestPactUrl = function (consumerName, providerName) { //TODO send this with the relationship data - return '<%= base_url %>/pacts/provider/' + providerName + '/consumer/' + consumerName + '/latest'; + return '<%= base_url %>' + '/pacts/provider/' + encodeURIComponent(providerName) + '/consumer/' + encodeURIComponent(consumerName) + '/latest'; }; var relationshipData = function(pacticipant, relationships) { diff --git a/lib/pact_broker/ui/views/index/show-with-tags.haml b/lib/pact_broker/ui/views/index/show-with-tags.haml index 45c060af4..88ea4a7aa 100644 --- a/lib/pact_broker/ui/views/index/show-with-tags.haml +++ b/lib/pact_broker/ui/views/index/show-with-tags.haml @@ -1,9 +1,9 @@ %body - = render :haml, :'index/_css_and_js', :layout => false + != render :haml, :'index/_css_and_js', :layout => false .container - = render :haml, :'index/_navbar', :layout => false, locals: {tag_toggle: false, base_url: base_url} + != render :haml, :'index/_navbar', :layout => false, locals: {tag_toggle: false, base_url: base_url} - if index_items.empty? - = render :haml, :'index/_getting-started', :layout => false + != render :haml, :'index/_getting-started', :layout => false %h1.page-header Pacts %table.table.table-bordered.table-striped{ id: 'relationships' } @@ -37,10 +37,10 @@ %tr{'data-pact-versions-url': index_item.pact_versions_url, 'data-consumer-name': index_item.consumer_name, 'data-provider-name': index_item.provider_name, 'data-integration-url': index_item.integration_url} %td.consumer %a{:href => index_item.consumer_group_url } - = escape_html(index_item.consumer_name) + = index_item.consumer_name %td.consumer-version-number{"data-text": index_item.consumer_version_order} %div.clippable - = escape_html(index_item.consumer_version_number) + = index_item.consumer_version_number - if index_item.consumer_version_number %button.clippy.invisible{ title: "Copy to clipboard" } %span.glyphicon.glyphicon-copy @@ -49,7 +49,7 @@ latest - index_item.consumer_version_latest_tag_names.each do | tag_name | .tag.label.label-primary - = escape_html(tag_name) + = tag_name %td.pact %span.pact %a{ href: index_item.pact_url, title: "View pact" } @@ -57,16 +57,16 @@ %a{ href: index_item.pact_matrix_url, title: "View pact matrix" } %td.provider %a{ href: index_item.provider_group_url } - = escape_html(index_item.provider_name) + = index_item.provider_name %td.provider-version-number %div.clippable - = escape_html(index_item.provider_version_number) + = index_item.provider_version_number - if index_item.provider_version_number %button.clippy.invisible{ title: "Copy to clipboard" } %span.glyphicon.glyphicon-copy - index_item.provider_version_latest_tag_names.each do | tag_name | .tag.label.label-primary - = escape_html(tag_name) + = tag_name %td{"data-text": index_item.publication_date_of_latest_pact_order} = index_item.publication_date_of_latest_pact.gsub("about ", "") %td{ class: index_item.webhook_status } @@ -86,7 +86,7 @@ %div.pagination - pagination_locals = { page_number: page_number, page_size: page_size, pagination_record_count: pagination_record_count, current_page_size: current_page_size } - = render :haml, :'index/_pagination', :layout => false, locals: pagination_locals + != render :haml, :'index/_pagination', :layout => false, locals: pagination_locals :javascript $(function(){ diff --git a/lib/pact_broker/ui/views/index/show.haml b/lib/pact_broker/ui/views/index/show.haml index 059a01208..9cd62e6b4 100644 --- a/lib/pact_broker/ui/views/index/show.haml +++ b/lib/pact_broker/ui/views/index/show.haml @@ -1,9 +1,9 @@ %body - = render :haml, :'index/_css_and_js', :layout => false + != render :haml, :'index/_css_and_js', :layout => false .container - = render :haml, :'index/_navbar', :layout => false, locals: {tag_toggle: true, base_url: base_url} + != render :haml, :'index/_navbar', :layout => false, locals: {tag_toggle: true, base_url: base_url} - if index_items.empty? - = render :haml, :'index/_getting-started', :layout => false + != render :haml, :'index/_getting-started', :layout => false %h1.page-header Pacts %table.table.table-bordered.table-striped{ id: 'relationships' } @@ -32,7 +32,7 @@ %td %td.consumer %a{ href: index_item.consumer_group_url } - = escape_html(index_item.consumer_name) + = index_item.consumer_name %td.pact %span.pact %a{ href: index_item.latest_pact_url, :title => "View pact" } @@ -40,7 +40,7 @@ %a{ href: index_item.pact_matrix_url, title: "View pact matrix" } %td.provider %a{ href: index_item.provider_group_url } - = escape_html(index_item.provider_name) + = index_item.provider_name %td %td{"data-text": index_item.publication_date_of_latest_pact_order} = index_item.publication_date_of_latest_pact @@ -58,7 +58,7 @@ %div.pagination - pagination_locals = { page_number: page_number, page_size: page_size, pagination_record_count: pagination_record_count, current_page_size: current_page_size } - = render :haml, :'index/_pagination', :layout => false, locals: pagination_locals + != render :haml, :'index/_pagination', :layout => false, locals: pagination_locals :javascript diff --git a/lib/pact_broker/ui/views/layouts/main.haml b/lib/pact_broker/ui/views/layouts/main.haml index d5f5e3fdc..748e0714f 100644 --- a/lib/pact_broker/ui/views/layouts/main.haml +++ b/lib/pact_broker/ui/views/layouts/main.haml @@ -6,4 +6,4 @@ %link{rel: 'stylesheet', href: "#{base_url}/css/bootstrap.min.css"} %script{type: 'text/javascript', src:"#{base_url}/javascripts/jquery-3.3.1.min.js"} %script{type: 'text/javascript', src:"#{base_url}/js/bootstrap.min.js"} - = yield + != yield diff --git a/lib/pact_broker/ui/views/matrix/show.haml b/lib/pact_broker/ui/views/matrix/show.haml index 92334bc8f..7475c3de3 100644 --- a/lib/pact_broker/ui/views/matrix/show.haml +++ b/lib/pact_broker/ui/views/matrix/show.haml @@ -20,15 +20,14 @@ - if defined?(errors) && errors.any? - errors.each do | error | %div.alert.alert-danger - = escape_html(error) - + = error %form{action: "#{base_url}/matrix", onsubmit:'return onSubmit()'} - selectors.each_with_index do | selector, index | .selector %label{for: "pacticipant#{index}"} Pacticipant name - %input{name: 'q[]pacticipant', id: "pacticipant1#{index}", class: 'pacticipant_name', value: escape_html(selector.pacticipant_name)} + %input{name: 'q[]pacticipant', id: "pacticipant1#{index}", class: 'pacticipant_name', value: selector.pacticipant_name} .input-group @@ -45,9 +44,9 @@ %option{ value: 'specify-all-tagged', selected: selector.specify_all_tagged } All versions with tag... - %input{name: 'q[]version', type: 'text', id: "pacticipant#{index}_version", class: 'version', value: escape_html(selector.pacticipant_version_number)} + %input{name: 'q[]version', type: 'text', id: "pacticipant#{index}_version", class: 'version', value: selector.pacticipant_version_number} - %input{name: 'q[]tag', type: 'text', id: "pacticipant#{index}_tag", class: 'tag', value: escape_html(selector.tag)} + %input{name: 'q[]tag', type: 'text', id: "pacticipant#{index}_tag", class: 'tag', value: selector.tag} %input{name: 'q[]latest', value: 'true', hidden: true, class: 'latest-flag'} diff --git a/public/javascripts/pact.js b/public/javascripts/pact.js index 68528242c..bcfcddccb 100644 --- a/public/javascripts/pact.js +++ b/public/javascripts/pact.js @@ -39,10 +39,14 @@ $(document).ready(function() { }); }); +function h(string) { + return jQuery('
').text(string).html(); +} + function createDeletionConfirmationText(data) { return `Do you wish to delete the pact for version ${ - data.consumerVersionNumber - } of ${data.consumerName}?`; + h(data.consumerVersionNumber) + } of ${h(data.consumerName)}?`; } function confirmDeleteResource( diff --git a/script/seed.rb b/script/seed.rb index ed4e0c288..6ad1fd139 100755 --- a/script/seed.rb +++ b/script/seed.rb @@ -58,13 +58,11 @@ url: "http://localhost:9292/verification-published-webhook", body: webhook_body.to_json) .set_now(Date.today - 101) - .tap{ | it | - 2.times do | i | - it.create_pact_with_verification("Foo", i.to_s, "Bar", i.to_s) - .create_pact_with_verification("Bar", i.to_s, "Foo", i.to_s) - .add_day - end - }.create_pact_with_hierarchy("Foo", "100", "Bar") + .create_pact_with_hierarchy("Foo/Foo", "100", "Bar/Bar") + .create_pact_with_hierarchy("Foo", "1", "Bar") + .create_pact_with_hierarchy("", "", "Bar/Bar") + .create_consumer_version_tag("prod") + .create_verification(provider_version: "1", tag_names: "prod") # .create_certificate(path: 'spec/fixtures/certificates/self-signed.badssl.com.pem') diff --git a/spec/lib/pact_broker/api/renderers/html_pact_renderer_spec.rb b/spec/lib/pact_broker/api/renderers/html_pact_renderer_spec.rb index 8895aa324..65120a4f5 100644 --- a/spec/lib/pact_broker/api/renderers/html_pact_renderer_spec.rb +++ b/spec/lib/pact_broker/api/renderers/html_pact_renderer_spec.rb @@ -23,22 +23,26 @@ module Renderers Timecop.return end - let(:consumer) { double('consumer', name: 'Consumer')} - let(:provider) { double('provider', name: 'Provider')} + let(:consumer_name) { 'Consumer' } + let(:provider_name) { 'Provider' } + let(:consumer_version_number) { '1.2.3' } + let(:consumer) { double('consumer', name: consumer_name) } + let(:provider) { double('provider', name: provider_name) } let(:consumer_version) { double('consumer version') } let(:created_at) { DateTime.new(2014, 02, 27) } let(:json_content) { load_fixture('renderer_pact.json') } let(:pact) do double('pact', json_content: json_content, - consumer_version_number: '1.2.3', + consumer_version_number: consumer_version_number, consumer: consumer, provider: provider, - consumer_version_tag_names: ['prod', 'master'], + consumer_version_tag_names: consumer_version_tag_names, created_at: created_at, consumer_version: consumer_version ) end + let(:consumer_version_tag_names) { ['prod', 'master'] } let(:pact_url) { '/pact/url' } let(:matrix_url) { '/matrix/url' } let(:options) do @@ -49,7 +53,7 @@ module Renderers end let(:logger) { double('logger').as_null_object } - subject { HtmlPactRenderer.call pact, options } + subject { HtmlPactRenderer.call(pact, options) } describe ".call" do it "renders the pact as HTML" do @@ -69,7 +73,7 @@ module Renderers end it "renders the badge image" do - expect(subject).to include "" + expect(subject).to include "" end it "renders a text area with the badge markdown" do @@ -81,6 +85,20 @@ module Renderers expect(subject).to include matrix_url end + context "with dodgey data" do + let(:consumer_name) { '' } + let(:provider_name) { '' } + let(:consumer_version_number) { '' } + let(:consumer_version_tag_names) { [''] } + + it "does not contain the literal / Pact Status](http://badge)]' + end + end + context "when enable_public_badge_access is false" do before do PactBroker.configuration.enable_public_badge_access = false diff --git a/spec/lib/pact_broker/badges/service_spec.rb b/spec/lib/pact_broker/badges/service_spec.rb index cb13befb8..fff9016d7 100644 --- a/spec/lib/pact_broker/badges/service_spec.rb +++ b/spec/lib/pact_broker/badges/service_spec.rb @@ -13,7 +13,7 @@ module Badges let(:expected_url) { "https://img.shields.io/badge/#{expected_left_text}-#{expected_right_text}-#{expected_color}.svg" } let(:expected_color) { "brightgreen" } let(:expected_right_text) { "verified" } - let(:expected_left_text) { "foo--bar%2fthing__blah%20pact" } + let(:expected_left_text) { "foo--bar%2Fthing__blah%20pact" } let(:response_status) { 200 } let!(:http_request) do stub_request(:get, expected_url).to_return(:status => response_status, :body => "svg") @@ -62,7 +62,7 @@ module Badges end context "when initials is true" do - let(:expected_left_text) { "fb%2ftb%20pact" } + let(:expected_left_text) { "fb%2Ftb%20pact" } let(:initials) { true } it "creates a badge with the consumer and provider initials" do @@ -73,7 +73,7 @@ module Badges end context "when initials is true but the consumer and provider names only contain one word" do - let(:expected_left_text) { "foo%2fbar%20pact" } + let(:expected_left_text) { "foo%2Fbar%20pact" } let(:initials) { true } let(:pact) { double("pact", consumer_name: "Foo", provider_name: "Bar") } @@ -85,7 +85,7 @@ module Badges end context "when initials is true but the consumer and provider names are one camel cased word" do - let(:expected_left_text) { "fa%2fbp%20pact" } + let(:expected_left_text) { "fa%2Fbp%20pact" } let(:initials) { true } let(:pact) { double("pact", consumer_name: "FooApp", provider_name: "barProvider") } @@ -97,7 +97,7 @@ module Badges end context "when initials is true but the consumer and provider names are one camel cased word" do - let(:expected_left_text) { "fa%2fdat%20pact" } + let(:expected_left_text) { "fa%2Fdat%20pact" } let(:initials) { true } let(:pact) { double("pact", consumer_name: "FooApp", provider_name: "doAThing") } @@ -111,7 +111,7 @@ module Badges context "when the tags are supplied" do let(:tags) { { consumer_tag: "prod", provider_tag: "master" } } - let(:expected_left_text) { "foo--bar%20(prod)%2fthing__blah%20(master)%20pact" } + let(:expected_left_text) { "foo--bar%20%28prod%29%2Fthing__blah%20%28master%29%20pact" } it "creates a badge with the consumer and provider names, not initials" do subject