[wp-trac] [WordPress Trac] #41593: Document every function parameter that expects slashed data
WordPress Trac
noreply at wordpress.org
Fri Nov 3 22:22:03 UTC 2017
#41593: Document every function parameter that expects slashed data
----------------------------+-----------------------------
Reporter: johnbillion | Owner:
Type: task (blessed) | Status: new
Priority: normal | Milestone: Future Release
Component: General | Version:
Severity: normal | Resolution:
Keywords: needs-patch | Focuses: docs
----------------------------+-----------------------------
Comment (by jdgrimes):
[attachment:41593.diff] is a snapshot of the changes relating to slashing
that I've explored so far. It isn't intended to be committed as it is, or
even really be used as the base for future patches. Probably the next step
is really splitting it up.
Basically what I've done in the patch is identify a list of functions that
expect slashed data, and fed them to a PHPCS sniff. I then ran that sniff
over core, to check whether those functions were passed slashed data as
expected in each place that they were used. This revealed more functions
that expect slashed data because they pass args directly to these
functions, and so on.
In the patch, I have added `expected_slashed` comments for some of these
functions. I have also reviewed every place where these functions were
used, and either added slashing if necessary, or a `// WPCS: slashing OK`
comment where it isn't (this causes the sniff to longer issue an error
there). The sniff is smart about detecting cases where slashing isn't
needed, so in appropriate cases I've silenced the error by casting the
value in question to an integer (the sniff then ignores that because
integers don't need to be slashed). For cases where the value isn't an
integer, I've erred on the side of slashing even if it might not be
necessary, since it does no harm and decreases the chance of future
breakage.
The WPCS sniff I've used to get this far can now be found here:
https://github.com/WordPress-Coding-Standards/WordPress-Coding-
Standards/pull/1222
That includes a list of functions that expect slashed data.
However, I don't think that it is as simple as just documenting this.
Ideally, all of these functions should have slashing-related unit tests,
to verify that they do indeed expect slashed data. Only a handful of them
actually do:
{{{
\Tests_Attachment_Slashes::test_wp_insert_attachment
\Tests_Comment_Slashes::test_wp_new_comment
\Tests_Comment_Slashes::test_wp_insert_comment
\Tests_Comment_Slashes::test_wp_update_comment
\Tests_Meta_Slashes::test_add_comment_meta
\Tests_Meta_Slashes::test_update_comment_meta
\Tests_Meta_Slashes::test_add_user_meta
\Tests_Meta_Slashes::test_update_user_meta
\Tests_Post_Slashes::test_wp_insert_post
\Tests_Post_Slashes::test_wp_update_post
\Tests_Term_Slashes::test_wp_insert_term
\Tests_Term_Slashes::test_wp_update_term
\Tests_User_Slashes::test_wp_insert_user
\Tests_User_Slashes::test_wp_update_user
}}}
(In the process I've written tests for a few of the additional functions,
which I'll also attach here shortly.)
In some cases, a decision may need to be made as to whether a function
should continue to expect slashed data, or whether it should be changed to
not. Since this decision will need to be made on a case-by-case basis, I
suggest that perhaps we should split this ticket, creating a ticket for
each function or set of functions. This may also help to keep the patches
to manageable size and scope.
Because some functions use other functions that expect slashed data, it
would be necessary to begin with the functions that call `wp_unslash()` or
`stripslashes()` directly, and then work outward from there.
On that note, I'd like to suggest that all uses of `stripslashes()` be
replaced with `wp_unslash()` where this is appropriate. It simplifies
things. If there are some places that `wp_unslash()` shouldn't be used
though, it would be helpful for me to understand why.
I have more thoughts on particular functions, and I'll probably also think
of some other things that I forgot to note, which I'll post tomorrow.
--
Ticket URL: <https://core.trac.wordpress.org/ticket/41593#comment:6>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list