WordPress / WordPress-Coding-Standards

PHP_CodeSniffer rules (sniffs) to enforce WordPress coding conventions
MIT License
2.56k stars 485 forks source link

WordPress.PHP.IniSet.Risky: additional whitelisted options #1993

Closed rebeccahum closed 3 years ago

rebeccahum commented 3 years ago

Describe the solution you'd like

https://github.com/WordPress/WordPress-Coding-Standards/blob/41f5a9c66ff814863bc479fb52fd6cd1abc87e28/WordPress/Sniffs/PHP/IniSetSniff.php#L55-L65

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

jrfnl commented 3 years ago

@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™.

rebeccahum commented 3 years ago

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?

jrfnl commented 3 years ago

@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.

rebeccahum commented 3 years ago

@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!