Skip to content

Commit 4cd955c

Browse files
jsotuyodjsotuyod
authored
Merge pull request #122 from adangel/issue-121
Escape violation messages
2 parents 322be92 + 9317d85 commit 4cd955c

13 files changed

+148
-3
lines changed

‎History.md

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
## Enhancements
66

77
## Fixed Issues
8+
* [#121](https://github.com/pmd/pmd-regression-tester/issues/121): Violation messages should be escaped for html
89

910
## External Contributions
1011

‎README.rdoc

+1
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ The tool creates the following folders:
127127
gem install pmdtester --pre
128128

129129
== DEVELOPERS:
130+
130131
git clone https://github.com/pmd/pmd-regression-tester.git
131132
cd pmd-regression-tester
132133
gem install bundler

‎lib/pmdtester/builders/project_hasher.rb

+10-3
Original file line numberDiff line numberDiff line change
@@ -117,15 +117,22 @@ def make_violation_hash(file_ref, violation, is_diff = TRUE)
117117
'l' => violation.line,
118118
'f' => file_ref,
119119
'r' => violation.rule_name,
120-
'm' => is_diff && violation.changed? ? diff_fragments(violation) : violation.message
120+
'm' => create_violation_message(violation, is_diff && violation.changed?)
121121
}
122122
h['ol'] = violation.old_line if is_diff && violation.changed? && violation.line != violation.old_line
123123
h
124124
end
125125

126-
def diff_fragments(violation)
127-
diff = Differ.diff_by_word(violation.message, violation.old_message)
126+
def create_violation_message(violation, is_diff)
127+
return escape_html(violation.message) unless is_diff
128+
129+
diff = Differ.diff_by_word(escape_html(violation.message),
130+
escape_html(violation.old_message))
128131
diff.format_as(:html)
129132
end
133+
134+
def escape_html(string)
135+
CGI.escapeHTML(string)
136+
end
130137
end
131138
end

‎test/resources/summary_report_builder_issue121/base-report.xml

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<pmd xmlns="http://pmd.sourceforge.net/report/2.0.0"
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
xsi:schemaLocation="http://pmd.sourceforge.net/report/2.0.0 http://pmd.sourceforge.net/report_2_0_0.xsd"
5+
version="6.3.0-SNAPSHOT" timestamp="2018-04-16T22:41:45.065">
6+
<file name="Same1.java">
7+
<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">
8+
The method 'foo(List&lt;SObject&gt;, Map&lt;Id,SObject&gt;)' has a cyclomatic complexity of 19.
9+
</violation>
10+
</file>
11+
</file>
12+
</pmd>

‎test/resources/summary_report_builder_issue121/base_branch_info.json

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"branch_last_sha":"test sha",
3+
"branch_last_message":"test message",
4+
"branch_name":"base_branch",
5+
"timestamp":"foo",
6+
"execution_time":121.123,
7+
"jdk_version":"test version",
8+
"language":"test language",
9+
"pull_request":42
10+
}

‎test/resources/summary_report_builder_issue121/empty_config.xml

Whitespace-only changes.

‎test/resources/summary_report_builder_issue121/expected_base_data.js

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
let project = {
2+
"source_link_base":"https://github.com/pmd/sample_project/tree/main",
3+
"source_link_template":"https://github.com/pmd/sample_project/tree/main/{file}#L{line}",
4+
"file_index":[
5+
"Same1.java"
6+
],
7+
"violations":[
8+
{
9+
"t":"+",
10+
"l":402,
11+
"f":0,
12+
"r":"CyclomaticComplexity",
13+
"m":"The method &#39;foo(List&lt;SObject&gt;, Map&lt;Id,SObject&gt;)&#39; has a cyclomatic complexity of 19."
14+
}
15+
]
16+
}

‎test/resources/summary_report_builder_issue121/expected_patch_data.js

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
let project = {
2+
"source_link_base":"https://github.com/pmd/sample_project/tree/main",
3+
"source_link_template":"https://github.com/pmd/sample_project/tree/main/{file}#L{line}",
4+
"file_index":[
5+
"Same1.java"
6+
],
7+
"violations":[
8+
{
9+
"t":"+",
10+
"l":402,
11+
"f":0,
12+
"r":"CyclomaticComplexity",
13+
"m":"The method &#39;foo(List&lt;SObject&gt;, Map&lt;Id, SObject&gt;)&#39; has a cyclomatic complexity of 19."
14+
}
15+
]
16+
}

