Skip to content

Media: Skip the multisite upload space check for sideloaded sub-sizes - #15954

Open
adamsilverstein wants to merge 5 commits into
WordPress:trunkfrom
adamsilverstein:trac-66680/ms-sideload-quota
Open

adamsilverstein wants to merge 5 commits into
WordPress:trunkfrom
adamsilverstein:trac-66680/ms-sideload-quota

Conversation

@adamsilverstein

Copy link
Copy Markdown
Member

Claude Code wrote the fix on top of the tests from #15860, and this description:

On multisite, a sub-size sideloaded by client-side media processing is no longer checked against the site's upload space. That matches the classic path, where only the original upload is checked and the server writes the sub-sizes regardless.

Before this, a site close to its quota accepted the original and then rejected some or all of the sub-sizes with rest_upload_limited_space, leaving the attachment with missing sizes.

This includes the two tests from #15860 by @desrosj (his commit is kept as is), so it supersedes that PR.

What changed

  • WP_REST_Attachments_Controller::sideload_item() sets a private flag while the sideloaded file is stored, and check_upload_size() skips its two space checks (space left, and site over quota) when the flag is set. Both upload handlers, multipart and raw body, go through that method.
  • The network's maximum upload file size (fileupload_maxk) still applies to a sideloaded file.
  • A new upload through POST /wp/v2/media is checked as before. Sideloaded files count toward the space used, so the next upload on a full site is still rejected.
  • Tests:

Questions for review

  • The space check is skipped for every file the sideload endpoint accepts. That includes the source_original (the HEIC or JXL kept next to its JPEG) and the animated_video companions, which are not files the classic path would generate. Should those still count against the quota?
  • Nothing limits how far one attachment's sideloads can push a site past its quota, other than the per-file limit and what the client sends. The classic path can overshoot too, but only by the sizes the server decides to generate. Is that acceptable, or does the endpoint need a bound of its own?
  • The Gutenberg plugin has its own copy of this endpoint in lib/media/class-gutenberg-rest-attachments-controller.php and needs the same change. Not done yet.

Testing

npm run test:php -- -c tests/phpunit/multisite.xml --filter 'multisite_upload_space|sideload_sub_size_exceeds_multisite_max_filesize'

The full WP_Test_REST_Attachments_Controller class passes on single site and multisite. This was not tested in a browser on a multisite install.

Trac ticket: https://core.trac.wordpress.org/ticket/66680

AI Use

The fix and this description came from 🤖 Claude Code, building on the tests in #15860. I will review and test.


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.

desrosj and others added 5 commits November 12, 2026 10:25
On the classic path, only the original upload is checked against the site's upload space. The image editor then writes sub-sizes even when the original used the last of it. With client-side media processing, every sub-size is sideloaded and checked against the quota on its own, so it is rejected with "Not enough space to upload." and the attachment is left with missing sub-sizes, or deleted when they all fail.

The first test is the classic baseline and passes. The second fails until the sideload endpoint stops checking the quota for an attachment's own sub-sizes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On multisite, the classic upload path checks only the original file against the site's upload space. The sub-sizes the server generates from it are written without a check. With client-side media processing each sub-size is sideloaded and was checked on its own, so a site close to its quota rejected some of them and left the attachment with missing sizes.

Disable the space check while a sideloaded file is stored, so both paths treat an attachment's sub-sizes the same way. New uploads are still checked.

See #66680.

Claude-Session: https://claude.ai/code/session_018vwYjSXLpsi7LrGqx7UsrL
Add the `@ticket` annotation, assert the types PHPStan cannot infer, and drop the sentence describing the sideload check, which no longer runs.

See #66680.

Claude-Session: https://claude.ai/code/session_018vwYjSXLpsi7LrGqx7UsrL
The browser sends each sub-size as multipart form data, which takes a different upload handler than the raw request body the existing test uses. Also confirm the upload space check still applies to the next new upload.

See #66680.

Claude-Session: https://claude.ai/code/session_018vwYjSXLpsi7LrGqx7UsrL
Disabling the whole upload space check for a sideload also dropped the network's per-file size limit, which is unrelated to the space a site has left. Skip only the two space checks with a flag set while the file is stored, so a sideloaded file is still held to `fileupload_maxk`.

See #66680.

Claude-Session: https://claude.ai/code/session_018vwYjSXLpsi7LrGqx7UsrL
@github-actions

Copy link
Copy Markdown

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 or WordPress.org 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.

@github-actions

Copy link
Copy Markdown

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.

@adamsilverstein adamsilverstein left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, running manual tests.

@adamsilverstein

Copy link
Copy Markdown
Member Author

Manual testing on a multisite install confirms the bug on trunk and the fix in this PR, screencasts below.

Claude ran the before and after on a multisite Playground site:

Setup: WordPress Playground with today's nightly (7.2-beta5-65405) as the "before", and the same build with this PR's class-wp-rest-attachments-controller.php dropped in as the "after". Multisite, site upload space limited to 1.6 MB, max upload file size 10 MB. Each run uploads an 1800x1200, 1.5 MB JPEG to an Image block in the post editor, with client-side media processing active (crossOriginIsolated is true).

Before:

  • The original upload (201) and three sub-size sideloads (200) went through, then two sideloads came back 400 rest_upload_limited_space ("Not enough space to upload. 67 KB needed." and "276 KB needed.").
  • The attachment ended up with thumbnail, medium and medium_large, but no large or 1536x1536.
  • The editor still showed "Upload complete", so nothing tells the user that sizes are missing. That part is separate from this ticket, but probably worth a Gutenberg follow-up.

After:

  • All five sideloads returned 200 and the attachment has every sub-size.
  • The site ended at about 2.0 MB against the 1.6 MB quota, the kind of overshoot the classic path already allows when the server writes sub-sizes.
  • A new upload through POST /wp/v2/media right after was still rejected with rest_upload_limited_space, so the quota still applies to new files.

The dark panel in the top right of each screencast mirrors the media POST responses as they happen, and the table at the end is the attachment's media_details.sizes from GET /wp/v2/media/<id>?context=edit.

Before (trunk):

trac-66680-before.mp4

After (this PR):

trac-66680-after.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants