[wp-trac] [WordPress Trac] #50413: Update code and comments to remove "blacklist" and "whitelist"

[WordPress Trac]

#50413: Update code and comments to remove "blacklist" and "whitelist"
---------------------------------------------------+-----------------------
 Reporter:  strangerstudios                        |       Owner:  desrosj
     Type:  defect (bug)                           |      Status:  reopened
 Priority:  normal                                 |   Milestone:  5.5
Component:  General                                |     Version:
 Severity:  normal                                 |  Resolution:
 Keywords:  has-patch needs-dev-note dev-feedback  |     Focuses:
---------------------------------------------------+-----------------------
Changes (by desrosj):

 * keywords:  has-patch needs-dev-note => has-patch needs-dev-note dev-
     feedback


Comment:

 [attachment:"approach-1.diff"] is a first pass at seamlessly switching the
 option key to the new one when calling `(get|update|add)_option()`. I
 account for both 'blacklist_keys' and `comment_whitelist`, which were both
 changed to accomplish the goal of this ticket.

 Some notes on this approach specific to `blacklist_keys`:
 - If someone is running WordPress 5.5, we can safely assume that the value
 of `blacklist_keys` has been moved to the new key during the upgrade
 routine. Any future option API calls can be automatically switched to the
 new key and issues would only arise if plugins are directly updating the
 keys in the database (which is not proper usage, they should be using the
 API).
 - I added the code directly in the options API functions to avoid having
 to introduce a new function (needed if using the
 `pre_option_blacklist_keys` hook). The reasons being it's late in the
 release cycle, and the future intention is to replace this patch with a
 more solid `_deprecated_option()` alternative, so adding a function now
 would just mean another deprecated function in a future release. An
 anonymous function could work, but would not allow the code to be unhooked
 if desired.
 - I did not add this workaround in the `delete_option()` function. If
 someone is trying to delete the old key, it's OK to let it fail. Deleting
 this key is not really a good idea anyway. Core options will be re-added
 with any future update through `populate_options()` and this option is re-
 added any time a user saves the Options -> Discussion page.
 - The patch uses `_doing_it_wrong()`, which is meant for using functions
 incorrectly. I used `%s option` to change the message returned by the
 function a bit to fit this scenario better.
 - Any custom code accessing the `blacklist_keys` option in the cache
 directly using `wp_get_cache( 'blacklist_keys', 'options' )` will not
 receive the correct values.

 The other negative to this approach that I can think of is that any filter
 or action hooks related to the old key will not run. I did some research,
 and the number of plugins using these hooks seems small enough for this
 specific option that we could probably afford to not account for them.

 I also have a developer note mostly drafted that will be going out to
 illuminate the changes made in this ticket with the hopes that any plugins
 or custom code in either of these two edge cases will be adjusted.

 Here are the searches I performed on the directory (many searches return
 repeat plugins):

 **Hooks throughout option API**
 - `default_option_blacklist_keys` filter
 ([https://wpdirectory.net/search/01EDVGYDC42MD32S9FVD07RCX2 0 plugins -
 20,000 installs])

 **Get option related**
 - `pre_option_blacklist_keys` filter
 ([https://wpdirectory.net/search/01EDVDHEMY146VBSPTY5MKSCJ0 1 plugin])
 - `option_blacklist_keys` filter
 ([https://wpdirectory.net/search/01EDVDPS2K1AM7JSARRP9WJH92 3 plugins -
 ~20,200 installs])

 **Update option related**
 - `pre_update_option_blacklist_keys` filter
 ([https://wpdirectory.net/search/01EDVDT7459QA0KXEAZ4T8Q4Q6 1 plugin] -
 100 installs)
 - `update_option_blacklist_keys` action
 ([https://wpdirectory.net/search/01EDVE05TY3Z7ZKQ65ZN8TZVFR1 plugin] - 100
 installs)

 **Add option related**
 - `add_option_blacklist_keys` action -
 ([https://wpdirectory.net/search/01EDVE15N2F03BHT5JEXJPT2D5 0 installs])

 There are some general hooks like `pre_update_option`, `update_option`,
 `add_option`, etc. that searches of the directory cannot reliably
 determine if the hooked code is modifying this key. But it seems that this
 key is not widely modified through actions and filters.

 I repeated this process for the other key changed in this ticket,
 `comment_whitelist`, and the same appears to be true. The implications of
 this comment key being renamed is not as bad. `blacklist_keys` not being
 seamless could result in some comments with undesired content or from
 undesired IPs.

 I'll also note that I have not done any performance testing for this
 approach, though it does not seem that any large changes to performance
 are introduced by this approach.

 I'm also attaching a first pass at adding a `_deprecated_option()` based
 approach. It is not yet handling database queries (which key should be
 checked retrieved/updated/deleted when calling a deprecated option? The
 new one? The old one?), but it does ensure that both the new and old
 actions and filters run.

 I think that this should receive wider feedback, though. So I am not a big
 fan of using this approach for 5.5. I do think that we should get
 something similar in for 5.6, replacing the temporary fix
 [attachment:"approach-1.diff"] provides.

 @johnjamesjacoby let me know if you think this will work!

 @SergeyBiryukov I would also love your feedback on this as the Core Tech
 Lead of 5.5

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/50413#comment:38>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform