Merge pull request #122 from adangel/issue-121

Escape violation messages

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
 

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

lib/pmdtester/builders/project_hasher.rb

@@ -117,15 +117,22 @@ 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 && violation.changed?)
       }
       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)
+      CGI.escapeHTML(string)
+    end
   end
 end

test/resources/summary_report_builder_issue121/base-report.xml

@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<pmd xmlns="http://pmd.sourceforge.net/report/2.0.0"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://pmd.sourceforge.net/report/2.0.0 http://pmd.sourceforge.net/report_2_0_0.xsd"
+    version="6.3.0-SNAPSHOT" timestamp="2018-04-16T22:41:45.065">
+<file name="Same1.java">
+<violation beginline="402" endline="402" begincolumn="22" endcolumn="36" rule="CyclomaticComplexity" ruleset="Design" externalInfoUrl="https://docs.pmd-code.org/snapshot/pmd_rules_apex_design.html#cyclomaticcomplexity" priority="3">
+The method 'foo(List&lt;SObject&gt;, Map&lt;Id,SObject&gt;)' has a cyclomatic complexity of 19.
+</violation>
+</file>
+</file>
+</pmd>

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
+}

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."
+        }
+    ]
+}

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."
+        }
+    ]
+}

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 &#39;foo(List&lt;SObject&gt;, Map&lt;Id<del class=\"differ\">,</del><ins class=\"differ\">, </ins>SObject&gt;)&#39; has a cyclomatic complexity of 19."
+        }
+    ]
+}

test/resources/summary_report_builder_issue121/patch-report.xml

@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<pmd xmlns="http://pmd.sourceforge.net/report/2.0.0"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://pmd.sourceforge.net/report/2.0.0 http://pmd.sourceforge.net/report_2_0_0.xsd"
+    version="6.3.0-SNAPSHOT" timestamp="2018-04-16T22:41:45.065">
+<file name="Same1.java">
+<violation beginline="402" endline="402" begincolumn="22" endcolumn="36" rule="CyclomaticComplexity" ruleset="Design" externalInfoUrl="https://docs.pmd-code.org/snapshot/pmd_rules_apex_design.html#cyclomaticcomplexity" priority="3">
+The method 'foo(List&lt;SObject&gt;, Map&lt;Id, SObject&gt;)' has a cyclomatic complexity of 19.
+</violation>
+</file>
+</file>
+</pmd>

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
+}

test/resources/summary_report_builder_issue121/project-list.xml

@@ -0,0 +1,14 @@
+<?xml version="1.0"?>
+
+<projectlist xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+xsi:noNamespaceSchemaLocation="projectlist_1_2_0.xsd">
+  <description>Standard Projects</description>
+
+  <project>
+    <name>sample_project</name>
+    <type>git</type>
+    <connection>https://github.com/pmd/sample_project</connection>
+    <tag>main</tag>
+  </project>
+
+</projectlist>

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