Make WordPress Core

Opened 7 weeks ago

Closed 4 weeks ago

Last modified 4 weeks ago

#58157 closed defect (bug) (fixed)

PHP Deprecated: str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in wp-includes/formatting.php

Reported by: salvoaranzulla's profile salvoaranzulla Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.3 Priority: normal
Severity: normal Version:
Component: Media Keywords: php81 has-patch needs-dev-note
Focuses: Cc:

Description (last modified by sabernhardt)

I'm using WordPress 6.2 and PHP 8.1.

If I go on Media page on admin (/wp-admin/upload.php) I see this kind of error on my PHP log:

[19-Apr-2023 07:07:44 UTC] PHP Deprecated: str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in .../wp-includes/formatting.php on line 4280

Everytime I change page on Media page on admin, I get that PHP Deprecated error.

I have disabled all plugins and I still see that kind of PHP Deprecated error.

Attachments (1)

58157.diff (1.9 KB) - added by SergeyBiryukov 4 weeks ago.

Download all attachments as: .zip

Change History (20)

#1 @sabernhardt
7 weeks ago

  • Description modified (diff)
  • Keywords php81 added

Possibly related: #58066

Line 4280 is at the end of the ent2ncr() function.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


7 weeks ago

#3 @antpb
7 weeks ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 6.3

This is a valid issue and can be fixed by bailing early in the function if the value is null. This should prevent any chance of a null value being used in parameter 3.

Moving to 6.3 so we can get this fixed. Should be really straightforward. :)

#4 @jrf
7 weeks ago

This issue needs a backtrace to see exactly where the function is called from.

Hiding the error in the ent2ncr() function is not the way to go. This should be solved at the point the function is called with invalid data.

#5 @SergeyBiryukov
7 weeks ago

  • Keywords reporter-feedback added

Hi there, thanks for the report!

FWIW, I could not reproduce the issue on a clean install. In core, ent2ncr() is mostly applied to RSS filters but never appears to be called in media context.

I have disabled all plugins and I still see that kind of PHP Deprecated error.

Does it still happen with the default theme (Twenty Twenty-Three) activated?

#6 @salvoaranzulla
7 weeks ago

Hi,

I have disabled my custom template and I have setup default template (twentytwentyone). I have also disabled all plugins.

I confirm that if I open Media pages, I see that errors.

PHP Deprecated:  str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in /wp-includes/formatting.php on line 4280

#7 @Soean
4 weeks ago

I can reproduce it. It occurs when a post has no author.

  • Create a post via cli: wp post create --post_title='A post'

The filter the_author has the parameter $display_name which is a string or null.

The filter is used with ent2ncr:

<?php
add_filter( 'the_author', 'ent2ncr', 8 );

But ent2ncr( string $text ) only accepts a string and is doing a str_replace which fails with null.

Last edited 4 weeks ago by Soean (previous) (diff)

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


4 weeks ago

#9 follow-up: @sabernhardt
4 weeks ago

The author filter was added in [2517] with RSS filters. Should it run only in the feed context?

#10 in reply to: ↑ 9 @sabernhardt
4 weeks ago

Should it run only in the feed context?

That probably would still have a problem if the post does not have an author.

#11 follow-up: @Soean
4 weeks ago

What would be some other possible solutions?

  1. Change the filter value from null to empty string ''. This breaks backward compatibility.
    <?php
    return apply_filters( 'the_author', is_object( $authordata ) ? $authordata->display_name : '' );
    
  1. Add a anonym function to convert the parameter. We can't remove the filter anymore.
    <?php
    add_filter( 'the_author', function ( $text ) {
            return ent2ncr( (string) $text );
    }, 8 );
    
    
  1. Allow null as parameter type and convert it to an empty string.
    <?php
    
    function ent2ncr( $text ) {
            $text = (string) $text;
            // ...
    }
    

I think that the last solution would be the best one in this case, right? Or what else could we do in this case?

@SergeyBiryukov
4 weeks ago

#12 in reply to: ↑ 11 @SergeyBiryukov
4 weeks ago

Replying to Soean:

I think that the last solution would be the best one in this case, right?

On one hand, yes, since the $display_name parameter is documented as string|null in the_author filter, and ent2ncr() is hooked to that filter by default, it should be able to handle the null value as well.

One the other hand, the current behavior is inconsistent:

  • get_the_author() returns null if there is no author name.
  • get_the_modified_author() returns an empty string if there is no author name.

Some history here:

  • [2858] / #1531 added the_author filter.
  • [12284] / #10758 added null as a default value if there is no author.
  • [53187] / #55420 added an empty string if there is no author in a similar fix for the_modified_author filter.

So maybe get_the_author() could be updated to use an empty string too instead of null? That would be a minor backward compatibility break, but would result in a more consistent API.

At a glance, I don't see any issues with returning an empty string here, it should not cause a fatal error anywhere as far as I can tell, but should be mentioned in a dev note.

58157.diff implements this if we go in that direction.

Last edited 4 weeks ago by SergeyBiryukov (previous) (diff)

#13 @Soean
4 weeks ago

This looks good. You're right, if it's okay from a backward compatibility perspective, then that's the best solution and makes it a bit more consistent.

#14 @jrf
4 weeks ago

  • Keywords has-patch added; needs-patch removed

@SergeyBiryukov I can get behind that solution. It solves the problem in a clean way and improves the type consistency in the code base. 👍🏻

#15 @SergeyBiryukov
4 weeks ago

  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

#16 @SergeyBiryukov
4 weeks ago

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

In 55755:

Code Modernization: Correct fallback return value in get_the_author().

If the $authordata global is not set, get_the_author() returned null, causing a PHP 8.1 "null to non-nullable" deprecation notice in ent2ncr() hooked via the_author filter:

str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated

This commit updates get_the_author() to return an empty string if called before $authordata is set, bringing consistency with a few other similar functions which also return an empty string in this case:

  • get_the_author_meta()
  • get_the_author_posts_link()
  • get_the_modified_author()

Follow-up to [695/tests], [2858], [11138], [12284], [20575], [34677], [44616], [53187].

Props Soean, jrf, sabernhardt, salvoaranzulla, antpb, ebai4, sajjad67, tijmensmit, SergeyBiryukov.
Fixes #58157.

#17 @SergeyBiryukov
4 weeks ago

  • Keywords needs-dev-note added; reporter-feedback removed

#18 @SergeyBiryukov
4 weeks ago

#58066 was marked as a duplicate.

#19 @SergeyBiryukov
4 weeks ago

In 55756:

Tests: Remove expectation of a deprecation notice from WP_Posts_List_Table tests.

With get_the_author() returning an empty string instead of null if called before $authordata is set, and no longer causing a PHP 8.1 "null to non-nullable" deprecation notice, there is no need for these tests to ignore the notice.

Follow-up to [51968], [55755].

See #58157.

Note: See TracTickets for help on using tickets.