Media: Report failed image writes to streams.

When saving an image to a stream, `WP_Image_Editor::make_image()` currently ignores the return values of `fwrite()` and `fclose()`. A save that failed to write the image still reported success. `WP_Image_Editor_GD` uses this path for all stream destinations.

The buffered image is now written with `file_put_contents()`, and `make_image()` returns `false` when that fails. This happens when the stream cannot be opened or the image is only partially written. As of PHP 8.6, it also happens when flushing or closing the stream fails. For stream wrappers that only send data on flush or close, a failed upload was previously reported as a successful save.

Streams are now written the same way as in `WP_Image_Editor_Imagick::write_image()`, so both editors report the same failures.

Developed in #15477.

Props adamsilverstein.
Fixes #66486.



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

Author: desrosj

src/wp-includes/class-wp-image-editor.php

@@ -559,6 +559,7 @@ public function maybe_exif_rotate() {
 	 * Either calls editor's save function or handles file as a stream.
 	 *
 	 * @since 3.5.0
+	 * @since 7.2.0 Returns false when the image cannot be fully written to a stream.
 	 *
 	 * @param string   $filename
 	 * @param callable $callback
@@ -579,15 +580,17 @@ protected function make_image( $filename, $callback, $arguments ) {
 		if ( $result && $stream ) {
 			$contents = ob_get_contents();
 
-			$fp = fopen( $filename, 'w' );
-
-			if ( ! $fp ) {
-				ob_end_clean();
-				return false;
+			/*
+			 * file_put_contents() returns false if the stream cannot be opened, or if fewer
+			 * bytes are written than the image contains.
+			 *
+			 * As of PHP 8.6, false is also returned when flushing or closing the stream fails.
+			 * For stream wrappers that only send data on flush or close, such a failure means
+			 * the image was never saved.
+			 */
+			if ( false === file_put_contents( $filename, $contents ) ) {
+				$result = false;
 			}
-
-			fwrite( $fp, $contents );
-			fclose( $fp );
 		}
 
 		if ( $stream ) {

tests/phpunit/tests/image/editorGd.php

@@ -8,6 +8,7 @@
  * @group wp-image-editor-gd
  */
 require_once __DIR__ . '/base.php';
+require_once DIR_TESTROOT . '/includes/class-wp-test-stream.php';
 
 class Tests_Image_Editor_GD extends WP_Image_UnitTestCase {
 
@@ -674,4 +675,102 @@ public function test_image_non_existent_extension() {
 
 		$this->assertTrue( $loaded );
 	}
+
+	/**
+	 * Tests that images can be saved to a stream.
+	 *
+	 * @covers WP_Image_Editor::make_image
+	 */
+	public function test_save_to_stream() {
+		$saved = $this->save_to_stream( 'wptestgd', 'WP_Test_Stream' );
+
+		$this->assertNotWPError( $saved );
+		$this->assertSame( 'wptestgd://Tests_Image_Editor_GD/write.jpg', $saved['path'] );
+		$this->assertNotEmpty( WP_Test_Stream::$data['Tests_Image_Editor_GD']['/write.jpg'], 'The image should have been written to the stream.' );
+	}
+
+	/**
+	 * Tests that saving fails when the image cannot be fully written to the stream.
+	 *
+	 * @covers WP_Image_Editor::make_image
+	 */
+	public function test_save_to_stream_fails_when_write_fails() {
+		// file_put_contents() warns about the incomplete write. $this->expectWarning() is deprecated in PHPUnit 10.
+		$warnings = array();
+		set_error_handler(
+			static function ( int $errno, string $errstr ) use ( &$warnings ) {
+				$warnings[] = $errstr;
+				return true;
+			},
+			E_WARNING
+		);
+
+		try {
+			$saved = $this->save_to_stream( 'wptestgdwritefail', 'WP_Test_Stream_GD_Write_Failure' );
+		} finally {
+			restore_error_handler();
+		}
+
+		$this->assertCount( 1, $warnings, 'Expected one warning about the incomplete write.' );
+		$this->assertStringContainsString( 'bytes written', $warnings[0] );
+		$this->assertWPError( $saved );
+		$this->assertSame( 'image_save_error', $saved->get_error_code() );
+	}
+
+	/**
+	 * Tests that saving fails when the stream cannot be flushed.
+	 *
+	 * PHP 8.6 is the first version where fclose() reports flush and close failures.
+	 *
+	 * @requires PHP >= 8.6
+	 *
+	 * @covers WP_Image_Editor::make_image
+	 */
+	public function test_save_to_stream_fails_when_flush_fails() {
+		$saved = $this->save_to_stream( 'wptestgdflushfail', 'WP_Test_Stream_GD_Flush_Failure' );
+
+		$this->assertWPError( $saved );
+		$this->assertSame( 'image_save_error', $saved->get_error_code() );
+	}
+
+	/**
+	 * Loads a JPEG image with the GD editor and saves it to a stream.
+	 *
+	 * @param string $protocol      The stream protocol to register.
+	 * @param string $wrapper_class The stream wrapper class to register for the protocol.
+	 * @return array|WP_Error The result of WP_Image_Editor_GD::save().
+	 */
+	private function save_to_stream( $protocol, $wrapper_class ) {
+		stream_wrapper_register( $protocol, $wrapper_class );
+		WP_Test_Stream::$data = array();
+
+		$gd_image_editor = new WP_Image_Editor_GD( DIR_TESTDATA . '/images/waffles.jpg' );
+		$loaded          = $gd_image_editor->load();
+
+		$saved = $gd_image_editor->save( $protocol . '://Tests_Image_Editor_GD/write.jpg', 'image/jpeg' );
+
+		stream_wrapper_unregister( $protocol );
+
+		$this->assertTrue( $loaded );
+
+		return $saved;
+	}
+}
+
+/**
+ * A stream wrapper that fails to write any data.
+ */
+class WP_Test_Stream_GD_Write_Failure extends WP_Test_Stream {
+	public function stream_write( $data ) {
+		return 0;
+	}
+}
+
+/**
+ * A stream wrapper that fails to flush, like a remote storage wrapper whose upload fails.
+ */
+class WP_Test_Stream_GD_Flush_Failure extends WP_Test_Stream {
+	public function stream_flush() {
+		return false;
+	}
 }