Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation for all the whitelist comments #446

Closed
johnbillion opened this issue Sep 9, 2015 · 7 comments
Closed

Documentation for all the whitelist comments #446

johnbillion opened this issue Sep 9, 2015 · 7 comments

Comments

@johnbillion
Copy link
Member

It's difficult to determine which whitelist comments (eg. WPCS: sanitization ok) are available to use. These should be documented, and it would be good if the documentation recommended a standard format such as // WPCS: sanitization ok.

Here's the list I have. Are there more?

  • Escaping: XSS
  • Sanitising: sanitization
  • Nonce verification: CSRF
  • Loose comparison: loose comparison
  • Overriding WordPress globals: override
  • Use of superglobal: input var
@JDGrimes
Copy link
Contributor

JDGrimes commented Sep 9, 2015

Previously: #361

@JDGrimes JDGrimes added this to the Future Release milestone Sep 9, 2015
@JDGrimes
Copy link
Contributor

JDGrimes commented Sep 9, 2015

Related: #228

@westonruter
Copy link
Member

Note also that I've proposed a way to standardize these whitelisting comments in a model following JSCS, as opposed to our current ad hoc system: squizlabs/PHP_CodeSniffer#604

Example:

echo $do_you_trust_me; // @codingStandardsIgnoreLine WordPress.XSS.EscapeOutput 

@JDGrimes
Copy link
Contributor

JDGrimes commented Sep 9, 2015

Uses of ::has_whitelist_comment().

Uses of preg_match().

There are two more to add to your list from the direct database query sniff:

  • Direct database query: db call
  • Database query without caching: cache

The reason that these don't use ::has_whitelist_comment() is because they check for comments differently than the other sniffs did, so the transition would not have been backward compatible.

@JDGrimes
Copy link
Contributor

JDGrimes commented Sep 9, 2015

Note also: there is a wiki, and you are welcome to contribute to it. 😄

@JDGrimes
Copy link
Contributor

JDGrimes commented Sep 9, 2015

Thanks @johnbillion. I've added a section cautioning against overuse, and explaining how to refactor the example to actually fix it without adding the whitelisting flag.

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

No branches or pull requests

4 participants