From 4ee50a17a880020a7afb96108f303cd54ccc1065 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 9 Nov 2023 11:49:25 +0100 Subject: [PATCH 1/3] Escape violation messages Fixes #121 --- History.md | 1 + lib/pmdtester/builders/project_hasher.rb | 15 ++++++++-- .../base-report.xml | 12 ++++++++ .../base_branch_info.json | 10 +++++++ .../empty_config.xml | 0 .../expected_base_data.js | 16 ++++++++++ .../expected_patch_data.js | 16 ++++++++++ .../expected_project_data.js | 16 ++++++++++ .../patch-report.xml | 12 ++++++++ .../patch_branch_info.json | 10 +++++++ .../project-list.xml | 14 +++++++++ test/test_summary_report_builder.rb | 30 +++++++++++++++++++ 12 files changed, 149 insertions(+), 3 deletions(-) create mode 100644 test/resources/summary_report_builder_issue121/base-report.xml create mode 100644 test/resources/summary_report_builder_issue121/base_branch_info.json create mode 100644 test/resources/summary_report_builder_issue121/empty_config.xml create mode 100644 test/resources/summary_report_builder_issue121/expected_base_data.js create mode 100644 test/resources/summary_report_builder_issue121/expected_patch_data.js create mode 100644 test/resources/summary_report_builder_issue121/expected_project_data.js create mode 100644 test/resources/summary_report_builder_issue121/patch-report.xml create mode 100644 test/resources/summary_report_builder_issue121/patch_branch_info.json create mode 100644 test/resources/summary_report_builder_issue121/project-list.xml diff --git a/History.md b/History.md index 6bc3443c..7b85ec36 100644 --- a/History.md +++ b/History.md @@ -5,6 +5,7 @@ ## Enhancements ## Fixed Issues +* [#121](https://github.com/pmd/pmd-regression-tester/issues/121): Violation messages should be escaped for html ## External Contributions diff --git a/lib/pmdtester/builders/project_hasher.rb b/lib/pmdtester/builders/project_hasher.rb index 63997a78..30b4148f 100644 --- a/lib/pmdtester/builders/project_hasher.rb +++ b/lib/pmdtester/builders/project_hasher.rb @@ -117,15 +117,24 @@ def make_violation_hash(file_ref, violation, is_diff = TRUE) 'l' => violation.line, 'f' => file_ref, 'r' => violation.rule_name, - 'm' => is_diff && violation.changed? ? diff_fragments(violation) : violation.message + 'm' => create_violation_message(violation, is_diff) } h['ol'] = violation.old_line if is_diff && violation.changed? && violation.line != violation.old_line h end - def diff_fragments(violation) - diff = Differ.diff_by_word(violation.message, violation.old_message) + def create_violation_message(violation, is_diff) + return escape_html(violation.message) unless is_diff + + diff = Differ.diff_by_word(escape_html(violation.message), + escape_html(violation.old_message)) diff.format_as(:html) end + + def escape_html(string) + return '' if string.nil? + + CGI.escapeHTML(string) + end end end diff --git a/test/resources/summary_report_builder_issue121/base-report.xml b/test/resources/summary_report_builder_issue121/base-report.xml new file mode 100644 index 00000000..f4b9a2f0 --- /dev/null +++ b/test/resources/summary_report_builder_issue121/base-report.xml @@ -0,0 +1,12 @@ + + + + +The method 'foo(List<SObject>, Map<Id,SObject>)' has a cyclomatic complexity of 19. + + + + diff --git a/test/resources/summary_report_builder_issue121/base_branch_info.json b/test/resources/summary_report_builder_issue121/base_branch_info.json new file mode 100644 index 00000000..a752cd72 --- /dev/null +++ b/test/resources/summary_report_builder_issue121/base_branch_info.json @@ -0,0 +1,10 @@ +{ + "branch_last_sha":"test sha", + "branch_last_message":"test message", + "branch_name":"base_branch", + "timestamp":"foo", + "execution_time":121.123, + "jdk_version":"test version", + "language":"test language", + "pull_request":42 +} diff --git a/test/resources/summary_report_builder_issue121/empty_config.xml b/test/resources/summary_report_builder_issue121/empty_config.xml new file mode 100644 index 00000000..e69de29b diff --git a/test/resources/summary_report_builder_issue121/expected_base_data.js b/test/resources/summary_report_builder_issue121/expected_base_data.js new file mode 100644 index 00000000..4dd0bd5a --- /dev/null +++ b/test/resources/summary_report_builder_issue121/expected_base_data.js @@ -0,0 +1,16 @@ +let project = { + "source_link_base":"https://github.com/pmd/sample_project/tree/main", + "source_link_template":"https://github.com/pmd/sample_project/tree/main/{file}#L{line}", + "file_index":[ + "Same1.java" + ], + "violations":[ + { + "t":"+", + "l":402, + "f":0, + "r":"CyclomaticComplexity", + "m":"The method 'foo(List<SObject>, Map<Id,SObject>)' has a cyclomatic complexity of 19." + } + ] +} diff --git a/test/resources/summary_report_builder_issue121/expected_patch_data.js b/test/resources/summary_report_builder_issue121/expected_patch_data.js new file mode 100644 index 00000000..923eb646 --- /dev/null +++ b/test/resources/summary_report_builder_issue121/expected_patch_data.js @@ -0,0 +1,16 @@ +let project = { + "source_link_base":"https://github.com/pmd/sample_project/tree/main", + "source_link_template":"https://github.com/pmd/sample_project/tree/main/{file}#L{line}", + "file_index":[ + "Same1.java" + ], + "violations":[ + { + "t":"+", + "l":402, + "f":0, + "r":"CyclomaticComplexity", + "m":"The method 'foo(List<SObject>, Map<Id, SObject>)' has a cyclomatic complexity of 19." + } + ] +} diff --git a/test/resources/summary_report_builder_issue121/expected_project_data.js b/test/resources/summary_report_builder_issue121/expected_project_data.js new file mode 100644 index 00000000..af4e4617 --- /dev/null +++ b/test/resources/summary_report_builder_issue121/expected_project_data.js @@ -0,0 +1,16 @@ +let project = { + "source_link_base":"https://github.com/pmd/sample_project/tree/main", + "source_link_template":"https://github.com/pmd/sample_project/tree/main/{file}#L{line}", + "file_index":[ + "Same1.java" + ], + "violations":[ + { + "t":"~", + "l":402, + "f":0, + "r":"CyclomaticComplexity", + "m":"The method 'foo(List<SObject>, Map<Id,, SObject>)' has a cyclomatic complexity of 19." + } + ] +} diff --git a/test/resources/summary_report_builder_issue121/patch-report.xml b/test/resources/summary_report_builder_issue121/patch-report.xml new file mode 100644 index 00000000..8f2ee592 --- /dev/null +++ b/test/resources/summary_report_builder_issue121/patch-report.xml @@ -0,0 +1,12 @@ + + + + +The method 'foo(List<SObject>, Map<Id, SObject>)' has a cyclomatic complexity of 19. + + + + diff --git a/test/resources/summary_report_builder_issue121/patch_branch_info.json b/test/resources/summary_report_builder_issue121/patch_branch_info.json new file mode 100644 index 00000000..1ddb1230 --- /dev/null +++ b/test/resources/summary_report_builder_issue121/patch_branch_info.json @@ -0,0 +1,10 @@ +{ + "branch_last_sha":"test sha", + "branch_last_message":"test message", + "branch_name":"patch_branch", + "timestamp":"foo2", + "execution_time":121.123, + "jdk_version":"test version", + "language":"test language", + "pull_request":42 +} diff --git a/test/resources/summary_report_builder_issue121/project-list.xml b/test/resources/summary_report_builder_issue121/project-list.xml new file mode 100644 index 00000000..017bc2f9 --- /dev/null +++ b/test/resources/summary_report_builder_issue121/project-list.xml @@ -0,0 +1,14 @@ + + + + Standard Projects + + + sample_project + git + https://github.com/pmd/sample_project + main + + + diff --git a/test/test_summary_report_builder.rb b/test/test_summary_report_builder.rb index 3df3bc0a..6b15394c 100644 --- a/test/test_summary_report_builder.rb +++ b/test/test_summary_report_builder.rb @@ -50,4 +50,34 @@ def test_summary_report_builder_with_filter assert_file_equals('test/resources/summary_report_builder/expected_filtered_index.html', 'target/reports/diff/index.html') end + + # See https://github.com/pmd/pmd-regression-tester/issues/121 + def test_summary_report_builder_issue121 + test_resources_path = 'test/resources/summary_report_builder_issue121' + projects = PmdTester::ProjectsParser.new.parse("#{test_resources_path}/project-list.xml") + + base_path = 'target/reports/base_branch' + FileUtils.mkdir_p(base_path) + FileUtils.cp("#{test_resources_path}/base_branch_info.json", "#{base_path}/branch_info.json") + FileUtils.cp("#{test_resources_path}/empty_config.xml", "#{base_path}/config.xml") + FileUtils.mkdir_p("#{base_path}/sample_project") + FileUtils.cp("#{test_resources_path}/base-report.xml", "#{base_path}/sample_project/pmd_report.xml") + + patch_path = 'target/reports/patch_branch' + FileUtils.mkdir_p(patch_path) + FileUtils.cp("#{test_resources_path}/patch_branch_info.json", "#{patch_path}/branch_info.json") + FileUtils.cp("#{test_resources_path}/empty_config.xml", "#{patch_path}/config.xml") + FileUtils.mkdir_p("#{patch_path}/sample_project") + FileUtils.cp("#{test_resources_path}/patch-report.xml", "#{patch_path}/sample_project/pmd_report.xml") + + build_html_reports(projects, PmdTester::PmdBranchDetail.load('base_branch', nil), + PmdTester::PmdBranchDetail.load('patch_branch', nil)) + + assert_file_equals("#{test_resources_path}/expected_base_data.js", + 'target/reports/diff/sample_project/base_data.js') + assert_file_equals("#{test_resources_path}/expected_patch_data.js", + 'target/reports/diff/sample_project/patch_data.js') + assert_file_equals("#{test_resources_path}/expected_project_data.js", + 'target/reports/diff/sample_project/project_data.js') + end end From 41b41ec659de4a286a945ebbabd03f00c927f9b4 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 9 Nov 2023 12:09:38 +0100 Subject: [PATCH 2/3] trigger new build --- README.rdoc | 1 + 1 file changed, 1 insertion(+) diff --git a/README.rdoc b/README.rdoc index 62fbf5cf..dba29566 100644 --- a/README.rdoc +++ b/README.rdoc @@ -127,6 +127,7 @@ The tool creates the following folders: gem install pmdtester --pre == DEVELOPERS: + git clone https://github.com/pmd/pmd-regression-tester.git cd pmd-regression-tester gem install bundler From 9317d850f6b2b3b9728b7659217d97e4ec19ec25 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Tue, 14 Nov 2023 19:27:38 +0100 Subject: [PATCH 3/3] Only create diff if violation.changed? --- lib/pmdtester/builders/project_hasher.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/pmdtester/builders/project_hasher.rb b/lib/pmdtester/builders/project_hasher.rb index 30b4148f..b2d9b8dc 100644 --- a/lib/pmdtester/builders/project_hasher.rb +++ b/lib/pmdtester/builders/project_hasher.rb @@ -117,7 +117,7 @@ def make_violation_hash(file_ref, violation, is_diff = TRUE) 'l' => violation.line, 'f' => file_ref, 'r' => violation.rule_name, - 'm' => create_violation_message(violation, is_diff) + 'm' => create_violation_message(violation, is_diff && violation.changed?) } h['ol'] = violation.old_line if is_diff && violation.changed? && violation.line != violation.old_line h @@ -132,8 +132,6 @@ def create_violation_message(violation, is_diff) end def escape_html(string) - return '' if string.nil? - CGI.escapeHTML(string) end end