Media: Check the result of writing an image to a stream in `WP_Image_Editor::make_image()` by desrosj · Pull Request #15477 · WordPress/wordpress-develop

desrosj · 2026-11-04T21:46:03Z

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

…Editor::make_image()`. When saving an image to a stream, `make_image()` ignored the return values of `fwrite()` and `fclose()`, so a save that failed to write the image still reported success. This is used by `WP_Image_Editor_GD` for stream destinations. Some stream wrappers, such as those for remote storage, only send the data when the stream is flushed or closed. As of PHP 8.6, `fclose()` returns `false` when flushing or closing a stream fails, which is the only way such a failure can be detected. `make_image()` now returns `false` when the image is not fully written or the stream cannot be closed. This matches `WP_Image_Editor_Imagick::write_image()`, which already checks the return value of `file_put_contents()`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Y86gwJZGdkh78pggUy4zs

github-actions · 2026-11-04T21:51:28Z

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

All changes will be lost when closing a tab with a Playground instance.
All changes will be lost when refreshing the page.
A fresh instance is created each time the link below is clicked.
Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

Copilot

🟢 Approval recommended

No active review issues are recorded for the changes that report incomplete stream writes and close failures.

No open issues 🎉

What changed in this PR

This PR makes image saves to streams report failure when writing is incomplete or closing the stream fails, allowing existing GD callers to return image_save_error.

Changes:

Checks the number of bytes written and the result of fclose() in WP_Image_Editor::make_image().
Adds GD tests for successful saves, write failures, and flush failures detected on PHP 8.6+. The successful-save test depends on #15476 on PHP 8.6.

File	Description
`tests/phpunit/tests/image/editorGd.php`	Adds stream-save tests and failure-simulating wrappers.
`src/wp-includes/class-wp-image-editor.php`	Checks stream write and close results and documents the behavior change.

_{Review effort: Balanced}

github-actions · 2026-11-05T02:10:38Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs.

Core Committers: Use this line as a base for the props when committing in SVN:

Props desrosj, adamsilverstein.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Props Bot commands

You can trigger Props Bot actions by commenting on this PR:

@propsbot add <username> to give props to a GitHub user. Requires write access.
@propsbot remove <username> to leave a user out of the props. Requires write access.
@propsbot refresh to refresh this list. Adding the props-bot label works too.

desrosj · 2026-11-05T02:12:08Z

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

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 Media: Check the result of writing an image to a stream in WP_Image_Editor::make_image() #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).

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.

adamsilverstein · 2026-11-05T18:03:09Z

+			 */
+			$closed = fclose( $fp );
+
+			if ( strlen( $contents ) !== $bytes_written || ! $closed ) {


I noticed that in addition to the false return check, this PR adds a length check (comparing the contents length to the written length. While this seems fine, I did wonder if there would be unanticipated edge cases where this might not be true even after a successful write (not sure how, just a feeling). I'll research this a bit further with Claude before approving. I'll also check if Imagick has a similar guard.

Imagick uses file_put_contents instead of fopen, fwrite and fclose. Can we use that same approach here? does that function properly return an error now if stream writing fails?

Claude made the same suggestion when reviewing:

Replace the fopen/fwrite/fclose block with file_put_contents( $filename, $contents ) !== false. That gives the length check, the 8.6 close-failure detection, and exact parity with Imagick in one call that PHP maintains. It's also less code.

adamsilverstein

Overall looks good, left a small suggestion.

Replaces the `fopen()`, `fwrite()`, and `fclose()` calls in `WP_Image_Editor::make_image()` with a single `file_put_contents()` call, matching `WP_Image_Editor_Imagick::write_image()`. `file_put_contents()` returns `false` when the stream cannot be opened or the image cannot be fully written, and as of PHP 8.6, when flushing or closing the stream fails. That covers the same failures as the separate byte count and `fclose()` checks, using the length check PHP itself maintains. An incomplete write now also triggers the PHP warning that `file_put_contents()` raises, as it already does for Imagick. `test_save_to_stream_fails_when_write_fails()` captures and asserts that warning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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

github-actions · 2026-11-06T01:48:16Z

A commit was made that fixes the Trac ticket referenced in the description of this pull request.

SVN changeset: 65294
GitHub commit: 39e403e

This PR will be closed, but please confirm the accuracy of this and reopen if there is more work to be done.

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 WordPress/wordpress-develop#15477. Props adamsilverstein. Fixes #66486. Built from https://develop.svn.wordpress.org/trunk@65294 git-svn-id: http://core.svn.wordpress.org/trunk@64443 1a063a9b-81f0-0310-95a4-ce76da25c4cd

desrosj requested a balanced review from Copilot November 5, 2026 00:48

Copilot started reviewing on behalf of desrosj November 5, 2026 00:49

View session

desrosj self-assigned this Nov 5, 2026

Copilot AI reviewed Nov 5, 2026

View reviewed changes

desrosj marked this pull request as ready for review November 5, 2026 02:10

adamsilverstein reviewed Nov 5, 2026

View reviewed changes

adamsilverstein approved these changes Nov 5, 2026

View reviewed changes

adamsilverstein reviewed Nov 6, 2026

View reviewed changes

Comment thread src/wp-includes/class-wp-image-editor.php Resolved

Improve the precision of the stream write inline comment.

a491ac8

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions Bot closed this Nov 6, 2026

Navigation Menu

Repository navigation

Media: Check the result of writing an image to a stream in `WP_Image_Editor::make_image()` - #15477

Media: Check the result of writing an image to a stream in `WP_Image_Editor::make_image()`#15477
desrosj wants to merge 3 commits into
WordPress:trunkfrom
desrosj:image-editor-check-stream-write

desrosj commented Nov 4, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Nov 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented Nov 5, 2026 •

edited

Loading

Uh oh!

desrosj commented Nov 5, 2026

Uh oh!

adamsilverstein Nov 5, 2026

Uh oh!

adamsilverstein Nov 5, 2026

Uh oh!

adamsilverstein Nov 5, 2026

Uh oh!

adamsilverstein left a comment

Uh oh!

Uh oh!

github-actions Bot commented Nov 6, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

desrosj commented Nov 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Dependency

Testing

Use of AI Tools

Uh oh!

github-actions Bot commented Nov 4, 2026

Test using WordPress Playground

Some things to be aware of

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

github-actions Bot commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

desrosj commented Nov 5, 2026

Uh oh!

adamsilverstein Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

adamsilverstein Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

adamsilverstein Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

adamsilverstein left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Nov 6, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

desrosj commented Nov 4, 2026 •

edited

Loading

github-actions Bot commented Nov 5, 2026 •

edited

Loading