Skip to content

fix: route the upload server on the path, not the full target - #557

Merged
dcalhoun merged 5 commits into
fix/media-upload-review-followupsfrom
fix/upload-server-query-string-routing
Jul 21, 2026
Merged

fix: route the upload server on the path, not the full target#557
dcalhoun merged 5 commits into
fix/media-upload-review-followupsfrom
fix/upload-server-query-string-routing

Conversation

@dcalhoun

@dcalhoun dcalhoun commented Jul 21, 2026

Copy link
Copy Markdown
Member

What?

Media uploads through the native upload server fail with a 404.

Why?

Uploads that route through the native server never reach WordPress — the editor surfaces an upload failure instead of inserting the image.

@wordpress/media-utils uploads to /wp/v2/media?_embed=wp:featuredmedia, and #546 started forwarding that query on to the native server so it could be relayed to WordPress. The route guard still compared the full request target against "/upload", so the appended query string made the match fail and the server returned 404 before the upload handler ever ran:

POST /upload?_embed=wp:featuredmedia → 404

Both platforms had the bug. Notably, each handler already expected a query — both sliced it off the target to relay upstream — so only the guards were out of step.

How?

Adds path and query accessors to ParsedHTTPRequest (iOS) and HttpRequest (Android), then routes on path and takes the relayed query from query instead of re-deriving it in each handler.

A bare trailing ? carries no parameters, so all three sides normalize it to an empty query and the value can be appended to an upstream URL unconditionally. The middleware that builds the target derived its query by hand and kept the ?, sending POST /upload? — a target the native accessors treat as impossible. It now shares the same rule via requestQuery.

Testing Instructions

  1. Run the demo app against a site, so uploads route through the native upload server.
  2. Insert an Image block and pick a photo from the library.
  3. Confirm the image uploads and renders in the editor.
  4. In the Xcode/Android console, confirm the request is POST /upload?_embed=wp:featuredmedia and returns 201, not 404.

Automated coverage:

make test-swift-library
make test-android
npx vitest run src/utils/api-fetch-upload-middleware.test.js

Both native suites include a regression test that uploads through /upload?_embed=wp:featuredmedia and asserts the query reaches the uploader unchanged. Reverting either route guard to match on target makes them fail with the original 404. Each also pins which upload branch ran, since the mock records the query on both — without that, a regression collapsing routing onto one branch would still pass.

The middleware suite covers the bare-? normalization; restoring the hand-rolled indexOf/slice makes it fail.

Accessibility Testing Instructions

Not applicable — no user interface changes.

dcalhoun and others added 2 commits July 20, 2026 15:05
The request target carries both the path and the query string, so callers
that want to route on the path have to split it themselves. Expose `path`
and `query` on both platforms' request types instead.

A bare trailing "?" yields an empty query on both platforms, so the value
can be appended to an upstream URL unconditionally.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Media uploads fail with a 404. `@wordpress/media-utils` uploads to
`/wp/v2/media?_embed=wp:featuredmedia`, and the middleware now forwards
that query on to the native server so it can be relayed to WordPress.
The route guard still compared the full request target against
"/upload", so a query string made it miss and return 404 before the
upload handler ever ran.

Match on `path` instead, and take the relayed query from `query` rather
than re-deriving it in the handler.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the [Type] Bug An existing feature does not function as intended label Jul 21, 2026
@wpmobilebot

wpmobilebot commented Jul 21, 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/557")

Built from b1a776b

dcalhoun and others added 3 commits July 21, 2026 11:05
The native `query` accessors treat a bare trailing "?" as carrying no
parameters and yield an empty string, so the value can be appended to an
upstream URL unconditionally. The middleware that produces the target
still derived the query with `indexOf`/`slice`, which keeps the "?" —
`/wp/v2/media?` became `POST /upload?`, a target both platforms document
as impossible.

Extract `requestQuery` so the rule is stated once on this side and named
against its native counterparts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both `upload` and `passthroughUpload` record `lastQuery` on the mock, so
asserting the query alone passes whichever branch ran — a regression that
collapsed routing onto one path would still go green.

Assert the passthrough branch explicitly, matching the sibling tests.
Forcing `processFile` to return `.processed` now fails these tests; the
query assertion alone did not.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The explicit `startIndex..<separator` range says the same thing as
`prefix(upTo:)` with more moving parts, and reads further from its
Android counterpart (`substringBefore`).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@dcalhoun
dcalhoun marked this pull request as ready for review July 21, 2026 15:24
@dcalhoun
dcalhoun requested a review from jkmassel July 21, 2026 15:25
@dcalhoun

Copy link
Copy Markdown
Member Author

Requested review for awareness, but also merging now given the target branch is broken, in-progress work currently under review.

@dcalhoun
dcalhoun merged commit 4214615 into fix/media-upload-review-followups Jul 21, 2026
19 checks passed
@dcalhoun
dcalhoun deleted the fix/upload-server-query-string-routing branch July 21, 2026 15:28
dcalhoun added a commit that referenced this pull request Jul 21, 2026
… CORS) (#546)

* fix: fall back to default upload path when native server is unreachable

The native media upload middleware rethrew on any fetch failure, so a local upload server that was never started, restarted on a new port, or torn down turned every upload into a hard failure.

Distinguish a connection-level failure (fall back to next()) from a non-ok response (a real server error that must surface), and re-throw AbortError so an explicit cancellation is not retried.

Amplifier fix for the iOS/Android upload-server lifecycle issues.

* fix(ios): stop upload server on deinit and hold the delegate weakly

viewDidDisappear fires whenever another view controller is pushed or presented over the editor. HTTPServer.stop() cancels the NWListener, which is terminal, so uploads stayed broken after the user returned. Move stop() to deinit.

Hold the media upload delegate weakly in UploadContext (re-read per request): mediaUploadDelegate is declared weak, and the strong capture both defeated that contract and risked a retain cycle that kept the view controller — and the server — alive so deinit never fired.

* fix(android): re-advertise upload server port and token on restart

setGlobalJavaScriptVariables only runs from onPageStarted, so when the media upload delegate setter restarts the server after the page has loaded (e.g. a Compose host constructing a new delegate instance per recomposition), the new port and token were never injected and JS kept fetching a dead port.

Patch the two fields into window.GBKit after each (re)start; the window.GBKit guard makes it a no-op before the page loads, where onPageStarted still handles the initial injection.

* fix(android): use a 60s timeout for media uploads

The upload OkHttpClient used the bare defaults, including a 10s read timeout. WordPress generates image sub-sizes synchronously inside POST /wp/v2/media, so >10s responses are routine on shared hosting — and because the attachment row is created before sub-size generation, a client-side timeout orphans the attachment server-side and duplicates it on retry. Match EditorHTTPClient's 60s policy.

* fix(android): return CORS headers on all upload error responses

processAndRespond only caught MediaUploadException; an IOException from the upload call, JSON parse errors, a throwing delegate, and "no uploader configured" escaped to HttpServer's header-less 500 fallback, so the browser rejected the preflighted cross-origin fetch with an opaque "Failed to fetch" and hid the real error from the editor.

Add a catch-all mapping to the CORS-bearing errorResponse (mirroring iOS), rethrowing coroutine cancellation so it is not swallowed.

* fix(android): start upload server for cookie-auth hosts with an upload delegate

startUploadServer bare-returned when authHeader was empty, but empty authHeader is in-contract for cookie-auth hosts (auth via setCookies), and a delegate implementing uploadFile can own the upload without the default REST uploader. This made an uploadFile-implementing cookie-auth host silently take the unprocessed WebView path, while the same setup worked on iOS.

Build the default uploader only when siteApiRoot and authHeader are present (MediaUploadServer already accepts a null default uploader), and start the server whenever a delegate can handle uploads.

* fix: relay WordPress's raw upload response instead of a synthesized shape

The native upload server parsed WordPress's media response into a 9-field MediaUploadResult (duplicated in Swift, Kotlin, and JS) and the JS middleware re-synthesized an attachment from it. That dropped media_details.sizes — so every native-uploaded image fell back to sizeSlug: full and embedded the full-resolution original — plus the attachment-page link, distinct raw/rendered fields, and _embedded, and flattened WordPress's status to a local 500.

Replace the schema with MediaUploadResponse (status + raw body) and relay WordPress's response verbatim. DefaultMediaUploader returns the raw bytes + status and no longer throws on non-2xx (iOS via a new non-throwing EditorHTTPClientProtocol.performRaw; Android reads the OkHttp response directly). The server relays (status, body) with CORS headers, and errorResponse now emits a {code, message} JSON body so its own errors normalize like a relayed WordPress error.

The JS middleware returns response.json() unchanged on success, and on a non-2xx rejects with the parsed WordPress error body (like @wordpress/api-fetch) so media-utils surfaces WordPress's real message and code, falling back to invalid_json on a non-JSON body.

The uploadFile delegate now returns MediaUploadResponse? (the raw response); nil still selects the default uploader. Forwarding post/additionalData and ?_embed is a follow-up.

* fix: forward media upload fields and query through the native server

The native media upload middleware rebuilt the request body with only the file field and dropped the URL query, so post (the attachment's post association), any additionalData, and ?_embed never reached WordPress — the attachment was created with no post_parent and _embedded was always absent.

Forward the original request body (all fields) and the original query string from JS. On the native side, carry the incoming query onto the WordPress media URL, and — on the re-encode path where processFile changed the file — preserve the non-file parts in the rebuilt multipart body. The passthrough path already forwards them verbatim once the body is relayed unchanged.

Completes the finding-3/4/18 upload-fidelity work; the multipart re-encode dedup is tracked separately in #545.

* fix: clean up upload temp files on failure and sweep crash orphans

When processFile produced a new file but the upload failed, the processed file leaked (the caller only cleaned up the original); partial writes leaked too; and there was no sweep for files orphaned by a crash. Android also staged temp files under java.io.tmpdir instead of the injected cache dir.

Clean up the processed file inside processAndUpload (defer/finally) so the throw paths are covered, and delete the original in the write-failure catch. iOS handleUpload now uses defer + straight-line do/catch instead of the Result/mutable-var dance. Android stages temp files under the injected cacheDir (system-temp fallback). Both platforms sweep upload temp files older than an hour at startup, so crash orphans are reclaimed without disturbing in-flight uploads.

Findings 13 and 21.

* fix: regenerate Package.resolved against the committed manifest

Package.resolved pinned wordpress-rs (branch alpha-20260313), which the root Package.swift never declares — so every swift build/test pruned it and dirtied the tree. Regenerate it from the manifest. The Demo-iOS Xcode project resolves wordpress-rs through its own package reference, so this does not affect the demo.

Finding 16.

* refactor(ios): remove dead MediaUploadError cases and Data.append extension

After the raw-relay refactor, MediaUploadError.uploadFailed/unexpectedResponse are unused (the uploader no longer parses or throws on non-2xx), leaving only streamReadFailed — which UploadError already defines. Fold the two streamReadFailed uses into UploadError and delete MediaUploadError, plus the private Data.append(String) extension the multipart builder no longer uses (the test keeps its own copy).

Finding 22 (partial).

* fix(android): bake EXIF orientation into resized demo images

The demo resize decodes with BitmapFactory (which ignores EXIF orientation) and re-encodes via compress() (which writes no EXIF), so a portrait photo — stored with landscape pixels plus an orientation tag — uploaded rotated. Read the tag and rotate the bitmap before compressing.

Demo-only, but it is the reference processFile implementation hosts copy. Finding 15.

* fix: normalize the media endpoint URL for unslashed root and namespace

The upload endpoint URL was built by raw concatenation, so an unslashed siteApiRoot ("...wp-json") or namespace ("sites/123") produced a malformed URL ("...wp-jsonwp/v2/..." or ".../v2/sites/123media") and a 404. Apply the same normalization RESTAPIRepository uses — trim the root's trailing slash and give the namespace a trailing slash.

Latent (in-repo producers pass slashed values today). Finding 7.

* fix: give processFile an explicit result with corrected metadata

processFile returned a bare URL/File, and the framework inferred "did it change?" by path equality — which conflated "unchanged" with "edited in place" and gave the delegate no way to report a new filename or MIME type. So an in-place EXIF/GPS strip was silently discarded (the original body was forwarded instead), and a format-changing transcode (MOV->MP4) uploaded with the original extension and mime_type.

Replace the bare return with ProcessedProxyFile (.original / .processed(url, mimeType:, filename:)). Passthrough is now explicit, so an in-place edit is uploaded rather than dropped, and the delegate reports the resulting metadata, which is used verbatim. processFile also gains a filename parameter so the delegate has the original name to echo or rewrite — without it that data was simply lost.

Breaking change to the public MediaUploadDelegate; both demos updated. Findings 11 and 12.

* feat: add an opt-in permissive CORS policy to the HTTP server

The HTTP library generated some responses itself — the read timeout (408) and pre-handler errors — without CORS headers, so the browser blocked the WebView from reading them and a timeout surfaced as an opaque "Failed to fetch" (and, with the JS fallback, a silent re-upload). Every handler also had to remember to add CORS to its own responses.

Add an opt-in cors: .permissive policy to HTTPServer (iOS) / HttpServer (Android). When enabled the library answers the OPTIONS preflight itself and stamps permissive CORS headers on every response at the single send choke point — covering handler responses and the library's own. MediaUploadServer opts in and deletes its bespoke corsHeaders / corsPreflightResponse / OPTIONS handling, so CORS lives in one place.

This also de-fangs the finding-14 contract change: the library's error responses now carry CORS regardless of whether a handler inspects serverError. Default policy is .none, so existing consumers are unaffected.

Findings 10 and 14.

* refactor: classify parse-error disposition (fatal vs recoverable) as an enum

The parser decided which parse errors abort the connection vs. are surfaced to the handler with a single implicit condition (!= payloadTooLarge), and the fixture runners accepted an expected error via EITHER channel — so a refactor routing a smuggling-relevant error (e.g. conflicting Content-Length) into the recoverable/handler path would have kept every suite green while letting a malformed request reach the handler before auth.

Make the classification typed data: HTTPRequestParseError gains a Disposition (fatal / recoverable), declared per case so a new error can't compile without one, and the parser throws on disposition == fatal. A lock test pins that only payloadTooLarge is recoverable, and the fixture runners now assert the channel matches the error's disposition (threw => fatal, pendingParseError => recoverable) on both platforms.

Finding 17.

* refactor: extract a shared namespaced REST URL builder

RESTAPIRepository's site-root/namespace normalization was duplicated by the media
uploader — the one endpoint that bypassed the canonical builder, so the two could
drift. Extract it to a shared, tested helper (WordPressRESTURL on iOS,
RestUrlBuilder on Android) and route RESTAPIRepository through it. No behavior
change; pinned by the existing repository tests plus new builder tests.

* fix: harden the native media upload server

- Relay non-file multipart fields (post, additionalData) as raw bytes rather than
  round-tripping through String, which silently mangled any non-UTF-8 value.
- Build the media endpoint through the shared URL builder, and carry the request
  query via percentEncodedQuery on iOS so a non-URL-safe value can't drop it.
- Sweep crash-orphaned temp files off the caller's thread via an injectable scope
  and dispatcher (cancelled on stop; tests inject Unconfined to run it inline).

Tests: iOS non-2xx relay, processed-file cleanup (both platforms), orphan-sweep
age threshold, and the weak-delegate lifetime that keeps deinit teardown working.

* fix(android): gate the upload server on reachability and align its timeouts

- Only start when a REST uploader can be built (drop the unreachable cookie-auth
  branch that 500'd), and clear the advertised port on stop so the JS middleware
  routes to the default path instead of a dead port.
- Skip start when cleartext to localhost is blocked, so a misconfigured host
  degrades gracefully rather than failing every upload.
- Drop the total callTimeout for per-operation inactivity timeouts matching
  URLSession (15s connect, 60s read/write) so a large upload isn't capped.
- Post the JS port/token sync to the WebView's UI thread.

* fix: drop the media-upload fallback and relay errors faithfully

The connection-error fallback re-ran a non-idempotent POST /wp/v2/media, which
could duplicate an attachment, and pointed the wrong way under Lockdown Mode.
Reachability is now gated natively, so remove it. Detect cancellation via
signal.aborted and rethrow signal.reason (catches AbortSignal.timeout, not just
AbortError). Normalize a non-JSON 2xx body to invalid_json like the non-ok path.

* fix(android): report the re-encoded image type in the demo delegate

The resize demo normalizes everything but PNG to JPEG (Bitmap.compress can't
round-trip WebP/HEIC), but returned the original mime/filename — so a WebP upload
was JPEG bytes labelled image/webp, which WordPress rejects. Report the actual
output type and extension.

* fix: route the upload server on the path, not the full target (#557)

* feat(http): add path and query accessors to parsed requests

The request target carries both the path and the query string, so callers
that want to route on the path have to split it themselves. Expose `path`
and `query` on both platforms' request types instead.

A bare trailing "?" yields an empty query on both platforms, so the value
can be appended to an upstream URL unconditionally.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: route the upload server on the path, not the full target

Media uploads fail with a 404. `@wordpress/media-utils` uploads to
`/wp/v2/media?_embed=wp:featuredmedia`, and the middleware now forwards
that query on to the native server so it can be relayed to WordPress.
The route guard still compared the full request target against
"/upload", so a query string made it miss and return 404 before the
upload handler ever ran.

Match on `path` instead, and take the relayed query from `query` rather
than re-deriving it in the handler.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(upload): normalize a bare trailing "?" to an empty query

The native `query` accessors treat a bare trailing "?" as carrying no
parameters and yield an empty string, so the value can be appended to an
upstream URL unconditionally. The middleware that produces the target
still derived the query with `indexOf`/`slice`, which keeps the "?" —
`/wp/v2/media?` became `POST /upload?`, a target both platforms document
as impossible.

Extract `requestQuery` so the rule is stated once on this side and named
against its native counterparts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(upload): pin which upload branch relays the query

Both `upload` and `passthroughUpload` record `lastQuery` on the mock, so
asserting the query alone passes whichever branch ran — a regression that
collapsed routing onto one path would still go green.

Assert the passthrough branch explicitly, matching the sibling tests.
Forcing `processFile` to return `.processed` now fails these tests; the
query assertion alone did not.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(http): express `path` with prefix(upTo:)

The explicit `startIndex..<separator` range says the same thing as
`prefix(upTo:)` with more moving parts, and reads further from its
Android counterpart (`substringBefore`).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: David Calhoun <github@davidcalhoun.me>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jkmassel added a commit that referenced this pull request Aug 12, 2026
… CORS) (#546)

* fix: fall back to default upload path when native server is unreachable

The native media upload middleware rethrew on any fetch failure, so a local upload server that was never started, restarted on a new port, or torn down turned every upload into a hard failure.

Distinguish a connection-level failure (fall back to next()) from a non-ok response (a real server error that must surface), and re-throw AbortError so an explicit cancellation is not retried.

Amplifier fix for the iOS/Android upload-server lifecycle issues.

* fix(ios): stop upload server on deinit and hold the delegate weakly

viewDidDisappear fires whenever another view controller is pushed or presented over the editor. HTTPServer.stop() cancels the NWListener, which is terminal, so uploads stayed broken after the user returned. Move stop() to deinit.

Hold the media upload delegate weakly in UploadContext (re-read per request): mediaUploadDelegate is declared weak, and the strong capture both defeated that contract and risked a retain cycle that kept the view controller — and the server — alive so deinit never fired.

* fix(android): re-advertise upload server port and token on restart

setGlobalJavaScriptVariables only runs from onPageStarted, so when the media upload delegate setter restarts the server after the page has loaded (e.g. a Compose host constructing a new delegate instance per recomposition), the new port and token were never injected and JS kept fetching a dead port.

Patch the two fields into window.GBKit after each (re)start; the window.GBKit guard makes it a no-op before the page loads, where onPageStarted still handles the initial injection.

* fix(android): use a 60s timeout for media uploads

The upload OkHttpClient used the bare defaults, including a 10s read timeout. WordPress generates image sub-sizes synchronously inside POST /wp/v2/media, so >10s responses are routine on shared hosting — and because the attachment row is created before sub-size generation, a client-side timeout orphans the attachment server-side and duplicates it on retry. Match EditorHTTPClient's 60s policy.

* fix(android): return CORS headers on all upload error responses

processAndRespond only caught MediaUploadException; an IOException from the upload call, JSON parse errors, a throwing delegate, and "no uploader configured" escaped to HttpServer's header-less 500 fallback, so the browser rejected the preflighted cross-origin fetch with an opaque "Failed to fetch" and hid the real error from the editor.

Add a catch-all mapping to the CORS-bearing errorResponse (mirroring iOS), rethrowing coroutine cancellation so it is not swallowed.

* fix(android): start upload server for cookie-auth hosts with an upload delegate

startUploadServer bare-returned when authHeader was empty, but empty authHeader is in-contract for cookie-auth hosts (auth via setCookies), and a delegate implementing uploadFile can own the upload without the default REST uploader. This made an uploadFile-implementing cookie-auth host silently take the unprocessed WebView path, while the same setup worked on iOS.

Build the default uploader only when siteApiRoot and authHeader are present (MediaUploadServer already accepts a null default uploader), and start the server whenever a delegate can handle uploads.

* fix: relay WordPress's raw upload response instead of a synthesized shape

The native upload server parsed WordPress's media response into a 9-field MediaUploadResult (duplicated in Swift, Kotlin, and JS) and the JS middleware re-synthesized an attachment from it. That dropped media_details.sizes — so every native-uploaded image fell back to sizeSlug: full and embedded the full-resolution original — plus the attachment-page link, distinct raw/rendered fields, and _embedded, and flattened WordPress's status to a local 500.

Replace the schema with MediaUploadResponse (status + raw body) and relay WordPress's response verbatim. DefaultMediaUploader returns the raw bytes + status and no longer throws on non-2xx (iOS via a new non-throwing EditorHTTPClientProtocol.performRaw; Android reads the OkHttp response directly). The server relays (status, body) with CORS headers, and errorResponse now emits a {code, message} JSON body so its own errors normalize like a relayed WordPress error.

The JS middleware returns response.json() unchanged on success, and on a non-2xx rejects with the parsed WordPress error body (like @wordpress/api-fetch) so media-utils surfaces WordPress's real message and code, falling back to invalid_json on a non-JSON body.

The uploadFile delegate now returns MediaUploadResponse? (the raw response); nil still selects the default uploader. Forwarding post/additionalData and ?_embed is a follow-up.

* fix: forward media upload fields and query through the native server

The native media upload middleware rebuilt the request body with only the file field and dropped the URL query, so post (the attachment's post association), any additionalData, and ?_embed never reached WordPress — the attachment was created with no post_parent and _embedded was always absent.

Forward the original request body (all fields) and the original query string from JS. On the native side, carry the incoming query onto the WordPress media URL, and — on the re-encode path where processFile changed the file — preserve the non-file parts in the rebuilt multipart body. The passthrough path already forwards them verbatim once the body is relayed unchanged.

Completes the finding-3/4/18 upload-fidelity work; the multipart re-encode dedup is tracked separately in #545.

* fix: clean up upload temp files on failure and sweep crash orphans

When processFile produced a new file but the upload failed, the processed file leaked (the caller only cleaned up the original); partial writes leaked too; and there was no sweep for files orphaned by a crash. Android also staged temp files under java.io.tmpdir instead of the injected cache dir.

Clean up the processed file inside processAndUpload (defer/finally) so the throw paths are covered, and delete the original in the write-failure catch. iOS handleUpload now uses defer + straight-line do/catch instead of the Result/mutable-var dance. Android stages temp files under the injected cacheDir (system-temp fallback). Both platforms sweep upload temp files older than an hour at startup, so crash orphans are reclaimed without disturbing in-flight uploads.

Findings 13 and 21.

* fix: regenerate Package.resolved against the committed manifest

Package.resolved pinned wordpress-rs (branch alpha-20260313), which the root Package.swift never declares — so every swift build/test pruned it and dirtied the tree. Regenerate it from the manifest. The Demo-iOS Xcode project resolves wordpress-rs through its own package reference, so this does not affect the demo.

Finding 16.

* refactor(ios): remove dead MediaUploadError cases and Data.append extension

After the raw-relay refactor, MediaUploadError.uploadFailed/unexpectedResponse are unused (the uploader no longer parses or throws on non-2xx), leaving only streamReadFailed — which UploadError already defines. Fold the two streamReadFailed uses into UploadError and delete MediaUploadError, plus the private Data.append(String) extension the multipart builder no longer uses (the test keeps its own copy).

Finding 22 (partial).

* fix(android): bake EXIF orientation into resized demo images

The demo resize decodes with BitmapFactory (which ignores EXIF orientation) and re-encodes via compress() (which writes no EXIF), so a portrait photo — stored with landscape pixels plus an orientation tag — uploaded rotated. Read the tag and rotate the bitmap before compressing.

Demo-only, but it is the reference processFile implementation hosts copy. Finding 15.

* fix: normalize the media endpoint URL for unslashed root and namespace

The upload endpoint URL was built by raw concatenation, so an unslashed siteApiRoot ("...wp-json") or namespace ("sites/123") produced a malformed URL ("...wp-jsonwp/v2/..." or ".../v2/sites/123media") and a 404. Apply the same normalization RESTAPIRepository uses — trim the root's trailing slash and give the namespace a trailing slash.

Latent (in-repo producers pass slashed values today). Finding 7.

* fix: give processFile an explicit result with corrected metadata

processFile returned a bare URL/File, and the framework inferred "did it change?" by path equality — which conflated "unchanged" with "edited in place" and gave the delegate no way to report a new filename or MIME type. So an in-place EXIF/GPS strip was silently discarded (the original body was forwarded instead), and a format-changing transcode (MOV->MP4) uploaded with the original extension and mime_type.

Replace the bare return with ProcessedProxyFile (.original / .processed(url, mimeType:, filename:)). Passthrough is now explicit, so an in-place edit is uploaded rather than dropped, and the delegate reports the resulting metadata, which is used verbatim. processFile also gains a filename parameter so the delegate has the original name to echo or rewrite — without it that data was simply lost.

Breaking change to the public MediaUploadDelegate; both demos updated. Findings 11 and 12.

* feat: add an opt-in permissive CORS policy to the HTTP server

The HTTP library generated some responses itself — the read timeout (408) and pre-handler errors — without CORS headers, so the browser blocked the WebView from reading them and a timeout surfaced as an opaque "Failed to fetch" (and, with the JS fallback, a silent re-upload). Every handler also had to remember to add CORS to its own responses.

Add an opt-in cors: .permissive policy to HTTPServer (iOS) / HttpServer (Android). When enabled the library answers the OPTIONS preflight itself and stamps permissive CORS headers on every response at the single send choke point — covering handler responses and the library's own. MediaUploadServer opts in and deletes its bespoke corsHeaders / corsPreflightResponse / OPTIONS handling, so CORS lives in one place.

This also de-fangs the finding-14 contract change: the library's error responses now carry CORS regardless of whether a handler inspects serverError. Default policy is .none, so existing consumers are unaffected.

Findings 10 and 14.

* refactor: classify parse-error disposition (fatal vs recoverable) as an enum

The parser decided which parse errors abort the connection vs. are surfaced to the handler with a single implicit condition (!= payloadTooLarge), and the fixture runners accepted an expected error via EITHER channel — so a refactor routing a smuggling-relevant error (e.g. conflicting Content-Length) into the recoverable/handler path would have kept every suite green while letting a malformed request reach the handler before auth.

Make the classification typed data: HTTPRequestParseError gains a Disposition (fatal / recoverable), declared per case so a new error can't compile without one, and the parser throws on disposition == fatal. A lock test pins that only payloadTooLarge is recoverable, and the fixture runners now assert the channel matches the error's disposition (threw => fatal, pendingParseError => recoverable) on both platforms.

Finding 17.

* refactor: extract a shared namespaced REST URL builder

RESTAPIRepository's site-root/namespace normalization was duplicated by the media
uploader — the one endpoint that bypassed the canonical builder, so the two could
drift. Extract it to a shared, tested helper (WordPressRESTURL on iOS,
RestUrlBuilder on Android) and route RESTAPIRepository through it. No behavior
change; pinned by the existing repository tests plus new builder tests.

* fix: harden the native media upload server

- Relay non-file multipart fields (post, additionalData) as raw bytes rather than
  round-tripping through String, which silently mangled any non-UTF-8 value.
- Build the media endpoint through the shared URL builder, and carry the request
  query via percentEncodedQuery on iOS so a non-URL-safe value can't drop it.
- Sweep crash-orphaned temp files off the caller's thread via an injectable scope
  and dispatcher (cancelled on stop; tests inject Unconfined to run it inline).

Tests: iOS non-2xx relay, processed-file cleanup (both platforms), orphan-sweep
age threshold, and the weak-delegate lifetime that keeps deinit teardown working.

* fix(android): gate the upload server on reachability and align its timeouts

- Only start when a REST uploader can be built (drop the unreachable cookie-auth
  branch that 500'd), and clear the advertised port on stop so the JS middleware
  routes to the default path instead of a dead port.
- Skip start when cleartext to localhost is blocked, so a misconfigured host
  degrades gracefully rather than failing every upload.
- Drop the total callTimeout for per-operation inactivity timeouts matching
  URLSession (15s connect, 60s read/write) so a large upload isn't capped.
- Post the JS port/token sync to the WebView's UI thread.

* fix: drop the media-upload fallback and relay errors faithfully

The connection-error fallback re-ran a non-idempotent POST /wp/v2/media, which
could duplicate an attachment, and pointed the wrong way under Lockdown Mode.
Reachability is now gated natively, so remove it. Detect cancellation via
signal.aborted and rethrow signal.reason (catches AbortSignal.timeout, not just
AbortError). Normalize a non-JSON 2xx body to invalid_json like the non-ok path.

* fix(android): report the re-encoded image type in the demo delegate

The resize demo normalizes everything but PNG to JPEG (Bitmap.compress can't
round-trip WebP/HEIC), but returned the original mime/filename — so a WebP upload
was JPEG bytes labelled image/webp, which WordPress rejects. Report the actual
output type and extension.

* fix: route the upload server on the path, not the full target (#557)

* feat(http): add path and query accessors to parsed requests

The request target carries both the path and the query string, so callers
that want to route on the path have to split it themselves. Expose `path`
and `query` on both platforms' request types instead.

A bare trailing "?" yields an empty query on both platforms, so the value
can be appended to an upstream URL unconditionally.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: route the upload server on the path, not the full target

Media uploads fail with a 404. `@wordpress/media-utils` uploads to
`/wp/v2/media?_embed=wp:featuredmedia`, and the middleware now forwards
that query on to the native server so it can be relayed to WordPress.
The route guard still compared the full request target against
"/upload", so a query string made it miss and return 404 before the
upload handler ever ran.

Match on `path` instead, and take the relayed query from `query` rather
than re-deriving it in the handler.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(upload): normalize a bare trailing "?" to an empty query

The native `query` accessors treat a bare trailing "?" as carrying no
parameters and yield an empty string, so the value can be appended to an
upstream URL unconditionally. The middleware that produces the target
still derived the query with `indexOf`/`slice`, which keeps the "?" —
`/wp/v2/media?` became `POST /upload?`, a target both platforms document
as impossible.

Extract `requestQuery` so the rule is stated once on this side and named
against its native counterparts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(upload): pin which upload branch relays the query

Both `upload` and `passthroughUpload` record `lastQuery` on the mock, so
asserting the query alone passes whichever branch ran — a regression that
collapsed routing onto one path would still go green.

Assert the passthrough branch explicitly, matching the sibling tests.
Forcing `processFile` to return `.processed` now fails these tests; the
query assertion alone did not.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(http): express `path` with prefix(upTo:)

The explicit `startIndex..<separator` range says the same thing as
`prefix(upTo:)` with more moving parts, and reads further from its
Android counterpart (`substringBefore`).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: David Calhoun <github@davidcalhoun.me>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jkmassel added a commit that referenced this pull request Aug 14, 2026
* feat(js): route media uploads through native server via api-fetch middleware

Add nativeMediaUploadMiddleware that intercepts POST /wp/v2/media
requests when a native upload port is configured in GBKit. The
middleware forwards uploads to the local HTTP server with
Relay-Authorization bearer token auth, then transforms the native
response into the WordPress REST API attachment shape.

Includes user-friendly error handling for 413 (file too large) and
generic upload failures.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(ios): add media upload server and delegate

Add MediaUploadServer backed by the GutenbergKitHTTP library, which
handles TCP binding, HTTP parsing, bearer token auth, and multipart
parsing. The upload server provides a thin handler that routes file
uploads through a native delegate pipeline for processing (e.g. image
resize, video transcode) before uploading to WordPress.

- MediaUploadDelegate protocol with processFile and uploadFile hooks
- DefaultMediaUploader for WordPress REST API uploads with namespace support
- EditorViewController integration with async server lifecycle
- GBKitGlobal nativeUploadPort/nativeUploadToken injection
- GutenbergKitHTTP added as dependency of GutenbergKit target

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(android): add media upload server and delegate

Add MediaUploadServer backed by the HttpServer library, which handles
TCP binding, HTTP parsing, bearer token auth, and connection management.
The upload server provides a thin handler that routes file uploads
through a native delegate pipeline for processing (e.g. image resize,
video transcode) before uploading to WordPress.

- MediaUploadDelegate interface with processFile and uploadFile hooks
- DefaultMediaUploader for WordPress REST API uploads with namespace support
- GutenbergView integration with synchronous server lifecycle
- GBKitGlobal nativeUploadPort/nativeUploadToken injection
- org.json test dependency added

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat: add image resize delegate to iOS and Android demo apps

Add DemoMediaUploadDelegate implementations that resize images to a
maximum dimension of 2000px before upload. Includes a toggle in the
site preparation screen to enable/disable native media upload
processing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test(ios): add MediaUploadServer tests

Integration tests covering server lifecycle, bearer token auth (407 on
missing/wrong token), CORS preflight, routing (404 for unknown paths),
delegate processing pipeline, and fallback to default uploader.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test(android): add MediaUploadServer tests

Integration tests covering server lifecycle, bearer token auth (407 on
missing/wrong token), CORS preflight, routing (404 for unknown paths),
delegate processing pipeline, fallback to default uploader,
DefaultMediaUploader request format, and error handling for bad
requests and non-multipart content types.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test(js): add nativeMediaUploadMiddleware tests

Tests covering passthrough behavior (missing port, non-POST, non-media
paths, sub-paths, non-FormData), upload interception with
Relay-Authorization auth, response transformation to WordPress REST API
shape, error handling (413 file too large, generic failures), and abort
signal forwarding.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: surface server error messages in upload failure snackbar

Add LocalizedError conformance to EditorHTTPClient.ClientError so that
WordPress error messages (e.g. "This file is too large. The maximum
upload size is 10 KB.") are surfaced to the user instead of a cryptic
Swift type description.

Remove the dead 413-specific handling from the JS middleware — the HTTP
library rejects oversized uploads at the connection level (never
producing an HTTP response the browser can read), so the 413 branch
was unreachable. All upload errors now go through the generic path
which includes the server's error message.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(android): extract human-readable message from WordPress error responses

Parse the WordPress JSON error body to extract the message field (e.g.
"This file is too large. The maximum upload size is 10 KB.") instead
of showing the raw JSON in the upload failure snackbar. Falls back to
the raw body for non-JSON error responses.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: deduplicate CORS headers in upload server responses

Compose corsPreflightResponse() from the shared corsHeaders constant
instead of re-declaring origin and allowed-headers values. Also removes
a redundant "what" comment on the multipart parsing call.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(android): skip upload server restart on redundant delegate assignment

Guard the mediaUploadDelegate setter with an identity check so that
assigning the same delegate instance does not needlessly stop and
restart the upload server.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: improve upload server error handling and memory efficiency (#419)

* fix: Avoid "native" term in user-facing errors messages

The term "native" is likely unfamiliar to users, provides no tangible
value, and may cause confusion.

* fix: drain oversized request body before sending 413 response

When Content-Length exceeds maxBodySize, the server now reads and
discards the full request body before responding with 413. This ensures
the client (WebView fetch) receives the error response cleanly instead
of a connection reset (RFC 9110 §15.5.14).

Adds a `.draining` parser state that tracks consumed bytes without
buffering them, keeping memory and disk usage at zero for rejected
uploads.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: include CORS headers on server-generated error responses

Add an errorResponseHeaders parameter to HTTPServer (iOS) and
HttpServer (Android) so that callers can specify headers to include on
all server-generated error responses (413, 407, 408, etc.).
MediaUploadServer passes its CORS headers through this parameter so
the browser does not block error responses due to missing
Access-Control-Allow-Origin.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Revert "fix: include CORS headers on server-generated error responses"

This reverts commit 0b6c67b.

* fix: route 413 response through handler for CORS headers

Instead of the HTTP server library building 413 responses directly
(which lacked CORS headers), payloadTooLarge is now treated as a
non-fatal parse error. parseRequest() returns the parsed headers as a
partial request and exposes the error via a new parseError property.
The server passes it to the handler via a serverError field on the
request, letting MediaUploadServer build the response with CORS
headers — consistent with how OPTIONS preflight is handled.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* perf(ios): stream multipart upload body from disk instead of memory

Replace Data(contentsOf:) + httpBody with a streaming InputStream via
httpBodyStream in DefaultMediaUploader. The multipart body (preamble,
file content, epilogue) is written through a bound stream pair on a
background thread, keeping peak memory at ~65 KB regardless of file
size — down from ~2x file size previously.

Android already streams via OkHttp's file.asRequestBody() and needs
no changes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* perf: passthrough upload when delegate does not modify the file

When the delegate's processFile returns the original file unchanged
(e.g., GIFs, non-images, files already within size limits), the
original request body is forwarded directly to WordPress — skipping
multipart re-encoding and the extra file read.

Detection: after processFile, compare the returned URL/File to the
input. If unchanged and uploadFile returns nil, signal passthrough
back to handleUpload which streams the original body via
passthroughUpload().

Also extracts shared response parsing into performUpload() on both
platforms to avoid duplication between upload() and
passthroughUpload().

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor(android): fix Detekt lint violations

Extract readUntil() helper from HttpServer.handleRequest() to reduce
nesting depth and throw count. Extract performPassthroughUpload() from
MediaUploadServer.processAndRespond() to consolidate throw statements.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(ios): prevent integer overflow in drain mode byte tracking

Cast `bytesWritten` and `offset` to Int64 individually before
subtracting, avoiding a potential Int overflow when the difference
is computed before the widening cast.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor(ios): replace deprecated URL.path with path(percentEncoded:)

URL.path is deprecated on iOS 16+. Use path(percentEncoded: false)
to get the file system path without percent-encoding.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor(ios): localize "file too large" error via EditorLocalization

Add a `fileTooLarge` case to `EditorLocalizableString` so host apps
can provide translations for the 413 error message. The hardcoded
string in MediaUploadServer now reads from the localization system.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Revert "refactor(ios): localize "file too large" error via EditorLocalization"

This reverts commit 71440d9.

* fix(ios): review adjustments for #419 (#441)

* fix(ios): use Int64 for HTTPRequestParser.bytesWritten

Change `bytesWritten` from `Int` to `Int64` for consistency with
`expectedContentLength` and `maxBodySize`, which are already `Int64`.

* test(ios): add end-to-end test for 413 response with CORS headers

Expose maxRequestBodySize on MediaUploadServer.start() and add an
integration test that sends an oversized request and verifies the
response includes both a 413 status and CORS headers.

* fix(test): increase maxRequestBodySize for 413 test

The multipart overhead (~191 bytes) plus auth headers meant the
previous limit of 100 bytes caused the connection to reset before
the drain could complete. Use 1024 bytes with a 2048-byte payload
so the parser can drain the body and deliver the 413 response.

* fix(test): use raw TCP for 413 test to avoid URLSession connection reset

URLSession treats a server response during upload as a connection
error (NSURLErrorNetworkConnectionLost). Use a raw NWConnection
to send the request and read the response directly, which correctly
receives the 413 with CORS headers.

* fix: complete drain immediately when body arrives with headers

When the entire HTTP request (headers + body) arrives in a single
read, the parser enters DRAINING but never completes because the
body bytes were already counted in bytesWritten. Subsequent reads
find no more data, causing a timeout.

Check the drain condition immediately when entering the draining
state, transitioning to complete if all body bytes have already
been received. Fixes both iOS and Android parsers.

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Jeremy Massel <1123407+jkmassel@users.noreply.github.com>

* fix: native media upload review follow-ups (relay, delegate contract, CORS) (#546)

* fix: fall back to default upload path when native server is unreachable

The native media upload middleware rethrew on any fetch failure, so a local upload server that was never started, restarted on a new port, or torn down turned every upload into a hard failure.

Distinguish a connection-level failure (fall back to next()) from a non-ok response (a real server error that must surface), and re-throw AbortError so an explicit cancellation is not retried.

Amplifier fix for the iOS/Android upload-server lifecycle issues.

* fix(ios): stop upload server on deinit and hold the delegate weakly

viewDidDisappear fires whenever another view controller is pushed or presented over the editor. HTTPServer.stop() cancels the NWListener, which is terminal, so uploads stayed broken after the user returned. Move stop() to deinit.

Hold the media upload delegate weakly in UploadContext (re-read per request): mediaUploadDelegate is declared weak, and the strong capture both defeated that contract and risked a retain cycle that kept the view controller — and the server — alive so deinit never fired.

* fix(android): re-advertise upload server port and token on restart

setGlobalJavaScriptVariables only runs from onPageStarted, so when the media upload delegate setter restarts the server after the page has loaded (e.g. a Compose host constructing a new delegate instance per recomposition), the new port and token were never injected and JS kept fetching a dead port.

Patch the two fields into window.GBKit after each (re)start; the window.GBKit guard makes it a no-op before the page loads, where onPageStarted still handles the initial injection.

* fix(android): use a 60s timeout for media uploads

The upload OkHttpClient used the bare defaults, including a 10s read timeout. WordPress generates image sub-sizes synchronously inside POST /wp/v2/media, so >10s responses are routine on shared hosting — and because the attachment row is created before sub-size generation, a client-side timeout orphans the attachment server-side and duplicates it on retry. Match EditorHTTPClient's 60s policy.

* fix(android): return CORS headers on all upload error responses

processAndRespond only caught MediaUploadException; an IOException from the upload call, JSON parse errors, a throwing delegate, and "no uploader configured" escaped to HttpServer's header-less 500 fallback, so the browser rejected the preflighted cross-origin fetch with an opaque "Failed to fetch" and hid the real error from the editor.

Add a catch-all mapping to the CORS-bearing errorResponse (mirroring iOS), rethrowing coroutine cancellation so it is not swallowed.

* fix(android): start upload server for cookie-auth hosts with an upload delegate

startUploadServer bare-returned when authHeader was empty, but empty authHeader is in-contract for cookie-auth hosts (auth via setCookies), and a delegate implementing uploadFile can own the upload without the default REST uploader. This made an uploadFile-implementing cookie-auth host silently take the unprocessed WebView path, while the same setup worked on iOS.

Build the default uploader only when siteApiRoot and authHeader are present (MediaUploadServer already accepts a null default uploader), and start the server whenever a delegate can handle uploads.

* fix: relay WordPress's raw upload response instead of a synthesized shape

The native upload server parsed WordPress's media response into a 9-field MediaUploadResult (duplicated in Swift, Kotlin, and JS) and the JS middleware re-synthesized an attachment from it. That dropped media_details.sizes — so every native-uploaded image fell back to sizeSlug: full and embedded the full-resolution original — plus the attachment-page link, distinct raw/rendered fields, and _embedded, and flattened WordPress's status to a local 500.

Replace the schema with MediaUploadResponse (status + raw body) and relay WordPress's response verbatim. DefaultMediaUploader returns the raw bytes + status and no longer throws on non-2xx (iOS via a new non-throwing EditorHTTPClientProtocol.performRaw; Android reads the OkHttp response directly). The server relays (status, body) with CORS headers, and errorResponse now emits a {code, message} JSON body so its own errors normalize like a relayed WordPress error.

The JS middleware returns response.json() unchanged on success, and on a non-2xx rejects with the parsed WordPress error body (like @wordpress/api-fetch) so media-utils surfaces WordPress's real message and code, falling back to invalid_json on a non-JSON body.

The uploadFile delegate now returns MediaUploadResponse? (the raw response); nil still selects the default uploader. Forwarding post/additionalData and ?_embed is a follow-up.

* fix: forward media upload fields and query through the native server

The native media upload middleware rebuilt the request body with only the file field and dropped the URL query, so post (the attachment's post association), any additionalData, and ?_embed never reached WordPress — the attachment was created with no post_parent and _embedded was always absent.

Forward the original request body (all fields) and the original query string from JS. On the native side, carry the incoming query onto the WordPress media URL, and — on the re-encode path where processFile changed the file — preserve the non-file parts in the rebuilt multipart body. The passthrough path already forwards them verbatim once the body is relayed unchanged.

Completes the finding-3/4/18 upload-fidelity work; the multipart re-encode dedup is tracked separately in #545.

* fix: clean up upload temp files on failure and sweep crash orphans

When processFile produced a new file but the upload failed, the processed file leaked (the caller only cleaned up the original); partial writes leaked too; and there was no sweep for files orphaned by a crash. Android also staged temp files under java.io.tmpdir instead of the injected cache dir.

Clean up the processed file inside processAndUpload (defer/finally) so the throw paths are covered, and delete the original in the write-failure catch. iOS handleUpload now uses defer + straight-line do/catch instead of the Result/mutable-var dance. Android stages temp files under the injected cacheDir (system-temp fallback). Both platforms sweep upload temp files older than an hour at startup, so crash orphans are reclaimed without disturbing in-flight uploads.

Findings 13 and 21.

* fix: regenerate Package.resolved against the committed manifest

Package.resolved pinned wordpress-rs (branch alpha-20260313), which the root Package.swift never declares — so every swift build/test pruned it and dirtied the tree. Regenerate it from the manifest. The Demo-iOS Xcode project resolves wordpress-rs through its own package reference, so this does not affect the demo.

Finding 16.

* refactor(ios): remove dead MediaUploadError cases and Data.append extension

After the raw-relay refactor, MediaUploadError.uploadFailed/unexpectedResponse are unused (the uploader no longer parses or throws on non-2xx), leaving only streamReadFailed — which UploadError already defines. Fold the two streamReadFailed uses into UploadError and delete MediaUploadError, plus the private Data.append(String) extension the multipart builder no longer uses (the test keeps its own copy).

Finding 22 (partial).

* fix(android): bake EXIF orientation into resized demo images

The demo resize decodes with BitmapFactory (which ignores EXIF orientation) and re-encodes via compress() (which writes no EXIF), so a portrait photo — stored with landscape pixels plus an orientation tag — uploaded rotated. Read the tag and rotate the bitmap before compressing.

Demo-only, but it is the reference processFile implementation hosts copy. Finding 15.

* fix: normalize the media endpoint URL for unslashed root and namespace

The upload endpoint URL was built by raw concatenation, so an unslashed siteApiRoot ("...wp-json") or namespace ("sites/123") produced a malformed URL ("...wp-jsonwp/v2/..." or ".../v2/sites/123media") and a 404. Apply the same normalization RESTAPIRepository uses — trim the root's trailing slash and give the namespace a trailing slash.

Latent (in-repo producers pass slashed values today). Finding 7.

* fix: give processFile an explicit result with corrected metadata

processFile returned a bare URL/File, and the framework inferred "did it change?" by path equality — which conflated "unchanged" with "edited in place" and gave the delegate no way to report a new filename or MIME type. So an in-place EXIF/GPS strip was silently discarded (the original body was forwarded instead), and a format-changing transcode (MOV->MP4) uploaded with the original extension and mime_type.

Replace the bare return with ProcessedProxyFile (.original / .processed(url, mimeType:, filename:)). Passthrough is now explicit, so an in-place edit is uploaded rather than dropped, and the delegate reports the resulting metadata, which is used verbatim. processFile also gains a filename parameter so the delegate has the original name to echo or rewrite — without it that data was simply lost.

Breaking change to the public MediaUploadDelegate; both demos updated. Findings 11 and 12.

* feat: add an opt-in permissive CORS policy to the HTTP server

The HTTP library generated some responses itself — the read timeout (408) and pre-handler errors — without CORS headers, so the browser blocked the WebView from reading them and a timeout surfaced as an opaque "Failed to fetch" (and, with the JS fallback, a silent re-upload). Every handler also had to remember to add CORS to its own responses.

Add an opt-in cors: .permissive policy to HTTPServer (iOS) / HttpServer (Android). When enabled the library answers the OPTIONS preflight itself and stamps permissive CORS headers on every response at the single send choke point — covering handler responses and the library's own. MediaUploadServer opts in and deletes its bespoke corsHeaders / corsPreflightResponse / OPTIONS handling, so CORS lives in one place.

This also de-fangs the finding-14 contract change: the library's error responses now carry CORS regardless of whether a handler inspects serverError. Default policy is .none, so existing consumers are unaffected.

Findings 10 and 14.

* refactor: classify parse-error disposition (fatal vs recoverable) as an enum

The parser decided which parse errors abort the connection vs. are surfaced to the handler with a single implicit condition (!= payloadTooLarge), and the fixture runners accepted an expected error via EITHER channel — so a refactor routing a smuggling-relevant error (e.g. conflicting Content-Length) into the recoverable/handler path would have kept every suite green while letting a malformed request reach the handler before auth.

Make the classification typed data: HTTPRequestParseError gains a Disposition (fatal / recoverable), declared per case so a new error can't compile without one, and the parser throws on disposition == fatal. A lock test pins that only payloadTooLarge is recoverable, and the fixture runners now assert the channel matches the error's disposition (threw => fatal, pendingParseError => recoverable) on both platforms.

Finding 17.

* refactor: extract a shared namespaced REST URL builder

RESTAPIRepository's site-root/namespace normalization was duplicated by the media
uploader — the one endpoint that bypassed the canonical builder, so the two could
drift. Extract it to a shared, tested helper (WordPressRESTURL on iOS,
RestUrlBuilder on Android) and route RESTAPIRepository through it. No behavior
change; pinned by the existing repository tests plus new builder tests.

* fix: harden the native media upload server

- Relay non-file multipart fields (post, additionalData) as raw bytes rather than
  round-tripping through String, which silently mangled any non-UTF-8 value.
- Build the media endpoint through the shared URL builder, and carry the request
  query via percentEncodedQuery on iOS so a non-URL-safe value can't drop it.
- Sweep crash-orphaned temp files off the caller's thread via an injectable scope
  and dispatcher (cancelled on stop; tests inject Unconfined to run it inline).

Tests: iOS non-2xx relay, processed-file cleanup (both platforms), orphan-sweep
age threshold, and the weak-delegate lifetime that keeps deinit teardown working.

* fix(android): gate the upload server on reachability and align its timeouts

- Only start when a REST uploader can be built (drop the unreachable cookie-auth
  branch that 500'd), and clear the advertised port on stop so the JS middleware
  routes to the default path instead of a dead port.
- Skip start when cleartext to localhost is blocked, so a misconfigured host
  degrades gracefully rather than failing every upload.
- Drop the total callTimeout for per-operation inactivity timeouts matching
  URLSession (15s connect, 60s read/write) so a large upload isn't capped.
- Post the JS port/token sync to the WebView's UI thread.

* fix: drop the media-upload fallback and relay errors faithfully

The connection-error fallback re-ran a non-idempotent POST /wp/v2/media, which
could duplicate an attachment, and pointed the wrong way under Lockdown Mode.
Reachability is now gated natively, so remove it. Detect cancellation via
signal.aborted and rethrow signal.reason (catches AbortSignal.timeout, not just
AbortError). Normalize a non-JSON 2xx body to invalid_json like the non-ok path.

* fix(android): report the re-encoded image type in the demo delegate

The resize demo normalizes everything but PNG to JPEG (Bitmap.compress can't
round-trip WebP/HEIC), but returned the original mime/filename — so a WebP upload
was JPEG bytes labelled image/webp, which WordPress rejects. Report the actual
output type and extension.

* fix: route the upload server on the path, not the full target (#557)

* feat(http): add path and query accessors to parsed requests

The request target carries both the path and the query string, so callers
that want to route on the path have to split it themselves. Expose `path`
and `query` on both platforms' request types instead.

A bare trailing "?" yields an empty query on both platforms, so the value
can be appended to an upstream URL unconditionally.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: route the upload server on the path, not the full target

Media uploads fail with a 404. `@wordpress/media-utils` uploads to
`/wp/v2/media?_embed=wp:featuredmedia`, and the middleware now forwards
that query on to the native server so it can be relayed to WordPress.
The route guard still compared the full request target against
"/upload", so a query string made it miss and return 404 before the
upload handler ever ran.

Match on `path` instead, and take the relayed query from `query` rather
than re-deriving it in the handler.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(upload): normalize a bare trailing "?" to an empty query

The native `query` accessors treat a bare trailing "?" as carrying no
parameters and yield an empty string, so the value can be appended to an
upstream URL unconditionally. The middleware that produces the target
still derived the query with `indexOf`/`slice`, which keeps the "?" —
`/wp/v2/media?` became `POST /upload?`, a target both platforms document
as impossible.

Extract `requestQuery` so the rule is stated once on this side and named
against its native counterparts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(upload): pin which upload branch relays the query

Both `upload` and `passthroughUpload` record `lastQuery` on the mock, so
asserting the query alone passes whichever branch ran — a regression that
collapsed routing onto one path would still go green.

Assert the passthrough branch explicitly, matching the sibling tests.
Forcing `processFile` to return `.processed` now fails these tests; the
query assertion alone did not.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(http): express `path` with prefix(upTo:)

The explicit `startIndex..<separator` range says the same thing as
`prefix(upTo:)` with more moving parts, and reads further from its
Android counterpart (`substringBefore`).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: David Calhoun <github@davidcalhoun.me>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: harden native upload middleware guard and post sentinel

Two fixes to `nativeMediaUploadMiddleware`, both aligning it with the
existing `mediaUploadMiddleware` it short-circuits past.

Strip the `post` field when it is the auto-draft sentinel (`-1`).
WordPress rejects attaching media to post `-1`, and because this
middleware returns the relay fetch without calling `next()`,
`mediaUploadMiddleware` (which normally strips the sentinel) never runs
— so the value reached WordPress verbatim and broke uploads on the
common new-post flow.

Require `nativeUploadToken` in the activation guard. The server always
requires authentication, so a port-without-token state would send
`Bearer undefined` and 407 instead of falling through to the default
upload path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(ios): don't start upload server without an auth header

`startUploadServer` built `DefaultMediaUploader` whenever a media upload
delegate was set, ignoring whether an auth header was present. It uploads
to `/wp/v2/media` with the host's `Authorization` header (the WebView has
none of its own), so with an empty header every relayed upload would 401
instead of falling back to the default WebView path.

Guard on a non-empty auth header before starting the server, matching the
Android guard in `GutenbergView.startUploadServer`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(ios): throw instead of trapping when the upload temp file can't open

`writeStream` force-unwrapped `OutputStream(url:append:)`, which returns
nil if the file can't be opened for writing (uploads directory removed
after creation, or a permissions/sandbox failure). That trapped and
crashed the process instead of returning the clean 500 the caller already
builds. Throw `streamWriteFailed` so it flows into the existing
do/catch — temp file removed, 500 relayed to the editor.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: remove redundant post sentinel strip from native upload middleware

Partially reverts 93aa2d1. api-fetch runs middleware in reverse
registration order, so `mediaUploadMiddleware` (registered later) runs
first and has already stripped the auto-draft `post: -1` sentinel by the
time `nativeMediaUploadMiddleware` runs — the strip was dead code based
on an inverted ordering assumption. The `nativeUploadToken` guard from
that commit remains.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: check upload server auth before draining oversized bodies

The oversized-payload path (drain plus serverError handler dispatch) ran
before the bearer-token check, so unauthenticated clients could make the
server read an arbitrarily large declared body and reach the handler.

Reorder the connection flow on both platforms so auth is validated on
headers alone first: unauthenticated oversized requests are rejected with
an immediate 407, while authenticated ones keep the full drain and
CORS-stamped 413 (RFC 9110 §15.5.14).

Add regression tests pinning the ordering — an unauthenticated oversized
request must return 407, not the handler's 413.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(ios): bound the HTTP server's listener readiness wait

HTTPServer.start awaited NWListener readiness with no bound and treated
.waiting as "keep waiting" — a listener stuck in that state (which emits
no further updates) suspended the caller forever. Since the editor awaits
the upload server start before loading the WebView, a stuck listener
blocked editor startup indefinitely behind the activity spinner.

Race the readiness wait against a startTimeout (default 5s; loopback
binds normally complete in milliseconds) and throw failedToStart on
expiry, cancelling the half-started listener. Callers already degrade
gracefully on failedToStart, falling back to the default upload path.

The wait is extracted into an internal firstTerminalState helper because
a stuck listener cannot be reproduced deterministically with a real
NWListener — tests feed it a hand-built state stream instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(ios): don't cancel the fast-path editor load on view disappearance

The fast path (dependencies provided at init) became an async Task when
loadEditor started awaiting the upload server, and it was tracked in
dependencyTaskHandle — which viewDidDisappear cancels. A transient
disappearance (e.g. a full-screen modal presented right after the editor)
cancelled the load mid startUploadServer(), surfacing as failedToStart
and silently disabling native uploads for the session with no retry.

Run the fast path untracked: the disappear-cancel exists to abort the
async dependency *fetch*, while the fast path is cheap local work that
must run to completion — restoring the guarantee it had when it was
synchronous. The weak self capture still makes it a no-op after teardown.

Leaving the task untracked is safe because the preceding commit bounds
the upload server's listener readiness wait (startTimeout) — the only
await here that could hang — so the task always terminates. Reverting
that commit alone would reintroduce an unbounded, now-uncancellable
suspension on this path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* perf(ios): move orphan temp file sweeps off the editor-startup path

Both startup sweeps — cleanOrphanedUploads in MediaUploadServer.start and
cleanOrphanedTempFiles in HTTPServer.start — ran synchronous directory
scans on the editor-load critical path. Detach them to a utility-priority
task instead, mirroring Android's cleanupJob; each server exposes the
task so tests can await completion.

Detaching cleanOrphanedTempFiles required giving it the same one-hour
age threshold the upload sweep already uses: it previously deleted every
file in the server's temp subdirectory, which a detached sweep could
race against a live instance's in-flight temp buffers. The threshold
also removes the documented hazard of same-name instances wiping each
other's files.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* style: satisfy pinned SwiftLint --strict after trunk rebase

Rebasing onto trunk pulled in the pinned SwiftLint CI step (#575), which
runs `--strict` and promotes these previously-latent warnings to errors:

- `MediaUploadServer.swift`: extra trailing newline and a blank line
  before a closing brace (auto-corrected).
- `EditorView.swift` (demo): a blank line before a closing brace
  (auto-corrected).
- `HTTPServer.swift`: `redundant_nil_coalescing` on `group.next() ?? nil`.
  This is a false positive on a double optional — `TaskGroup.next()`
  returns `State??` and `?? nil` flattens it to `State?`. Rewrite as
  `.flatMap { $0 }` to keep the type while dropping the flagged operator.

No behavior change. Verified `make lint-swift` is clean and the package
compiles for the iOS Simulator SDK.

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Jeremy Massel <1123407+jkmassel@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants