feat: proxy media uploads through native delegate for processing#357
feat: proxy media uploads through native delegate for processing#357
Conversation
df05265 to
64d64cb
Compare
| return response.text().then( ( body ) => { | ||
| const message = | ||
| response.status === 413 | ||
| ? `The file is too large to upload. Please choose a smaller file.` |
There was a problem hiding this comment.
We should probably print whatever the server sends back – it should be WP_Error-shaped, but some hosts might have messaging like "The max is ${SOME_NUMBER}" or "You've reached your quota".
WDYT?
There was a problem hiding this comment.
Yes, that makes sense.
This was strictly implemented to handle the 250 MB maximum upload restriction of the local server, but I agree it should be made more robust. If we do not add specific handling for 413, the default user-facing message is something like "Unable to get a valid response from the server."
4cd2cac to
7295a51
Compare
…dleware 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>
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>
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>
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>
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>
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>
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>
5a78344 to
18f81b7
Compare
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>
…ponses 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>
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>
…ment 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>
|
@jkmassel this now relies upon the GBK HTTP server library. This is ready for another review. |
* 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>
Resolve conflicts by keeping both the native media upload plumbing (enableNativeMediaUpload) from this branch and the account/post-editing features (accountId, apiClient, POST_FALLBACKS) from trunk. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@jkmassel the latest changes tested well for me. I believe this is ready for another review. 🙇🏻♂️ |
What?
Adds a native media upload pipeline that routes file uploads through a local HTTP server on iOS and Android, enabling the host app to process files (e.g., resize images, transcode video) before they are uploaded to WordPress.
Why?
Ref CMM-1249.
Gutenberg's built-in upload path sends files directly from the WebView to the WordPress REST API with no opportunity for native processing. Host apps need to resize images, enforce upload size limits, or apply other transformations before upload. This pipeline gives the native layer full control over media processing while keeping the existing Gutenberg upload UX (blob previews, save locking, entity caching) unchanged.
How?
Architecture: A localhost HTTP server runs on each platform, built on the
GutenbergKitHTTPlibrary (iOS) andHttpServer(Android) from #367. The library handles TCP binding, HTTP/1.1 parsing, bearer token authentication (Relay-Authorization), multipart form-data parsing, connection limits, and disk-backed body buffering. The upload server is a thin handler on top.JS layer:
nativeMediaUploadMiddlewareinapi-fetch.jsinterceptsPOST /wp/v2/mediarequests whennativeUploadPortis configured inwindow.GBKit, forwarding files to the local server and transforming responses to WordPress REST API attachment shape (source_url,caption.raw/rendered,title.raw/rendered,media_details, etc.) so the existing Gutenberg upload pipeline works unchanged/wp/v2/mediabut not/wp/v2/media/123or/wp/v2/media-categories) to avoid intercepting non-upload requestsNative layer:
MediaUploadDelegateprotocol/interface withprocessFile(resize/transcode) and optionaluploadFile(custom upload)DefaultMediaUploaderas fallback, uploading to/wp/v2/mediavia the host's HTTP client, with site API namespace support for namespaced sitesContent-Dispositionvaluesupload_failederror codeDemo apps:
Key design decisions
MediaUploadDelegate, keeping the default behavior unchangedapi-fetchmiddleware overmediaUploadeditor setting: Ideally, media uploads would be handled via themediaUploadeditor setting (see the Gutenberg Framework guides), but GutenbergKit uses Gutenberg'sEditorProviderwhich overwrites that setting internally. Until GutenbergKit is refactored to useBlockEditorProvider, theapi-fetchmiddleware approach is necessary.Alternatives considered
JS Canvas resize + native inserter resize — Two separate implementations:
createImageBitmap()+OffscreenCanvasin JS for web uploads,CGImageSourceCreateThumbnailAtIndexinMediaFileManager.import()for the native inserter. Ships fastest and lowest complexity, but Canvas resize quality is lower than native, two codepaths to maintain, and a dead-end for video (client-side transcoding in a WebView is impractical).Native upload pipeline via local HTTP server (this PR) — A single
api-fetchmiddleware intercepts allPOST /wp/v2/mediarequests and routes files through a localhost server for native processing. Covers every upload path (file picker, drag-and-drop, paste, programmatic, plugin blocks) with native-quality processing. Scales to video transcoding. More upfront work than option 1.Replace
MediaPlaceholderviawithFiltershook — Useeditor.MediaPlaceholderandeditor.MediaReplaceFlowfilters withhandleUpload={false}to deliver rawFileobjects toonSelect, then route to native. Incomplete coverage: misses block drag-and-drop re-uploads (handleBlocksDropcallsmediaUploaddirectly), directmediaUploadcalls from plugins, and loses blob previews whenhandleUploadis false.instanceof FileListchecks are fragile in WebView contexts.Redirect to native UI on large files — Keep the web upload button, but show a native dialog when files exceed limits. Awkward UX (user already picked a file, now asked to pick again differently). On iOS, the already-selected JS
Filecan't be handed to native for optimization. Two parallel upload paths add complexity.JS resize for images + hide web upload for video blocks — JS Canvas resize for images, hide the "Upload" button on video-accepting blocks via
editor.MediaPlaceholderfilter (forcing users to Media Library for video). Users can't drag-and-drop videos, blocks accepting both image and video (Cover) get complicated, and it's a dead-end architecture.Client-side processing via
@wordpress/upload-media(WASM libvips) — Gutenberg's experimental@wordpress/upload-mediapackage includes a WASM build of libvips for high-quality client-side image resizing, rotation, format transcoding, and thumbnail generation. Quality is comparable to server-side ImageMagick. However, it requiresSharedArrayBufferfor WASM threading, which is only available in cross-origin isolated contexts — WKWebView loads GutenbergKit's HTML locally with no HTTP headers, soSharedArrayBufferis unavailable. WebKit also lackscredentiallessiframe support, meaning cross-origin isolation would break third-party embeds (YouTube, Twitter, etc.). Single-threaded WASM fallback is unvalidated, and the package's memory footprint (50-100MB+ per image) is a concern under iOS jetsam pressure. Not viable today, but worth revisiting if the package decouples its store/queue management from WASM processing (tracked upstream).A key constraint is platform asymmetry: Android can intercept web
<input type="file">viaonShowFileChooser(), but iOS cannot —WKWebViewhandles file selection internally. This rules out purely native interception strategies for web-originated uploads and motivated the localhost server approach, which works identically on both platforms.Testing Instructions
Accessibility Testing Instructions
The toggle follows the same pattern as the existing "Enable Native Inserter" toggle — no new UI beyond that.
Screenshots or screencast
N/A — backend/infrastructure change with no visible UI changes beyond the demo app toggle.