WordPress.org

Make WordPress Core

Opened 9 months ago

Closed 8 months ago

#39774 closed defect (bug) (fixed)

php 7.1 media uploader bug

Reported by: drrobotnik Owned by: SergeyBiryukov
Milestone: 4.7.3 Priority: normal
Severity: normal Version: 4.8
Component: Upload Keywords: has-patch fixed-major
Focuses: Cc:

Description

In PHP7.1, when you upload a file with the same name multiple times there is a PHP Warning: A non-numeric value encountered in /wp-includes/functions.php on line 2077.

The warning is a type casting related. All you need to do to fix is change the default string value to null.

I've included a very tiny patch and have tested that it fixes the issue and has no backwards compatibility implications.

Attachments (3)

functions.diff (504 bytes) - added by drrobotnik 9 months ago.
wp-includes/functions.php, changes default type to null in order to fix bug in WP while using PHP 7.1
functions.2.diff (465 bytes) - added by drrobotnik 9 months ago.
updated patch changes null to 0.
39774.patch (1.2 KB) - added by SergeyBiryukov 9 months ago.

Download all attachments as: .zip

Change History (13)

@drrobotnik
9 months ago

wp-includes/functions.php, changes default type to null in order to fix bug in WP while using PHP 7.1

#1 @SergeyBiryukov
9 months ago

  • Milestone changed from Awaiting Review to 4.7.3

Hi @drrobotnik, thanks for the ticket and the patch!

$number = '' was added in [3255] and has not changed since then.

Shouldn't it be $number = 0 instead of null though?

#2 @drrobotnik
9 months ago

Hey @SergeyBiryukov,

Null shouldn't hurt anything, but yeah that makes sense. I was looking at an empty string and thinking unset.

I've attached the updated patch.

While I was testing the change I came across another issue a few lines down:

PHP Fatal error: Maximum execution time of 30 seconds exceeded in /wp-includes/functions.php on line 2098

<?php
while ( file_exists( $dir . "/$filename" ) ) {
    if ( '' === "$number$ext" ) {
        $filename = "$filename-" . ++$number;
    } else {
        $filename = str_replace( array( "-$number$ext", "$number$ext" ), "-" . ++$number . $ext, $filename );
    }
}

It was fixed by changing while() to if(). I see a few of these in functions.php. What's odd was it only happened when I uploaded a gif (963 KB), but not when I uploaded a jpg (1.3 MB). Should I open a separate ticket?

Last edited 9 months ago by drrobotnik (previous) (diff)

@drrobotnik
9 months ago

updated patch changes null to 0.

#3 @SergeyBiryukov
9 months ago

  • Keywords has-patch added

Ah, I was wrong, we can't set $number to 0, because it needs to be an empty string on first iteration. Otherwise, it causes the infinite loop issue that you experienced.

functions.diff works fine, I'd just like to make the type casting explicit just in case.

39774.patch also makes it more consistent with the preceding block.

#4 @drrobotnik
9 months ago

+1 for explicit type casting. Tested the additional patch, works for me!

#5 @SergeyBiryukov
9 months ago

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

In 40039:

Media: In wp_unique_filename(), use explicit type casting when incrementing $number.

This prevents the "non-numeric value encountered" warning in PHP 7.1, caused by trying to increment an empty string on the first loop iteration.

Props drrobotnik for initial patch.
Fixes #39774.

#6 @SergeyBiryukov
9 months ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for 4.7.3 consideration.

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


8 months ago

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


8 months ago

#9 @mikeschroder
8 months ago

I like the idea of fixing this in 4.7.3. Patch/commit looks good.

#10 @SergeyBiryukov
8 months ago

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

In 40075:

Media: In wp_unique_filename(), use explicit type casting when incrementing $number.

This prevents the "non-numeric value encountered" warning in PHP 7.1, caused by trying to increment an empty string on the first loop iteration.

Props drrobotnik for initial patch.
Merges [40039] to the 4.7 branch.
Fixes #39774.

Note: See TracTickets for help on using tickets.