From edeabc3faf638716842a562bc63e0e72c7937281 Mon Sep 17 00:00:00 2001
From: Tim McCormack <tmccormack@edx.org>
Date: Fri, 11 Dec 2020 12:34:04 +0000
Subject: [PATCH] Add --summary-format=json option to XSS linter (#25851)
This will simplify updating of the linter thresholds file after XSS linter
violations are addressed.
---
scripts/xsslint/tests/test_main.py | 35 +++++++++++++++++++++
scripts/xsslint/xsslint/main.py | 6 ++++
scripts/xsslint/xsslint/reporting.py | 47 ++++++++++++++++++++++------
3 files changed, 78 insertions(+), 10 deletions(-)
diff --git a/scripts/xsslint/tests/test_main.py b/scripts/xsslint/tests/test_main.py
index d437441f03b..3e026b62abc 100644
--- a/scripts/xsslint/tests/test_main.py
+++ b/scripts/xsslint/tests/test_main.py
@@ -4,6 +4,7 @@ Tests for main.py
"""
+import json
import re
from six import StringIO
from unittest import TestCase
@@ -69,6 +70,7 @@ class TestXSSLinter(TestCase):
'list_files': False,
'verbose': False,
'rule_totals': False,
+ 'summary_format': 'eslint',
'skip_dirs': ()
},
summary_results=self.summary_results,
@@ -107,6 +109,7 @@ class TestXSSLinter(TestCase):
'list_files': False,
'verbose': True,
'rule_totals': False,
+ 'summary_format': 'eslint',
'skip_dirs': ()
},
summary_results=self.summary_results,
@@ -139,6 +142,7 @@ class TestXSSLinter(TestCase):
'list_files': False,
'verbose': False,
'rule_totals': True,
+ 'summary_format': 'eslint',
'skip_dirs': ()
},
summary_results=self.summary_results,
@@ -153,6 +157,36 @@ class TestXSSLinter(TestCase):
self.assertIsNotNone(re.search(r'{}:\s*{} violations'.format(self.ruleset.python_wrap_html.rule_id, 1), output))
self.assertIsNotNone(re.search(r'{} violations total'.format(5), output))
+ def test_lint_with_json_output(self):
+ """
+ Tests the top-level linting with JSON summary format.
+ """
+ _lint(
+ 'scripts/xsslint/tests/templates',
+ template_linters=self.template_linters,
+ options={
+ 'list_files': False,
+ 'verbose': False,
+ 'rule_totals': True,
+ 'summary_format': 'json',
+ 'skip_dirs': ()
+ },
+ summary_results=self.summary_results,
+ out=self.out,
+ )
+
+ output = self.out.getvalue()
+ self.assertIsNotNone(re.search(r'test\.py.*{}'.format(self.ruleset.python_wrap_html.rule_id), output))
+
+ # Find something that looks like pretty-printed JSON
+ json_match = re.search(r'\n\{.*\n\}', output, re.DOTALL)
+ self.assertIsNotNone(json_match)
+ data = json.loads(json_match.group())
+ # Check for rule counts (including zero-instance ones) and total
+ self.assertEqual(1, data['rules']['javascript-concat-html'])
+ self.assertEqual(0, data['rules']['python-concat-html'])
+ self.assertEqual(5, data['total'])
+
def test_lint_with_list_files(self):
"""
Tests the top-level linting with list files option.
@@ -164,6 +198,7 @@ class TestXSSLinter(TestCase):
'list_files': True,
'verbose': False,
'rule_totals': False,
+ 'summary_format': 'eslint',
'skip_dirs': ()
},
summary_results=self.summary_results,
diff --git a/scripts/xsslint/xsslint/main.py b/scripts/xsslint/xsslint/main.py
index 5f8b9c65efd..1eefc690953 100644
--- a/scripts/xsslint/xsslint/main.py
+++ b/scripts/xsslint/xsslint/main.py
@@ -151,6 +151,11 @@ def main():
'--rule-totals', dest='rule_totals', action='store_true',
help='Display the totals for each rule.'
)
+ parser.add_argument(
+ '--summary-format', dest='summary_format',
+ choices=['eslint', 'json'], default='eslint',
+ help='Choose the display format for the summary.'
+ )
parser.add_argument(
'--verbose', dest='verbose', action='store_true',
help='Print multiple lines where possible for additional context of violations.'
@@ -166,6 +171,7 @@ def main():
options = {
'list_files': args.list_files,
'rule_totals': args.rule_totals,
+ 'summary_format': args.summary_format,
'verbose': args.verbose,
'skip_dirs': getattr(config, 'SKIP_DIRS', ())
}
diff --git a/scripts/xsslint/xsslint/reporting.py b/scripts/xsslint/xsslint/reporting.py
index b50fe76d905..a1690e58d5c 100644
--- a/scripts/xsslint/xsslint/reporting.py
+++ b/scripts/xsslint/xsslint/reporting.py
@@ -3,6 +3,7 @@ Utility classes for reporting linter results.
"""
+import json
import os
import re
@@ -275,17 +276,43 @@ class SummaryResults(object):
"""
if options['list_files'] is False:
- if options['rule_totals']:
- max_rule_id_len = max(len(rule_id) for rule_id in self.totals_by_rule)
- print("", file=out)
- for rule_id in sorted(self.totals_by_rule.keys()):
- padding = " " * (max_rule_id_len - len(rule_id))
- print("{}: {}{} violations".format(rule_id, padding, self.totals_by_rule[rule_id]), file=out)
- print("", file=out)
-
- # matches output of eslint for simplicity
+ if options['summary_format'] == 'json':
+ self._print_json_format(options, out)
+ else:
+ self._print_eslint_format(options, out)
+
+ def _print_eslint_format(self, options, out):
+ """
+ Implementation of print_results with eslint-style output.
+ """
+ if options['rule_totals']:
+ max_rule_id_len = max(len(rule_id) for rule_id in self.totals_by_rule)
print("", file=out)
- print("{} violations total".format(self.total_violations), file=out)
+ for rule_id in sorted(self.totals_by_rule.keys()):
+ padding = " " * (max_rule_id_len - len(rule_id))
+ print("{}: {}{} violations".format(rule_id, padding, self.totals_by_rule[rule_id]), file=out)
+ print("", file=out)
+
+ # matches output of eslint for simplicity
+ print("", file=out)
+ print("{} violations total".format(self.total_violations), file=out)
+
+ def _print_json_format(self, options, out):
+ """
+ Implementation of print_results with JSON output.
+ """
+ print("", file=out)
+ print("Violation counts:", file=out)
+ data = {'rules': self.totals_by_rule}
+ if options['rule_totals']:
+ data['total'] = self.total_violations
+ json.dump(data, fp=out, indent=4, sort_keys=True)
+ print("", file=out)
+ print(
+ "If you've fixed some XSS issues and these numbers have gone down, "
+ "you can use this to update scripts/xsslint_thresholds.json",
+ file=out
+ )
class FileResults(object):
--
GitLab