Make WordPress Core

Opened 14 months ago

Last modified 14 months ago

#56694 new defect (bug)

Uncached database read for logged in users when Privacy Policy Page is deleted

Reported by: johnjamesjacoby's profile johnjamesjacoby Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.7
Component: Privacy Keywords: needs-patch needs-unit-tests
Focuses: administration, performance Cc:

Description

As a logged in WordPress user navigating around WordPress Admin, a hook fires on every admin page that attempts to notify you whether or not the suggested Privacy Policy text (in WordPress) has changed, sourced from the "Privacy Policy Guide" settings page.

The Privacy Policy itself is a Page, and the numeric ID of that page is saved as a Setting.

If the WordPress Page (that matches the ID that is set as the Privacy Policy Page setting) is permanently deleted, WordPress still attempts to query the database to look for this Page.

This results in a single database read query that will never find what it is looking for, and is not cached because it does not exist, causing it to happen again on every subsequent page that any logged in user navigates to inside of WordPress Admin.

  • This user would normally never know this database chatter is happening because it isn't anything they are intentionally looking for
  • I discovered this misbehavior while using the Query Monitor plugin on a dev site, and noticed that there was always 1 uncached database read

Screenshot of the query & call stack will be attached below.

Attachments (2)

jjj-on-2022-09-29-at-15-17-33@2x.png (285.4 KB) - added by johnjamesjacoby 14 months ago.
Query Monitor showing call stack to uncacheable database read
jjj-on-2022-10-01-at-23-44-45@2x.png (320.8 KB) - added by johnjamesjacoby 14 months ago.
Default themes call the_privacy_policy_link() in footer.php, triggering 2 uncachable database reads

Download all attachments as: .zip

Change History (4)

@johnjamesjacoby
14 months ago

Query Monitor showing call stack to uncacheable database read

#1 @johnjamesjacoby
14 months ago

Recommend a two-part fix for this:

  1. Add a method to the WP_Privacy_Policy_Content class to check if the Page exists with the ID that the Setting (wp_page_for_privacy_policy) is set to. If it does not exist, update the setting value to 0 and halt any additional stuff related to it. This will self-heal the problem for existing sites without any kind of upgrade routine, and also just seems smart to do here.
  1. Add a similar hook (likely to wp_trash_post and before_delete_post) with a helper function to update the wp_page_for_privacy_policy option to 0. This matches what the Front Page options also do. See: _reset_front_page_settings_for_post() – this requires auditing WordPress for any existing places where the Privacy Policy Page being in the Trash is accounted for and removing them (I found 1, in options-privacy.php – may be more).

#2 @johnjamesjacoby
14 months ago

Out of scope idea: deprecate _reset_front_page_settings_for_post() and replace it with something more robust that plugins can also easily use to "disconnect" their own Page settings 💗

@johnjamesjacoby
14 months ago

Default themes call the_privacy_policy_link() in footer.php, triggering 2 uncachable database reads

Note: See TracTickets for help on using tickets.