Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#39774 closed defect (bug) (fixed)

php 7.1 media uploader bug

Reported by: drrobotnik's profile drrobotnik Owned by: sergeybiryukov's profile 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 7 years 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 7 years ago.
updated patch changes null to 0.
39774.patch (1.2 KB) - added by SergeyBiryukov 7 years ago.

Download all attachments as: .zip

Change History (13)

@drrobotnik
7 years ago

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

#1 @SergeyBiryukov
7 years 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
7 years 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 7 years ago by drrobotnik (previous) (diff)

@drrobotnik
7 years ago

updated patch changes null to 0.

#3 @SergeyBiryukov
7 years 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
7 years ago

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

#5 @SergeyBiryukov
7 years 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
7 years 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.


7 years ago

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


7 years ago

#9 @kirasong
7 years ago

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

#10 @SergeyBiryukov
7 years 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.