diff --git a/.changeset/blockset-cleanup-on-failed-extraction.md b/.changeset/blockset-cleanup-on-failed-extraction.md new file mode 100644 index 000000000..f99669046 --- /dev/null +++ b/.changeset/blockset-cleanup-on-failed-extraction.md @@ -0,0 +1,5 @@ +--- +"@faustwp/wordpress-plugin": patch +--- + +fix[faustwp]: clean up uploaded blockset zip when extraction fails diff --git a/plugins/faustwp/includes/blocks/functions.php b/plugins/faustwp/includes/blocks/functions.php index 716e2ff71..8253f4053 100644 --- a/plugins/faustwp/includes/blocks/functions.php +++ b/plugins/faustwp/includes/blocks/functions.php @@ -53,6 +53,7 @@ function process_and_replace_blocks( $wp_filesystem, $file, $dirs ) { $unzip_result = unzip_uploaded_file( $target_file, $dirs['target'] ); if ( is_wp_error( $unzip_result ) ) { + $wp_filesystem->delete( $target_file ); return $unzip_result; } diff --git a/plugins/faustwp/tests/unit/BlockFunctionTests.php b/plugins/faustwp/tests/unit/BlockFunctionTests.php index 8b3139a63..97f8cf372 100644 --- a/plugins/faustwp/tests/unit/BlockFunctionTests.php +++ b/plugins/faustwp/tests/unit/BlockFunctionTests.php @@ -136,4 +136,92 @@ public function test_ensure_directories_exist() { $this->assertTrue( Blocks\ensure_directories_exist( $dirs ) ); } + /** + * Regression guard for #2311: when unzip_uploaded_file() returns a WP_Error, + * process_and_replace_blocks() must call $wp_filesystem->delete( $target_file ) + * before returning, so the orphaned zip doesn't accumulate at the predictable + * path under wp-content/uploads/faustwp/blocks/. + * + * Verified red-on-revert: removing the delete line from + * includes/blocks/functions.php fails this test's Mockery once() expectation. + */ + public function test_process_and_replace_blocks_cleans_up_on_unzip_failure() { + $file = [ + 'name' => 'mock.zip', + 'type' => 'application/zip', + 'tmp_name' => '/tmp/mock.zip', + ]; + $dirs = [ + 'target' => '/uploads/blocks', + 'temp' => '/uploads/blocks/tmp', + ]; + $target_file = '/uploads/blocks/mock.zip'; + $error = new WP_Error( 'unzip_error', 'mock unzip failure' ); + + stubs([ + // Internal helpers: success on move, error on unzip. cleanup must NOT + // run on the error path; if it does, fail loudly so we catch a future + // regression where the early return is removed. + 'WPE\FaustWP\Blocks\move_uploaded_file' => true, + 'WPE\FaustWP\Blocks\unzip_uploaded_file' => $error, + 'WPE\FaustWP\Blocks\cleanup_temp_directory' => function () { + throw new \LogicException( 'cleanup_temp_directory must not run on the unzip-failure path' ); + }, + // WP global helpers: keep the target-file path deterministic so the + // Mockery ->with() expectation matches without depending on Brain + // Monkey's default passthroughs. + 'trailingslashit' => function ( $s ) { return rtrim( $s, '/' ) . '/'; }, + 'sanitize_file_name' => function ( $s ) { return $s; }, + ]); + + $filesystem = Mockery::mock( 'WP_Filesystem_Base' ); + $filesystem->shouldReceive( 'delete' ) + ->with( $target_file ) + ->once(); + + $result = Blocks\process_and_replace_blocks( $filesystem, $file, $dirs ); + + $this->assertInstanceOf( WP_Error::class, $result ); + $this->assertSame( $error, $result ); + } + + /** + * Success path: when both move and unzip succeed, process_and_replace_blocks() + * must NOT call $wp_filesystem->delete( $target_file ) (the extracted blocks + * remain in place) and must invoke cleanup_temp_directory() for the staging dir. + * + * Inverse of the regression-guard test above: catches a future change that + * over-aggressively deletes the target on the happy path. + */ + public function test_process_and_replace_blocks_success_path_does_not_delete_target() { + $file = [ + 'name' => 'mock.zip', + 'type' => 'application/zip', + 'tmp_name' => '/tmp/mock.zip', + ]; + $dirs = [ + 'target' => '/uploads/blocks', + 'temp' => '/uploads/blocks/tmp', + ]; + + $cleanup_called = false; + stubs([ + 'WPE\FaustWP\Blocks\move_uploaded_file' => true, + 'WPE\FaustWP\Blocks\unzip_uploaded_file' => true, + 'WPE\FaustWP\Blocks\cleanup_temp_directory' => function () use ( &$cleanup_called ) { + $cleanup_called = true; + }, + 'trailingslashit' => function ( $s ) { return rtrim( $s, '/' ) . '/'; }, + 'sanitize_file_name' => function ( $s ) { return $s; }, + ]); + + $filesystem = Mockery::mock( 'WP_Filesystem_Base' ); + $filesystem->shouldNotReceive( 'delete' ); + + $result = Blocks\process_and_replace_blocks( $filesystem, $file, $dirs ); + + $this->assertTrue( $result ); + $this->assertTrue( $cleanup_called, 'cleanup_temp_directory must run on the success path.' ); + } + }