wordpress-sdk icon indicating copy to clipboard operation
wordpress-sdk copied to clipboard

Proper escaping as per WPCS

Open Luehrsen opened this issue 5 years ago • 3 comments

During the theme review it was mentioned, that some values in the freemius sdk are not escaped properly. For example: https://github.com/Freemius/wordpress-sdk/blob/master/templates/js/style-premium-theme.php#L26

These should be escaped properly (or more visible) to make theme review happy.

Luehrsen avatar Jan 03 '19 21:01 Luehrsen

@Luehrsen this is an ongoing back and forth with wp.org. Not everything needs to be sanitized/escaped, especially when you know the expected format/structure of the value. If we start to sanitize/escape everything, it will just consume more memory and processing resources for no valid reason except bypassing the theme sniff tests. Currently, we believe that performance is more important. Maybe we need to add contextual notes for reviewers explaining to them why a value was not escaped/sanitized in some places. From your experience, do you think it would help?

vovafeldman avatar Feb 28 '19 13:02 vovafeldman

Maybe we need to add contextual notes for reviewers explaining to them why a value was not escaped/sanitized in some places. From your experience, do you think it would help?

Those will not be read. Especially new reviewers will always look at the sniffer and every instance of echo $... will be flagged. The amount of those will freak people out.

So while it makes no sense at all, if TRT is a problem I would spend the time on eliminating the raw echoing of variables and put escaping there, so the sniffer doesn't complain.

Luehrsen avatar Feb 28 '19 19:02 Luehrsen

Relevant PR: https://github.com/Freemius/wordpress-sdk/pull/308

vovafeldman avatar Mar 05 '19 06:03 vovafeldman