[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