‎test/resources/summary_report_builder_issue121/expected_project_data.js

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
let project = {
2+
"source_link_base":"https://github.com/pmd/sample_project/tree/main",
3+
"source_link_template":"https://github.com/pmd/sample_project/tree/main/{file}#L{line}",
4+
"file_index":[
5+
"Same1.java"
6+
],
7+
"violations":[
8+
{
9+
"t":"~",
10+
"l":402,
11+
"f":0,
12+
"r":"CyclomaticComplexity",
13+
"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."
14+
}
15+
]
16+
}

‎test/resources/summary_report_builder_issue121/patch-report.xml

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<pmd xmlns="http://pmd.sourceforge.net/report/2.0.0"
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
xsi:schemaLocation="http://pmd.sourceforge.net/report/2.0.0 http://pmd.sourceforge.net/report_2_0_0.xsd"
5+
version="6.3.0-SNAPSHOT" timestamp="2018-04-16T22:41:45.065">
6+
<file name="Same1.java">
7+
<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">
8+
The method 'foo(List&lt;SObject&gt;, Map&lt;Id, SObject&gt;)' has a cyclomatic complexity of 19.
9+
</violation>
10+
</file>
11+
</file>
12+
</pmd>

‎test/resources/summary_report_builder_issue121/patch_branch_info.json

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"branch_last_sha":"test sha",
3+
"branch_last_message":"test message",
4+
"branch_name":"patch_branch",
5+
"timestamp":"foo2",
6+
"execution_time":121.123,
7+
"jdk_version":"test version",
8+
"language":"test language",
9+
"pull_request":42
10+
}

‎test/resources/summary_report_builder_issue121/project-list.xml

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?xml version="1.0"?>
2+
3+
<projectlist xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
xsi:noNamespaceSchemaLocation="projectlist_1_2_0.xsd">
5+
<description>Standard Projects</description>
6+
7+
<project>
8+
<name>sample_project</name>
9+
<type>git</type>
10+
<connection>https://github.com/pmd/sample_project</connection>
11+
<tag>main</tag>
12+
</project>
13+
14+
</projectlist>

‎test/test_summary_report_builder.rb

+30
Original file line numberDiff line numberDiff line change
@@ -50,4 +50,34 @@ def test_summary_report_builder_with_filter
5050
assert_file_equals('test/resources/summary_report_builder/expected_filtered_index.html',
5151
'target/reports/diff/index.html')
5252
end
53+
54+
# See https://github.com/pmd/pmd-regression-tester/issues/121
55+
def test_summary_report_builder_issue121
56+
test_resources_path = 'test/resources/summary_report_builder_issue121'
57+
projects = PmdTester::ProjectsParser.new.parse("#{test_resources_path}/project-list.xml")
58+
59+
base_path = 'target/reports/base_branch'
60+
FileUtils.mkdir_p(base_path)
61+
FileUtils.cp("#{test_resources_path}/base_branch_info.json", "#{base_path}/branch_info.json")
62+
FileUtils.cp("#{test_resources_path}/empty_config.xml", "#{base_path}/config.xml")
63+
FileUtils.mkdir_p("#{base_path}/sample_project")
64+
FileUtils.cp("#{test_resources_path}/base-report.xml", "#{base_path}/sample_project/pmd_report.xml")
65+
66+
patch_path = 'target/reports/patch_branch'
67+
FileUtils.mkdir_p(patch_path)
68+
FileUtils.cp("#{test_resources_path}/patch_branch_info.json", "#{patch_path}/branch_info.json")
69+
FileUtils.cp("#{test_resources_path}/empty_config.xml", "#{patch_path}/config.xml")
70+
FileUtils.mkdir_p("#{patch_path}/sample_project")
71+
FileUtils.cp("#{test_resources_path}/patch-report.xml", "#{patch_path}/sample_project/pmd_report.xml")
72+
73+
build_html_reports(projects, PmdTester::PmdBranchDetail.load('base_branch', nil),
74+
PmdTester::PmdBranchDetail.load('patch_branch', nil))
75+
76+
assert_file_equals("#{test_resources_path}/expected_base_data.js",
77+
'target/reports/diff/sample_project/base_data.js')
78+
assert_file_equals("#{test_resources_path}/expected_patch_data.js",
79+
'target/reports/diff/sample_project/patch_data.js')
80+
assert_file_equals("#{test_resources_path}/expected_project_data.js",
81+
'target/reports/diff/sample_project/project_data.js')
82+
end
5383
end

0 commit comments

Comments
 (0)