Make WordPress Core

Opened 13 days ago

Closed 12 days ago

#54815 closed defect (bug) (fixed)

Warning found in post-thumbnail-template.php on PHP 8.0.14

Reported by: SierraTR Owned by: audrasjb
Milestone: 5.9 Priority: high
Severity: normal Version: 5.9
Component: Media Keywords: has-patch dev-feedback fixed-major
Focuses: Cc:

Description

Warning thrown when displaying post category page that includes thumbnail images.
WP 5.9 RC2 PHP 8.0.14, not browser-specific, not theme-specific

Warning: preg_match(): No ending delimiter '/' found in ..../wp-includes/post-thumbnail-template.php on line 198

Attachments (1)

54815.diff (700 bytes) - added by costdev 13 days ago.
Add missing / to regex.

Download all attachments as: .zip

Change History (14)

#1 @audrasjb
13 days ago

  • Milestone changed from Awaiting Review to 5.9

Assigning to milestone 5.9 for further investigation.

#2 @audrasjb
13 days ago

  • Summary changed from post-thumbnail-template.php to Warning found in post-thumbnail-template.php on PHP 8.0.14

#3 @costdev
13 days ago

The warning occurs in other PHP versions as well. See preg_match.


The regex in question:

'/(^|&)loading='

It's simply missing a trailing /.


"Why are we only finding this out now?"

No tests hit the condition on Line 198. See the report (this is subject to change, of course). As a result, the faulty preg_match() is never triggered and so the warning is not thrown.

This is why tests should at least achieve line coverage for all new code.


To trigger the warning (+hint for unit tests):

  1. Switch to Twenty Twenty-One.
  2. Create a page and add a Featured Image.
  3. Add this to page.php:
<?php

if ( have_posts() ) : while ( have_posts() ) : the_post();
        get_the_post_thumbnail(
                get_the_ID(),
                'post-thumbnail',
                'does_not_contain_loading=true'
        );
endwhile; endif;
  1. Load the page.
  2. You should see this warning:

Warning: preg_match(): No ending delimiter '/' found in /wp-includes/post-thumbnail-template.php on line 198

@costdev
13 days ago

Add missing / to regex.

#4 @costdev
13 days ago

  • Keywords has-patch added; needs-patch removed

#5 @costdev
13 days ago

  • Keywords commit added

#6 @audrasjb
12 days ago

  • Component changed from General to Media
  • Owner set to audrasjb
  • Priority changed from normal to high
  • Status changed from new to assigned

You're right, I'm seeing this now and this is a high priority, thanks @costdev.

The issue was introduced in [52065].
The proposed patch fixes the issue.

Self-assigning for commit asap in 5.9.

#8 @audrasjb
12 days ago

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

In 52574:

Media: Add a missing / in post thumbnail lazy loading regex.

This change avoids a warning thrown by a missing slash in a post thumbnail lazyload related regular expression.

Follow-up to [52065].

Props SierraTR, audrasjb, costdev.
Fixes #54815.

#9 @audrasjb
12 days ago

  • Keywords dev-feedback fixed-major added; commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backport.

We'll need a second committer sign-off before backporting this change to 5.9.

Pinging @adamsilverstein and/or @flixos90 for second sign-off as they worked on [52065].

#11 @flixos90
12 days ago

The change in [52574] looks good to me - thank you @SierraTR for catching it and @costdev for the fix!

@audrasjb Would you like me to backport or are you going to do that?

#12 @audrasjb
12 days ago

Thanks! I’ll backport but we needed a second committer sign off :)

#13 @audrasjb
12 days ago

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

In 52579:

Media: Add a missing / in post thumbnail lazy loading regex.

This change avoids a warning thrown by a missing slash in a post thumbnail lazyload related regular expression.

Follow-up to [52065].

Props SierraTR, audrasjb, costdev, flixos90.
Merges [52563] to the 5.9 branch.
Fixes #54815.

Note: See TracTickets for help on using tickets.