Make WordPress Core

Opened 2 years ago

Closed 22 months ago

#55888 closed defect (bug) (fixed)

False is not a valid cache key in community-events

Reported by: malthert's profile malthert Owned by: davidbaumwald's profile davidbaumwald
Milestone: 6.1 Priority: normal
Severity: normal Version:
Component: Administration Keywords: has-patch changes-requested dev-feedback
Focuses: Cc:

Description

There is a check for this in 1 place, but it's missing in the other, causing issues, since you pass an invalid type to get_site_transient

Attachments (1)

55888-1.patch (1.0 KB) - added by rafiahmedd 2 years ago.
2nd opinion.

Download all attachments as: .zip

Change History (11)

This ticket was mentioned in PR #2764 on WordPress/wordpress-develop by kkmuffme.


2 years ago
#1

  • Keywords has-patch added

Add check for false transient key, which is not a valid key.
There already is a check in the other place in this file where this is used. Added it in all places now.

Trac ticket: https://core.trac.wordpress.org/ticket/55888#ticket

#2 @SergeyBiryukov
2 years ago

  • Component changed from General to Administration
  • Milestone changed from Awaiting Review to 6.1

#3 @davidbaumwald
2 years ago

  • Owner set to davidbaumwald
  • Status changed from new to reviewing

@rafiahmedd
2 years ago

2nd opinion.

#4 @malthert
2 years ago

Early return is better and also it's more in the style of the existing check of line 337

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


23 months ago

#6 @audrasjb
23 months ago

  • Keywords changes-requested added

Reviewed during today's bug scrub.

The patch doesn’t look bad, but I’d say the last comment would probably provide a better approach for this (early return when falsey transient key).

#7 @costdev
23 months ago

  • Keywords dev-feedback added

Calling get_site_transient( false ) doesn't cause any notices/warning/errors. What am I missing?

#8 @malthert
22 months ago

@costdev I can't remember it's been a while and I don't have time to dive into it atm, but since the function arg is string, we shouldn't pass a bool at least.

#9 @davidbaumwald
22 months ago

  • Severity changed from critical to normal

#10 @davidbaumwald
22 months ago

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

In 54338:

Administration: Guard against false transient key in get_cached_events().

Inside WP_Community_Events::get_cached_events(), WP_Community_Events::get_events_transient_key() is used to retrieve the transient key name, based on the user's location. However, the transient key can potentially return false, resulting in a call to get_site_transient() with the $key being false.

This change first attempts to evaluate and guard against a false return from WP_Community_Events::get_events_transient_key(). The result is an early false return from WP_Community_Events::get_cached_events().

Props malthert, rafiahmedd, audrasjb, costdev.
Fixes #55888.

Note: See TracTickets for help on using tickets.