Changeset 46836

Timestamp:

12/09/2019 12:19:19 PM (5 years ago)

Author:

SergeyBiryukov

Message:

Upload: fix wp_unique_filename() to prevent name collisions with existing or future image sub-size file names, and add unit tests.

Props Viper007Bond, pbiron, azaozz.
Merges [46822], [46835] to the 5.3 branch.
Fixes #42437.

Location:

branches/5.3

Files:

• . (modified) (1 prop)
• src/wp-includes/functions.php (modified) (3 diffs)
• tests/phpunit/data/images/one-blue-pixel-1-100x100.png (copied) (copied from trunk/tests/phpunit/data/images/one-blue-pixel-1-100x100.png)
• tests/phpunit/tests/functions.php (modified) (1 diff)

branches/5.3/src/wp-includes/functions.php

@@ -2410 +2410 @@
     $ext  = pathinfo( $filename, PATHINFO_EXTENSION );
     $name = pathinfo( $filename, PATHINFO_BASENAME );
+
     if ( $ext ) {
         $ext = '.' . $ext;
@@ -2427 +2428 @@
     } else {
         $number = '';
+        $fname  = pathinfo( $filename, PATHINFO_FILENAME );
+
+        // Always append a number to file names that can potentially match image sub-size file names.
+        if ( $fname && preg_match( '/-(?:\d+x\d+|scaled|rotated)$/', $fname ) ) {
+            $number = 1;
+
+            // At this point the file name may not be unique. This is tested below and the $number is incremented.
+            $filename = str_replace( "{$fname}{$ext}", "{$fname}-{$number}{$ext}", $filename );
+        }
 
         // Change '.ext' to lower case.
@@ -2434 +2444 @@
 
             // Check for both lower and upper case extension or image sub-sizes may be overwritten.
-            while ( file_exists( $dir . "/$filename" ) || file_exists( $dir . "/$filename2" ) ) {
+            while ( file_exists( $dir . "/{$filename}" ) || file_exists( $dir . "/{$filename2}" ) ) {
                 $new_number = (int) $number + 1;
-                $filename   = str_replace( array( "-$number$ext", "$number$ext" ), "-$new_number$ext", $filename );
-                $filename2  = str_replace( array( "-$number$ext2", "$number$ext2" ), "-$new_number$ext2", $filename2 );
+                $filename   = str_replace( array( "-{$number}{$ext}", "{$number}{$ext}" ), "-{$new_number}{$ext}", $filename );
+                $filename2  = str_replace( array( "-{$number}{$ext2}", "{$number}{$ext2}" ), "-{$new_number}{$ext2}", $filename2 );
                 $number     = $new_number;
             }
 
-            /**
-             * Filters the result when generating a unique file name.
-             *
-             * @since 4.5.0
-             *
-             * @param string        $filename                 Unique file name.
-             * @param string        $ext                      File extension, eg. ".png".
-             * @param string        $dir                      Directory path.
-             * @param callable|null $unique_filename_callback Callback function that generates the unique file name.
-             */
-            return apply_filters( 'wp_unique_filename', $filename2, $ext, $dir, $unique_filename_callback );
-        }
-
-        while ( file_exists( $dir . "/$filename" ) ) {
-            $new_number = (int) $number + 1;
-            if ( '' == "$number$ext" ) {
-                $filename = "$filename-" . $new_number;
-            } else {
-                $filename = str_replace( array( "-$number$ext", "$number$ext" ), '-' . $new_number . $ext, $filename );
+            $filename = $filename2;
+        } else {
+            while ( file_exists( $dir . "/{$filename}" ) ) {
+                $new_number = (int) $number + 1;
+
+                if ( '' === "{$number}{$ext}" ) {
+                    $filename = "{$filename}-{$new_number}";
+                } else {
+                    $filename = str_replace( array( "-{$number}{$ext}", "{$number}{$ext}" ), "-{$new_number}{$ext}", $filename );
+                }
+
+                $number = $new_number;
             }
-            $number = $new_number;
-        }
-    }
-
-    /** This filter is documented in wp-includes/functions.php */
+        }
+
+        // Prevent collisions with existing file names that contain dimension-like strings
+        // (whether they are subsizes or originals uploaded prior to #42437).
+
+        // The (resized) image files would have name and extension, and will be in the uploads dir.
+        if ( @is_dir( $dir ) && $name && $ext ) {
+            // List of all files and directories contained in $dir (with the "dot" files removed).
+            $files = array_diff( scandir( $dir ), array( '.', '..' ) );
+
+            if ( ! empty( $files ) ) {
+                while ( _wp_check_existing_file_names( $filename, $files ) ) {
+                    $new_number = (int) $number + 1;
+                    $filename   = str_replace( array( "-{$number}{$ext}", "{$number}{$ext}" ), "-{$new_number}{$ext}", $filename );
+                    $number     = $new_number;
+                }
+            }
+        }
+    }
+
+    /**
+     * Filters the result when generating a unique file name.
+     *
+     * @since 4.5.0
+     *
+     * @param string        $filename                 Unique file name.
+     * @param string        $ext                      File extension, eg. ".png".
+     * @param string        $dir                      Directory path.
+     * @param callable|null $unique_filename_callback Callback function that generates the unique file name.
+     */
     return apply_filters( 'wp_unique_filename', $filename, $ext, $dir, $unique_filename_callback );
+}
+
+/**
+ * Helper function to check if a file name could match an existing image sub-size file name.
+ *
+ * @since 5.3.1
+ * @access private
+ *
+ * @param string $filename The file name to check.
+ * $param array  $files    An array of existing files in the directory.
+ * $return bool True if the tested file name could match an existing file, false otherwise.
+ */
+function _wp_check_existing_file_names( $filename, $files ) {
+    $fname = pathinfo( $filename, PATHINFO_FILENAME );
+    $ext   = pathinfo( $filename, PATHINFO_EXTENSION );
+
+    // Edge case, file names like `.ext`
+    if ( empty( $fname ) ) {
+        return false;
+    }
+
+    if ( $ext ) {
+        $ext = ".$ext";
+    }
+
+    $regex = '/^' . preg_quote( $fname ) . '-(?:\d+x\d+|scaled|rotated)' . preg_quote( $ext ) . '$/';
+
+    foreach ( $files as $file ) {
+        if ( preg_match( $regex, $file ) ) {
+            return true;
+        }
+    }
+
+    return false;
 }
 

branches/5.3/tests/phpunit/tests/functions.php

@@ -194 +194 @@
         $this->assertEquals( 'abcdefg.png', wp_unique_filename( $testdir, 'abcde\\fg.png' ), 'Double slashed not removed' );
         $this->assertEquals( 'abcdefg.png', wp_unique_filename( $testdir, 'abcde\\\fg.png' ), 'Tripple slashed not removed' );
+    }
+
+    /**
+     * @ticket 42437
+     */
+    function test_unique_filename_with_dimension_like_filename() {
+        $testdir = DIR_TESTDATA . '/images/';
+
+        // Test collision with "dimension-like" original filename.
+        $this->assertEquals( 'one-blue-pixel-100x100-1.png', wp_unique_filename( $testdir, 'one-blue-pixel-100x100.png' ) );
+        // Test collision with existing sub-size filename.
+        // Existing files: one-blue-pixel-100x100.png, one-blue-pixel-1-100x100.png.
+        $this->assertEquals( 'one-blue-pixel-2.png', wp_unique_filename( $testdir, 'one-blue-pixel.png' ) );
     }