Skip to content

Split the media delegate into MediaProcessor and MediaUploader - #621

Draft
jkmassel wants to merge 1 commit into
fix/register-core-media-upload-middlewarefrom
refactor/media-processor-uploader
Draft

Split the media delegate into MediaProcessor and MediaUploader#621
jkmassel wants to merge 1 commit into
fix/register-core-media-upload-middlewarefrom
refactor/media-processor-uploader

Conversation

@jkmassel

@jkmassel jkmassel commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Stacked on #594. Makes performing a media upload — and retrying it — a single, all-or-nothing responsibility: either GutenbergKit performs the upload and owns its retries, or the host does (say, to run it through its own networking so it can log every request). Both go to the same configured site; the only difference is who executes the requests. There's no in-between where the host performs the upload but GutenbergKit retries it.

Summary

Two protocols replace the single MediaUploadDelegate:

  • MediaProcessor — transform the file before upload; GutenbergKit performs the upload and owns its retries. The common, safe extension point.
  • MediaUploader — perform the upload yourself (your own networking, logging, retry policy, a background session) and own its whole lifecycle: retries, recovery, and cleanup.

Breaking: hosts migrate mediaUploadDelegatemediaProcessor / mediaUploader.

The problem with the old design

When an upload fatals in server-side post-processing, it has to be retried: core retries POST …/post-process up to 5×, and cleans up the orphan if that fails.

MediaUploadDelegate.uploadFile let a host perform the upload itself by returning the raw response it received. But that split one upload's HTTP across two owners: the host performed the POST /wp/v2/media, then core — reading that raw response — drove the post-process retries (and the orphan cleanup) behind it. A host that took over uploads to run them through its own stack still didn't own the retries; those went out through the browser, not the host. Delivery and its retries were owned by different parties.

The fix

Make the upload and its retries one unit with one owner:

  • MediaProcessor (handlesFile, processFile) only transforms the file. It never performs the upload, so GutenbergKit performs it and owns the retries. The extension point almost every host wants.
  • MediaUploader (upload) performs the upload on the host's own stack and owns the whole lifecycle. upload returns the finished attachment or throws — there's no raw response for core to retry behind it, so the host drives its own post-process recovery and force-deletes its own orphan on terminal failure.

So it's all-or-nothing: the host performs the upload and its retries, or GutenbergKit does — never a split. An uploader and GutenbergKit's built-in default both target the same configured site; the choice is only who executes the requests.

Media deletes always relay to the default uploader (the configured site) — every attachment lives there, even one a host uploader delivered, so there's no per-host delete path. EditorViewController / GutenbergView expose mediaProcessor + mediaUploader; the server starts if either is set and builds a default uploader whenever site credentials are present (it delivers GutenbergKit's own uploads and relays every delete). MediaUploadResponse is now internal — it's no longer on any public API.

Accepted Risk / Out of Scope

  • The configured-site delete relay isn't scoped. GutenbergKit relays core's cleanup DELETE to the configured site, but the relay can't distinguish it from any other DELETE the WebView sends — a client-side-compromised editor (a supply-chain-tampered JS bundle, or editor XSS) holding the loopback token could force-delete arbitrary media there. We accept this: such a script already has broad write access via allowed methods, and a server-side compromise (a malicious plugin) deletes media directly without the editor. An earlier revision carried a per-session ledger to scope the relay; it's dropped as not worth the cost for a client-side-only threat.

Test Plan

  • iOS host suite green — 964 tests
  • Android unit suites green — MediaUploadServer, GutenbergView
  • SwiftLint + Detekt clean
  • iOS demo app builds (xcodebuild, Xcode 26.4.1); Android demo builds (Detekt compiles it)
  • Migrate mediaUploadDelegate in WordPress-iOS / WordPress-Android / Jetpack

Related

@jkmassel jkmassel added [Type] Breaking Change For PRs that introduce a change that will break existing functionality iOS Android labels Sep 3, 2026
@jkmassel jkmassel self-assigned this Sep 3, 2026
@wpmobilebot

wpmobilebot commented Sep 3, 2026

Copy link
Copy Markdown

XCFramework Build

This PR's XCFramework is available for testing. Add the following to your Package.swift:

.package(url: "https://github.com/wordpress-mobile/GutenbergKit", branch: "pr-build/621")

Built from 1247d09

@jkmassel
jkmassel force-pushed the refactor/media-processor-uploader branch 4 times, most recently from 9fe481e to 9f00177 Compare September 3, 2026 21:50
Make performing a media upload -- and retrying it -- a single, all-or-nothing
responsibility: either GutenbergKit performs the upload and owns its retries, or
the host does (say, to run it through its own networking so it can log every
request). Both go to the same configured site; the only difference is who executes
the requests. There is no in-between where the host performs the upload but
GutenbergKit retries it.

When an upload fatals in server-side post-processing it has to be retried: core
retries POST .../post-process up to 5x, and cleans up the orphan if that fails.
The old `MediaUploadDelegate.uploadFile` let a host perform the upload itself by
returning the raw response it received -- which split one upload's HTTP across two
owners: the host performed the POST /wp/v2/media, then core, reading that raw
response, drove the post-process retries (and the orphan cleanup) behind it. A
host that took over uploads to run them through its own stack still didn't own the
retries; those went out through the browser, not the host. Delivery and its
retries were owned by different parties.

Make the upload and its retries one unit with one owner:

- `MediaProcessor` (handlesFile, processFile) only transforms the file. It never
  performs the upload, so GutenbergKit performs it and owns the retries -- the
  extension point almost every host wants.
- `MediaUploader` (upload) performs the upload on the host's own stack (its
  networking, logging, retry policy, a background session) and owns the whole
  lifecycle. `upload` returns the finished attachment or throws: there's no raw
  response for core to retry behind it, so the host drives its own post-process
  recovery and force-deletes its own orphan on terminal failure.

All-or-nothing: the host performs the upload and its retries, or GutenbergKit does
-- never a split. An uploader and GutenbergKit's built-in default both target the
same configured site; the choice is only who executes the requests.

Media deletes always relay to the default uploader (the configured site): every
attachment lives there, even one a host uploader delivered, so there is no per-host
delete path. The relay is left unscoped -- core issues its cleanup DELETE there,
but the relay can't tell it from any other DELETE the WebView sends, so a
client-side-compromised editor holding the loopback token could force-delete media
on the site. Accepted -- such a script already has broad write access, and a
server-side compromise deletes media directly without the editor. An earlier
revision carried a per-session ledger to scope the relay; dropped as not worth the
cost for a client-side-only threat.

`EditorViewController`/`GutenbergView` expose `mediaProcessor` + `mediaUploader`
in place of `mediaUploadDelegate`; the server starts if either is set and builds a
default uploader whenever site credentials are present (it delivers GutenbergKit's
own uploads and relays every media delete). `MediaUploadResponse` drops to internal
-- it is no longer on any public API. Both demos and all tests move to the new
protocols.

Breaking change: hosts must migrate `mediaUploadDelegate` (WordPress-iOS/Android,
Jetpack). iOS and Android suites green; SwiftLint and Detekt clean.
@jkmassel
jkmassel force-pushed the refactor/media-processor-uploader branch from 9f00177 to 1247d09 Compare September 3, 2026 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Android iOS [Type] Breaking Change For PRs that introduce a change that will break existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants