Skip to:

Opened 4 years ago

Closed 4 years ago

#3166 closed defect (fixed)

New cache priming functions error on bad meta

Reported by: thebrandonallen Owned by: johnjamesjacoby
Milestone: 2.6 Priority: high
Severity: minor Version: trunk
Component: API - Cache Keywords: commit


Our new cache priming functions don't allow for the possibility of bad meta, like a reference to a post id that doesn't exist. Attached patch fixes this.

Attachments (1)

3166.01.diff (1023 bytes) - added by thebrandonallen 4 years ago.

Download all attachments as: .zip

Change History (4)

#1 @johnjamesjacoby
4 years ago

This is an interesting condition, but I think you’re right that we should check for it here. Good eye!

#2 @johnjamesjacoby
4 years ago

  • Keywords commit added; has-patch removed
  • Owner set to johnjamesjacoby
  • Priority changed from normal to high
  • Severity changed from normal to minor

A few clarifying points:

  • It seems really unlikely that a post would go missing in the milliseconds between the original query and these functions, but it's possible on large forums so we should continue to protect against this.
  • Bad metadata is protected against inside of _prime_post_caches(), via _get_non_cached_ids() and later via the IN () query that's performed. If a post ID no longer exists in the database, it won't be returned to get passed into update_post_caches()
  • bbp_get_unique_array_values() calls array_filter() to help remove strays
  • Completely empty arrays will halt each function early

The comparisons in the patch are backwards, which led me down a slightly different path for bbp_update_post_author_caches(). We can skip get_post_field() and use the $object directly, because we don't care about formatting the post_author value, because cache_users() knows what to do. We should do this specifically for posts that may have anonymous users for authors anyways, which would have been a bug.

Last edited 4 years ago by johnjamesjacoby (previous) (diff)

#3 @johnjamesjacoby
4 years ago

  • Resolution set to fixed
  • Status changed from new to closed

In 6711:

Common: add some sanity checks to cache priming helpers.

This change adds more checks to avoid caching post & post author data that may have been deleted since the IDs were last sourced from the database.

We also remove a call to get_post_field() to reference the local object directly. This adds an empty() but removes a more complex function call when we already have the post in local scope anyways.

Fixes #3166. Props thebrandonallen.

Note: See TracTickets for help on using tickets.