WordPress-Coding-Standards icon indicating copy to clipboard operation
WordPress-Coding-Standards copied to clipboard

WordPress.Security.EscapeOutput: Wiki misses hint about lower case entry for customEscapingFunctions

Open datengraben opened this issue 1 year ago • 3 comments

phpcs version: 3.10.3 wp-cs version: 3.1.0

If I want to add a custom escaping function (for example myCustomEscapingFunction) in my phpcs.xml config via array parameter customEscapingFunctions of the WordPress.Security.EscapeOutput rule, I need to add it lower case, because the implementation forces the compare operation to use lowercase names.

See implementation:

https://github.com/WordPress/WordPress-Coding-Standards/blob/ffec7bfced474ed83c4e1b66225d37f84fdb65a3/WordPress/Helpers/EscapingFunctionsTrait.php#L254

So the problem is, if I provide a function name in an case sensitive way, at the moment of parsing the config value and instatiating the object, it does not get transformed into lowercase string. So the (above) line in the code of EscapingFunctionsTrait compares it the later to it's lowercase'd counterpart and thus will not match and my custom escaping function does not get recognized as configured.

Of course it gets recognized, if I use already the lower case variant in the phpcs.xml config.

Config Example:

    <rule ref="WordPress.Security.EscapeOutput">
        <properties>
            <property name="customEscapingFunctions" type="array">
                <element value="myCustomEscapingFunction"/><!-- won't work -->
                <element vlaue="mycustomescapingfunction" /> <!-- works -->
            </property>
        </properties>
    </rule>

I'm unsure if you can call it a bug. But I would suggest that the Customizable sniff wiki page in the section of custom escape output functions misses a hint about this behaviour. I couldn't get my head around it and went debugging, instead of remembering that php is case insensitive. So maybe the wiki can include a passage about this, at least as long as https://github.com/WordPress/WordPress-Coding-Standards/pull/2391 is not merged.

Anyway, thank you for your work!

datengraben avatar Nov 10 '24 14:11 datengraben

Seems like something we should handle, rather than the developer discovering it should be lowercase.

GaryJones avatar Nov 11 '24 07:11 GaryJones

@datengraben did you test out this patch: https://github.com/WordPress/WordPress-Coding-Standards/pull/2370 and see if that would fix your issue?

dingo-d avatar Nov 11 '24 09:11 dingo-d

@dingo-d no I did not test your patch, but I fixed it locally by myself, using pretty much the same approach as you have in your patch.

datengraben avatar Nov 11 '24 10:11 datengraben