Make WordPress Core

Opened 12 months ago

Closed 11 months ago

Last modified 6 months 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 has-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 11 months ago.

Download all attachments as: .zip

Change History (22)

#1 @sabernhardt
12 months 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.


12 months ago

#3 @antpb
12 months 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
12 months 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
12 months 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
12 months 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
12 months 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 12 months ago by Soean (previous) (diff)

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


12 months ago

#9 follow-up: @sabernhardt
12 months 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
12 months 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
11 months 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?

#12 in reply to: ↑ 11 @SergeyBiryukov
11 months 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 11 months ago by SergeyBiryukov (previous) (diff)

#13 @Soean
11 months 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
11 months 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
11 months ago

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

#16 @SergeyBiryukov
11 months 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
11 months ago

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

#18 @SergeyBiryukov
11 months ago

#58066 was marked as a duplicate.

#19 @SergeyBiryukov
11 months 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.

#20 @stevenlinx
9 months ago

  • Keywords has-dev-note added; needs-dev-note removed

#21 @blackstar1991
6 months ago

Got the same problem for php 8.2.8

Note: See TracTickets for help on using tickets.