Changeset 51912

Timestamp:

10/15/2021 10:57:47 PM (3 months ago)

Author:

hellofromTonya

Message:

FileSystem API: Add safeguard for invalid return from get_attached_file() in wp_delete_attachment().

The get_attached_file() function is supposed to return the path to the file, but could:

1. Return false if the file doesn't exist.
2. Return literally anything else, as a filter is being applied to the value on return.

As the clean_dirsize_cache() now has input validation, passing anything but a non-empty string to clean_dirsize_cache() will result in a PHP error notice.

This was exposed by the Tests_Post_GetPostStatus::wpSetUpBeforeClass() method which started generating unexpected output (the doing it wrong message) during the test run.

While this indicates that there is a flaw in the mocking being done in the test suite, debugging that is outside of the scope of the current patch.

At the same time, as based on the above point, this could potentially happen in a real-world situation as well, adding additional conditions to the if in the wp_delete_attachment() function before calling the clean_dirsize_cache() function, is warranted.

As there are no tests for the wp_delete_attachment() function at all at this time, we're not adding a test specifically for this change for now. This should however be addressed in the future, when tests will be added to cover the wp_delete_attachment() function completely.

Follow-up to [32619], [49212], [51910].

Props jrf, hellofromTonya.
See #52241.

File:

• trunk/src/wp-includes/post.php (modified) (1 diff)

trunk/src/wp-includes/post.php

@@ -6163 +6163 @@
     $file         = get_attached_file( $post_id );
 
-    if ( is_multisite() ) {
+    if ( is_multisite() && is_string( $file ) && ! empty( $file ) ) {
         clean_dirsize_cache( $file );
     }