-
Notifications
You must be signed in to change notification settings - Fork 15
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
Fix new PHPCS errors #324
Comments
These values are actually being sanitized, but because `wp_unslashed()` is (rightly) being used, WPCS is flagging them. We’ll remove these comments once this is fixed upstream. See WordPress/WordPress-Coding-Standards#172 See #324
None of these locations are vulnerable. Two have nonce checks later on, in the file included within the statement. The other is hooked to an action that is called only after nonce validation on the settings screen. However, I opted to add an explicit nonce check here, for good measure. See #324
Also updates a whitelisting comment for points settings. While we might want to consider performing sanitization of some sort here in the future, it is no easy task when we are dealing with an arbitrarily deep array that might contain HTML snippets. See #324
In this case, nonce verification isn’t necessary, because we aren’t performing a restricted action. See #324
In this case, the verification check is performed by the primary functions which call these methods. See #324
So the concern about possible CSRF isn’t valid. See #324
These were pointed out thanks to changes in PHPCS. See #324
Fixed for now. |
This has broken the Ajax tests:
The problem is this: $points_types = array_map( 'sanitize_key', $_POST['points_types'] );
|
Actually, that is incorrect. It is a simple array. The hooks are a comma-delimited string. The problem is that I'll still refactor it to use the list of points types from the database. |
These lists are an array of comma-delimited hook IDs. We were sanitizing them with `sanitize_key()`. That is causing the function not to work correctly, and our tests are failing. The problem is that `sanitize_key()` will strip out the commas. We need to use `sanitize_text_field()` instead. In addition, I’ve refactored the logic to get the list of points types from the database. Previously, the function was not verifying whether the points types were valid. Now it will only save the lists for valid points types. See #324
Updates to WPCS mean we're seeing some new errors.
The text was updated successfully, but these errors were encountered: