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

0.5.0 #370

Merged
merged 67 commits into from
Jun 1, 2015
Merged

0.5.0 #370

merged 67 commits into from
Jun 1, 2015

Conversation

JDGrimes
Copy link
Contributor

@JDGrimes JDGrimes commented May 1, 2015

PR to queue up commits for 0.5.0

@JDGrimes JDGrimes added this to the 0.5.0 milestone May 1, 2015
JDGrimes and others added 14 commits May 1, 2015 15:45
Say you have code like this:

```php
if ( isset( $_REQUEST['foo'] ) && in_array( $_REQUEST['foo'], $array ) ) {
	$foo = sanitize_key( $_REQUEST['foo'] );
}
```
This is safe, but the use of the raw `$_REQUEST['foo']` value in `in_array()` would be flagged. Adding `in_array()` to the list of auto-escaped functions avoids this.
…wercase

Add check for lowercase "as" in foreach
This makes it available to all sniffs. We also move another method to
the main sniff from the ValidatedSantizedInput sniff.

In addition, I’ve refactored the sanitization portion of this sniff to
use the `is_sanitized()` method that I recently introduced.
Fix fatal error for array_map() in sanitization check
This redundant check for sanitization was introduced sometime a while
ago. It is actually a bug, because it makes it so that no validation
check is performed if sanitization isn’t applied.

This is fixed by removing it, as demonstrated by the fact that we now
get two errors on line 33, instead of just the one for missing
sanitization.

I’ve added another test on line 85, for a sanitized but not validated
variable, just to demonstrate that it works correctly. This behavior is
unchanged; an error would have been given here previously.
Comparison of a super global to another value (e.g. `if ( ‘a’ ===
$_POST[‘foo’] )` or `switch ( $_POST[‘foo’] )`) doesn’t require
sanitization. Previously it was flagged anyway, but this fixes that,
and introduces a new method to check for comparisons.

Fixes #186
Sometimes input vars are an array:

```php
if ( isset( $_POST['foo'] ) && is_array( $_POST['foo'] ) ) {
    /* ... */
}
```

That shouldn't be flagged as needing sanitization. So let's add `is_array()` to the list of safe functions.
Add in_array() to list of auto-escaped functions
This basically synchronizes it with `Squiz.Arrays.ArrayDeclaration`.
The upstream version is similar, except that we exclude a few errors.
Unfortunately we have to actually comment out the code rather than just
using the upstream sniff and `<exclude>` in our ruleset, due to a bug
(squizlabs/PHP_CodeSniffer#582). (I’ve also included a fix for another
bug, squizlabs/PHP_CodeSniffer#584.) Because of this, we cannot yet
eliminate duplicated logic from this child sniff.

I think this pretty much maintains the previous behavior, except that I
have added a warning for extra whitespace at the edges of multiline
arrays.
GaryJones and others added 28 commits May 9, 2015 10:34
Left in and commented out with a note so that future us will know why it's been excluded.
Even when the value is being escaped for direct output. Previously
sanitization wasn’t required in this case.
…entation-standards

Add Squiz and Generic Commenting with exclusions
That was my original intention.

Also, the branch name is 3.0, not 3.0.0
We are incompatible with 2.0.0:

```
Fatal error: Call to undefined method
PHP_CodeSniffer_File::findEndOfStatement() in
/home/travis/build/WordPress-Coding-Standards/WordPress-Coding-Standards
/WordPress/Sniffs/XSS/EscapeOutputSniff.php on line 347
```

We can run against 2.1.0, however, there are some oddities, possibly in
the parser or something, that cause the tests for
`WordPress.XSS.EscapeOutput` to fail.

Fo more details you can see the travis build:
https://travis-ci.org/WordPress-Coding-Standards/WordPress-Coding-Standa
rds/builds/63017423
…-test

Have Travis CI test against multiple verions of PHPCS
Separate sanitizing and escaping functions
…-declaration

Add support for error fixing to ArrayDeclaration
Otherwise we end up doing a lot of extra work when a class doesn’t
implement any interfaces.
The validated/sanitized input sniff used not to handle super globals in
ternary conditions properly. Now it does, so I’ve added tests which
demonstrate this.

Closes #168
Add tests demonstrating ternary operator
This updates the validated/sanitized input sniff to also check for
slashing. This could have been made into another sniff instead,
however, it would have required lots of duplicated logic and this sniff
would need to be updated to accommodate the use of `wp_unslash()`
anyway.

Currently only `wp_unslash()` is recognized as an unlashing function,
but this can be changed in the future if needed.

The sniff currently requires that `wp_unslash()` be used *before* the
data is passed through a sanitizing function. Sanitizing first and then
wrapping that in `wp_unslash()` is not accepted.

The error for missing the use of `wp_unslash()` is independent of the
missing sanitizing function error, so an error will be given for
missing use of an unslashing function whether or not a sanitizing
function is used, and vice versa.

Unslashing is not required when sanitization is provided via casting,
or when certain sanitization functions are used which implicitly or
explicitly perform an unslash or for which unslashing isn’t necessary.
`absint()` implicitly unslashes, and `sanitize_key()` will remove
slashes explicitly. And unslashing isn’t necessary when testing a value
with `is_array()`. This list can be expanded in the future, and is
configurable via the `customUnslashingSanitizingFunctions` property.

See #172
…-camelcase-methods-in-implements-classes

Allow camelCase methods in classes that implement others
Require unlashing of input superglobals
JDGrimes added a commit that referenced this pull request Jun 1, 2015
@JDGrimes JDGrimes merged commit 0460804 into master Jun 1, 2015
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