-
-
Notifications
You must be signed in to change notification settings - Fork 483
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
WordPress.PHP.IniSet.Risky: additional whitelisted options #1993
Comments
@rebeccahum Could you explain why you think those values should be allowed to be set ? WP itself manages sessions, so AFAIK generally speaking any plugin or themes which touches anything "session" related, whether it is session functions or ini values, is doing it wrong™. |
Ah, this would be for PHP sessions in custom code, as some clients are creating custom sessions. I was thinking since VIPCS inherits from WPCS, maybe a way for us to customize the list would be a nice feature. Perhaps it would be better to have VIPCS have its own type of IniSetSniff instead then? |
@rebeccahum Well, again, I come back to my original question: why are those clients creating custom sessions and not using the WP session management ? Whatever they are doing may well create conflicts with the WP session management and cause hard to trace and debug bugs. But yes, from a WPCS point of view, any customization of the list of "allowed to be adjusted" ini settings should be handled in a custom standard and not in WPCS. |
@jrfnl Agreed — I don't have the specific use cases on hand, but there are some sessions that clients want to manage outside of WP (which I do understand is not best practices). Got it, I will close this then! Thank you for your input! |
Describe the solution you'd like
WordPress-Coding-Standards/WordPress/Sniffs/PHP/IniSetSniff.php
Lines 55 to 65 in 41f5a9c
For
$whitelisted_options
, it would be cool if we could have the below added or have a property we can configure with the below:Note: valid values will need to be
true
The text was updated successfully, but these errors were encountered: