`WP_Image_Editor::make_image()` ignores stream write and close failures

[desrosj]

When saving to a stream (for example, a plugin that offloads uploads to remote storage), WP_Image_Editor::make_image() buffers the image and writes it with fwrite() and fclose(). It ignores both return values and reports success as long as the image callback succeeded.

As a result, the GD editor can report a successful save when the image was truncated or never stored:

• If fwrite() writes fewer bytes than the image size, the stored file is incomplete.
• Some stream wrappers send data in stream_flush() or stream_close(). The AWS SDK S3 stream wrapper, for example, sends its PutObject request in stream_flush(). If that fails, the image is lost. Starting in PHP 8.6, fclose() returns false in this case (php-src commit ​c07cc85), but WordPress never checks.

The Imagick editor already handles this: WP_Image_Editor_Imagick::write_image() checks file_put_contents() === false and returns an image_save_error. GD should do the same.

On PHP 8.6, a stream wrapper that doesn't implement stream_flush() makes fclose() return false even when the write succeeded. With this change, GD saves to such wrappers will fail on PHP 8.6. This already happens with Imagick on PHP 8.6 because of the file_put_contents() check. The fix for affected wrappers is to implement stream_flush(), which may be worth a dev note.

As of PHP 8.6, fclose(), file_put_contents(), and copy() return false when flushing or closing a stream fails (​php/php-src@c07cc85, bug #60110). Before 8.6, these failures were silently ignored.

A userspace stream wrapper that does not implement stream_flush() is treated as failing to flush (php_userstreamop_flush() returns -1 when the method does not exist). WP_Test_Stream does not implement it, so on PHP 8.6 every write to it is reported as a failure, even though the data is written:

	file_put_contents()	Bytes stored	fclose()
PHP 8.5.12	100	100	true
PHP 8.6.0RC4	false	100	false

This causes Tests_Image_Editor_Imagick::test_streams to fail on PHP 8.6, because WP_Image_Editor_Imagick::write_image() (correctly) checks the return value of file_put_contents().

Changes

• Implements WP_Test_Stream::stream_flush(), returning true. Writes are stored directly in memory, so there is nothing to flush. WP_Test_Stream_Http_Mock extends this class and inherits the fix.
• Moves assertNotWPError( $saved ) in test_streams before $saved['path'] is accessed. Previously, a failed save surfaced as Error: Cannot use object of type WP_Error as array instead of the actual image_save_error.

This is a test-only change.

Testing

Ran against trunk with Imagick as the image editor (--group image,media):

	Before	After
PHP 8.6.0RC4	test_streams errors	Passes
PHP 8.5.12	Passes	Passes

Tests_Image_Editor_GD::test_flip also fails on PHP 8.6, both with and without this change, which is unrelated. PHPCS passes on the changed files.

Trac ticket: Core-66486.

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: Investigating the root cause, writing the change and the PR description, and running the tests.

---
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See ​GitHub Pull Requests for Code Review in the Core Handbook for more details.

🤖 Generated with ​Claude Code

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

This matters for stream wrappers that only send data when the stream is flushed or closed. For example, the AWS SDK's S3 stream wrapper performs the PutObject upload in stream_flush(). If that upload fails, GD reports the image as saved even though it was never stored.

As of PHP 8.6, fclose() returns false when flushing or closing a stream fails (​php/php-src@c07cc85), so this failure can now be detected. WP_Image_Editor_Imagick::write_image() already checks the return value of file_put_contents() for streams, so the two editors currently behave differently for the same failing stream.

Changes

• make_image() now returns false when fwrite() does not write the full image, or when fclose() fails. Callers already turn a false result into an image_save_error.
• New tests in Tests_Image_Editor_GD:
  • test_save_to_stream: a successful save to a stream.
  • test_save_to_stream_fails_when_write_fails: the stream accepts no data (all PHP versions).
  • test_save_to_stream_fails_when_flush_fails: the stream fails to flush (@requires PHP >= 8.6, as earlier versions cannot detect this).

Dependency

On PHP 8.6, test_save_to_stream requires #15476, which implements stream_flush() in WP_Test_Stream. Without it, writes to WP_Test_Stream fail to flush on PHP 8.6, and this change correctly reports them as failures.

Testing

Ran against trunk (--group image,media), with #15476 applied:

	New stream tests	Rest of image and media groups
PHP 8.6.0RC4	3 pass	Pass, except the unrelated Tests_Image_Editor_GD::test_flip failure, which also fails on trunk
PHP 8.5.12	2 pass, 1 skipped (requires PHP 8.6)	Pass

Without the make_image() change, test_save_to_stream_fails_when_write_fails and test_save_to_stream_fails_when_flush_fails fail on PHP 8.6. PHPCS passes on the changed files.

The @since 7.2.0 changelog entry may need adjusting, depending on the release this lands in.

Trac ticket: Core-66486

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: Investigating the issue, writing the change, tests, and PR description, and running the tests.

---
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See ​GitHub Pull Requests for Code Review in the Core Handbook for more details.

🤖 Generated with ​Claude Code

​https://claude.ai/code/session_011Y86gwJZGdkh78pggUy4zs

There are some potential risks to this change as summarized by Claude:

1. Main risk: stream wrappers without stream_flush() will fail GD saves on PHP 8.6

This is the same problem that broke WP_Test_Stream. On PHP 8.6, fclose() returns false whenever a userspace stream wrapper doesn't implement stream_flush(), even though the data was written. With #15477, a plugin that registers a wrapper without that method will now get image_save_error from the GD editor on PHP 8.6. Today the save "succeeds" because the result is ignored. Affected sites would lose generated sub-sizes and the ability to edit, rotate or crop images, for writes that actually worked.

What limits this:

• Imagick already does this on 8.6. WP_Image_Editor_Imagick::write_image() (class-wp-image-editor-imagick.php:1190) writes to streams with file_put_contents( ... ) === false. On PHP 8.6 that also returns false when the flush fails. Any site using Imagick (the default editor when available) will hit this with or without #15477. The PR makes GD match Imagick rather than adding new strictness.
• It only affects PHP 8.6. The flush check does nothing on earlier versions, so nothing changes for current production sites.
• The big offload wrappers implement stream_flush(). The AWS SDK StreamWrapper, used by S3 offload plugins, does its upload inside stream_flush(). For those wrappers the new check catches real data loss that GD currently hides.

Even so, smaller or custom wrappers without the method will start failing on 8.6. Adding stream_flush() fixes them, but the PR could make that easier to discover (see the last section).

2. Smaller items

• Short-write check on all PHP versions. If fwrite() returns fewer bytes than the image size, the save now fails on any PHP version. PHP keeps retrying a wrapper's partial writes until it reports zero bytes, so this only triggers when the wrapper actually stopped accepting data. In that case the image was already truncated, so this is a real failure being surfaced. A wrapper that wrongly returns 0 while still storing everything would also start failing, but that wrapper is broken under PHP's own rules.
• Generic error message. GD reports the existing generic "Image Editor Save Failed." It doesn't say the stream write or close failed, while Imagick names file_put_contents(). Whoever hits issue 1 will have little to go on.
• Partial files aren't cleaned up. On failure, whatever the wrapper received stays at the destination. That's unchanged from today, but the call now reports failure while a file exists at that path.
• Subclasses. Third-party editors that extend WP_Image_Editor and call make_image() inherit the stricter behavior. If they already check the return value, they'll start seeing failures in the same situations. There's no API or signature change.

One note: CI can't show the test_streams fix, because the PHP 8.6 jobs skip it with "The image editor engine WP_Image_Editor_Imagick is not supported on this system". Moving assertNotWPError() ahead of the $savedpath read is a good change on its own.

I forgot to note that technically no release of imagick supports PHP 8.6 yet. However, Claude built a Docker image locally with ImageMagick support by building directly from the project's primary branch.

[desrosj]

In 65294:

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 ​https://github.com/WordPress/wordpress-develop/pull/15477.

Props adamsilverstein.
Fixes #66486.