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.
Fixes #42437.

git-svn-id: https://develop.svn.wordpress.org/trunk@46822 602fd350-edb4-49c9-b593-d223f7449a82

src/wp-includes/functions.php

@@ -2409,6 +2409,7 @@ function wp_unique_filename( $dir, $filename, $unique_filename_callback = null )
 	// Separate the filename into a name and extension.
 	$ext  = pathinfo( $filename, PATHINFO_EXTENSION );
 	$name = pathinfo( $filename, PATHINFO_BASENAME );
+
 	if ( $ext ) {
 		$ext = '.' . $ext;
 	}
@@ -2426,6 +2427,15 @@ function wp_unique_filename( $dir, $filename, $unique_filename_callback = null )
 		$filename = call_user_func( $unique_filename_callback, $dir, $name, $ext );
 	} 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.
 		if ( $ext && strtolower( $ext ) != $ext ) {
@@ -2433,41 +2443,93 @@ function wp_unique_filename( $dir, $filename, $unique_filename_callback = null )
 			$filename2 = preg_replace( '|' . preg_quote( $ext ) . '$|', $ext2, $filename );
 
 			// 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 );
+				$filename   = str_replace( array( "-{$number}{$ext}", "{$number}{$ext}" ), "-{$new_number}{$ext}", $filename );
-				$filename2  = str_replace( array( "-$number$ext2", "$number$ext2" ), "-$new_number$ext2", $filename2 );
+				$filename2  = str_replace( array( "-{$number}{$ext2}", "{$number}{$ext2}" ), "-{$new_number}{$ext2}", $filename2 );
 				$number     = $new_number;
 			}
 
-			/**
+			$filename = $filename2;
-			 * Filters the result when generating a unique file name.
+		} else {
-			 *
+			while ( file_exists( $dir . "/{$filename}" ) ) {
-			 * @since 4.5.0
+				$new_number = (int) $number + 1;
-			 *
+
-			 * @param string        $filename                 Unique file name.
+				if ( '' === "{$number}{$ext}" ) {
-			 * @param string        $ext                      File extension, eg. ".png".
+					$filename = "{$filename}-{$new_number}";
-			 * @param string        $dir                      Directory path.
+				} else {
-			 * @param callable|null $unique_filename_callback Callback function that generates the unique file name.
+					$filename = str_replace( array( "-{$number}{$ext}", "{$number}{$ext}" ), "-{$new_number}{$ext}", $filename );
-			 */
+				}
-			return apply_filters( 'wp_unique_filename', $filename2, $ext, $dir, $unique_filename_callback );
+
+				$number = $new_number;
+			}
 		}
 
-		while ( file_exists( $dir . "/$filename" ) ) {
+		// Prevent collisions with existing file names that contain dimension-like strings
-			$new_number = (int) $number + 1;
+		// (whether they are subsizes or originals uploaded prior to #42437).
-			if ( '' == "$number$ext" ) {
+
-				$filename = "$filename-" . $new_number;
+		// The (resized) image files would have name and extension, and will be in the uploads dir.
-			} else {
+		if ( @is_dir( $dir ) && $name && $ext ) {
-				$filename = str_replace( array( "-$number$ext", "$number$ext" ), '-' . $new_number . $ext, $filename );
+			// 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;
+				}
 			}
-			$number = $new_number;
 		}
 	}
 
-	/** This filter is documented in wp-includes/functions.php */
+	/**
+	 * 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;
+}
+
 /**
  * Create a file in the upload folder with given content.
  *

tests/phpunit/tests/functions.php

@@ -195,6 +195,19 @@ class Tests_Functions extends WP_UnitTestCase {
 		$this->assertEquals( 'abcdefg.png', wp_unique_filename( $testdir, 'abcde\\\fg.png' ), 'Tripple slashed not removed' );
 	}
 
+	/**
+	 * @group 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' ) );
+	}
+
 	function test_is_serialized() {
 		$cases = array(
 			serialize( null ),