Skip to content

Raise xsslint threshold - #16719

Closed
efischer19 wants to merge 1 commit into
masterfrom
efischer/xss_limit
Closed

Raise xsslint threshold#16719
efischer19 wants to merge 1 commit into
masterfrom
efischer/xss_limit

Conversation

@efischer19

Copy link
Copy Markdown
Contributor

We've gotten ourselves into a broken state on master, this limit ought to be raised in the short term to unblock PRs.

FYI @edx/testeng

@efischer19
efischer19 requested a review from robrap November 30, 2017 14:05
Comment thread scripts/xsslint_thresholds.json Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@efischer19

  1. Can you just bump the total to 1771, which is the number of violations (see below):
  2. You need to find the individual rule that is passing the threshold (see below again), and also bump that by one.

The following is the report from master failing:

Number of xss_linter.py violations: 1771

javascript-concat-html violations: 160
javascript-escape violations: 7
javascript-interpolate violations: 29
javascript-jquery-append violations: 77
javascript-jquery-html violations: 189
javascript-jquery-insert-into-target violations: 23
javascript-jquery-insertion violations: 19
javascript-jquery-prepend violations: 7
mako-html-entities violations: 0
mako-invalid-html-filter violations: 11
mako-invalid-js-filter violations: 192
mako-js-html-string violations: 0
mako-js-missing-quotes violations: 0
mako-missing-default violations: 182
mako-multiple-page-tags violations: 0
mako-unknown-context violations: 0
mako-unparseable-expression violations: 0
mako-unwanted-html-filter violations: 0
python-close-before-format violations: 0
python-concat-html violations: 24
python-custom-escape violations: 13
python-deprecated-display-name violations: 41
python-interpolate-html violations: 64
python-parse-error violations: 0
python-requires-html-or-text violations: 0
python-wrap-html violations: 226
underscore-not-escaped violations: 507

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

roger 👍

@robrap

robrap commented Nov 30, 2017

Copy link
Copy Markdown
Contributor

@jzoldak: The console output for the quality builds used to tell you what thresholds were missed and by how much (just like diff quality). Now, the only thing it says is:

14:31:43 Running xss linter report.
14:32:06 Too many xsslint violations. You can view them in xsslint.log
14:32:06 Running xss commit linter report.

But that report just tells you the current number of violations. It doesn't compare against the thresholds for you and tell you what you need to fix.

See https://build.testeng.edx.org/job/edx-platform-quality-pr/43790/console

@jzoldak

jzoldak commented Nov 30, 2017

Copy link
Copy Markdown
Contributor

@efischer19 @robrap FWIW we just merged @UmanShahzad 's fix PR #16709 which should bring the total down by 20.

@efischer19

Copy link
Copy Markdown
Contributor Author

Ah excellent, thanks for that @jzoldak @UmanShahzad. I'll close this one then.

@efischer19 efischer19 closed this Nov 30, 2017
@efischer19
efischer19 deleted the efischer/xss_limit branch November 30, 2017 14:56
@jzoldak

jzoldak commented Nov 30, 2017

Copy link
Copy Markdown
Contributor

@robrap regarding the report now just saying to go check the log - that change was made in #16581, because writing all the information to the console was not what we wanted either.

Definitely open to suggestions/PR for a better solution.

@robrap

robrap commented Nov 30, 2017

Copy link
Copy Markdown
Contributor

Thanks @jzoldak. If we could append the information we lost to the end of the report we point people too, I think that would be helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants