fix: improve upload server error handling and memory efficiency by dcalhoun · Pull Request #419 · wordpress-mobile/GutenbergKit

dcalhoun · 2026-04-02T21:38:18Z

Note

These are improvements to #357 and the PR targets merging into that in-progress branch.

What?

Improves the upload server's handling of oversized requests and reduces memory pressure during file uploads.

Why?

Addresses PR #357 review feedback from @jkmassel:

413 responses were lost: When Content-Length exceeded the server's limit, the connection was closed immediately — the WebView's fetch() never received the 413 response.
413 responses lacked CORS headers: Even after fixing delivery, the browser blocked the 413 because the server built the response directly, bypassing the handler (which adds CORS headers).
iOS uploads loaded entire files into memory: DefaultMediaUploader used Data(contentsOf:) + httpBody, causing peak memory of ~2x file size.
Unnecessary multipart re-encoding: When the delegate didn't modify the file, the pipeline still extracted it to a temp file and re-encoded it into a new multipart body for WordPress.

How?

Drain oversized request bodies (RFC 9110 §15.5.14)

Adds a .draining parser state that reads and discards body bytes without buffering them. The server continues reading until Content-Length is satisfied, then sends a clean 413 response. Both iOS and Android parsers.

Route 413 through the handler for CORS headers

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

Stream multipart upload body from disk (iOS)

Replaces Data(contentsOf:) + httpBody with a streaming InputStream via httpBodyStream. 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. Android already streams via OkHttp's file.asRequestBody().

Passthrough upload when delegate doesn't modify the file

When 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 via passthroughUpload() — skipping multipart re-encoding and the extra file read. Both iOS and Android.

Testing Instructions

Open the iOS and Android demo apps with the Vite dev server running
Insert an Image block and select a file larger than the server's max body size
Verify the upload error snackbar shows a human-readable message (not a CORS error or connection reset)
Upload a normal-sized image and verify it succeeds as before (processed path)
Upload a GIF and verify it succeeds (passthrough path — check debug logs for "Passthrough: forwarding original request body to WordPress")

Screenshots or screencast

N/A — behavioral/performance changes, no UI modifications.

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

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>

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>

This reverts commit 0b6c67b.

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>

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>

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>

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>

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>

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>

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>

…lization" This reverts commit 71440d9.

dcalhoun · 2026-04-03T14:32:10Z

+        // Server-detected error (e.g., payload too large) — build the
+        // error response here so it includes CORS headers.
+        request.serverError?.let { error ->
+            val message = when (error) {
+                HTTPRequestParseError.PAYLOAD_TOO_LARGE -> "The file is too large to upload in the editor."
+                else -> error.errorId
+            }
+            return errorResponse(error.httpStatus, message)
+        }


The approach of passing server errors to the handler was taken rather than the adding a new errorResponseHeaders configuration (added in 0b6c67b, reverted in 0563a94). The configuration approach leaks host/caller concerns into the library. A similar consideration occurred when implementing authentication bypass for preflight OPTIONS requests.

Ultimately, the 413 responses need to include the correct CORS headers, otherwise the error code and message do not arrive to the client. It instead receives a CORS error.

@jkmassel what do you think of this approach?

dcalhoun · 2026-04-03T14:40:57Z

+        let (bodyStream, contentLength) = try Self.multipartBodyStream(
+            fileURL: fileURL, boundary: boundary, filename: filename, mimeType: mimeType
+        )


@jkmassel does this align with your vision for handling modified files?

dcalhoun · 2026-04-03T14:41:28Z

 			if ( ! response.ok ) {
 				return response.text().then( ( body ) => {
 					const error = new Error(
-						`Native upload failed (${ response.status }): ${


Exposing the term "native" to users feels inappropriate. It's an implementation detail.

jkmassel

I have some minor suggestions in #419. Let's land that, then this, then I think #357 will be ready

dcalhoun · 2026-04-09T18:13:23Z

I have some minor suggestions in #419. Let's land that, then this, then I think #357 will be ready

Sounds good. I believe you mean #441. That is next on my list to act on today. I'll work to move all three forward.

* 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.

dcalhoun and others added 6 commits April 2, 2026 14:36

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

e9193d6

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

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

0563a94

This reverts commit 0b6c67b.

github-actions bot added the [Type] Bug An existing feature does not function as intended label Apr 2, 2026

dcalhoun and others added 6 commits April 3, 2026 11:17

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

f3a08d1

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>

Revert "refactor(ios): localize "file too large" error via EditorLoca…

8015719

…lization" This reverts commit 71440d9.

dcalhoun commented Apr 3, 2026

View reviewed changes

dcalhoun marked this pull request as ready for review April 3, 2026 15:41

dcalhoun requested a review from jkmassel April 3, 2026 15:41

This was referenced Apr 8, 2026

iOS: OutputStream.write return value 0 treated as fatal in bound stream writers #437

Open

iOS: bound stream writer threads silently swallow errors #438

Open

fix(ios): review adjustments for #419 #440

Closed

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

Merged

jkmassel approved these changes Apr 9, 2026

View reviewed changes

dcalhoun merged commit 60234e0 into feat/leverage-host-media-processing Apr 9, 2026
1 of 5 checks passed

dcalhoun deleted the feat/improve-media-uploads-and-errors branch April 9, 2026 19:17

dcalhoun mentioned this pull request Apr 9, 2026

feat: proxy media uploads through native delegate for processing #357

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

fix: improve upload server error handling and memory efficiency#419
dcalhoun merged 13 commits intofeat/leverage-host-media-processingfrom
feat/improve-media-uploads-and-errors

dcalhoun commented Apr 2, 2026 •

edited

Loading

Uh oh!

dcalhoun Apr 3, 2026

Uh oh!

dcalhoun Apr 3, 2026

Uh oh!

dcalhoun Apr 3, 2026

Uh oh!

jkmassel left a comment

Uh oh!

dcalhoun commented Apr 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

dcalhoun commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What?

Why?

How?

Drain oversized request bodies (RFC 9110 §15.5.14)

Route 413 through the handler for CORS headers

Stream multipart upload body from disk (iOS)

Passthrough upload when delegate doesn't modify the file

Testing Instructions

Screenshots or screencast

Uh oh!

dcalhoun Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

dcalhoun Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

dcalhoun Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

jkmassel left a comment

Choose a reason for hiding this comment

Uh oh!

dcalhoun commented Apr 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dcalhoun commented Apr 2, 2026 •

edited

Loading