Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#54609 closed defect (bug) (duplicate)

wp_unique_filename issue/bug

Reported by: ultravibe's profile ultravibe Owned by:
Milestone: Priority: normal
Severity: major Version:
Component: Media Keywords: needs-patch dev-feedback
Focuses: Cc:

Description

Upgraded recently from much older version (5.4) to 5.8.1 - image uploads ceased to work, timeouts, 403 errors, etc. depending on which admin section it was attempted from.

Enterprise-level install with literally thousands of images in each monthly folder. We traced the issue to the update of the function wp_unique_filename() specifically this portion:

<?php

if ( $name && $ext && @is_dir( $dir ) && false !== strpos( $dir, $upload_dir['basedir'] ) ) {
        $files = apply_filters( 'pre_wp_unique_filename_file_list', null, $dir, $filename );

        if ( null === $files ) {
                // List of all files and directories contained in $dir.
                $files = @scandir( $dir );
        }

        if ( ! empty( $files ) ) {
                // Remove "dot" dirs.
                $files = array_diff( $files, array( '.', '..' ) );
        }

        if ( ! empty( $files ) ) {
                $count = count( $files );

                // Ensure this never goes into infinite loop
                // as it uses pathinfo() and regex in the check, but string replacement for the changes.
                $i = 0;

                while ( $i <= $count && _wp_check_existing_file_names( $filename, $files ) ) {
                        $new_number = (int) $number + 1;

                        // If $ext is uppercase it was replaced with the lowercase version after the previous loop.
                        $filename = str_replace( array( "-{$number}{$lc_ext}", "{$number}{$lc_ext}" ), "-{$new_number}{$lc_ext}", $filename );

                        $number = $new_number;
                        $i++;
                }
        }
}

On Dec 1 for us, image uploads were fine, but by the 6th image uploads stopped working again - The @scadir function will at some point in the month return an array that is just too large for the system to handle, causing timeouts and cascading failures.

It seems this portion of the method is attempting to check if a filename already exists and append a number if it does to avoid duplicate filenames, but that also seems to be handled well earlier in the method.

<?php
while ( file_exists( $_dir . $filename ) || ( $lc_filename && file_exists( $_dir . $lc_filename ) ) ) {
        $new_number = (int) $number + 1;

        if ( $lc_filename ) {
                $lc_filename = str_replace( array( "-{$number}{$lc_ext}", "{$number}{$lc_ext}" ), "-{$new_number}{$lc_ext}", $lc_filename );
        }

        if ( '' === "{$number}{$ext}" ) {
                $filename = "{$filename}-{$new_number}";
        } else {
                $filename = str_replace( array( "-{$number}{$ext}", "{$number}{$ext}" ), "-{$new_number}{$ext}", $filename );
        }

        $number = $new_number;
}

Apologies if this is improper bug reporting - my first one! But it does seem to be an issue for sites that may be working with large numbers of images in monthly folders (or potentially just the uploads folder if they don't set to separate.)

Change History (2)

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


2 years ago

#2 @antpb
2 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Hi @ultravibe ! This ticket was discussed last week and it seems there is an existing ticket that provides a workaround: https://core.trac.wordpress.org/ticket/50587

If you ever want to skip a path @joedolson provided a snippet in the meeting that may help you
add_filter( 'pre_wp_unique_filename_file_list', function() { return array(); } );

I am going to close this out in favor of the existing ticket at #50587 but feel free to open again if this is different.

Note: See TracTickets for help on using tickets.