diff --git a/.gitignore b/.gitignore index 2e700a94f..fef1eeb8a 100644 --- a/.gitignore +++ b/.gitignore @@ -216,6 +216,7 @@ playwright-report/ wp-env/mu-plugins/* !wp-env/mu-plugins/gutenbergkit-cors.php !wp-env/mu-plugins/gutenbergkit-jetpack-blocks.php +!wp-env/mu-plugins/gutenbergkit-media-failure.php # Claude .claude/settings.local.json diff --git a/Makefile b/Makefile index 6ac51f194..75fa08f9c 100644 --- a/Makefile +++ b/Makefile @@ -218,6 +218,10 @@ wp-env-android-reset: ## Remove the Android emulator URL remap and restart @rm -f wp-env/mu-plugins/gutenbergkit-android-urls.php @RESET=1 $(MAKE) wp-env-start +.PHONY: wp-env-media-failure +wp-env-media-failure: ## Report the media upload failure simulation mode (set via MODE=off|recover|always) + @MODE=$(MODE) bash bin/wp-env-media-failure.sh + ################################################################################ # Code Quality Targets ################################################################################ diff --git a/android/Gutenberg/detekt-baseline.xml b/android/Gutenberg/detekt-baseline.xml index 4f6c96915..ce3b4481a 100644 --- a/android/Gutenberg/detekt-baseline.xml +++ b/android/Gutenberg/detekt-baseline.xml @@ -11,6 +11,7 @@ ExplicitItLambdaParameter:EditorAssetsLibrary.kt$EditorAssetsLibrary${ str, it -> str + "%02x".format(it) } FunctionNaming:EditorURLCache.kt$EditorURLCache$private fun __store( response: EditorURLResponse, url: String, httpMethod: EditorHttpMethod, currentDate: Date ) LargeClass:GutenbergView.kt$GutenbergView : FrameLayout + LargeClass:MediaUploadServerTest.kt$MediaUploadServerTest LongMethod:FixtureTests.kt$FixtureTests$@Test fun `request parsing - all basic cases pass`() LongMethod:FixtureTests.kt$FixtureTests$@Test fun `request parsing - all incremental cases pass`() LongMethod:HTTPRequestParser.kt$HTTPRequestParser$fun append(data: ByteArray): Unit diff --git a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt index 0989879ac..96f999dfe 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt @@ -113,30 +113,54 @@ class GutenbergView : FrameLayout { var requestInterceptor: GutenbergRequestInterceptor = DefaultGutenbergRequestInterceptor() /** - * Optional delegate for customizing media upload behavior (resize, transcode, - * custom upload). + * Transforms media (resize, transcode, …) before GutenbergKit delivers it to + * the configured site. The safe, common extension point — a processor never + * performs the upload itself, so it cannot deliver media to the wrong place. * * Provide this **before the editor loads** — typically right after * construction (e.g. in the `AndroidView` factory). It is captured once, when - * the page begins loading, and advertised to the page then; setting it - * afterward has no effect, so the setter throws to surface the mistake. + * the page begins loading; setting it afterward has no effect, so the setter + * throws to surface the mistake. + * + * This view owns the processor for its lifetime, so you don't need to keep a + * reference after assigning it — and avoid strongly retaining this [GutenbergView] + * from your processor in return, so the two don't form a reference cycle. */ - var mediaUploadDelegate: MediaUploadDelegate? = null + var mediaProcessor: MediaProcessor? = null set(value) { - check(!hasStartedLoading) { - "mediaUploadDelegate must be set before the editor loads (e.g. right " + - "after construction). It is captured when the page begins loading; " + - "setting it afterward has no effect." - } + check(!hasStartedLoading) { lateMediaAssignmentMessage("mediaProcessor") } + field = value + } + + /** + * Takes over media upload on the host's own stack (background service, offline + * queue, resumable transport). Setting it makes the host own every upload and + * its whole lifecycle; GutenbergKit stays out of the network entirely for media. + * + * Same lifecycle rules as [mediaProcessor]: set it before the editor loads, and + * this view owns it for its lifetime — so you needn't retain it yourself, just + * don't strongly retain this [GutenbergView] from your uploader. + * + * Requires site credentials in the editor configuration: media deletes always + * relay to the configured site, so an uploader set without a site root and auth + * header is a configuration error and throws at load. + */ + var mediaUploader: MediaUploader? = null + set(value) { + check(!hasStartedLoading) { lateMediaAssignmentMessage("mediaUploader") } field = value } + private fun lateMediaAssignmentMessage(name: String) = + "$name must be set before the editor loads (e.g. right after construction). " + + "It is captured when the page begins loading; setting it afterward has no effect." + @Volatile private var uploadServer: MediaUploadServer? = null /** * True once the editor page has begun loading and the upload server's - * configuration has been captured. After this the [mediaUploadDelegate] can no - * longer take effect, so its setter throws. + * configuration has been captured. After this the [mediaProcessor]/[mediaUploader] + * can no longer take effect, so their setters throw. */ @Volatile private var hasStartedLoading = false @@ -638,13 +662,13 @@ class GutenbergView : FrameLayout { /** * Invoked when the editor page begins loading. Starts the upload server once — - * capturing the [mediaUploadDelegate] provided before load — then advertises - * the editor globals (including the server's port and token) to the page. + * capturing the [mediaProcessor]/[mediaUploader] provided before load — then + * advertises the editor globals (including the server's port and token) to the page. * * Starting the server here, on the UI thread, rather than from the - * [mediaUploadDelegate] setter keeps its whole lifecycle — start here, stop in - * [onDetachedFromWindow] — on the UI thread, so it can't race a - * background-thread delegate assignment. + * [mediaProcessor]/[mediaUploader] setters keeps its whole lifecycle — start + * here, stop in [onDetachedFromWindow] — on the UI thread, so it can't race a + * background-thread assignment. */ private fun onEditorPageStarted() { if (!hasStartedLoading) { @@ -671,17 +695,32 @@ class GutenbergView : FrameLayout { } private fun startUploadServer() { - // No delegate means nothing wants to customize uploads, so there's no reason - // to route them through the native server — leave it down and let uploads - // fall to the default WebView path. (Matches iOS.) - if (mediaUploadDelegate == null) return - - // The native upload server relays through DefaultMediaUploader, which needs a - // site root and an auth header (every host provides one — the editor injects - // it because the WebView has no auth cookies). Without both there is nothing - // to upload through, so leave the server down and let uploads fall to the - // default WebView path rather than start a server that could only fail. - if (configuration.siteApiRoot.isEmpty() || configuration.authHeader.isEmpty()) return + // Nothing to route through the native server unless the host provided a + // processor or an uploader. (Matches iOS.) + if (mediaProcessor == null && mediaUploader == null) return + + // An InternalMediaClient delivers GutenbergKit-owned uploads (when no uploader + // is set) and relays the editor's media DELETEs to the configured site — every + // attachment lives there, even one a host uploader delivered. It needs a site + // root and an auth header (the editor injects it because the WebView has no + // auth cookies). Without them the behavior forks by intent: + // + // - A mediaProcessor only enhances GutenbergKit-owned uploads; with no + // credentials there's nothing to deliver through, so nothing to process — + // leave the server down and let uploads fall to the default WebView path. + // + // - A mediaUploader means the host is taking over uploads. Falling back would + // silently drop it, and its media deletes still need the internal media client to + // reach the configured site. A host that sets an uploader must provide + // credentials too; omitting them is a configuration error, so fail fast. + if (configuration.siteApiRoot.isEmpty() || configuration.authHeader.isEmpty()) { + check(mediaUploader == null) { + "A mediaUploader needs site credentials so GutenbergKit can relay the " + + "editor's media deletes to the configured site. Set siteApiRoot and " + + "the auth header in the editor configuration." + } + return + } // The editor reaches the loopback server over cleartext http://localhost. If // the host app's network-security config doesn't permit cleartext to @@ -701,15 +740,19 @@ class GutenbergView : FrameLayout { } try { - val defaultUploader = DefaultMediaUploader( + // Credentials are present (checked above), so always build a default + // uploader: it delivers GutenbergKit-owned uploads and relays the editor's + // media DELETEs to the configured site. + val internalClient = InternalMediaClient( httpClient = uploadHttpClient, siteApiRoot = configuration.siteApiRoot, authHeader = configuration.authHeader, siteApiNamespace = configuration.siteApiNamespace.toList() ) uploadServer = MediaUploadServer( - uploadDelegate = mediaUploadDelegate, - defaultUploader = defaultUploader, + processor = mediaProcessor, + uploader = mediaUploader, + internalClient = internalClient, cacheDir = context.cacheDir, scope = coroutineScope ) diff --git a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/HttpServer.kt b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/HttpServer.kt index fefd16a3e..d41dede12 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/HttpServer.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/HttpServer.kt @@ -125,6 +125,12 @@ enum class CorsPolicy { "Access-Control-Allow-Origin" to "*", "Access-Control-Allow-Methods" to "GET, POST, PUT, DELETE, OPTIONS", "Access-Control-Allow-Headers" to "Authorization, Relay-Authorization, Content-Type", + // Only CORS-safelisted response headers are readable + // cross-origin by default. The editor needs to read + // `x-wp-upload-attachment-id` off a relayed media upload + // response to retry `post-process` and clean up an orphaned + // attachment, so it has to be exposed explicitly. + "Access-Control-Expose-Headers" to "x-wp-upload-attachment-id", "Access-Control-Max-Age" to "86400" ) } @@ -179,6 +185,7 @@ private fun HttpResponse.addingHeadersIfAbsent(newHeaders: Map): * Access-Control-Allow-Origin: * Access-Control-Allow-Methods: GET, POST, PUT, DELETE, OPTIONS * Access-Control-Allow-Headers: Authorization, Relay-Authorization, Content-Type + * Access-Control-Expose-Headers: x-wp-upload-attachment-id * Access-Control-Max-Age: 86400 * * Without these headers, browsers will reject the preflight and block diff --git a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt index 0fcfa5e90..08e01630c 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt @@ -31,18 +31,28 @@ import okio.source * so every consumer — image sub-sizes, attachment links, error notices — * behaves identically to a non-native upload. */ -class MediaUploadResponse( - /** The HTTP status code WordPress (or the host's upload service) returned. */ +internal class MediaUploadResponse( + /** The HTTP status code WordPress returned. */ val statusCode: Int, /** * The raw response body — a WordPress REST attachment on success, or a * WordPress REST error object (`{ "code", "message", "data" }`) on failure. */ - val body: ByteArray + val body: ByteArray, + /** + * The response headers to relay to the editor. + * + * `x-wp-upload-attachment-id` is the one that carries behavior: WordPress + * sets it on a failed upload whose attachment row was created before + * metadata generation fataled, and the editor's api-fetch middleware reads + * it to retry `post-process` and clean up the orphan. Dropping it turns a + * recoverable upload into a permanent failure. + */ + val headers: Map = emptyMap() ) /** - * The result of a delegate's [MediaUploadDelegate.processFile]. + * The result of a [MediaProcessor.processFile]. */ sealed class ProcessedProxyFile { /** The delegate did not modify the file; the original upload is forwarded unchanged. */ @@ -58,49 +68,113 @@ sealed class ProcessedProxyFile { } /** - * Interface for customizing media upload behavior. + * Transforms media before GutenbergKit delivers it. * - * The native host app can provide an implementation to resize images, - * transcode video, or use its own upload service. + * A processor only changes *bytes* — GutenbergKit still uploads the result to the + * configured site and owns the whole lifecycle (retries, cleanup). Because it + * never performs the upload itself, a processor cannot deliver media to the wrong + * place. Set [GutenbergView.mediaProcessor] to resize images, transcode video, + * strip EXIF, etc. This is the safe, common extension point: most hosts want only + * this. */ -interface MediaUploadDelegate { +interface MediaProcessor { /** - * Whether this delegate might handle a file with the given metadata — either - * processing it ([processFile]) or uploading it itself ([uploadFile]). - * - * A cheap, metadata-only gate the server consults *before* materializing the - * upload to a temp file. Return false to decline a file by type — e.g. an - * image-only delegate returning false for a video — so the server forwards - * the original upload to WordPress without first copying a file the delegate - * won't touch. Because it gates the temp-file copy needed by *both* - * [processFile] and [uploadFile], return true for any file the delegate will - * either process or upload itself. - * - * Defaults to true: every file is materialized and the full pipeline runs. A - * true here is not a commitment — [processFile] may still return - * [ProcessedProxyFile.Original] after inspecting the file's contents. + * Whether this processor might transform a file with the given metadata. A + * cheap, metadata-only gate consulted *before* the upload is materialized to a + * temp file; return false to pass a file straight through untouched — e.g. an + * image-only processor returning false for a video. Defaults to true; not a + * commitment, since [processFile] may still return [ProcessedProxyFile.Original] + * after inspecting the file's contents. */ fun handlesFile(mimeType: String, filename: String): Boolean = true /** - * Process a file before upload (e.g., resize image, transcode video). - * - * Return [ProcessedProxyFile.Original] to upload the file unchanged, or - * [ProcessedProxyFile.Processed] with the processed file and its metadata. - * When the format changes, report the new mimeType and filename so WordPress - * stores it with the correct extension and type. + * Transform a file before upload (e.g., resize image, transcode video). Return + * [ProcessedProxyFile.Original] to upload it unchanged, or + * [ProcessedProxyFile.Processed] with the new file and its metadata (report the + * new mimeType and filename when the format changes, so WordPress stores it + * with the correct extension and type). */ suspend fun processFile(file: File, mimeType: String, filename: String): ProcessedProxyFile = ProcessedProxyFile.Original +} + +/** + * One of the editor's non-file form fields, as sent with a media upload. + * + * A named type rather than a `Pair`, so the two platforms describe this the same way + * and reading a field says `name`/`value` rather than `first`/`second`. (On iOS the + * equivalent change is load-bearing: a tuple there would block Equatable/Hashable/ + * Codable synthesis on [MediaUpload] permanently.) + * + * @property name The field name, e.g. `post`. Not unique — a `field[]` array repeats it. + * @property value The field's value, decoded as UTF-8. + */ +data class MediaUploadField( + val name: String, + val value: String +) + +/** + * Everything a [MediaUploader] needs to reproduce a native upload: the file to send, + * its metadata, the editor's non-file form fields, and the request's query. + * + * @property file The file to upload — already processed, if a [MediaProcessor] ran. + * @property mimeType The file's MIME type. + * @property filename The file's name. + * @property fields The editor's non-file form fields, in order, each decoded as UTF-8 — + * most importantly `post`, the parent post's ID, without which the attachment is + * created unattached. A list, not a map, so repeated field names (e.g. a `field[]` + * array) survive verbatim. Send each as a form part on your `POST /wp/v2/media`, + * in the given order. + * @property query The request's query string (leading `?`, e.g. `?_embed=...`), or + * empty. Carry it on your request so the editor gets the response it expects. + */ +data class MediaUpload( + val file: File, + val mimeType: String, + val filename: String, + val fields: List, + val query: String +) +/** + * Takes over performing a media upload — on the host's own stack: its own + * networking (say, to log every request), a background service, an offline queue, + * a resumable transport, its own retry policy. + * + * This is a choice of who executes the requests, not where they go: an uploader + * and GutenbergKit's built-in default both target the same configured site. + * Setting [GutenbergView.mediaUploader] makes the host own that upload end-to-end — + * the request, its own retries, and its recovery and cleanup — with GutenbergKit + * out of the network entirely. Because the host does the retries itself, there's no + * raw response left for core to retry behind it. The attachment you return lives on + * that same configured site, where the editor reads and updates it by ID. + */ +interface MediaUploader { /** - * Upload a processed file to the remote WordPress site. + * Upload a (possibly processed) file and return the finished WordPress + * attachment JSON the editor inserts — the same object a direct + * `POST /wp/v2/media` returns. Return only once the upload is genuinely done, or + * throw on terminal failure: a returned value is taken as a completed attachment, + * and there is no GutenbergKit recovery behind you. + * + * The [MediaUpload] carries the file plus the editor's form fields (e.g. `post`) + * and query — send them all so the created attachment matches a native upload + * rather than landing as an unattached orphan. * - * Return the raw WordPress response (status code + body), which GutenbergKit - * relays to the editor unchanged, or null to use the default uploader. A host - * that uploads to WordPress should return the exact response it received so - * the editor sees a complete attachment object. + * That recovery is yours to run. When `POST /wp/v2/media` fatals in server-side + * post-processing it returns a 5xx carrying the attachment's ID in + * `x-wp-upload-attachment-id` — the attachment exists but is unfinished. Don't + * re-upload; drive `POST /wp/v2/media//post-process` to completion, the way + * core recovers its own uploads (up to 5 attempts), then return the finished + * attachment. + * + * Owning the upload means owning cleanup on the server too: if post-process + * can't be recovered, force-delete the orphan (`DELETE /wp/v2/media/?force=true`) + * before you throw, or it stays on the site — neither GutenbergKit nor core + * cleans up behind you. */ - suspend fun uploadFile(file: File, mimeType: String, filename: String): MediaUploadResponse? = null + suspend fun upload(upload: MediaUpload): ByteArray } /** @@ -116,9 +190,10 @@ interface MediaUploadDelegate { * stop on detach. */ internal class MediaUploadServer( - private val uploadDelegate: MediaUploadDelegate?, - private val defaultUploader: DefaultMediaUploader?, - cacheDir: File? = null, + private val processor: MediaProcessor?, + private val uploader: MediaUploader?, + private val internalClient: InternalMediaClient, + cacheDir: File, scope: CoroutineScope? = null, ioDispatcher: CoroutineDispatcher = Dispatchers.IO ) : HttpServerDelegate { @@ -131,11 +206,10 @@ internal class MediaUploadServer( private val server: HttpServer /** - * Directory for staging uploaded files, under the injected cache dir (with a - * system-temp fallback) so orphans share the app's managed cache lifecycle. + * Directory for staging uploaded files, under the injected cache dir so orphans + * share the app's managed cache lifecycle. */ - private val uploadsTempDir: File = - File(cacheDir ?: File(System.getProperty("java.io.tmpdir")), "gutenbergkit-uploads") + private val uploadsTempDir: File = File(cacheDir, "gutenbergkit-uploads") /** * The scope MediaUploadServer created itself because the caller supplied none. @@ -212,15 +286,70 @@ internal class MediaUploadServer( } private suspend fun handleRequest(request: HttpRequest): HttpResponse { - // Route: only POST /upload is handled. (OPTIONS preflight is answered by - // the HTTP library under its permissive CORS policy.) Match on the path - // alone — the target carries a query string (e.g. `?_embed`) that the - // upload handler relays on to WordPress. - if (request.method.uppercase() != "POST" || request.path != "/upload") { - return errorResponse(404, "Not found") + // Routes: POST /upload, and DELETE /media/ for the editor's orphan + // cleanup. (OPTIONS preflight is answered by the HTTP library under its + // permissive CORS policy.) Match on the path alone — the target carries + // a query string (e.g. `?_embed`, `?force=true`) relayed to WordPress. + val method = request.method.uppercase() + + if (method == "POST" && request.path == "/upload") { + return handleUpload(request) + } + + if (method == "DELETE") { + attachmentIdFromPath(request.path)?.let { attachmentId -> + return handleMediaDelete(attachmentId, request.query) + } } - return handleUpload(request) + return errorResponse(404, "Not found") + } + + /** + * The attachment ID in a `/media/` path, or `null` if the path is not one. + * + * Deliberately narrow: this server relays media operations, not arbitrary + * REST requests, so only a numeric attachment ID under `/media/` matches. + */ + private fun attachmentIdFromPath(path: String): String? { + val components = path.split("/").filter { it.isNotEmpty() } + if (components.size != 2 || components[0] != "media") return null + val id = components[1] + return if (id.isNotEmpty() && id.all { it.isDigit() }) id else null + } + + /** + * Relays a media deletion. + * + * The editor deletes an attachment when the user removes it, and core deletes + * an upload's orphan when every `post-process` retry fails. A cross-origin + * editor cannot issue `DELETE` directly — api-fetch tunnels it as a `POST` + * carrying `X-HTTP-Method-Override`, which core's CORS allow-list omits, so the + * browser blocks it at preflight; relaying it here lets the deletion run. + * + * Every attachment lives on the configured site — even one a host uploader + * delivered — so its deletion is relayed to the internal media client there. See + * the accepted-risk note in the body. + */ + @Suppress("TooGenericExceptionCaught") + private suspend fun handleMediaDelete(attachmentId: String, query: String): HttpResponse { + return try { + // Relay to the internal media client (the configured site) — every attachment + // lives there, even one a host uploader delivered. Core issues this only + // as orphan cleanup after failed recovery, but the relay can't tell that + // from any other DELETE the WebView sends: a compromised editor script + // holding the loopback token could force-delete arbitrary media on the + // configured site. Accepted risk — such a script already has broad write + // access, and a server-side compromise (a malicious plugin) deletes media + // directly without the editor, so scoping this with a per-session ledger + // buys little for the cost. + relayResponse(internalClient.deleteMedia(attachmentId, query)) + } catch (e: kotlin.coroutines.cancellation.CancellationException) { + throw e // Never swallow coroutine cancellation. + } catch (e: Exception) { + Log.e(TAG, "Media deletion failed", e) + errorResponse(500, e.message ?: "Deletion failed") + } } private suspend fun handleUpload(request: HttpRequest): HttpResponse { @@ -236,18 +365,20 @@ internal class MediaUploadServer( val mimeType = filePart.contentType val filename = filePart.filename ?: "upload" - // Ask the delegate — from metadata alone — whether it will touch a file - // like this. If not, forward the original upload to WordPress directly, - // skipping a full temp-file copy of a file the delegate won't process or - // upload (e.g. a video handed to an image-only delegate). - if (uploadDelegate?.handlesFile(mimeType, filename) != true) { + // Materialize a temp file only if someone will touch it: a processor that + // claims this file, or an uploader (which always delivers the file itself). + // If GutenbergKit will deliver (no uploader) and no processor wants the + // file, forward the original request body directly, skipping a temp copy of + // a file nobody will process (e.g. a video handed to an image-only processor). + val processorWantsFile = processor?.handlesFile(mimeType, filename) == true + if (uploader == null && !processorWantsFile) { return passthroughResponse(request, query) } val tempFile = writePartToTempFile(filePart) ?: return errorResponse(500, "Failed to save file") - return processAndRespond(request, tempFile, filePart, extraParts, query) + return processAndRespond(request, tempFile, filePart, extraParts, query, processorWantsFile) } @Suppress("TooGenericExceptionCaught") @@ -264,13 +395,23 @@ internal class MediaUploadServer( } /** - * Relays WordPress's exact status and body to the editor so it sees the same - * attachment object (or error) as a direct upload. + * Relays WordPress's exact status, body, and relayable headers to the editor + * so it sees the same attachment object (or error) as a direct upload. + * + * The headers matter for recovery: `x-wp-upload-attachment-id` is what lets + * the editor retry `post-process` for an upload whose metadata generation + * fataled server-side, rather than surfacing a permanent failure and leaving + * an orphaned attachment behind. + * + * The relayed body is always WordPress REST JSON — an attachment, or a + * `{code, message, data}` error — so the response is always `application/json`. + * The relayed headers are a content-type-free allowlist (`RELAYABLE_HEADER_NAMES`), + * so prepending the JSON default never collides with them. */ private fun relayResponse(response: MediaUploadResponse): HttpResponse { return HttpResponse( status = response.statusCode, - headers = mapOf("Content-Type" to "application/json"), + headers = mapOf("Content-Type" to "application/json") + response.headers, body = response.body ) } @@ -319,11 +460,12 @@ internal class MediaUploadServer( @Suppress("TooGenericExceptionCaught") private suspend fun processAndRespond( request: HttpRequest, tempFile: File, filePart: MultipartPart, - extraParts: List, query: String + extraParts: List, query: String, processorWantsFile: Boolean ): HttpResponse { try { val uploadResult = processAndUpload( - tempFile, filePart.contentType, filePart.filename ?: "upload", extraParts, query + tempFile, filePart.contentType, filePart.filename ?: "upload", + extraParts, query, processorWantsFile ) val response = when (uploadResult) { is UploadResult.Uploaded -> { @@ -331,8 +473,8 @@ internal class MediaUploadServer( uploadResult.response } is UploadResult.Passthrough -> { - // Delegate didn't modify the file — forward the original - // request body to WordPress without re-encoding. + // No uploader is set and the processor left the file unmodified — + // forward the original request body without re-encoding. Log.d(TAG, "Passthrough: forwarding original request body to WordPress") performPassthroughUpload(request, query) } @@ -357,7 +499,7 @@ internal class MediaUploadServer( } } - // MARK: - Delegate Pipeline + // MARK: - Process + Deliver Pipeline private sealed class UploadResult { data class Uploaded(val response: MediaUploadResponse) : UploadResult() @@ -367,21 +509,27 @@ internal class MediaUploadServer( private suspend fun performPassthroughUpload(request: HttpRequest, query: String): MediaUploadResponse { val body = request.body val contentType = request.header("Content-Type") - val uploader = defaultUploader - if (body == null || contentType == null || uploader == null) { - throw MediaUploadException("Passthrough upload requires a request body, Content-Type, and default uploader") + if (body == null || contentType == null) { + throw MediaUploadException("Passthrough upload requires a request body and Content-Type") } - return uploader.passthroughUpload(body, contentType, query) + return internalClient.passthroughUpload(body, contentType, query) } private suspend fun processAndUpload( file: File, mimeType: String, filename: String, - extraParts: List, query: String + extraParts: List, query: String, processorWantsFile: Boolean ): UploadResult { - val processed = uploadDelegate?.processFile(file, mimeType, filename) ?: ProcessedProxyFile.Original + // Transform (resize, transcode, …) if a processor claims the file. Reuse the + // gate's handlesFile decision from handleUpload rather than asking again — one + // metadata call per upload, and the admit and transform steps can't disagree. + val processed = if (processorWantsFile && processor != null) { + processor.processFile(file, mimeType, filename) + } else { + ProcessedProxyFile.Original + } // Resolve the file to upload and its metadata. Processed uses the - // delegate's values verbatim, so a format change is reported to WordPress. + // processor's values verbatim, so a format change is reported to WordPress. val targetFile: File val targetMimeType: String val targetFilename: String @@ -399,9 +547,28 @@ internal class MediaUploadServer( } try { - // If the delegate provided its own upload, use that. - uploadDelegate?.uploadFile(targetFile, targetMimeType, targetFilename)?.let { - return UploadResult.Uploaded(it) + // An uploader owns delivery on the host's own stack and returns the + // finished attachment JSON (or throws); GutenbergKit relays that as a + // success and never runs its own recovery behind it. + uploader?.let { up -> + // Hand the host the editor's non-file fields (e.g. `post`) and query + // too, so its own POST can reproduce a native upload — otherwise the + // attachment is created unattached and `?_embed` is lost. + // + // The UTF-8 decode is lossless because of an invariant nothing enforces: + // the only client is the editor's browser FormData. The server binds to + // loopback behind a per-session token; a FormData string value is a + // USVString, already well-formed at append time; and its only way to carry + // arbitrary bytes is a Blob, which always gets a filename and so is + // filtered out of extraParts above. Valid UTF-8 — emoji, any script — + // round-trips exactly. If that stops holding this silently substitutes + // U+FFFD, and differently from iOS (Java reports one replacement char for + // ED A0 80 where Swift's maximal-subpart rule reports three). + val fields = extraParts.map { part -> + MediaUploadField(part.name, String(part.body.readBytes(), Charsets.UTF_8)) + } + val upload = MediaUpload(targetFile, targetMimeType, targetFilename, fields, query) + return UploadResult.Uploaded(MediaUploadResponse(201, up.upload(upload))) } // Unmodified — forward the original request body directly, skipping @@ -410,8 +577,7 @@ internal class MediaUploadServer( return UploadResult.Passthrough } - val result = defaultUploader?.upload(targetFile, targetMimeType, targetFilename, extraParts, query) - ?: error("No upload delegate or default uploader configured") + val result = internalClient.upload(targetFile, targetMimeType, targetFilename, extraParts, query) return UploadResult.Uploaded(result) } finally { // The processed file (if the delegate produced a new one) is ours to @@ -468,7 +634,7 @@ internal class MediaUploadException(message: String, cause: Throwable? = null) : /** * Uploads files to the WordPress REST API using OkHttp. */ -internal open class DefaultMediaUploader( +internal open class InternalMediaClient( private val httpClient: okhttp3.OkHttpClient, private val siteApiRoot: String, private val authHeader: String, @@ -479,8 +645,26 @@ internal open class DefaultMediaUploader( * namespacing (so it matches every other REST URL) and carrying the original * request query (e.g. `?_embed`) through to WordPress. */ - private fun mediaEndpointUrl(query: String): String = - RestUrlBuilder.namespaced(siteApiRoot, siteApiNamespace.firstOrNull(), "/wp/v2/media") + query + private fun mediaEndpointUrl(query: String, attachmentId: String? = null): String { + val path = if (attachmentId != null) "/wp/v2/media/$attachmentId" else "/wp/v2/media" + return RestUrlBuilder.namespaced(siteApiRoot, siteApiNamespace.firstOrNull(), path) + query + } + + /** + * Deletes an attachment, relaying WordPress's response verbatim. + * + * Carries the editor's query through unchanged — core's cleanup sends + * `?force=true`, without which WordPress trashes rather than deletes. + */ + open suspend fun deleteMedia(attachmentId: String, query: String): MediaUploadResponse { + val request = okhttp3.Request.Builder() + .url(mediaEndpointUrl(query, attachmentId)) + .addHeader("Authorization", authHeader) + .delete() + .build() + + return performUpload(request) + } open suspend fun upload( file: File, mimeType: String, filename: String, @@ -489,8 +673,11 @@ internal open class DefaultMediaUploader( val mediaType = mimeType.toMediaType() val builder = okhttp3.MultipartBody.Builder().setType(okhttp3.MultipartBody.FORM) // Preserve the non-file parts (post, additionalData) through the re-encode. - // Append each field's raw bytes (not via String) so a non-UTF-8 value is - // forwarded verbatim rather than coerced. filename=null makes it a plain + // Each field's raw bytes are appended rather than round-tripped through String + // — not because malformed values are expected (they can't reach here; see the + // invariant where `fields` is built), but so this re-encode stays byte-identical + // to the passthrough it stands in for: a user's upload shouldn't change shape + // just because a processor resized the image. filename=null makes it a plain // field, matching okhttp's String overload byte-for-byte. for (part in extraParts) { builder.addFormDataPart(part.name, null, part.body.readBytes().toRequestBody()) @@ -559,7 +746,11 @@ internal open class DefaultMediaUploader( // holding a connection permit. val result = try { response.use { - MediaUploadResponse(it.code, it.body?.bytes() ?: ByteArray(0)) + MediaUploadResponse( + it.code, + it.body?.bytes() ?: ByteArray(0), + relayableHeaders(it) + ) } } catch (e: IOException) { if (!continuation.isCancelled) continuation.resumeWithException(e) @@ -577,4 +768,22 @@ internal open class DefaultMediaUploader( }) } } + + /** Picks the headers to relay out of an upstream response. */ + private fun relayableHeaders(response: okhttp3.Response): Map = + RELAYABLE_HEADER_NAMES.mapNotNull { name -> + response.header(name)?.let { name to it } + }.toMap() + + companion object { + /** + * The upstream headers worth relaying to the editor. + * + * Deliberately an allowlist rather than a filtered passthrough: the body + * is re-sent with a recomputed length, so relaying upstream entity or + * transport headers wholesale would risk contradicting what the server + * actually sends. + */ + private val RELAYABLE_HEADER_NAMES = listOf("x-wp-upload-attachment-id") + } } diff --git a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewUploadServerTest.kt b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewUploadServerTest.kt index a83cfe5f5..b319c107d 100644 --- a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewUploadServerTest.kt +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewUploadServerTest.kt @@ -2,10 +2,12 @@ package org.wordpress.gutenberg import android.os.Looper import android.view.View +import java.lang.reflect.InvocationTargetException import kotlinx.coroutines.test.TestScope import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertThrows +import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mockito.mock @@ -62,15 +64,15 @@ class GutenbergViewUploadServerTest { private fun idle() = shadowOf(Looper.getMainLooper()).idle() @Test - fun `the upload server starts when the page begins loading, capturing the delegate`() { + fun `the upload server starts when the page begins loading, capturing the processor`() { val view = makeView() try { - // A delegate provided before load is captured when the page starts. - view.mediaUploadDelegate = mock(MediaUploadDelegate::class.java) + // A processor provided before load is captured when the page starts. + view.mediaProcessor = mock(MediaProcessor::class.java) startLoading(view) idle() assertNotNull( - "a delegate provided before load should bring up the upload server", + "a processor provided before load should bring up the upload server", uploadServerOf(view) ) } finally { @@ -79,14 +81,14 @@ class GutenbergViewUploadServerTest { } @Test - fun `no delegate means no upload server`() { + fun `no processor or uploader means no upload server`() { val view = makeView() try { - // No delegate provided — uploads should use the default WebView path. + // Nothing provided — uploads should use the default WebView path. startLoading(view) idle() assertNull( - "with no delegate, no upload server should be started", + "with no processor or uploader, no upload server should be started", uploadServerOf(view) ) } finally { @@ -95,25 +97,94 @@ class GutenbergViewUploadServerTest { } @Test - fun `setting the delegate after the page has started loading throws`() { + fun `setting a media handler after the page has started loading throws`() { val view = makeView() try { startLoading(view) idle() - // The delegate is captured at load; a later assignment is a programmer + // The processor is captured at load; a later assignment is a programmer // error and must surface loudly rather than silently do nothing. assertThrows(IllegalStateException::class.java) { - view.mediaUploadDelegate = mock(MediaUploadDelegate::class.java) + view.mediaProcessor = mock(MediaProcessor::class.java) } } finally { detach(view) } } + @Test + fun `an uploader without site credentials is a configuration error`() { + // A mediaUploader owns uploads, but media deletes still relay to the configured + // site — which needs credentials to reach. Setting an uploader without them is + // a programmer error, surfaced loudly rather than starting a server whose every + // delete would fail. (A processor without credentials is fine — it just falls + // back to the default WebView path, covered above.) + val config = EditorConfiguration + .builder("https://example.com", "https://example.com/wp-json/") + .build() // deliberately no auth header + val view = GutenbergView( + config, + EditorDependencies.empty, + testScope, + RuntimeEnvironment.getApplication() + ) + try { + view.mediaUploader = mock(MediaUploader::class.java) + // startUploadServer runs inside onEditorPageStarted, so reflection wraps its throw. + val error = assertThrows(InvocationTargetException::class.java) { + startLoading(view) + } + assertTrue( + "an uploader without credentials should fail with IllegalStateException", + error.cause is IllegalStateException + ) + assertNull( + "no server should be left running after the configuration error", + uploadServerOf(view) + ) + } finally { + detach(view) + } + } + + @Test + fun `an uploader without a site api root is a configuration error`() { + // The other arm of the same gate: an auth header is no use without a site to + // send it to. Both fields have to be present and usable for the internal media + // client to reach the configured site, so either one missing traps — iOS gates + // on the same pair. + val config = EditorConfiguration + .builder("https://example.com", "") + .setAuthHeader("Bearer token") + .build() // deliberately no site API root + val view = GutenbergView( + config, + EditorDependencies.empty, + testScope, + RuntimeEnvironment.getApplication() + ) + try { + view.mediaUploader = mock(MediaUploader::class.java) + val error = assertThrows(InvocationTargetException::class.java) { + startLoading(view) + } + assertTrue( + "an uploader without a site api root should fail with IllegalStateException", + error.cause is IllegalStateException + ) + assertNull( + "no server should be left running after the configuration error", + uploadServerOf(view) + ) + } finally { + detach(view) + } + } + @Test fun `detaching the view stops and clears the upload server`() { val view = makeView() - view.mediaUploadDelegate = mock(MediaUploadDelegate::class.java) + view.mediaProcessor = mock(MediaProcessor::class.java) startLoading(view) idle() assertNotNull(uploadServerOf(view)) diff --git a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/MediaUploadServerTest.kt b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/MediaUploadServerTest.kt index e87290434..8da157973 100644 --- a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/MediaUploadServerTest.kt +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/MediaUploadServerTest.kt @@ -33,7 +33,12 @@ class MediaUploadServerTest { @Before fun setUp() { - server = MediaUploadServer(uploadDelegate = null, defaultUploader = null, cacheDir = tempFolder.root) + server = MediaUploadServer( + processor = null, + uploader = null, + internalClient = MockInternalMediaClient(), + cacheDir = tempFolder.root + ) } @After @@ -53,7 +58,12 @@ class MediaUploadServerTest { fun `stop cancels an internally-created scope but leaves a caller-supplied one alone`() { // No scope supplied → the server owns one, which stop() must cancel. val owningServer = - MediaUploadServer(uploadDelegate = null, defaultUploader = null, cacheDir = tempFolder.root) + MediaUploadServer( + processor = null, + uploader = null, + internalClient = MockInternalMediaClient(), + cacheDir = tempFolder.root + ) val ownedScope = ownedScopeOf(owningServer) assertNotNull("server should own a scope when none is supplied", ownedScope) assertTrue(ownedScope!!.isActive) @@ -63,8 +73,9 @@ class MediaUploadServerTest { // A caller-supplied scope belongs to the caller — stop() must not cancel it. val callerScope = CoroutineScope(Dispatchers.IO) val borrowingServer = MediaUploadServer( - uploadDelegate = null, - defaultUploader = null, + processor = null, + uploader = null, + internalClient = MockInternalMediaClient(), cacheDir = tempFolder.root, scope = callerScope ) @@ -140,12 +151,66 @@ class MediaUploadServerTest { assertTrue(response.statusLine.contains("404")) } + @Test + fun `relays a deletion to the internal media client even when an uploader owns uploads`() { + // An attachment lives on the configured site even when a host uploader + // delivered it, so its deletion goes to the internal media client — the host + // uploader owns uploads, not deletes. + val uploader = MockUploader() + val internalClient = MockInternalMediaClient() + server.stop() + server = MediaUploadServer( + processor = null, + uploader = uploader, + internalClient = internalClient, + cacheDir = tempFolder.root + ) + + val response = sendRawRequest( + method = "DELETE", + path = "/media/42?force=true", + headers = mapOf("Relay-Authorization" to "Bearer ${server.token}"), + body = ByteArray(0) + ) + + assertTrue("Expected 200 but got: ${response.statusLine}", response.statusLine.contains("200")) + assertTrue(internalClient.deleteMediaCalled) + assertEquals("42", internalClient.deletedAttachmentId) + } + + // MARK: - Media deletion + + @Test + fun `relays a deletion to the internal media client (configured site)`() { + // With no uploader set, GutenbergKit owns deletes: core's orphan cleanup + // DELETE is relayed to the internal media client (the configured site). + val mockUploader = MockInternalMediaClient() + server.stop() + server = MediaUploadServer( + processor = null, + uploader = null, + internalClient = mockUploader, + cacheDir = tempFolder.root + ) + + val response = sendRawRequest( + method = "DELETE", + path = "/media/512?force=true", + headers = mapOf("Relay-Authorization" to "Bearer ${server.token}"), + body = ByteArray(0) + ) + + assertTrue("Expected 200 but got: ${response.statusLine}", response.statusLine.contains("200")) + assertTrue(mockUploader.deleteMediaCalled) + assertEquals("512", mockUploader.deletedAttachmentId) + } + @Test fun `routes upload with a query string and relays the query`() { - val delegate = ProcessOnlyDelegate() - val mockUploader = MockDefaultUploader() + val processor = PassthroughProcessor() + val mockUploader = MockInternalMediaClient() server.stop() - server = MediaUploadServer(uploadDelegate = delegate, defaultUploader = mockUploader, cacheDir = tempFolder.root) + server = MediaUploadServer(processor = processor, uploader = null, internalClient = mockUploader, cacheDir = tempFolder.root) // `@wordpress/media-utils` uploads to `/wp/v2/media?_embed=wp:featuredmedia`, // so the middleware forwards that query on to the native server. Routing must @@ -164,7 +229,7 @@ class MediaUploadServerTest { ) assertTrue("Expected 201 but got: ${response.statusLine}", response.statusLine.contains("201")) - // The delegate returns Original, so this is the passthrough branch. + // The processor returns Original, so this is the passthrough branch. // Pin which branch ran — `lastQuery` is recorded by both, so without this // the query assertion would pass even if routing collapsed onto one path. assertTrue(mockUploader.passthroughUploadCalled) @@ -172,20 +237,31 @@ class MediaUploadServerTest { assertEquals("?_embed=wp:featuredmedia", mockUploader.lastQuery) } - // MARK: - Upload with delegate + // MARK: - Upload with a processor or uploader @Test - fun `calls delegate processFile and uploadFile`() { - val delegate = MockUploadDelegate() + fun `routes an upload to the uploader and relays its attachment`() { + // With an uploader set, GutenbergKit hands it the file and relays the finished + // attachment it returns — the internal media client (configured site) is never used. + val uploader = MockUploader() + val internalClient = MockInternalMediaClient() server.stop() - server = MediaUploadServer(uploadDelegate = delegate, defaultUploader = null, cacheDir = tempFolder.root) + server = MediaUploadServer( + processor = null, + uploader = uploader, + internalClient = internalClient, + cacheDir = tempFolder.root + ) val boundary = "test-boundary-123" - val body = buildMultipartBody(boundary, "photo.jpg", "image/jpeg", "fake image data".toByteArray()) + val body = buildMultipartBody( + boundary, "photo.jpg", "image/jpeg", "fake image data".toByteArray(), + fields = listOf(MediaUploadField("post", "123")) + ) val response = sendRawRequest( method = "POST", - path = "/upload", + path = "/upload?_embed=wp:featuredmedia", headers = mapOf( "Relay-Authorization" to "Bearer ${server.token}", "Content-Type" to "multipart/form-data; boundary=$boundary" @@ -194,12 +270,20 @@ class MediaUploadServerTest { ) assertTrue("Expected 201 but got: ${response.statusLine}", response.statusLine.contains("201")) - assertTrue(delegate.processFileCalled) - assertTrue(delegate.uploadFileCalled) - assertEquals("image/jpeg", delegate.lastMimeType) - assertEquals("photo.jpg", delegate.lastFilename) - - // The server relays WordPress's raw response body verbatim. + assertTrue(uploader.uploadCalled) + assertEquals("image/jpeg", uploader.lastMimeType) + assertEquals("photo.jpg", uploader.lastFilename) + // The editor's post association and query must reach the host uploader, so it + // can reproduce a native upload (attach to the post, honor ?_embed). + assertEquals("123", uploader.lastFields.first { it.name == "post" }.value) + assertEquals("?_embed=wp:featuredmedia", uploader.lastQuery) + // …and the actual file bytes the editor sent — the host uploads them itself. + assertEquals("fake image data", uploader.lastFileBytes?.decodeToString()) + // The host owns delivery — GutenbergKit must not upload to the configured site. + assertFalse(internalClient.uploadCalled) + assertFalse(internalClient.passthroughUploadCalled) + + // The server relays the exact attachment JSON the uploader returned. val json = JsonParser.parseString(response.body).asJsonObject assertEquals(42, json.get("id").asInt) assertEquals("https://example.com/photo.jpg", json.get("source_url").asString) @@ -207,11 +291,181 @@ class MediaUploadServerTest { } @Test - fun `forwards the delegate's processed metadata to the uploader`() { - val delegate = TranscodingDelegate() - val mockUploader = MockDefaultUploader() + fun `hands a host uploader repeated form field names in order, not collapsed`() { + // A `field[]`-style repeated name (e.g. a custom attachment taxonomy): WordPress + // builds an array from these, so both values must reach the host uploader in + // order. A map would drop the first — the ordered-list contract must not. + val uploader = MockUploader() + val internalClient = MockInternalMediaClient() + server.stop() + server = MediaUploadServer( + processor = null, + uploader = uploader, + internalClient = internalClient, + cacheDir = tempFolder.root + ) + + val boundary = "test-boundary-123" + val body = buildMultipartBody( + boundary, "photo.jpg", "image/jpeg", "fake image data".toByteArray(), + fields = listOf( + MediaUploadField("post", "123"), + MediaUploadField("media_folder[]", "12"), + MediaUploadField("media_folder[]", "45") + ) + ) + + val response = sendRawRequest( + method = "POST", + path = "/upload", + headers = mapOf( + "Relay-Authorization" to "Bearer ${server.token}", + "Content-Type" to "multipart/form-data; boundary=$boundary" + ), + body = body + ) + + assertTrue("Expected 201 but got: ${response.statusLine}", response.statusLine.contains("201")) + assertTrue(uploader.uploadCalled) + // Both repeated values survive, in order — not collapsed to the last. + assertEquals( + listOf("12", "45"), + uploader.lastFields.filter { it.name == "media_folder[]" }.map { it.value } + ) + assertEquals("123", uploader.lastFields.first { it.name == "post" }.value) + } + + @Test + fun `keeps a filename-bearing part out of the fields handed to an uploader`() { + // Pins the invariant that makes the UTF-8 decode of `fields` lossless. A browser + // FormData can only carry arbitrary bytes as a Blob, and a Blob always gets a + // filename, so the partition on `filename == null` is what keeps binary out of + // `fields`. Change it and the decode silently substitutes U+FFFD — and does so + // differently from iOS. + // + // Deliberately not asserted: what becomes of the second filename-bearing part. + // It is currently dropped rather than relayed, which is a separate open question. + val uploader = MockUploader() + val internalClient = MockInternalMediaClient() + server.stop() + server = MediaUploadServer( + processor = null, + uploader = uploader, + internalClient = internalClient, + cacheDir = tempFolder.root + ) + + val boundary = "test-boundary-123" + val out = java.io.ByteArrayOutputStream() + // A plain field — no filename, so it belongs in `fields`. + out.write("--$boundary\r\n".toByteArray()) + out.write("Content-Disposition: form-data; name=\"post\"\r\n\r\n".toByteArray()) + out.write("123".toByteArray()) + out.write("\r\n".toByteArray()) + // The file. + out.write("--$boundary\r\n".toByteArray()) + out.write("Content-Disposition: form-data; name=\"file\"; filename=\"photo.jpg\"\r\n".toByteArray()) + out.write("Content-Type: image/jpeg\r\n\r\n".toByteArray()) + out.write("fake image data".toByteArray()) + out.write("\r\n".toByteArray()) + // A Blob-shaped sidecar: filename present, bytes not valid UTF-8. If the + // partition admitted this to `fields`, the lone 0xFF would become U+FFFD. + out.write("--$boundary\r\n".toByteArray()) + out.write("Content-Disposition: form-data; name=\"sidecar\"; filename=\"blob\"\r\n".toByteArray()) + out.write("Content-Type: application/octet-stream\r\n\r\n".toByteArray()) + out.write(byteArrayOf(0x61, 0xFF.toByte(), 0x62)) + out.write("\r\n--$boundary--\r\n".toByteArray()) + + sendRawRequest( + method = "POST", + path = "/upload", + headers = mapOf( + "Relay-Authorization" to "Bearer ${server.token}", + "Content-Type" to "multipart/form-data; boundary=$boundary" + ), + body = out.toByteArray() + ) + + assertTrue(uploader.uploadCalled) + assertEquals( + "only filename-less parts belong in fields", + listOf("post"), + uploader.lastFields.map { it.name } + ) + assertTrue( + "no field value should have been lossily decoded", + uploader.lastFields.none { it.value.contains('�') } + ) + } + + @Test + fun `hands the processed file and its new metadata to the uploader`() { + // A processor transcodes the file; the host uploader must receive the processed + // bytes and the new metadata, not the original clip.mov. + val processor = TranscodingProcessor() + val uploader = MockUploader() server.stop() - server = MediaUploadServer(uploadDelegate = delegate, defaultUploader = mockUploader, cacheDir = tempFolder.root) + server = MediaUploadServer( + processor = processor, + uploader = uploader, + internalClient = MockInternalMediaClient(), + cacheDir = tempFolder.root + ) + + val boundary = "test-boundary-proc" + val body = buildMultipartBody(boundary, "clip.mov", "video/quicktime", "movie".toByteArray()) + + val response = sendRawRequest( + method = "POST", + path = "/upload", + headers = mapOf( + "Relay-Authorization" to "Bearer ${server.token}", + "Content-Type" to "multipart/form-data; boundary=$boundary" + ), + body = body + ) + + assertTrue("Expected 201 but got: ${response.statusLine}", response.statusLine.contains("201")) + assertTrue(uploader.uploadCalled) + assertEquals("processed", uploader.lastFileBytes?.decodeToString()) + assertEquals("video/mp4", uploader.lastMimeType) + assertEquals("clip.mp4", uploader.lastFilename) + } + + @Test + fun `relays a 500 when the host uploader throws`() { + val uploader = MockUploader(error = RuntimeException("upload failed")) + server.stop() + server = MediaUploadServer( + processor = null, + uploader = uploader, + internalClient = MockInternalMediaClient(), + cacheDir = tempFolder.root + ) + + val boundary = "test-boundary-err" + val body = buildMultipartBody(boundary, "photo.jpg", "image/jpeg", "fake image data".toByteArray()) + + val response = sendRawRequest( + method = "POST", + path = "/upload", + headers = mapOf( + "Relay-Authorization" to "Bearer ${server.token}", + "Content-Type" to "multipart/form-data; boundary=$boundary" + ), + body = body + ) + + assertTrue("Expected 500 but got: ${response.statusLine}", response.statusLine.contains("500")) + assertTrue(uploader.uploadCalled) + } + + @Test + fun `forwards the processor's processed metadata to the uploader`() { + val processor = TranscodingProcessor() + val mockUploader = MockInternalMediaClient() + server.stop() + server = MediaUploadServer(processor = processor, uploader = null, internalClient = mockUploader, cacheDir = tempFolder.root) val boundary = "test-boundary-meta" val body = buildMultipartBody(boundary, "clip.mov", "video/quicktime", "movie".toByteArray()) @@ -226,7 +480,7 @@ class MediaUploadServerTest { body = body ) - // The delegate changed the format, so the uploader must receive the new + // The processor changed the format, so the uploader must receive the new // metadata — not the original video/quicktime + clip.mov. assertTrue(mockUploader.uploadCalled) assertEquals("video/mp4", mockUploader.lastUploadMimeType) @@ -234,11 +488,11 @@ class MediaUploadServerTest { } @Test - fun `deletes the delegate's processed file after upload`() { - val delegate = TranscodingDelegate() - val mockUploader = MockDefaultUploader() + fun `deletes the processor's processed file after upload`() { + val processor = TranscodingProcessor() + val mockUploader = MockInternalMediaClient() server.stop() - server = MediaUploadServer(uploadDelegate = delegate, defaultUploader = mockUploader, cacheDir = tempFolder.root) + server = MediaUploadServer(processor = processor, uploader = null, internalClient = mockUploader, cacheDir = tempFolder.root) val boundary = "test-boundary-cleanup" val body = buildMultipartBody(boundary, "clip.mov", "video/quicktime", "movie".toByteArray()) @@ -253,10 +507,10 @@ class MediaUploadServerTest { body = body ) - // The server owns the file the delegate produced and must delete it once the + // The server owns the file the processor produced and must delete it once the // upload finishes — the finally in processAndUpload covers success and throw // paths alike. A leaked processed file is a full-size temp per upload. - val processed = requireNotNull(delegate.producedFile) { "processFile was not called" } + val processed = requireNotNull(processor.producedFile) { "processFile was not called" } assertFalse("Processed temp file should be deleted after upload", processed.exists()) } @@ -276,8 +530,9 @@ class MediaUploadServerTest { // one — a flipped comparison would do the opposite and wipe an in-flight upload. server.stop() server = MediaUploadServer( - uploadDelegate = null, - defaultUploader = null, + processor = null, + uploader = null, + internalClient = MockInternalMediaClient(), cacheDir = tempFolder.root, ioDispatcher = Dispatchers.Unconfined ) @@ -286,15 +541,15 @@ class MediaUploadServerTest { assertTrue("Fresh temp should be preserved", fresh.exists()) } - // MARK: - Fallback to default uploader + // MARK: - Fallback to internal media client @Test - fun `uses passthrough when delegate does not modify file`() { - val delegate = ProcessOnlyDelegate() - val mockUploader = MockDefaultUploader() + fun `uses passthrough when the processor does not modify the file`() { + val processor = PassthroughProcessor() + val mockUploader = MockInternalMediaClient() server.stop() - server = MediaUploadServer(uploadDelegate = delegate, defaultUploader = mockUploader, cacheDir = tempFolder.root) + server = MediaUploadServer(processor = processor, uploader = null, internalClient = mockUploader, cacheDir = tempFolder.root) val boundary = "test-boundary-456" val body = buildMultipartBody(boundary, "doc.pdf", "application/pdf", "fake pdf data".toByteArray()) @@ -310,7 +565,7 @@ class MediaUploadServerTest { ) assertTrue("Expected 201 but got: ${response.statusLine}", response.statusLine.contains("201")) - assertTrue(delegate.processFileCalled) + assertTrue(processor.processFileCalled) // Passthrough: original body forwarded directly, not re-encoded. assertTrue(mockUploader.passthroughUploadCalled) assertFalse(mockUploader.uploadCalled) @@ -320,12 +575,12 @@ class MediaUploadServerTest { } @Test - fun `skips processing and the temp copy when the delegate declines by metadata`() { - val delegate = DeclineByMetadataDelegate() - val mockUploader = MockDefaultUploader() + fun `skips processing and the temp copy when the processor declines by metadata`() { + val processor = DecliningProcessor() + val mockUploader = MockInternalMediaClient() server.stop() - server = MediaUploadServer(uploadDelegate = delegate, defaultUploader = mockUploader, cacheDir = tempFolder.root) + server = MediaUploadServer(processor = processor, uploader = null, internalClient = mockUploader, cacheDir = tempFolder.root) val boundary = "test-boundary-decline" val body = buildMultipartBody(boundary, "clip.mov", "video/quicktime", "fake movie".toByteArray()) @@ -341,17 +596,17 @@ class MediaUploadServerTest { ) assertTrue("Expected 201 but got: ${response.statusLine}", response.statusLine.contains("201")) - // Declined by metadata → the delegate is never asked to process (so the + // Declined by metadata → the processor is never asked to process (so the // file was never materialized), and the upload is passed through directly. - assertFalse(delegate.processFileCalled) + assertFalse(processor.processFileCalled) assertTrue(mockUploader.passthroughUploadCalled) assertFalse(mockUploader.uploadCalled) } - // MARK: - DefaultMediaUploader + // MARK: - InternalMediaClient @Test - fun `DefaultMediaUploader relays the WordPress response`() { + fun `InternalMediaClient relays the WordPress response`() { val mockWpServer = MockWebServer() val wpBody = """{"id":1,"source_url":"https://example.com/u.jpg","media_type":"image"}""" @@ -364,7 +619,7 @@ class MediaUploadServerTest { mockWpServer.start() val wpBaseUrl = mockWpServer.url("/wp-json/").toString() - val uploader = DefaultMediaUploader( + val uploader = InternalMediaClient( httpClient = okhttp3.OkHttpClient(), siteApiRoot = wpBaseUrl, authHeader = "Bearer test-token" @@ -389,13 +644,13 @@ class MediaUploadServerTest { } @Test - fun `DefaultMediaUploader relays a WordPress error response instead of throwing`() { + fun `InternalMediaClient relays a WordPress error response instead of throwing`() { val mockWpServer = MockWebServer() mockWpServer.enqueue(MockResponse().setResponseCode(500).setBody("Internal error")) mockWpServer.start() val wpBaseUrl = mockWpServer.url("/wp-json/").toString() - val uploader = DefaultMediaUploader( + val uploader = InternalMediaClient( httpClient = okhttp3.OkHttpClient(), siteApiRoot = wpBaseUrl, authHeader = "Bearer test-token" @@ -414,12 +669,68 @@ class MediaUploadServerTest { } @Test - fun `DefaultMediaUploader normalizes an unslashed root and namespace`() { + fun `InternalMediaClient relays the upload attachment ID header`() { + // WordPress sets this header on an upload whose attachment row was + // created before metadata generation fataled. The editor reads it to + // retry post-process and clean up the orphan, so it must survive the + // relay. Unrelated upstream headers are not relayed. + val mockWpServer = MockWebServer() + mockWpServer.enqueue( + MockResponse() + .setResponseCode(500) + .setBody("""{"code":"rest_upload_error"}""") + .setHeader("x-wp-upload-attachment-id", "4242") + .setHeader("X-Powered-By", "PHP/8.2") + ) + mockWpServer.start() + + val uploader = InternalMediaClient( + httpClient = okhttp3.OkHttpClient(), + siteApiRoot = mockWpServer.url("/wp-json/").toString(), + authHeader = "Bearer test-token" + ) + + val file = tempFolder.newFile("orphan.jpg") + file.writeBytes("data".toByteArray()) + + val response = runBlocking { uploader.upload(file, "image/jpeg", "orphan.jpg", emptyList(), "") } + + assertEquals(500, response.statusCode) + assertEquals(mapOf("x-wp-upload-attachment-id" to "4242"), response.headers) + + mockWpServer.shutdown() + } + + @Test + fun `InternalMediaClient deletes an attachment carrying namespace and force query`() { + val mockWpServer = MockWebServer() + mockWpServer.enqueue(MockResponse().setResponseCode(200).setBody("""{"deleted":true}""")) + mockWpServer.start() + + val uploader = InternalMediaClient( + httpClient = okhttp3.OkHttpClient(), + siteApiRoot = mockWpServer.url("/wp-json/").toString(), + authHeader = "Bearer test-token", + siteApiNamespace = listOf("sites/123") + ) + + val response = runBlocking { uploader.deleteMedia("42", "?force=true") } + + assertEquals(200, response.statusCode) + val recorded = mockWpServer.takeRequest() + assertEquals("DELETE", recorded.method) + assertEquals("/wp-json/wp/v2/sites/123/media/42?force=true", recorded.path) + + mockWpServer.shutdown() + } + + @Test + fun `InternalMediaClient normalizes an unslashed root and namespace`() { val mockWpServer = MockWebServer() mockWpServer.enqueue(MockResponse().setResponseCode(201).setBody("{}")) mockWpServer.start() - val uploader = DefaultMediaUploader( + val uploader = InternalMediaClient( httpClient = okhttp3.OkHttpClient(), siteApiRoot = mockWpServer.url("/wp-json").toString(), // no trailing slash authHeader = "Bearer test-token", @@ -451,7 +762,7 @@ class MediaUploadServerTest { ) mockWpServer.start() - val uploader = DefaultMediaUploader( + val uploader = InternalMediaClient( httpClient = okhttp3.OkHttpClient(), siteApiRoot = mockWpServer.url("/wp-json/").toString(), authHeader = "Bearer test-token" @@ -479,12 +790,12 @@ class MediaUploadServerTest { } @Test - fun `DefaultMediaUploader re-encode preserves extra parts and query`() { + fun `InternalMediaClient re-encode preserves extra parts and query`() { val mockWpServer = MockWebServer() mockWpServer.enqueue(MockResponse().setResponseCode(201).setBody("{}")) mockWpServer.start() - val uploader = DefaultMediaUploader( + val uploader = InternalMediaClient( httpClient = okhttp3.OkHttpClient(), siteApiRoot = mockWpServer.url("/wp-json/").toString(), authHeader = "Bearer test-token" @@ -519,7 +830,7 @@ class MediaUploadServerTest { mockWpServer.enqueue(MockResponse().setResponseCode(201).setBody("{}")) mockWpServer.start() - val uploader = DefaultMediaUploader( + val uploader = InternalMediaClient( httpClient = okhttp3.OkHttpClient(), siteApiRoot = mockWpServer.url("/wp-json/").toString(), authHeader = "Bearer test-token" @@ -582,8 +893,22 @@ class MediaUploadServerTest { private data class RawHttpResponse( val statusLine: String, val headers: Map, - val body: String - ) + val body: String, + /** The header lines exactly as received, before collapsing into [headers]. */ + val rawHeaderLines: List = emptyList() + ) { + /** + * Every value sent for [name], in order. Unlike [headers], this preserves + * repeats — the only way to catch a header emitted twice. + */ + fun rawHeaderValues(name: String): List = + rawHeaderLines.mapNotNull { line -> + val colonIndex = line.indexOf(':') + if (colonIndex <= 0) return@mapNotNull null + if (!line.substring(0, colonIndex).trim().equals(name, ignoreCase = true)) return@mapNotNull null + line.substring(colonIndex + 1).trim() + } + } private fun sendRawRequest( method: String, @@ -638,16 +963,23 @@ class MediaUploadServerTest { } } - return RawHttpResponse(statusLine, responseHeaders, responseBody) + return RawHttpResponse(statusLine, responseHeaders, responseBody, lines.drop(1)) } private fun buildMultipartBody( boundary: String, filename: String, mimeType: String, - data: ByteArray + data: ByteArray, + fields: List = emptyList() ): ByteArray { val out = java.io.ByteArrayOutputStream() + for ((name, value) in fields) { + out.write("--$boundary\r\n".toByteArray()) + out.write("Content-Disposition: form-data; name=\"$name\"\r\n\r\n".toByteArray()) + out.write(value.toByteArray()) + out.write("\r\n".toByteArray()) + } out.write("--$boundary\r\n".toByteArray()) out.write("Content-Disposition: form-data; name=\"file\"; filename=\"$filename\"\r\n".toByteArray()) out.write("Content-Type: $mimeType\r\n\r\n".toByteArray()) @@ -658,27 +990,35 @@ class MediaUploadServerTest { // MARK: - Mocks - private class MockUploadDelegate : MediaUploadDelegate { - @Volatile var processFileCalled = false - @Volatile var uploadFileCalled = false + /** + * A host uploader: it performs the upload on its own stack. `upload` returns the + * finished attachment JSON (or throws). + */ + private class MockUploader( + private val uploadBody: ByteArray = + """{"id":42,"source_url":"https://example.com/photo.jpg","media_type":"image"}""".toByteArray(), + private val error: Exception? = null + ) : MediaUploader { + @Volatile var uploadCalled = false @Volatile var lastMimeType: String? = null @Volatile var lastFilename: String? = null + @Volatile var lastFields: List = emptyList() + @Volatile var lastQuery: String? = null + @Volatile var lastFileBytes: ByteArray? = null - override suspend fun processFile(file: File, mimeType: String, filename: String): ProcessedProxyFile { - processFileCalled = true - lastMimeType = mimeType - return ProcessedProxyFile.Original - } - - override suspend fun uploadFile(file: File, mimeType: String, filename: String): MediaUploadResponse? { - uploadFileCalled = true - lastFilename = filename - val json = """{"id":42,"source_url":"https://example.com/photo.jpg","media_type":"image"}""" - return MediaUploadResponse(201, json.toByteArray()) + override suspend fun upload(upload: MediaUpload): ByteArray { + uploadCalled = true + lastMimeType = upload.mimeType + lastFilename = upload.filename + lastFields = upload.fields + lastQuery = upload.query + lastFileBytes = upload.file.readBytes() + error?.let { throw it } + return uploadBody } } - private class ProcessOnlyDelegate : MediaUploadDelegate { + private class PassthroughProcessor : MediaProcessor { @Volatile var processFileCalled = false override suspend fun processFile(file: File, mimeType: String, filename: String): ProcessedProxyFile { @@ -691,7 +1031,7 @@ class MediaUploadServerTest { * Declines every file by metadata via [handlesFile], so the server must pass * through without materializing the file or calling [processFile]. */ - private class DeclineByMetadataDelegate : MediaUploadDelegate { + private class DecliningProcessor : MediaProcessor { @Volatile var processFileCalled = false override fun handlesFile(mimeType: String, filename: String): Boolean = false @@ -702,9 +1042,9 @@ class MediaUploadServerTest { } } - /** A delegate that produces a new file with changed metadata (e.g. a transcode). */ - private class TranscodingDelegate : MediaUploadDelegate { - /** The processed file this delegate wrote, for cleanup assertions. */ + /** A processor that produces a new file with changed metadata (e.g. a transcode). */ + private class TranscodingProcessor : MediaProcessor { + /** The processed file this processor wrote, for cleanup assertions. */ @Volatile var producedFile: File? = null override suspend fun processFile(file: File, mimeType: String, filename: String): ProcessedProxyFile { @@ -715,7 +1055,13 @@ class MediaUploadServerTest { } } - private class MockDefaultUploader : DefaultMediaUploader( + private class MockInternalMediaClient( + /** The response `upload`/`passthroughUpload` return. Defaults to a 201 success. */ + private val uploadResponse: MediaUploadResponse = MediaUploadResponse( + 201, + """{"id":99,"source_url":"https://example.com/doc.pdf","media_type":"file"}""".toByteArray() + ) + ) : InternalMediaClient( httpClient = okhttp3.OkHttpClient(), siteApiRoot = "https://example.com/wp-json/", authHeader = "Bearer mock" @@ -725,6 +1071,8 @@ class MediaUploadServerTest { @Volatile var lastUploadMimeType: String? = null @Volatile var lastUploadFilename: String? = null @Volatile var lastQuery: String? = null + @Volatile var deleteMediaCalled = false + @Volatile var deletedAttachmentId: String? = null override suspend fun upload( file: File, mimeType: String, filename: String, @@ -734,7 +1082,7 @@ class MediaUploadServerTest { lastUploadMimeType = mimeType lastUploadFilename = filename lastQuery = query - return mockResponse() + return uploadResponse } override suspend fun passthroughUpload( @@ -744,13 +1092,14 @@ class MediaUploadServerTest { ): MediaUploadResponse { passthroughUploadCalled = true lastQuery = query - return mockResponse() + return uploadResponse } - private fun mockResponse() = MediaUploadResponse( - 201, - """{"id":99,"source_url":"https://example.com/doc.pdf","media_type":"file"}""".toByteArray() - ) + override suspend fun deleteMedia(attachmentId: String, query: String): MediaUploadResponse { + deleteMediaCalled = true + deletedAttachmentId = attachmentId + return MediaUploadResponse(200, """{"deleted":true}""".toByteArray()) + } } } diff --git a/android/app/src/main/java/com/example/gutenbergkit/DemoMediaUploadDelegate.kt b/android/app/src/main/java/com/example/gutenbergkit/DemoMediaProcessor.kt similarity index 93% rename from android/app/src/main/java/com/example/gutenbergkit/DemoMediaUploadDelegate.kt rename to android/app/src/main/java/com/example/gutenbergkit/DemoMediaProcessor.kt index 572836e4c..ea7de6ca4 100644 --- a/android/app/src/main/java/com/example/gutenbergkit/DemoMediaUploadDelegate.kt +++ b/android/app/src/main/java/com/example/gutenbergkit/DemoMediaProcessor.kt @@ -5,19 +5,18 @@ import android.graphics.BitmapFactory import android.graphics.Matrix import android.media.ExifInterface import android.util.Log -import org.wordpress.gutenberg.MediaUploadDelegate +import org.wordpress.gutenberg.MediaProcessor import org.wordpress.gutenberg.ProcessedProxyFile import java.io.File import java.io.IOException /** - * Demo media upload delegate that resizes images to a maximum dimension of 2000px. - * - * Only overrides [processFile] — [uploadFile] returns null so the default uploader is used. + * Demo media processor that resizes images to a maximum dimension of 2000px, then + * lets GutenbergKit deliver the result to the configured site. */ -class DemoMediaUploadDelegate : MediaUploadDelegate { +class DemoMediaProcessor : MediaProcessor { companion object { - private const val TAG = "DemoMediaUploadDelegate" + private const val TAG = "DemoMediaProcessor" } // Only non-GIF images are ever resized (see processFile), so decline diff --git a/android/app/src/main/java/com/example/gutenbergkit/EditorActivity.kt b/android/app/src/main/java/com/example/gutenbergkit/EditorActivity.kt index 20f3e84b2..c2a48ac0f 100644 --- a/android/app/src/main/java/com/example/gutenbergkit/EditorActivity.kt +++ b/android/app/src/main/java/com/example/gutenbergkit/EditorActivity.kt @@ -338,7 +338,7 @@ fun EditorScreen( } }) if (enableNativeMediaUpload) { - mediaUploadDelegate = DemoMediaUploadDelegate() + mediaProcessor = DemoMediaProcessor() } onGutenbergViewCreated(this) } diff --git a/bin/wp-env-media-failure.sh b/bin/wp-env-media-failure.sh new file mode 100755 index 000000000..b4254cd61 --- /dev/null +++ b/bin/wp-env-media-failure.sh @@ -0,0 +1,141 @@ +#!/usr/bin/env bash +# +# wp-env-media-failure.sh — Read or set the media upload failure simulation mode. +# +# Drives the `gutenbergkit-media-failure` mu-plugin, which fatals during image +# sub-size generation so the editor's post-process retry can be exercised +# locally. See docs/code/local-wordpress.md. +# +# Usage: +# bash bin/wp-env-media-failure.sh # Report the current mode +# MODE=recover bash bin/wp-env-media-failure.sh # Fail once, then succeed +# MODE=always bash bin/wp-env-media-failure.sh # Fail every attempt +# MODE=off bash bin/wp-env-media-failure.sh # Normal uploads + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PROJECT_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +CREDENTIALS_FILE="$PROJECT_ROOT/.wp-env.credentials.json" + +# Addressed to localhost rather than the credentials file's siteUrl: after +# `make wp-env-android` that URL points at 10.0.2.2, which only resolves inside +# the emulator. The server is always reachable on localhost from the host. +ENDPOINT="http://localhost:8888/wp-json/gutenbergkit/v1/media-failure" + +VALID_MODES="off recover always" +MODE="${MODE:-}" + +# --------------------------------------------------------------------------- +# Validate the requested mode before touching the network +# --------------------------------------------------------------------------- + +if [ -n "$MODE" ]; then + case " $VALID_MODES " in + *" $MODE "*) ;; + *) + echo "Error: invalid MODE '$MODE'." >&2 + echo "Valid modes: $VALID_MODES" >&2 + exit 1 + ;; + esac +fi + +# --------------------------------------------------------------------------- +# Preconditions +# --------------------------------------------------------------------------- + +if [ ! -f "$CREDENTIALS_FILE" ]; then + echo "Error: credentials not found at $CREDENTIALS_FILE" >&2 + echo 'Run "make wp-env-start" to provision the local WordPress environment.' >&2 + exit 1 +fi + +# The path arrives as an argument rather than interpolated into the source: a +# checkout under a path containing a quote or backslash would otherwise produce +# a syntax error instead of the failure message below. +AUTH_HEADER=$(node -e ' + const fs = require("fs"); + try { + const c = JSON.parse(fs.readFileSync(process.argv[1], "utf8")); + if (!c.authHeader) process.exit(1); + process.stdout.write(c.authHeader); + } catch { + process.exit(1); + } +' "$CREDENTIALS_FILE") || { + echo "Error: could not read authHeader from $CREDENTIALS_FILE" >&2 + echo 'The file may be malformed. Run "make wp-env-start RESET=1" to regenerate it.' >&2 + exit 1 +} + +# --------------------------------------------------------------------------- +# Request +# --------------------------------------------------------------------------- + +if [ -n "$MODE" ]; then + METHOD="POST" + URL="$ENDPOINT?mode=$MODE" +else + METHOD="GET" + URL="$ENDPOINT" +fi + +# Separate the body from the status code so each failure can be reported with +# the fix that actually applies to it. +RESPONSE=$(curl -sS -X "$METHOD" -H "Authorization: $AUTH_HEADER" \ + -w $'\n%{http_code}' "$URL" 2>/dev/null) || { + echo "Error: could not reach wp-env at $ENDPOINT" >&2 + echo 'Run "make wp-env-start" to start the local WordPress environment.' >&2 + exit 1 +} + +STATUS="${RESPONSE##*$'\n'}" +BODY="${RESPONSE%$'\n'*}" + +case "$STATUS" in + 200) ;; + 401|403) + echo "Error: credentials were rejected (HTTP $STATUS)." >&2 + echo 'They are likely stale. Run "make wp-env-start RESET=1" to regenerate them.' >&2 + exit 1 + ;; + 404) + echo "Error: the media-failure endpoint is not registered (HTTP 404)." >&2 + echo 'Check that wp-env/mu-plugins/gutenbergkit-media-failure.php exists,' >&2 + echo 'then restart with "make wp-env-start".' >&2 + exit 1 + ;; + *) + echo "Error: unexpected response (HTTP $STATUS) from $ENDPOINT" >&2 + echo "$BODY" >&2 + exit 1 + ;; +esac + +CURRENT=$(echo "$BODY" | node -e " + let input = ''; + process.stdin.on('data', (chunk) => { input += chunk; }); + process.stdin.on('end', () => { + try { + process.stdout.write(JSON.parse(input).mode ?? 'unknown'); + } catch { + process.stdout.write('unknown'); + } + }); +") + +if [ -n "$MODE" ]; then + echo "Media upload failure mode set to: $CURRENT" +else + echo "Media upload failure mode: $CURRENT" +fi + +case "$CURRENT" in + recover) + echo "Uploads fail once per attachment, then succeed on the post-process retry." + ;; + always) + echo "Uploads fail every attempt, exhausting all five retries before the orphan is deleted." + ;; +esac diff --git a/docs/code/local-wordpress.md b/docs/code/local-wordpress.md index 694e4e03a..a8fac7eea 100644 --- a/docs/code/local-wordpress.md +++ b/docs/code/local-wordpress.md @@ -43,6 +43,7 @@ The `.wp-env.json` file at the project root configures the environment: - **Gutenberg plugin** is installed for the `/wp-block-editor/v1/settings` editor settings REST API endpoint. - **Jetpack plugin** is installed with the blocks module auto-activated via a mu-plugin (`wp-env/mu-plugins/gutenbergkit-jetpack-blocks.php`), providing the `/wpcom/v2/editor-assets` endpoint and additional editor blocks. - A **CORS mu-plugin** (`wp-env/mu-plugins/gutenbergkit-cors.php`) adds CORS headers to REST API responses, allowing requests from the Vite dev server, preview server, and native WebViews. +- A **media failure mu-plugin** (`wp-env/mu-plugins/gutenbergkit-media-failure.php`) can simulate a server-side image processing fatal on demand. Inactive by default — see [Simulating media upload failures](#simulating-media-upload-failures). - **WP_DEBUG** and **WP_DEBUG_LOG** are enabled for development. ### Credential Provisioning @@ -104,6 +105,34 @@ Physical devices cannot reach `localhost`. You'll need to: 2. Update the CORS mu-plugin to allow your IP. 3. Manually create or edit `.wp-env.credentials.json` with the correct IP-based URLs. +## Simulating Media Upload Failures + +When `wp_generate_attachment_metadata()` fatals server-side — commonly a PHP `memory_limit` or `max_execution_time` fatal on a large image — WordPress has already created the attachment row and sent an `X-WP-Upload-Attachment-ID` header. The editor reads that header off the resulting 5xx and retries `POST /wp/v2/media//post-process` up to five times, deleting the orphaned attachment if every attempt fails. + +`gutenbergkit-media-failure.php` reproduces that failure on demand so the retry can be exercised without a large image or a resource-starved host: + +```bash +# Report the current mode. +make wp-env-media-failure + +# Fail once per attachment, then succeed: the retry recovers the upload. +make wp-env-media-failure MODE=recover + +# Fail every time: exhausts all five retries, then the orphan is deleted. +make wp-env-media-failure MODE=always + +# Restore normal uploads. +make wp-env-media-failure MODE=off +``` + +The mode is stored server-side, so it persists across uploads and retries until changed. It does not survive restarting wp-env — the Playground runtime starts from a fresh database, so the mode returns to `off` (and credentials need `make wp-env-start RESET=1`). + +Then upload an image from a demo app and watch the network requests. In `recover` mode the upload 500s and the following `post-process` call succeeds, leaving a complete attachment; in `always` mode you should see five `post-process` attempts followed by a `DELETE`. + +**Only the native upload server path recovers locally.** Reading `X-WP-Upload-Attachment-ID` cross-origin requires the site to list it in `Access-Control-Expose-Headers`, and WordPress core's `rest_send_cors_headers()` does not. Uploads routed through the native upload server recover on both platforms, since that server exposes the header itself. + +A **direct** upload (native media upload disabled) never recovers on iOS, which loads the editor from `file://`. It does not recover against wp-env on Android either: `GutenbergView` derives the asset domain from the site's _host_, which drops the port, so the editor at `http://10.0.2.2` is cross-origin with the site at `http://10.0.2.2:8888`. Direct uploads are only same-origin — and therefore only recover — when the site runs on the scheme's default port, as production sites do. + ## WordPress Admin Access the WordPress admin dashboard at **http://localhost:8888/wp-admin/**: diff --git a/ios/Demo-iOS/Sources/Views/EditorView.swift b/ios/Demo-iOS/Sources/Views/EditorView.swift index 0f9b56ca4..c358db526 100644 --- a/ios/Demo-iOS/Sources/Views/EditorView.swift +++ b/ios/Demo-iOS/Sources/Views/EditorView.swift @@ -136,7 +136,7 @@ private struct _EditorView: UIViewControllerRepresentable { let viewController = EditorViewController(configuration: configuration, dependencies: dependencies) viewController.delegate = context.coordinator if enableNativeMediaUpload { - viewController.mediaUploadDelegate = context.coordinator + viewController.mediaProcessor = context.coordinator } viewController.webView.isInspectable = true @@ -189,7 +189,7 @@ private struct _EditorView: UIViewControllerRepresentable { } @MainActor - class Coordinator: NSObject, EditorViewControllerDelegate, MediaUploadDelegate { + class Coordinator: NSObject, EditorViewControllerDelegate, MediaProcessor { let viewModel: EditorViewModel init(viewModel: EditorViewModel) { @@ -295,11 +295,11 @@ private struct _EditorView: UIViewControllerRepresentable { return nil } - // MARK: - MediaUploadDelegate + // MARK: - MediaProcessor /// Only non-GIF images are ever resized (see `processFile`), so decline /// everything else by metadata — the server then skips copying a file - /// this delegate would only pass through. + /// this processor would only pass through. nonisolated func handlesFile(ofType mimeType: String, named _: String) -> Bool { mimeType.hasPrefix("image/") && mimeType != "image/gif" } diff --git a/ios/Sources/GutenbergKit/Sources/EditorViewController.swift b/ios/Sources/GutenbergKit/Sources/EditorViewController.swift index da4c1fefe..cd9cc5e53 100644 --- a/ios/Sources/GutenbergKit/Sources/EditorViewController.swift +++ b/ios/Sources/GutenbergKit/Sources/EditorViewController.swift @@ -105,52 +105,60 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro private let isWarmupMode: Bool /// Set once the editor has begun loading and captured its configuration - /// (including ``mediaUploadDelegate``). After this, that delegate can no longer - /// take effect, so its setter traps if written. + /// (including ``mediaProcessor`` and ``mediaUploader``). After this, they can no + /// longer take effect, so their setters trap if written. private var hasStartedLoading = false - /// Whether a non-nil ``mediaUploadDelegate`` was ever assigned. Lets the load - /// path tell "the delegate was released before load" (a retention mistake to - /// trap) apart from "no delegate was configured" (a valid opt-out). - private var mediaUploadDelegateWasAssigned = false + /// Transforms media (resize, transcode, …) before GutenbergKit delivers it to + /// the configured site. The safe, common extension point — a processor never + /// performs the upload itself, so it cannot deliver media to the wrong place. + /// + /// Provide this **before the editor loads** — typically right after `init`. It + /// is captured once, when the editor begins loading; setting it afterward has no + /// effect, so the setter traps. + /// + /// The editor **owns** this for its lifetime and releases it on `deinit`, so you + /// don't need to keep a reference after assigning it. The one rule: your processor + /// must not strongly retain this `EditorViewController` in return, or the two form + /// a retain cycle and neither is freed. + public var mediaProcessor: (any MediaProcessor)? { + didSet { + precondition(!hasStartedLoading, Self.lateMediaAssignmentMessage("mediaProcessor")) + } + } - /// Delegate for customizing media file processing and upload behavior. + /// Takes over media upload on the host's own stack (background session, offline + /// queue, resumable transport). Setting it makes the host own every upload and + /// its whole lifecycle; GutenbergKit stays out of the network entirely for media. /// - /// Provide this **before the editor loads** — typically right after `init`, the - /// same way the rest of the editor configuration is supplied. It is captured - /// once, when the editor begins loading, and injected into the page's initial - /// configuration; setting it afterward has no effect, so the setter traps. + /// Same lifecycle rules as ``mediaProcessor``: set it before the editor loads. + /// The editor owns it for its lifetime (releasing it on `deinit`), so you needn't + /// retain it yourself — just don't strongly retain this `EditorViewController` + /// from your uploader. /// - /// - Important: This is a `weak` reference — you must hold a strong reference to - /// your delegate until the editor has loaded, or native uploads are silently - /// disabled. To surface that mistake, the editor traps at load time if a - /// delegate that was assigned here has already been deallocated. - public weak var mediaUploadDelegate: (any MediaUploadDelegate)? { + /// Requires site credentials in the editor configuration: media deletes always + /// relay to the configured site, so an uploader set without an auth header is a + /// configuration error and traps at load. + public var mediaUploader: (any MediaUploader)? { didSet { - // Record whether a delegate was provided so the load path can tell a - // premature deallocation apart from a deliberate opt-out (see - // `startUploadServer`). - mediaUploadDelegateWasAssigned = mediaUploadDelegate != nil - // Deliberate fail-fast, not a defensive check. The delegate is captured - // into the page's initial configuration when the editor begins loading, - // so a delegate assigned afterward would silently never take effect; - // trapping surfaces that misuse loudly instead of failing quietly. - // - // `hasStartedLoading` flips at the start of the async load (see - // `loadEditor`), which runs at or after `viewDidLoad` — so this only - // *widens* the safe window versus a synchronous flip. A host that - // follows the documented contract (set right after `init`, before - // presenting) can never race it; the trap fires only on a genuinely - // late assignment. Do not soften this to a no-op or a log — silently - // dropping the delegate is exactly the failure this is here to catch. - precondition( - !hasStartedLoading, - "mediaUploadDelegate must be set before the editor loads (e.g. right after init). " - + "It is captured into the editor configuration at load; setting it afterward has no effect." - ) + precondition(!hasStartedLoading, Self.lateMediaAssignmentMessage("mediaUploader")) } } + /// Message for the fail-fast when media handling is assigned too late. + /// + /// Deliberate fail-fast, not a defensive check: the processor/uploader is + /// captured into the page's initial configuration when the editor begins + /// loading, so one assigned afterward would silently never take effect. + /// `hasStartedLoading` flips at the start of the async load, which runs at or + /// after `viewDidLoad`, so a host that follows the contract (set right after + /// `init`) can never race it. Do not soften this to a no-op or a log — silently + /// dropping the host's media handling is exactly the failure this catches. + private static func lateMediaAssignmentMessage(_ name: String) -> String { + "\(name) must be set before the editor loads (e.g. right after init). " + + "It is captured into the editor configuration at load; setting it afterward has no effect." + } + // MARK: - Private Properties (Services) private let editorService: EditorService private let httpClient: any EditorHTTPClientProtocol @@ -384,8 +392,8 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro /// @MainActor private func loadEditor(dependencies: EditorDependencies) async throws { - // From here on the editor configuration — including `mediaUploadDelegate` — - // is captured, so the delegate setter traps if written after this point. + // From here on the editor configuration — including `mediaProcessor` and + // `mediaUploader` — is captured, so their setters trap if written after this point. self.hasStartedLoading = true self.displayActivityView() @@ -451,28 +459,33 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro /// falls back to Gutenberg's default upload behavior (the JS override won't activate /// because `nativeUploadPort` will be nil in GBKit). private func startUploadServer() async { - // A delegate that was provided but is already nil here was deallocated before - // the editor finished loading — the host didn't hold a strong reference to it. - // That silently disables native uploads, so trap loudly instead. - precondition( - !(mediaUploadDelegateWasAssigned && mediaUploadDelegate == nil), - "mediaUploadDelegate was released before the editor loaded — hold a strong reference to it." - ) - - guard mediaUploadDelegate != nil else { + // Nothing to route through the native server unless the host provided a + // processor or an uploader. The editor owns whichever it was given — its + // `mediaProcessor`/`mediaUploader` are strong — so there's no + // released-before-load case to guard against; they live as long as it does. + guard mediaProcessor != nil || mediaUploader != nil else { return } - // The native upload server relays through DefaultMediaUploader, which needs a - // site root and an auth header (every host provides one — the editor injects - // it because the WebView has no auth cookies). Without both there is nothing - // to upload through, so leave the server down and let uploads fall to the - // default WebView path rather than start a server that could only fail. - guard !configuration.authHeader.isEmpty else { + // An InternalMediaClient does two jobs: it delivers GutenbergKit-owned + // uploads (when no `mediaUploader` is set), and it relays the editor's media + // DELETEs to the configured site — every attachment lives there, even one a + // host uploader delivered, so that's where its deletion goes. It needs both a + // site API root to address and an auth header (the editor injects the latter + // because the WebView has no auth cookies); with either missing, every media + // request it makes fails. + // + // `MediaServerCredentials` owns that check and the fail-fast behind it, so the + // policy is reachable from the host test suite — this file is not. + guard MediaServerCredentials.canStartServer( + siteApiRoot: configuration.siteApiRoot, + authHeader: configuration.authHeader, + hasUploader: mediaUploader != nil + ) else { return } - let defaultUploader = DefaultMediaUploader( + let internalClient = InternalMediaClient( httpClient: httpClient.uploadClient(), siteApiRoot: configuration.siteApiRoot, siteApiNamespace: configuration.siteApiNamespace @@ -480,8 +493,9 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro do { self.uploadServer = try await MediaUploadServer.start( - uploadDelegate: mediaUploadDelegate, - defaultUploader: defaultUploader + processor: mediaProcessor, + uploader: mediaUploader, + internalClient: internalClient ) } catch { Logger.uploadServer.error("Failed to start upload server: \(error). Falling back to default upload behavior.") diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaHandlers.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaHandlers.swift new file mode 100644 index 000000000..83d3a09c2 --- /dev/null +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaHandlers.swift @@ -0,0 +1,166 @@ +import Foundation + +/// A raw response from the WordPress REST API media endpoint. +/// +/// GutenbergKit relays this to the editor verbatim — it does not interpret the +/// body. The editor therefore receives the exact attachment object (on success) +/// or WordPress REST error object (on failure) it would get from a direct +/// upload, so every consumer — image sub-sizes, attachment links, error notices — +/// behaves identically to a non-native upload. +struct MediaUploadResponse: Sendable { + /// The HTTP status code WordPress returned. + let statusCode: Int + + /// The raw response body — a WordPress REST attachment on success, or a + /// WordPress REST error object (`{ "code", "message", "data" }`) on failure. + let body: Data + + /// The response headers to relay to the editor. + /// + /// `x-wp-upload-attachment-id` is the one that carries behavior: WordPress + /// sets it on a failed upload whose attachment row was created before + /// metadata generation fataled, and the editor's api-fetch middleware reads + /// it to retry `post-process` and clean up the orphan. Dropping it turns a + /// recoverable upload into a permanent failure. + let headers: [String: String] + + init(statusCode: Int, body: Data, headers: [String: String] = [:]) { + self.statusCode = statusCode + self.body = body + self.headers = headers + } +} + +/// The result of a ``MediaProcessor/processFile(at:mimeType:filename:)``. +public enum ProcessedProxyFile: Sendable { + /// The delegate did not modify the file; the original upload is forwarded + /// to WordPress unchanged. + case original + + /// The delegate produced a file to upload, along with its MIME type and + /// filename. Both are used verbatim, so a format change (e.g. transcoding + /// MOV to MP4, or an in-place EXIF strip) must report the resulting type and + /// filename for WordPress to store the file correctly. + case processed(URL, mimeType: String, filename: String) +} + +/// Transforms media before GutenbergKit delivers it. +/// +/// A processor only changes *bytes* — GutenbergKit still uploads the result to +/// the configured site and owns the whole lifecycle (retries, cleanup). Because +/// it never performs the upload itself, a processor cannot deliver media to the +/// wrong place. Set ``EditorViewController/mediaProcessor`` to resize images, +/// transcode video, strip EXIF, etc. +/// +/// This is the safe, common extension point: most hosts want only this. +public protocol MediaProcessor: AnyObject, Sendable { + /// Whether this processor might transform a file with the given metadata. + /// + /// A cheap, metadata-only gate the server consults *before* materializing the + /// upload to a temp file. Return `false` to pass a file straight through + /// untouched — e.g. an image-only processor returning `false` for a video — + /// so the server never copies a file the processor won't touch. + /// + /// Defaults to `true`. A `true` here is not a commitment — `processFile` may + /// still return `.original` after inspecting the file's contents. + func handlesFile(ofType mimeType: String, named filename: String) -> Bool + + /// Transform a file before upload (e.g., resize image, transcode video). + /// + /// Return ``ProcessedProxyFile/original`` to upload the file unchanged, or + /// ``ProcessedProxyFile/processed(_:mimeType:filename:)`` with the processed + /// file and its metadata. When the format changes, report the new mimeType + /// and filename so WordPress stores it with the correct extension and type. + func processFile(at url: URL, mimeType: String, filename: String) async throws -> ProcessedProxyFile +} + +/// One of the editor's non-file form fields, as sent with a media upload. +/// +/// A named type rather than a `(name, value)` tuple: tuples are not nominal, so a +/// tuple-typed property would permanently block `Equatable`/`Hashable`/`Codable` +/// synthesis on ``MediaUpload`` — including inside GutenbergKit, and not fixable +/// later without a source break for every host. +public struct MediaUploadField: Sendable, Hashable, Codable { + /// The field name, e.g. `post`. Not unique — a `field[]` array repeats it. + public let name: String + + /// The field's value, decoded as UTF-8. + public let value: String + + public init(name: String, value: String) { + self.name = name + self.value = value + } +} + +/// Everything a ``MediaUploader`` needs to reproduce a native upload: the file to +/// send, its metadata, the editor's non-file form fields, and the request's query. +public struct MediaUpload: Sendable { + /// The file to upload — already processed, if a ``MediaProcessor`` ran. + public let fileURL: URL + + /// The file's MIME type. + public let mimeType: String + + /// The file's name. + public let filename: String + + /// The editor's non-file form fields, in order, each decoded as UTF-8 — most + /// importantly `post`, the parent post's ID, without which the attachment is created + /// unattached. A list, not a dictionary, so repeated field names (e.g. a `field[]` + /// array) survive verbatim. Send each as a form part on your `POST /wp/v2/media`, + /// in the given order. + public let fields: [MediaUploadField] + + /// The request's query string (leading `?`, e.g. `?_embed=wp:featuredmedia`), or + /// empty. Carry it on your request so the editor gets the response it expects. + public let query: String +} + +/// Takes over *performing* a media upload — on the host's own stack: its own +/// networking (say, to log every request), a background session, an offline queue, +/// a resumable transport, its own retry policy. +/// +/// This is a choice of *who executes the requests*, not where they go: an uploader +/// and GutenbergKit's built-in default both target the same configured site. +/// Setting ``EditorViewController/mediaUploader`` makes the host own that upload +/// end-to-end — the request, its own retries, and its recovery and cleanup — with +/// GutenbergKit out of the network entirely. Because the host does the retries +/// itself, there's no raw response left for core to retry behind it. The attachment +/// you return lives on that same configured site, where the editor reads and +/// updates it by ID. +public protocol MediaUploader: AnyObject, Sendable { + /// Upload a (possibly processed) file and return the finished WordPress + /// attachment JSON the editor inserts — the same object a direct + /// `POST /wp/v2/media` returns. Return only once the upload is genuinely done, + /// or `throw` on terminal failure: a returned value is taken as a completed + /// attachment, and there is no GutenbergKit recovery behind you. + /// + /// The ``MediaUpload`` carries the file plus the editor's form fields (e.g. + /// `post`) and query — send them all so the created attachment matches a native + /// upload rather than landing as an unattached orphan. + /// + /// That recovery is yours to run. When `POST /wp/v2/media` fatals in server-side + /// post-processing it returns a 5xx carrying the attachment's ID in + /// `x-wp-upload-attachment-id` — the attachment exists but is unfinished. Don't + /// re-upload; drive `POST /wp/v2/media//post-process` to completion, the way + /// core recovers its own uploads (up to 5 attempts), then return the finished + /// attachment. + /// + /// Owning the upload means owning cleanup on the server too: if post-process + /// can't be recovered, force-delete the orphan + /// (`DELETE /wp/v2/media/?force=true`) before you `throw`, or it stays on the + /// site — neither GutenbergKit nor core cleans up behind you. + func upload(_ upload: MediaUpload) async throws -> Data +} + +/// Default implementations for the optional ``MediaProcessor`` methods. +extension MediaProcessor { + public func handlesFile(ofType mimeType: String, named filename: String) -> Bool { + true + } + + public func processFile(at url: URL, mimeType: String, filename: String) async throws -> ProcessedProxyFile { + .original + } +} diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaServerCredentials.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaServerCredentials.swift new file mode 100644 index 000000000..5ec9ee854 --- /dev/null +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaServerCredentials.swift @@ -0,0 +1,55 @@ +import Foundation + +/// Whether the editor configuration can reach the configured site for media, and the +/// fail-fast that enforces it. +/// +/// Deliberately outside `EditorViewController`. That type is `#if canImport(UIKit)`, +/// so on the macOS host it does not exist and nothing in it can be tested — including +/// this policy, which is a *crash* policy and already diverged silently between iOS +/// and Android once. Living here, it is reachable from the host test suite, where +/// Swift Testing's exit tests (unavailable on iOS/simulator) can assert the trap +/// itself rather than only the predicate. +enum MediaServerCredentials { + /// Whether an ``InternalMediaClient`` built from this configuration could actually + /// reach the site. + /// + /// Both fields are required. The client delivers GutenbergKit-owned uploads and + /// relays every media delete to the configured site, so it needs somewhere to send + /// them and credentials to be accepted; with either missing, every media request it + /// makes fails. + /// + /// `siteApiRoot` is a `URL` here where Android types it as a `String`, so the + /// equivalent of Android's `isEmpty()` check is "not absolute" — a URL with no + /// scheme or host cannot address the site, and every request built from it fails at + /// the URLSession layer. + static func areUsable(siteApiRoot: URL, authHeader: String) -> Bool { + siteApiRoot.scheme != nil && siteApiRoot.host() != nil && !authHeader.isEmpty + } + + /// Returns whether the upload server can start, trapping if the host set a + /// ``MediaUploader`` without usable credentials. + /// + /// The behavior forks by intent: + /// + /// - A ``MediaProcessor`` only enhances GutenbergKit-owned uploads. With no + /// credentials there is nothing to deliver through, so nothing to process — the + /// caller leaves the server down and uploads fall to the default WebView path. + /// + /// - A ``MediaUploader`` means the host is *taking over* uploads. Falling back would + /// silently drop it, and its media deletes still need the internal media client to + /// reach the configured site. A host that sets an uploader must provide credentials + /// too; omitting them is a configuration error, so trap rather than start a server + /// whose every delete would fail. (Matches Android's `check`.) + static func canStartServer(siteApiRoot: URL, authHeader: String, hasUploader: Bool) -> Bool { + if areUsable(siteApiRoot: siteApiRoot, authHeader: authHeader) { + return true + } + precondition( + !hasUploader, + "A mediaUploader needs site credentials so GutenbergKit can relay the " + + "editor's media deletes to the configured site. Set an absolute " + + "siteApiRoot and the auth header in the editor configuration." + ) + return false + } +} diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadDelegate.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadDelegate.swift deleted file mode 100644 index 56b1cf2b6..000000000 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadDelegate.swift +++ /dev/null @@ -1,90 +0,0 @@ -import Foundation - -/// A raw response from the WordPress REST API media endpoint. -/// -/// GutenbergKit relays this to the editor verbatim — it does not interpret the -/// body. The editor therefore receives the exact attachment object (on success) -/// or WordPress REST error object (on failure) it would get from a direct -/// upload, so every consumer — image sub-sizes, attachment links, error notices — -/// behaves identically to a non-native upload. -public struct MediaUploadResponse: Sendable { - /// The HTTP status code WordPress (or the host's upload service) returned. - public let statusCode: Int - - /// The raw response body — a WordPress REST attachment on success, or a - /// WordPress REST error object (`{ "code", "message", "data" }`) on failure. - public let body: Data - - public init(statusCode: Int, body: Data) { - self.statusCode = statusCode - self.body = body - } -} - -/// The result of a delegate's ``MediaUploadDelegate/processFile(at:mimeType:filename:)``. -public enum ProcessedProxyFile: Sendable { - /// The delegate did not modify the file; the original upload is forwarded - /// to WordPress unchanged. - case original - - /// The delegate produced a file to upload, along with its MIME type and - /// filename. Both are used verbatim, so a format change (e.g. transcoding - /// MOV to MP4, or an in-place EXIF strip) must report the resulting type and - /// filename for WordPress to store the file correctly. - case processed(URL, mimeType: String, filename: String) -} - -/// Protocol for customizing media upload behavior. -/// -/// The native host app can provide an implementation to resize images, -/// transcode video, or use its own upload service. Default implementations -/// pass files through unchanged and upload via the WordPress REST API. -public protocol MediaUploadDelegate: AnyObject, Sendable { - /// Whether this delegate might handle a file with the given metadata — either - /// processing it (``processFile(at:mimeType:filename:)``) or uploading it - /// itself (``uploadFile(at:mimeType:filename:)``). - /// - /// A cheap, metadata-only gate the server consults *before* materializing the - /// upload to a temp file. Return `false` to decline a file by type — e.g. an - /// image-only delegate returning `false` for a video — so the server forwards - /// the original upload to WordPress without first copying a file the delegate - /// won't touch. Because it gates the temp-file copy needed by *both* - /// `processFile` and `uploadFile`, return `true` for any file the delegate - /// will either process or upload itself. - /// - /// Defaults to `true`: every file is materialized and the full pipeline runs. - /// A `true` here is not a commitment — `processFile` may still return - /// `.original` after inspecting the file's contents. - func handlesFile(ofType mimeType: String, named filename: String) -> Bool - - /// Process a file before upload (e.g., resize image, transcode video). - /// - /// Return ``ProcessedProxyFile/original`` to upload the file unchanged, or - /// ``ProcessedProxyFile/processed(_:mimeType:filename:)`` with the processed - /// file and its metadata. When the format changes, report the new mimeType - /// and filename so WordPress stores it with the correct extension and type. - func processFile(at url: URL, mimeType: String, filename: String) async throws -> ProcessedProxyFile - - /// Upload a processed file to the remote WordPress site. - /// - /// Return the raw WordPress response (status code + body), which GutenbergKit - /// relays to the editor unchanged, or `nil` to use the default uploader. A - /// host that uploads to WordPress should return the exact response it - /// received so the editor sees a complete attachment object. - func uploadFile(at url: URL, mimeType: String, filename: String) async throws -> MediaUploadResponse? -} - -/// Default implementations. -extension MediaUploadDelegate { - public func handlesFile(ofType mimeType: String, named filename: String) -> Bool { - true - } - - public func processFile(at url: URL, mimeType: String, filename: String) async throws -> ProcessedProxyFile { - .original - } - - public func uploadFile(at url: URL, mimeType: String, filename: String) async throws -> MediaUploadResponse? { - nil - } -} diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift index cbab7d742..b0ac09f42 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift @@ -29,13 +29,16 @@ final class MediaUploadServer: Sendable { /// Creates and starts a new upload server. /// /// - Parameters: - /// - uploadDelegate: Optional delegate for customizing file processing and upload. - /// - defaultUploader: Fallback uploader used when no delegate provides `uploadFile`. + /// - processor: Optional processor for transforming files before upload. + /// - uploader: Optional uploader that takes over delivery on the host's own stack. + /// - internalClient: GutenbergKit's client for the configured site — delivers + /// GutenbergKit's own uploads (when no uploader is set) and relays every delete. /// - maxRequestBodySize: The maximum allowed request body size in bytes. /// Requests exceeding this limit receive a 413 response. Defaults to 4 GB. static func start( - uploadDelegate: (any MediaUploadDelegate)? = nil, - defaultUploader: DefaultMediaUploader? = nil, + processor: (any MediaProcessor)? = nil, + uploader: (any MediaUploader)? = nil, + internalClient: InternalMediaClient, maxRequestBodySize: Int64 = HTTPRequestParser.defaultMaxBodySize ) async throws -> MediaUploadServer { // Sweep temp files orphaned by a prior crash, off the editor-startup @@ -45,7 +48,7 @@ final class MediaUploadServer: Sendable { cleanOrphanedUploads() } - let context = UploadContext(uploadDelegate: uploadDelegate, defaultUploader: defaultUploader) + let handler = Handler(processor: processor, uploader: uploader, internalClient: internalClient) // A generous ceiling for receiving the upload body. The body read is // primarily bounded by the per-read idle timeout (which reaps a stalled @@ -62,9 +65,7 @@ final class MediaUploadServer: Sendable { bodyReadTimeout: bodyReadTimeout, cors: .permissive, delegate: ServerDelegate(), - handler: { request in - await Self.handleRequest(request, context: context) - } + handler: handler ) return MediaUploadServer(server: server, cleanupTask: cleanupTask) @@ -84,199 +85,334 @@ final class MediaUploadServer: Sendable { // MARK: - Request Handling - private static func handleRequest(_ request: HTTPServer.Request, context: UploadContext) async -> HTTPResponse { - let parsed = request.parsed + /// Serves each upload request, holding the media collaborators it needs. + /// + /// A value type, and deliberately separate from ``MediaUploadServer``: the handler + /// is captured by the `HTTPServer` that the server itself owns, so a handler that + /// *was* the server would form `MediaUploadServer -> HTTPServer -> handler -> + /// MediaUploadServer` and `deinit` would never run `stop()`. A struct can't + /// participate in a reference cycle, so that question doesn't arise — and its + /// dependencies are stored properties, so the request methods are ordinary + /// instance methods rather than statics threading a context through every call. + /// + /// Everything is held strongly. `EditorViewController` owns `mediaProcessor` / + /// `mediaUploader` strongly too, so a host object retaining the view controller + /// already cycles through the view controller's own properties — something this + /// handler can neither create nor prevent. Holding weak here bought no leak + /// protection, only the risk of a reference vanishing mid-request. Immutable + /// strong references also make the admission gate and delivery agree by + /// construction. (Matches Android, which holds plain `val`s.) + private struct Handler: HTTPRequestHandler { + let processor: (any MediaProcessor)? + let uploader: (any MediaUploader)? + let internalClient: InternalMediaClient + + func handle(_ request: HTTPServer.Request) async -> HTTPResponse { + let parsed = request.parsed + + // Routes: POST /upload, and DELETE /media/ for the editor's orphan + // cleanup. (OPTIONS preflight is answered by the HTTP library under its + // permissive CORS policy.) Match on the path alone — the target carries + // a query string (e.g. `?_embed`, `?force=true`) relayed to WordPress. + let method = parsed.method.uppercased() + + if method == "POST", parsed.path == "/upload" { + return await handleUpload(request) + } + + if method == "DELETE", let attachmentId = Self.attachmentId(fromPath: parsed.path) { + return await handleMediaDelete(attachmentId, query: parsed.query) + } - // Route: only POST /upload is handled. (OPTIONS preflight is answered by - // the HTTP library under its permissive CORS policy.) Match on the path - // alone — the target carries a query string (e.g. `?_embed`) that the - // upload handler relays on to WordPress. - guard parsed.method.uppercased() == "POST", parsed.path == "/upload" else { - return errorResponse(status: 404, message: "Not found") + return MediaUploadServer.errorResponse(status: 404, message: "Not found") } - return await handleUpload(request, context: context) - } + private func handleUpload(_ request: HTTPServer.Request) async -> HTTPResponse { + let parts: [MultipartPart] + do { + parts = try request.parsed.multipartParts() + } catch { + Logger.uploadServer.error("Multipart parse failed: \(error)") + return MediaUploadServer.errorResponse(status: 400, message: "Expected multipart/form-data") + } - private static func handleUpload(_ request: HTTPServer.Request, context: UploadContext) async -> HTTPResponse { - let parts: [MultipartPart] - do { - parts = try request.parsed.multipartParts() - } catch { - Logger.uploadServer.error("Multipart parse failed: \(error)") - return errorResponse(status: 400, message: "Expected multipart/form-data") - } + // Find the file part (the first part with a filename). + guard let filePart = parts.first(where: { $0.filename != nil }) else { + return MediaUploadServer.errorResponse(status: 400, message: "No file found in request") + } - // Find the file part (the first part with a filename). - guard let filePart = parts.first(where: { $0.filename != nil }) else { - return errorResponse(status: 400, message: "No file found in request") - } + // The non-file parts (post, additionalData) and the original query + // (e.g. ?_embed) must reach WordPress too — relay them alongside the file. + let extraParts = parts.filter { $0.filename == nil } + let query = request.parsed.query + + let filename = filePart.filename ?? "upload" + let mimeType = filePart.contentType + + // Materialize a temp file only if someone will touch it: a processor that + // claims this file, or an uploader (which always delivers the file itself). + // If GutenbergKit will deliver (no uploader) and no processor wants the + // file, forward the original request body directly, skipping a temp copy of + // a file nobody will process (e.g. a video handed to an image-only processor). + let processorWantsFile = processor?.handlesFile(ofType: mimeType, named: filename) ?? false + if uploader == nil, !processorWantsFile { + do { + return try await passthroughResponse(request, query: query) + } catch { + return Self.uploadErrorResponse(error) + } + } + + // The delegate wants the file. Stream the part body to a dedicated temp + // file for it — the library's RequestBody may be a byte-range slice of a + // larger temp file whose lifecycle is tied to ARC, so the delegate needs a + // standalone file that outlives the handler return. + let tempDir = MediaUploadServer.uploadsTempDirectory + try? FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) - // The non-file parts (post, additionalData) and the original query - // (e.g. ?_embed) must reach WordPress too — relay them alongside the file. - let extraParts = parts.filter { $0.filename == nil } - let query = request.parsed.query + let fileURL = tempDir.appending(component: "\(UUID().uuidString)-\(MediaUploadServer.sanitizeFilename(filename))") + do { + let inputStream = try filePart.body.makeInputStream() + try MediaUploadServer.writeStream(inputStream, to: fileURL) + } catch { + try? FileManager.default.removeItem(at: fileURL) + Logger.uploadServer.error("Failed to write upload to disk: \(error)") + return MediaUploadServer.errorResponse(status: 500, message: "Failed to save file") + } - let filename = filePart.filename ?? "upload" - let mimeType = filePart.contentType + // From here on always clean up the original temp file. The processed + // file (if the delegate produced a new one) is cleaned up inside + // processAndUpload so its throw paths are covered too. + defer { try? FileManager.default.removeItem(at: fileURL) } - // Ask the delegate — from metadata alone — whether it will touch a file - // like this. If not, forward the original upload to WordPress directly, - // skipping a full temp-file copy of a file the delegate won't process or - // upload (e.g. a video handed to an image-only delegate). - guard context.uploadDelegate?.handlesFile(ofType: mimeType, named: filename) ?? false else { do { - return try await passthroughResponse(request, query: query, context: context) + let uploadResult = try await processAndUpload( + fileURL: fileURL, mimeType: mimeType, filename: filename, + extraParts: extraParts, query: query, + processorWantsFile: processorWantsFile + ) + switch uploadResult { + case .uploaded(let uploaded): + Logger.uploadServer.debug("Uploaded file to WordPress") + return Self.relayResponse(uploaded) + case .passthrough: + // Delegate didn't modify the file — forward the original request + // body to WordPress without re-encoding. + return try await passthroughResponse(request, query: query) + } } catch { - return uploadErrorResponse(error) + return Self.uploadErrorResponse(error) } } - // The delegate wants the file. Stream the part body to a dedicated temp - // file for it — the library's RequestBody may be a byte-range slice of a - // larger temp file whose lifecycle is tied to ARC, so the delegate needs a - // standalone file that outlives the handler return. - let tempDir = uploadsTempDirectory - try? FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) - - let fileURL = tempDir.appending(component: "\(UUID().uuidString)-\(sanitizeFilename(filename))") - do { - let inputStream = try filePart.body.makeInputStream() - try writeStream(inputStream, to: fileURL) - } catch { - try? FileManager.default.removeItem(at: fileURL) - Logger.uploadServer.error("Failed to write upload to disk: \(error)") - return errorResponse(status: 500, message: "Failed to save file") + /// Forwards the original request body to WordPress unchanged (no multipart + /// re-encoding) and relays the response. Used on the no-uploader path when the + /// processor won't touch the file — it declined by metadata (`handlesFile` + /// returned false) or `processFile` returned `.original`. + private func passthroughResponse( + _ request: HTTPServer.Request, query: String + ) async throws -> HTTPResponse { + // As in `processAndUpload`: don't put bytes on the wire for a torn-down + // editor, regardless of whether the HTTP client honors cancellation. + try Task.checkCancellation() + + Logger.uploadServer.debug("Passthrough: forwarding original request body to WordPress") + guard let body = request.parsed.body, + let contentType = request.parsed.header("Content-Type") else { + return MediaUploadServer.errorResponse(status: 500, message: "Passthrough upload requires a request body and Content-Type") + } + let response = try await internalClient.passthroughUpload(body: body, contentType: contentType, query: query) + return Self.relayResponse(response) } - // From here on always clean up the original temp file. The processed - // file (if the delegate produced a new one) is cleaned up inside - // processAndUpload so its throw paths are covered too. - defer { try? FileManager.default.removeItem(at: fileURL) } + /// The attachment ID in a `/media/` path, or `nil` if the path is not one. + /// + /// Deliberately narrow: this server relays media operations, not arbitrary + /// REST requests, so only a numeric attachment ID under `/media/` matches. + private static func attachmentId(fromPath path: String) -> String? { + let components = path.split(separator: "/", omittingEmptySubsequences: true) + guard components.count == 2, components[0] == "media" else { return nil } + let id = String(components[1]) + guard !id.isEmpty, id.allSatisfy(\.isNumber) else { return nil } + return id + } - do { - let uploadResult = try await processAndUpload( - fileURL: fileURL, mimeType: mimeType, filename: filename, - extraParts: extraParts, query: query, context: context - ) - switch uploadResult { - case .uploaded(let uploaded): - Logger.uploadServer.debug("Uploaded file to WordPress") - return relayResponse(uploaded) - case .passthrough: - // Delegate didn't modify the file — forward the original request - // body to WordPress without re-encoding. - return try await passthroughResponse(request, query: query, context: context) + /// Relays a media deletion. + /// + /// The editor deletes an attachment when the user removes it, and core deletes + /// an upload's orphan when every `post-process` retry fails. A cross-origin + /// editor cannot issue `DELETE` directly — api-fetch tunnels it as a `POST` + /// carrying `X-HTTP-Method-Override`, which core's CORS allow-list omits, so the + /// browser blocks it at preflight; relaying it here lets the deletion run. + /// + /// Every attachment lives on the configured site — even one a host uploader + /// delivered — so its deletion is relayed to the internal media client there. See + /// the accepted-risk note in the body. + private func handleMediaDelete( + _ attachmentId: String, query: String + ) async -> HTTPResponse { + do { + // Relay to the internal media client (the configured site) — every attachment + // lives there, even one a host uploader delivered. Core issues this only + // as orphan cleanup after failed recovery, but the relay can't tell that + // from any other DELETE the WebView sends: a compromised editor script + // holding the loopback token could force-delete arbitrary media on the + // configured site. Accepted risk — such a script already has broad write + // access, and a server-side compromise (a malicious plugin) deletes media + // directly without the editor, so scoping this with a per-session ledger + // buys little for the cost. + let response = try await internalClient.deleteMedia(attachmentId: attachmentId, query: query) + return Self.relayResponse(response) + } catch { + return Self.uploadErrorResponse(error) } - } catch { - return uploadErrorResponse(error) } - } - /// Forwards the original request body to WordPress unchanged (no multipart - /// re-encoding) and relays the response. Used when the delegate won't touch - /// the file — it declined by metadata (`handlesFile` returned false) or - /// `processFile` returned `.original`. - private static func passthroughResponse( - _ request: HTTPServer.Request, query: String, context: UploadContext - ) async throws -> HTTPResponse { - Logger.uploadServer.debug("Passthrough: forwarding original request body to WordPress") - guard let body = request.parsed.body, - let contentType = request.parsed.header("Content-Type"), - let defaultUploader = context.defaultUploader else { - return errorResponse(status: 500, message: UploadError.noUploader.localizedDescription) + /// Relays WordPress's exact status, body, and relayable headers to the editor + /// so it sees the same attachment object (or error) as a direct upload. + /// + /// The headers matter for recovery: `x-wp-upload-attachment-id` is what lets + /// the editor retry `post-process` for an upload whose metadata generation + /// fataled server-side, rather than surfacing a permanent failure and + /// leaving an orphaned attachment behind. + /// + /// The relayed body is always WordPress REST JSON — an attachment, or a + /// `{code, message, data}` error — so the response is always `application/json`. + /// The relayed headers are a content-type-free allowlist (`relayableHeaderNames`), + /// so prepending the JSON default never collides with them. + private static func relayResponse(_ response: MediaUploadResponse) -> HTTPResponse { + return HTTPResponse( + status: response.statusCode, + headers: [("Content-Type", "application/json")] + response.headers.map { ($0.key, $0.value) }, + body: response.body + ) } - let response = try await defaultUploader.passthroughUpload(body: body, contentType: contentType, query: query) - return relayResponse(response) - } - - /// Relays WordPress's exact status and body to the editor so it sees the same - /// attachment object (or error) as a direct upload. - private static func relayResponse(_ response: MediaUploadResponse) -> HTTPResponse { - HTTPResponse( - status: response.statusCode, - headers: [("Content-Type", "application/json")], - body: response.body - ) - } - /// Builds the 500 response for a failed upload. A cancelled connection task - /// (editor abort / server stop) surfaces here too — as CancellationError or - /// URLError.cancelled — but isn't a failure and the server closes the - /// connection without sending this response (see HTTPServer's cancellation - /// check), so log that quietly. - private static func uploadErrorResponse(_ error: any Error) -> HTTPResponse { - if Task.isCancelled { - Logger.uploadServer.debug("Upload cancelled") - } else { - Logger.uploadServer.error("Upload processing failed: \(error)") + /// Builds the 500 response for a failed upload. A cancelled connection task + /// (editor abort / server stop) surfaces here too — as CancellationError or + /// URLError.cancelled — but isn't a failure and the server closes the + /// connection without sending this response (see HTTPServer's cancellation + /// check), so log that quietly. + private static func uploadErrorResponse(_ error: any Error) -> HTTPResponse { + if Task.isCancelled { + Logger.uploadServer.debug("Upload cancelled") + } else { + Logger.uploadServer.error("Upload processing failed: \(error)") + } + return MediaUploadServer.errorResponse(status: 500, message: error.localizedDescription) } - return errorResponse(status: 500, message: error.localizedDescription) - } - // MARK: - Delegate Pipeline - - /// Result of the delegate processing + upload pipeline. - private enum UploadResult { - /// The delegate (or default uploader) completed the upload; carries the - /// raw WordPress response to relay. - case uploaded(MediaUploadResponse) - /// The delegate didn't modify the file and `uploadFile` returned nil. - /// The caller should forward the original request body to WordPress. - case passthrough - } + // MARK: - Delegate Pipeline - private static func processAndUpload( - fileURL: URL, mimeType: String, filename: String, - extraParts: [MultipartPart], query: String, context: UploadContext - ) async throws -> UploadResult { - // Step 1: Process (resize, transcode, etc.) - let processed: ProcessedProxyFile - if let delegate = context.uploadDelegate { - processed = try await delegate.processFile(at: fileURL, mimeType: mimeType, filename: filename) - } else { - processed = .original + /// Result of the process + deliver pipeline. + private enum UploadResult { + /// The uploader or internal media client completed the upload; carries the + /// response to relay. + case uploaded(MediaUploadResponse) + /// No uploader is set and the processor left the file unmodified, so the + /// caller should forward the original request body to the configured site. + case passthrough } - // Resolve the file to upload and its metadata. `.processed` uses the - // delegate's values verbatim, so a format change is reported to WordPress. - let uploadURL: URL - let uploadMimeType: String - let uploadFilename: String - switch processed { - case .original: - uploadURL = fileURL - uploadMimeType = mimeType - uploadFilename = filename - case let .processed(url, processedMimeType, processedFilename): - uploadURL = url - uploadMimeType = processedMimeType - uploadFilename = processedFilename - } + private func processAndUpload( + fileURL: URL, mimeType: String, filename: String, + extraParts: [MultipartPart], query: String, + processorWantsFile: Bool + ) async throws -> UploadResult { + // Step 1: transform (resize, transcode, …) if a processor claims the file. + // Reuse the gate's `handlesFile` decision from `handleUpload` rather than + // asking again — one metadata call per upload, and the admit and transform + // steps can't disagree. + let processed: ProcessedProxyFile + if processorWantsFile, let processor { + processed = try await processor.processFile(at: fileURL, mimeType: mimeType, filename: filename) + } else { + processed = .original + } + + // Resolve the file to upload and its metadata. `.processed` uses the + // processor's values verbatim, so a format change is reported to WordPress. + let uploadURL: URL + let uploadMimeType: String + let uploadFilename: String + switch processed { + case .original: + uploadURL = fileURL + uploadMimeType = mimeType + uploadFilename = filename + case let .processed(url, processedMimeType, processedFilename): + uploadURL = url + uploadMimeType = processedMimeType + uploadFilename = processedFilename + } - // The processed file (if the delegate produced a new one) is ours to - // clean up — on success it has been uploaded, on failure it is abandoned. - // Cleaning up here rather than in the caller covers the throw paths too. - defer { - if uploadURL != fileURL { - try? FileManager.default.removeItem(at: uploadURL) + // The processed file (if the processor produced a new one) is ours to + // clean up — on success it has been uploaded, on failure it is abandoned. + // Cleaning up here rather than in the caller covers the throw paths too. + defer { + if uploadURL != fileURL { + try? FileManager.default.removeItem(at: uploadURL) + } + } + + // The editor was torn down (or the client disconnected) while we processed. + // Don't start an outbound upload whose response nobody will read — it would + // create an attachment neither GutenbergKit nor the host knows to clean up. + // Checking here rather than relying on the HTTP client to notice cancellation + // keeps this true for a host-injected `URLSessionProtocol` that doesn't. + try Task.checkCancellation() + + // Step 2: deliver. An uploader owns delivery on the host's own stack and + // returns the finished attachment JSON (or throws); GutenbergKit relays that + // as a success and never runs its own recovery behind it. Otherwise the + // internal media client delivers to the configured site. + if let uploader { + // Hand the host the editor's non-file fields (e.g. `post`) and query too, + // so its own POST can reproduce a native upload — otherwise the attachment + // is created unattached and `?_embed` is lost. + let fields = try await Self.formFields(from: extraParts) + let upload = MediaUpload( + fileURL: uploadURL, mimeType: uploadMimeType, filename: uploadFilename, + fields: fields, query: query + ) + let body = try await uploader.upload(upload) + return .uploaded(MediaUploadResponse(statusCode: 201, body: body)) + } else { + // Unmodified — forward the original request body directly, skipping + // multipart re-encoding. + if case .original = processed { + return .passthrough + } + let result = try await internalClient.upload(fileURL: uploadURL, mimeType: uploadMimeType, filename: uploadFilename, extraParts: extraParts, query: query) + return .uploaded(result) } } - // Step 2: Upload to remote WordPress - if let delegate = context.uploadDelegate, - let result = try await delegate.uploadFile(at: uploadURL, mimeType: uploadMimeType, filename: uploadFilename) { - return .uploaded(result) - } else if let defaultUploader = context.defaultUploader { - // Unmodified — forward the original request body directly, skipping - // multipart re-encoding. - if case .original = processed { - return .passthrough + /// Decodes the editor's non-file form parts (e.g. `post`, additionalData) into an + /// ordered list of name/value pairs for a host uploader, so it can send them on its + /// own `POST /wp/v2/media`. A list, not a dictionary, so repeated field names survive + /// in order. + /// + /// The UTF-8 decode is lossless here because of an invariant worth stating, since + /// nothing in the type system enforces it: **the only client is the editor's + /// browser `FormData`.** The server binds to loopback behind a per-session token, + /// so nothing else can reach it; a `FormData` string value is a `USVString`, which + /// the browser has already made well-formed at `append` time; and its only way to + /// carry arbitrary bytes is a Blob, which always gets a filename and is therefore + /// filtered out of `extraParts` by `handleUpload`. Valid UTF-8 — including emoji + /// and any non-Latin script — round-trips exactly, so real captions and titles are + /// unaffected. + /// + /// If that ever stops holding, this decode starts substituting U+FFFD *and* the two + /// platforms disagree about how: for `ED A0 80`, Swift's maximal-subpart rule yields + /// three replacement characters where Java's decoder yields one. `MediaUploadServerTests` + /// pins the partition that keeps binary parts out of here. + private static func formFields(from parts: [MultipartPart]) async throws -> [MediaUploadField] { + var fields: [MediaUploadField] = [] + for part in parts { + fields.append(MediaUploadField(name: part.name, value: String(decoding: try await part.body.data, as: UTF8.self))) } - let result = try await defaultUploader.upload(fileURL: uploadURL, mimeType: uploadMimeType, filename: uploadFilename, extraParts: extraParts, query: query) - return .uploaded(result) - } else { - throw UploadError.noUploader + return fields } } @@ -387,47 +523,21 @@ final class MediaUploadServer: Sendable { /// Errors from the native media upload pipeline. enum UploadError: Error, LocalizedError { - case noUploader case streamReadFailed case streamWriteFailed var errorDescription: String? { switch self { - case .noUploader: "No upload delegate or default uploader configured" case .streamReadFailed: "Failed to read upload stream" case .streamWriteFailed: "Failed to write upload to disk" } } } -// MARK: - Upload Context - -/// Container for the upload delegate and default uploader, captured by the -/// HTTPServer handler closure and re-read on each request. -/// -/// The delegate is held **weakly**. `EditorViewController.mediaUploadDelegate` is -/// declared `weak` — the host owns the delegate's lifetime. Capturing it strongly -/// here would silently defeat that contract and, worse, risk a retain cycle -/// (`EditorViewController → uploadServer → HTTPServer → handler → UploadContext → -/// delegate → EditorViewController`) that would keep the view controller — and -/// therefore the server — alive forever, so `deinit` would never stop it. -/// -/// `@unchecked Sendable`: `uploadDelegate` is assigned once at init and only read -/// afterwards; weak-reference reads are thread-safe at runtime. -private final class UploadContext: @unchecked Sendable { - weak var uploadDelegate: (any MediaUploadDelegate)? - let defaultUploader: DefaultMediaUploader? - - init(uploadDelegate: (any MediaUploadDelegate)?, defaultUploader: DefaultMediaUploader?) { - self.uploadDelegate = uploadDelegate - self.defaultUploader = defaultUploader - } -} - // MARK: - Default Media Uploader /// Uploads files to the WordPress REST API using site credentials from EditorConfiguration. -class DefaultMediaUploader: @unchecked Sendable { +class InternalMediaClient: @unchecked Sendable { private let httpClient: EditorHTTPClientProtocol private let siteApiRoot: URL private let siteApiNamespace: String? @@ -441,8 +551,9 @@ class DefaultMediaUploader: @unchecked Sendable { /// The WordPress media endpoint URL, built through the shared /// ``WordPressRESTURL`` namespacing (so it matches every other REST URL) and /// carrying the original request query (e.g. `?_embed`) through to WordPress. - private func mediaEndpointURL(query: String) -> URL { - let base = WordPressRESTURL.namespaced(apiRoot: siteApiRoot, path: "/wp/v2/media", namespace: siteApiNamespace) + private func mediaEndpointURL(query: String, attachmentId: String? = nil) -> URL { + let path = attachmentId.map { "/wp/v2/media/\($0)" } ?? "/wp/v2/media" + let base = WordPressRESTURL.namespaced(apiRoot: siteApiRoot, path: path, namespace: siteApiNamespace) guard !query.isEmpty else { return base } // `query` is the raw request query in wire form (leading "?"). Set it via // `percentEncodedQuery` so a value that isn't URL-safe can't make @@ -489,6 +600,23 @@ class DefaultMediaUploader: @unchecked Sendable { return try await performUpload(request) } + /// Deletes an attachment, relaying WordPress's response verbatim. + /// + /// Carries the editor's query through unchanged — core's cleanup sends + /// `?force=true`, without which WordPress trashes rather than deletes. + func deleteMedia(attachmentId: String, query: String) async throws -> MediaUploadResponse { + var request = URLRequest(url: mediaEndpointURL(query: query, attachmentId: attachmentId)) + request.httpMethod = "DELETE" + + // Authorization is applied centrally by the HTTP client, as for uploads. + let (data, response) = try await httpClient.performRaw(request) + return MediaUploadResponse( + statusCode: response.statusCode, + body: data, + headers: Self.relayableHeaders(from: response) + ) + } + /// Sends the assembled upload request to WordPress and relays the response. /// /// The request body is a **one-shot** stream (a bound-pair pipe for the @@ -517,7 +645,30 @@ class DefaultMediaUploader: @unchecked Sendable { // the editor sees WordPress's real status and error body, exactly as a // direct upload would. `performRaw` does not throw on non-2xx. let (data, response) = try await httpClient.performRaw(request) - return MediaUploadResponse(statusCode: response.statusCode, body: data) + return MediaUploadResponse( + statusCode: response.statusCode, + body: data, + headers: Self.relayableHeaders(from: response) + ) + } + + /// The upstream headers worth relaying to the editor. + /// + /// Deliberately an allowlist rather than a filtered passthrough: the body is + /// re-sent with a recomputed length, so relaying upstream entity or + /// transport headers wholesale would risk contradicting what the server + /// actually sends. + private static let relayableHeaderNames = ["x-wp-upload-attachment-id"] + + /// Picks the headers to relay out of an upstream response. + private static func relayableHeaders(from response: HTTPURLResponse) -> [String: String] { + var headers: [String: String] = [:] + for name in relayableHeaderNames { + if let value = response.value(forHTTPHeaderField: name) { + headers[name] = value + } + } + return headers } // MARK: - Streaming Multipart Body @@ -538,9 +689,16 @@ class DefaultMediaUploader: @unchecked Sendable { ) throws -> (InputStream, Int) { // Serialize the non-file parts (post, additionalData) into the preamble // ahead of the streamed file. They are small, so keeping them in memory is - // fine; `contentLength` counts them via `preamble.count`. Field values are - // appended as raw bytes (not through String) so a non-UTF-8 value is - // forwarded verbatim rather than coerced to empty. + // fine; `contentLength` counts them via `preamble.count`. + // + // Field values are appended as raw bytes rather than round-tripped through + // `String`. This is not because malformed values are expected — they can't + // reach here; see the invariant on `formFields`. It is because the failable + // `String(data:encoding:)` returns nil on invalid UTF-8, and the obvious + // `?? ""` behind it would silently drop a whole field's value. Appending the + // bytes keeps this re-encode byte-identical to the passthrough it stands in + // for, so a user's upload doesn't change shape just because a processor + // resized the image. var preamble = Data() for field in extraFields { preamble.append(Data("--\(boundary)\r\n".utf8)) diff --git a/ios/Sources/GutenbergKitHTTP/CORSPolicy.swift b/ios/Sources/GutenbergKitHTTP/CORSPolicy.swift index 26524d11e..642053b1e 100644 --- a/ios/Sources/GutenbergKitHTTP/CORSPolicy.swift +++ b/ios/Sources/GutenbergKitHTTP/CORSPolicy.swift @@ -33,6 +33,12 @@ public enum CORSPolicy: Sendable { ("Access-Control-Allow-Origin", "*"), ("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS"), ("Access-Control-Allow-Headers", "Authorization, Relay-Authorization, Content-Type"), + // Only CORS-safelisted response headers are readable + // cross-origin by default. The editor needs to read + // `x-wp-upload-attachment-id` off a relayed media upload + // response to retry `post-process` and clean up an orphaned + // attachment, so it has to be exposed explicitly. + ("Access-Control-Expose-Headers", "x-wp-upload-attachment-id"), ("Access-Control-Max-Age", "86400"), ] } diff --git a/ios/Sources/GutenbergKitHTTP/HTTPRequestHandler.swift b/ios/Sources/GutenbergKitHTTP/HTTPRequestHandler.swift new file mode 100644 index 000000000..2caa266af --- /dev/null +++ b/ios/Sources/GutenbergKitHTTP/HTTPRequestHandler.swift @@ -0,0 +1,35 @@ +#if canImport(Network) + +import Foundation + +/// Serves requests for an ``HTTPServer``. +/// +/// The closure form of +/// ``HTTPServer/start(name:port:listenOnAllInterfaces:requiresAuthentication:maxRequestBodySize:maxConnections:readTimeout:bodyReadTimeout:idleTimeout:startTimeout:cors:delegate:handler:)-(_,_,_,_,_,_,_,_,_,_,_,_,@escaping@Sendable(HTTPServer.Request)async->HTTPResponse)`` +/// is the right tool for a handler that needs no state. Conform to this instead when +/// the handler has dependencies: they become stored properties, and the request +/// methods become ordinary instance methods rather than statics threading a context +/// parameter through every call. +/// +/// ## Lifetimes +/// +/// The server retains its handler for its lifetime, and the handler must not be the +/// object that owns the server: `owner → HTTPServer → handler → owner` is a cycle, +/// so the owner's `deinit` would never run and `stop()` would never be called. +/// +/// This protocol is deliberately **not** `AnyObject`-constrained, because the +/// straightforward way to avoid that is a `struct` handler holding the dependencies +/// it needs. A value type cannot participate in a reference cycle at all, so the +/// question doesn't arise. A `final class` conformer is fine too — just keep it a +/// leaf, the same discipline ``HTTPServerDelegate`` documents. +public protocol HTTPRequestHandler: Sendable { + /// The response for a request the server has parsed and authenticated. + /// + /// Called once per request, concurrently across connections — hence `Sendable`. + /// Cancellation is cooperative: the server cancels this task when the client + /// disconnects or the server stops, and discards whatever a cancelled task + /// returns, so check `Task.isCancelled` before any side effect you can't undo. + func handle(_ request: HTTPServer.Request) async -> HTTPResponse +} + +#endif // canImport(Network) diff --git a/ios/Sources/GutenbergKitHTTP/HTTPServer.swift b/ios/Sources/GutenbergKitHTTP/HTTPServer.swift index ac05fbb01..0f0a56f54 100644 --- a/ios/Sources/GutenbergKitHTTP/HTTPServer.swift +++ b/ios/Sources/GutenbergKitHTTP/HTTPServer.swift @@ -277,6 +277,46 @@ public final class HTTPServer: Sendable { } } + /// Starts a server that serves requests from an ``HTTPRequestHandler`` object + /// rather than a closure. + /// + /// Everything else behaves identically — this forwards to the closure form. Reach + /// for it when the handler has dependencies to hold: a `struct` conformer stores + /// them and serves from instance methods, instead of statics threading a context + /// parameter through every call. See ``HTTPRequestHandler`` for the (short) + /// lifetime rules. + public static func start( + name: String, + port: UInt16? = nil, + listenOnAllInterfaces: Bool = false, + requiresAuthentication: Bool = true, + maxRequestBodySize: Int64 = HTTPRequestParser.defaultMaxBodySize, + maxConnections: Int = HTTPServer.defaultMaxConnections, + readTimeout: Duration = HTTPServer.defaultReadTimeout, + bodyReadTimeout: Duration? = nil, + idleTimeout: Duration = HTTPServer.defaultIdleTimeout, + startTimeout: Duration = HTTPServer.defaultStartTimeout, + cors: CORSPolicy = .none, + delegate: HTTPServerDelegate? = nil, + handler: some HTTPRequestHandler + ) async throws -> HTTPServer { + try await start( + name: name, + port: port, + listenOnAllInterfaces: listenOnAllInterfaces, + requiresAuthentication: requiresAuthentication, + maxRequestBodySize: maxRequestBodySize, + maxConnections: maxConnections, + readTimeout: readTimeout, + bodyReadTimeout: bodyReadTimeout, + idleTimeout: idleTimeout, + startTimeout: startTimeout, + cors: cors, + delegate: delegate, + handler: { await handler.handle($0) } + ) + } + /// Races `operation` against `timeout`, throwing ``HTTPServerError/startTimeout`` /// if the timeout wins. Used to bound the wait for the listener to become ready /// so a caller — such as the editor load awaiting the upload server's bind — diff --git a/ios/Sources/GutenbergKitHTTP/README.md b/ios/Sources/GutenbergKitHTTP/README.md index 43721b305..42629aec0 100644 --- a/ios/Sources/GutenbergKitHTTP/README.md +++ b/ios/Sources/GutenbergKitHTTP/README.md @@ -46,6 +46,24 @@ server.stop() Pass `nil` (or omit `port`) to let the system assign an available port — useful for tests or when running multiple servers. +#### Handlers with state + +A closure is right for a handler that needs no state. When the handler has dependencies, conform a type to `HTTPRequestHandler` and pass it as `handler:` instead — the dependencies become stored properties and the request logic becomes instance methods, rather than statics threading a context parameter through every call. + +```swift +struct MediaHandler: HTTPRequestHandler { + let uploader: Uploader + + func handle(_ request: HTTPServer.Request) async -> HTTPResponse { + await uploader.upload(request.parsed.body) + } +} + +let server = try await HTTPServer.start(name: "media", handler: MediaHandler(uploader: uploader)) +``` + +The server retains its handler, so the handler must not be the object that owns the server — `owner → HTTPServer → handler → owner` is a cycle, and the owner's `deinit` would never run. `HTTPRequestHandler` is deliberately not `AnyObject`-constrained so a `struct` conformer sidesteps this entirely; a `final class` works too, as long as it stays a leaf. + When `requiresAuthentication` is enabled (the default), each request must include a `Proxy-Authorization: Bearer ` header carrying the server's randomly-generated token. The server uses `Proxy-Authorization` per RFC 9110 §11.7.1 rather than `Authorization`, so the client's `Authorization` header remains available for upstream credentials (e.g. HTTP Basic auth to the remote server). Unauthenticated requests receive a `407 Proxy Authentication Required` response with a `Proxy-Authenticate: Bearer` challenge header. ### Proxying via URLSession diff --git a/ios/Tests/GutenbergKitHTTPTests/HTTPServerStartTests.swift b/ios/Tests/GutenbergKitHTTPTests/HTTPServerStartTests.swift index 17be7d603..dda06a80e 100644 --- a/ios/Tests/GutenbergKitHTTPTests/HTTPServerStartTests.swift +++ b/ios/Tests/GutenbergKitHTTPTests/HTTPServerStartTests.swift @@ -38,6 +38,35 @@ struct HTTPServerStartTests { // rather than suspending its caller indefinitely. #expect(elapsed < .seconds(5)) } + + @Test("serves requests from an HTTPRequestHandler object, carrying its state") + func servesFromRequestHandlerObject() async throws { + // The point of the object overload: the handler holds its dependencies as + // stored properties and serves from an instance method, so a consumer with + // state doesn't need statics threading a context through every call. + let server = try await HTTPServer.start( + name: "handler-object-test", + requiresAuthentication: false, + handler: EchoHandler(greeting: "hello from a struct") + ) + defer { server.stop() } + + let url = URL(string: "http://127.0.0.1:\(server.port)/anything")! + let (data, response) = try await URLSession.shared.data(from: url) + + #expect((response as? HTTPURLResponse)?.statusCode == 200) + #expect(String(decoding: data, as: UTF8.self) == "hello from a struct") + } +} + +/// A value-type handler — it cannot form a reference cycle back to whatever owns +/// the server, which is why ``HTTPRequestHandler`` isn't `AnyObject`-constrained. +private struct EchoHandler: HTTPRequestHandler { + let greeting: String + + func handle(_ request: HTTPServer.Request) async -> HTTPResponse { + HTTPResponse(status: 200, body: Data(greeting.utf8)) + } } #endif diff --git a/ios/Tests/GutenbergKitTests/Media/EditorMediaHandlerOwnershipTests.swift b/ios/Tests/GutenbergKitTests/Media/EditorMediaHandlerOwnershipTests.swift new file mode 100644 index 000000000..0c685e85a --- /dev/null +++ b/ios/Tests/GutenbergKitTests/Media/EditorMediaHandlerOwnershipTests.swift @@ -0,0 +1,61 @@ +import Foundation +import Testing + +@testable import GutenbergKit + +#if canImport(UIKit) + +/// The editor takes **strong** ownership of the media handlers it is given, so a host +/// can assign one and immediately drop its own reference. These pin that ownership: if +/// the properties regressed to `weak`, the handler would deallocate the moment the host +/// released it and the expectations below would fail. +/// +/// The mirror invariant — that the upload *server* holds them **weakly**, so it can't +/// form a retain cycle back through the view controller — lives in +/// `MediaUploadServerTests.doesNotStronglyRetainProcessor`. +@Suite("Editor media-handler ownership") +struct EditorMediaHandlerOwnershipTests: MakesTestFixtures { + static let testSiteURL = URL(string: "https://test.example.com")! + static let testApiRoot = URL(string: "https://test.example.com/wp-json/wp/v2")! + + @MainActor + @Test("the editor retains its mediaProcessor after the host releases it") + func editorRetainsMediaProcessor() { + let editor = EditorViewController(configuration: makeConfiguration()) + weak var weakProcessor: OwnershipTestProcessor? + do { + let processor = OwnershipTestProcessor() + weakProcessor = processor + editor.mediaProcessor = processor + } + withExtendedLifetime(editor) { + #expect(weakProcessor != nil, "the editor must own its mediaProcessor for its lifetime") + } + } + + @MainActor + @Test("the editor retains its mediaUploader after the host releases it") + func editorRetainsMediaUploader() { + let editor = EditorViewController(configuration: makeConfiguration()) + weak var weakUploader: OwnershipTestUploader? + do { + let uploader = OwnershipTestUploader() + weakUploader = uploader + editor.mediaUploader = uploader + } + withExtendedLifetime(editor) { + #expect(weakUploader != nil, "the editor must own its mediaUploader for its lifetime") + } + } +} + +private final class OwnershipTestProcessor: MediaProcessor, @unchecked Sendable { + func handlesFile(ofType mimeType: String, named filename: String) -> Bool { false } + func processFile(at url: URL, mimeType: String, filename: String) async throws -> ProcessedProxyFile { .original } +} + +private final class OwnershipTestUploader: MediaUploader, @unchecked Sendable { + func upload(_ upload: MediaUpload) async throws -> Data { Data() } +} + +#endif diff --git a/ios/Tests/GutenbergKitTests/Media/MediaServerCredentialsTests.swift b/ios/Tests/GutenbergKitTests/Media/MediaServerCredentialsTests.swift new file mode 100644 index 000000000..18d51ba82 --- /dev/null +++ b/ios/Tests/GutenbergKitTests/Media/MediaServerCredentialsTests.swift @@ -0,0 +1,72 @@ +import Foundation +import Testing + +@testable import GutenbergKit + +/// The upload server's start policy: both a site API root and an auth header are +/// required, and a `mediaUploader` set without them is a configuration error rather +/// than a silent fallback. +/// +/// These run on the **host**, which is the point of `MediaServerCredentials` being +/// outside `EditorViewController` — that type is UIKit-gated and so untestable here, +/// and Swift Testing's exit tests (which is how the trap below is asserted) are +/// unavailable on iOS and the simulator. The Android counterparts live in +/// `GutenbergViewUploadServerTest`. +@Suite("Media server credentials") +struct MediaServerCredentialsTests { + static let apiRoot = URL(string: "https://example.com/wp-json/")! + + @Test("both a site API root and an auth header are usable") + func bothPresentIsUsable() { + #expect(MediaServerCredentials.areUsable(siteApiRoot: Self.apiRoot, authHeader: "Bearer t")) + } + + @Test("an empty auth header is not usable") + func emptyAuthHeaderIsNotUsable() { + #expect(!MediaServerCredentials.areUsable(siteApiRoot: Self.apiRoot, authHeader: "")) + } + + @Test("a relative site API root is not usable") + func relativeSiteApiRootIsNotUsable() { + // The iOS analogue of Android's `siteApiRoot.isEmpty()`. A host can't pass "", + // because the type is `URL` — but it can pass one with no scheme or host, which + // is just as unusable: every request built from it fails at the URLSession layer. + #expect(!MediaServerCredentials.areUsable(siteApiRoot: URL(string: "/wp-json/")!, authHeader: "Bearer t")) + } + + @Test("a scheme without a host is not usable") + func schemeWithoutHostIsNotUsable() { + #expect(!MediaServerCredentials.areUsable(siteApiRoot: URL(string: "https:///wp-json/")!, authHeader: "Bearer t")) + } + + @Test("a processor without credentials leaves the server down rather than trapping") + func processorWithoutCredentialsDoesNotTrap() { + // The other half of the fork: a processor only enhances GutenbergKit-owned + // uploads, so with nothing to deliver through there's nothing to process. The + // caller leaves the server down and uploads fall to the default WebView path. + #expect(!MediaServerCredentials.canStartServer(siteApiRoot: Self.apiRoot, authHeader: "", hasUploader: false)) + } + + @Test("credentials present means the server can start") + func credentialsPresentCanStart() { + #expect(MediaServerCredentials.canStartServer(siteApiRoot: Self.apiRoot, authHeader: "Bearer t", hasUploader: true)) + } + + @Test("an uploader without an auth header traps") + func uploaderWithoutAuthHeaderTraps() async { + await #expect(processExitsWith: .failure) { + _ = MediaServerCredentials.canStartServer( + siteApiRoot: MediaServerCredentialsTests.apiRoot, authHeader: "", hasUploader: true + ) + } + } + + @Test("an uploader without a usable site API root traps") + func uploaderWithoutSiteApiRootTraps() async { + await #expect(processExitsWith: .failure) { + _ = MediaServerCredentials.canStartServer( + siteApiRoot: URL(string: "/wp-json/")!, authHeader: "Bearer t", hasUploader: true + ) + } + } +} diff --git a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift index 97a300733..e6dc21cca 100644 --- a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift +++ b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift @@ -9,7 +9,7 @@ private let _canStartUploadServer: Bool = { let semaphore = DispatchSemaphore(value: 0) Task { do { - let server = try await MediaUploadServer.start() + let server = try await MediaUploadServer.start(internalClient: MockInternalMediaClient()) server.stop() result.value = true } catch { @@ -34,7 +34,7 @@ struct MediaUploadServerTests { @Test("starts and provides a port and token") func startAndStop() async throws { - let server = try await MediaUploadServer.start() + let server = try await MediaUploadServer.start(internalClient: MockInternalMediaClient()) #expect(server.port > 0) #expect(!server.token.isEmpty) server.stop() @@ -42,7 +42,7 @@ struct MediaUploadServerTests { @Test("rejects requests without auth token") func rejectsUnauthenticated() async throws { - let server = try await MediaUploadServer.start() + let server = try await MediaUploadServer.start(internalClient: MockInternalMediaClient()) defer { server.stop() } let url = URL(string: "http://127.0.0.1:\(server.port)/upload")! @@ -56,7 +56,7 @@ struct MediaUploadServerTests { @Test("rejects requests with wrong token") func rejectsWrongToken() async throws { - let server = try await MediaUploadServer.start() + let server = try await MediaUploadServer.start(internalClient: MockInternalMediaClient()) defer { server.stop() } let url = URL(string: "http://127.0.0.1:\(server.port)/upload")! @@ -71,7 +71,7 @@ struct MediaUploadServerTests { @Test("responds to OPTIONS preflight with CORS headers") func corsPreflightResponse() async throws { - let server = try await MediaUploadServer.start() + let server = try await MediaUploadServer.start(internalClient: MockInternalMediaClient()) defer { server.stop() } let url = URL(string: "http://127.0.0.1:\(server.port)/upload")! @@ -87,7 +87,7 @@ struct MediaUploadServerTests { @Test("returns 404 for unknown paths") func unknownPath() async throws { - let server = try await MediaUploadServer.start() + let server = try await MediaUploadServer.start(internalClient: MockInternalMediaClient()) defer { server.stop() } let url = URL(string: "http://127.0.0.1:\(server.port)/unknown")! @@ -102,9 +102,9 @@ struct MediaUploadServerTests { @Test("routes /upload with a query string and relays the query") func uploadWithQueryString() async throws { - let delegate = ProcessOnlyDelegate() - let mockUploader = MockDefaultUploader() - let server = try await MediaUploadServer.start(uploadDelegate: delegate, defaultUploader: mockUploader) + let processor = PassthroughProcessor() + let mockUploader = MockInternalMediaClient() + let server = try await MediaUploadServer.start(processor: processor, internalClient: mockUploader) defer { server.stop() } // `@wordpress/media-utils` uploads to `/wp/v2/media?_embed=wp:featuredmedia`, @@ -123,7 +123,7 @@ struct MediaUploadServerTests { let (_, response) = try await URLSession.shared.data(for: request) let httpResponse = try #require(response as? HTTPURLResponse) #expect(httpResponse.statusCode == 201) - // The delegate returns `.original`, so this is the passthrough branch. + // The processor returns `.original`, so this is the passthrough branch. // Pin which branch ran — `lastQuery` is recorded by both, so without this // the query assertion would pass even if routing collapsed onto one path. #expect(mockUploader.passthroughUploadCalled) @@ -131,17 +131,64 @@ struct MediaUploadServerTests { #expect(mockUploader.lastQuery == "?_embed=wp:featuredmedia") } - @Test("calls delegate and returns upload result") - func delegateProcessAndUpload() async throws { - let delegate = MockUploadDelegate() - let server = try await MediaUploadServer.start(uploadDelegate: delegate) + @Test("relays a deletion to the internal media client even when an uploader owns uploads") + func deletesGoToInternalClientNotUploader() async throws { + // An attachment lives on the configured site even when a host uploader delivered + // it, so its deletion goes to the internal media client — the host uploader owns + // uploads, not deletes. Held strongly: UploadContext keeps the uploader weakly. + let uploader = MockUploader() + let internalClient = MockInternalMediaClient() + let server = try await MediaUploadServer.start(uploader: uploader, internalClient: internalClient) + defer { server.stop() } + + let url = URL(string: "http://127.0.0.1:\(server.port)/media/42?force=true")! + var request = URLRequest(url: url) + request.httpMethod = "DELETE" + request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + + let (_, response) = try await URLSession.shared.data(for: request) + let httpResponse = try #require(response as? HTTPURLResponse) + + #expect(httpResponse.statusCode == 200) + #expect(internalClient.deleteMediaCalled) + #expect(internalClient.deletedAttachmentId == "42") + } + + @Test("relays a deletion to the internal media client (configured site)") + func relaysDeleteToInternalClient() async throws { + // With no uploader set, GutenbergKit owns deletes: core's orphan cleanup DELETE + // is relayed to the internal media client (the configured site). + let mockUploader = MockInternalMediaClient() + let server = try await MediaUploadServer.start(internalClient: mockUploader) + defer { server.stop() } + + let url = URL(string: "http://127.0.0.1:\(server.port)/media/512?force=true")! + var request = URLRequest(url: url) + request.httpMethod = "DELETE" + request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + let (_, response) = try await URLSession.shared.data(for: request) + + #expect((response as? HTTPURLResponse)?.statusCode == 200) + #expect(mockUploader.deleteMediaCalled) + #expect(mockUploader.deletedAttachmentId == "512") + } + + @Test("routes an upload to the uploader and relays its attachment") + func uploaderDeliversAttachment() async throws { + // With an uploader set, GutenbergKit hands it the file and relays the finished + // attachment it returns — the internal media client (configured site) is never used. + let uploader = MockUploader() + let internalClient = MockInternalMediaClient() + let server = try await MediaUploadServer.start(uploader: uploader, internalClient: internalClient) defer { server.stop() } let boundary = UUID().uuidString - let fileData = "fake image data".data(using: .utf8)! - let body = buildMultipartBody(boundary: boundary, filename: "photo.jpg", mimeType: "image/jpeg", data: fileData) + let body = buildMultipartBody( + boundary: boundary, filename: "photo.jpg", mimeType: "image/jpeg", + data: Data("fake image data".utf8), fields: [MediaUploadField(name: "post", value: "123")] + ) - let url = URL(string: "http://127.0.0.1:\(server.port)/upload")! + let url = URL(string: "http://127.0.0.1:\(server.port)/upload?_embed=wp:featuredmedia")! var request = URLRequest(url: url) request.httpMethod = "POST" request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") @@ -152,12 +199,20 @@ struct MediaUploadServerTests { let httpResponse = try #require(response as? HTTPURLResponse) #expect(httpResponse.statusCode == 201) - #expect(delegate.processFileCalled) - #expect(delegate.uploadFileCalled) - #expect(delegate.lastMimeType == "image/jpeg") - #expect(delegate.lastFilename == "photo.jpg") - - // The server relays WordPress's raw response body verbatim. + #expect(uploader.uploadCalled) + #expect(uploader.lastMimeType == "image/jpeg") + #expect(uploader.lastFilename == "photo.jpg") + // The editor's post association and query must reach the host uploader, so it can + // reproduce a native upload (attach to the post, honor ?_embed). + #expect(uploader.lastFields.first { $0.name == "post" }?.value == "123") + #expect(uploader.lastQuery == "?_embed=wp:featuredmedia") + // …and the actual file bytes the editor sent — the host uploads them itself. + #expect(uploader.lastFileData == Data("fake image data".utf8)) + // The host owns delivery — GutenbergKit must not upload to the configured site. + #expect(!internalClient.uploadCalled) + #expect(!internalClient.passthroughUploadCalled) + + // The server relays the exact attachment JSON the uploader returned. let object = try JSONSerialization.jsonObject(with: data) let json = try #require(object as? [String: Any]) #expect(json["id"] as? Int == 42) @@ -165,11 +220,99 @@ struct MediaUploadServerTests { #expect(json["media_type"] as? String == "image") } - @Test("uses passthrough when delegate does not modify file") - func delegatePassthrough() async throws { - let delegate = ProcessOnlyDelegate() - let mockUploader = MockDefaultUploader() - let server = try await MediaUploadServer.start(uploadDelegate: delegate, defaultUploader: mockUploader) + @Test("hands a host uploader repeated form field names in order, not collapsed") + func uploaderReceivesRepeatedFieldNames() async throws { + // A `field[]`-style repeated name (e.g. a custom attachment taxonomy): WordPress + // builds an array from these, so both values must reach the host uploader in order. + // A dictionary would drop the first — the ordered-list contract must not. + let uploader = MockUploader() + let internalClient = MockInternalMediaClient() + let server = try await MediaUploadServer.start(uploader: uploader, internalClient: internalClient) + defer { server.stop() } + + let boundary = UUID().uuidString + let body = buildMultipartBody( + boundary: boundary, filename: "photo.jpg", mimeType: "image/jpeg", + data: Data("fake image data".utf8), + fields: [ + MediaUploadField(name: "post", value: "123"), + MediaUploadField(name: "media_folder[]", value: "12"), + MediaUploadField(name: "media_folder[]", value: "45"), + ] + ) + + let url = URL(string: "http://127.0.0.1:\(server.port)/upload")! + var request = URLRequest(url: url) + request.httpMethod = "POST" + request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") + request.httpBody = body + + _ = try await URLSession.shared.data(for: request) + + #expect(uploader.uploadCalled) + // Both repeated values survive, in order — not collapsed to the last. + let folderValues = uploader.lastFields.filter { $0.name == "media_folder[]" }.map(\.value) + #expect(folderValues == ["12", "45"]) + #expect(uploader.lastFields.first { $0.name == "post" }?.value == "123") + } + + @Test("hands the processed file and its new metadata to the uploader") + func uploaderReceivesProcessedFile() async throws { + // A processor transcodes the file; the host uploader must receive the processed + // bytes and the new metadata, not the original clip.mov. + let processor = ResizingProcessor() + let uploader = MockUploader() + let server = try await MediaUploadServer.start( + processor: processor, uploader: uploader, internalClient: MockInternalMediaClient() + ) + defer { server.stop() } + + let boundary = UUID().uuidString + let body = buildMultipartBody(boundary: boundary, filename: "clip.mov", mimeType: "video/quicktime", data: Data("movie".utf8)) + + let url = URL(string: "http://127.0.0.1:\(server.port)/upload")! + var request = URLRequest(url: url) + request.httpMethod = "POST" + request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") + request.httpBody = body + + _ = try await URLSession.shared.data(for: request) + + #expect(uploader.uploadCalled) + #expect(uploader.lastFileData == Data("processed".utf8)) + #expect(uploader.lastMimeType == "video/mp4") + #expect(uploader.lastFilename == "clip.mp4") + } + + @Test("relays a 500 when the host uploader throws") + func uploaderErrorRelayedAs500() async throws { + struct UploaderFailure: Error {} + let uploader = MockUploader(error: UploaderFailure()) + let server = try await MediaUploadServer.start(uploader: uploader, internalClient: MockInternalMediaClient()) + defer { server.stop() } + + let boundary = UUID().uuidString + let body = buildMultipartBody(boundary: boundary, filename: "photo.jpg", mimeType: "image/jpeg", data: Data("fake image data".utf8)) + + let url = URL(string: "http://127.0.0.1:\(server.port)/upload")! + var request = URLRequest(url: url) + request.httpMethod = "POST" + request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") + request.httpBody = body + + let (_, response) = try await URLSession.shared.data(for: request) + #expect((response as? HTTPURLResponse)?.statusCode == 500) + #expect(uploader.uploadCalled) + } + + @Test("uses passthrough when the processor does not modify the file") + func processorPassthrough() async throws { + let processor = PassthroughProcessor() + let mockUploader = MockInternalMediaClient() + let server = try await MediaUploadServer.start(processor: processor, internalClient: mockUploader) defer { server.stop() } let boundary = UUID().uuidString @@ -187,7 +330,7 @@ struct MediaUploadServerTests { let httpResponse = try #require(response as? HTTPURLResponse) #expect(httpResponse.statusCode == 201) - #expect(delegate.processFileCalled) + #expect(processor.processFileCalled) // Passthrough: original body forwarded directly, not re-encoded. #expect(mockUploader.passthroughUploadCalled) #expect(!mockUploader.uploadCalled) @@ -198,11 +341,11 @@ struct MediaUploadServerTests { #expect(json["id"] as? Int == 99) } - @Test("skips processing and the temp copy when the delegate declines by metadata") - func delegateDeclinesByMetadata() async throws { - let delegate = DeclineByMetadataDelegate() - let mockUploader = MockDefaultUploader() - let server = try await MediaUploadServer.start(uploadDelegate: delegate, defaultUploader: mockUploader) + @Test("skips processing and the temp copy when the processor declines by metadata") + func processorDeclinesByMetadata() async throws { + let processor = DecliningProcessor() + let mockUploader = MockInternalMediaClient() + let server = try await MediaUploadServer.start(processor: processor, internalClient: mockUploader) defer { server.stop() } let boundary = UUID().uuidString @@ -219,18 +362,18 @@ struct MediaUploadServerTests { let httpResponse = try #require(response as? HTTPURLResponse) #expect(httpResponse.statusCode == 201) - // Declined by metadata → the delegate is never asked to process (so the file + // Declined by metadata → the processor is never asked to process (so the file // was never materialized), and the upload is passed through directly. - #expect(!delegate.processFileCalled) + #expect(!processor.processFileCalled) #expect(mockUploader.passthroughUploadCalled) #expect(!mockUploader.uploadCalled) } - @Test("forwards the delegate's processed metadata to the uploader") + @Test("forwards the processor's processed metadata to the uploader") func processedMetadataForwarded() async throws { - let delegate = ResizingDelegate() - let mockUploader = MockDefaultUploader() - let server = try await MediaUploadServer.start(uploadDelegate: delegate, defaultUploader: mockUploader) + let processor = ResizingProcessor() + let mockUploader = MockInternalMediaClient() + let server = try await MediaUploadServer.start(processor: processor, internalClient: mockUploader) defer { server.stop() } let boundary = UUID().uuidString @@ -245,18 +388,18 @@ struct MediaUploadServerTests { _ = try await URLSession.shared.data(for: request) - // The delegate changed the format, so the uploader must receive the new + // The processor changed the format, so the uploader must receive the new // metadata — not the original video/quicktime + clip.mov. #expect(mockUploader.uploadCalled) #expect(mockUploader.lastUploadMimeType == "video/mp4") #expect(mockUploader.lastUploadFilename == "clip.mp4") } - @Test("deletes the delegate's processed file after upload") + @Test("deletes the processor's processed file after upload") func deletesProcessedFile() async throws { - let delegate = ResizingDelegate() - let mockUploader = MockDefaultUploader() - let server = try await MediaUploadServer.start(uploadDelegate: delegate, defaultUploader: mockUploader) + let processor = ResizingProcessor() + let mockUploader = MockInternalMediaClient() + let server = try await MediaUploadServer.start(processor: processor, internalClient: mockUploader) defer { server.stop() } let boundary = UUID().uuidString @@ -271,16 +414,16 @@ struct MediaUploadServerTests { _ = try await URLSession.shared.data(for: request) - // The server owns the file the delegate produced and must delete it once the + // The server owns the file the processor produced and must delete it once the // upload finishes — the defer in processAndUpload covers the success and throw // paths alike. A leaked processed file is a full-size temp per upload. - let processedURL = try #require(delegate.producedURL) + let processedURL = try #require(processor.producedURL) #expect(!FileManager.default.fileExists(atPath: processedURL.path(percentEncoded: false))) } @Test("returns 413 with CORS headers when request body exceeds max size") func oversizedUploadReturns413WithCORSHeaders() async throws { - let server = try await MediaUploadServer.start(maxRequestBodySize: 1024) + let server = try await MediaUploadServer.start(internalClient: MockInternalMediaClient(), maxRequestBodySize: 1024) defer { server.stop() } let boundary = UUID().uuidString @@ -305,7 +448,7 @@ struct MediaUploadServerTests { @Test("unauthenticated oversized request returns 407, not 413 (auth precedes drain)") func oversizedUploadWithoutTokenReturns407() async throws { - let server = try await MediaUploadServer.start(maxRequestBodySize: 1024) + let server = try await MediaUploadServer.start(internalClient: MockInternalMediaClient(), maxRequestBodySize: 1024) defer { server.stop() } let boundary = UUID().uuidString @@ -351,7 +494,7 @@ struct MediaUploadServerTests { // start() kicks off cleanOrphanedUploads() off the editor-startup path. // The sweep must delete the aged file and keep the fresh one — a flipped // comparison would do the opposite and wipe an in-flight upload. - let server = try await MediaUploadServer.start() + let server = try await MediaUploadServer.start(internalClient: MockInternalMediaClient()) await server.cleanupTask.value server.stop() @@ -359,26 +502,157 @@ struct MediaUploadServerTests { #expect(FileManager.default.fileExists(atPath: fresh.path(percentEncoded: false))) } - @Test("does not strongly retain the upload delegate (weak — preserves deinit teardown)") - func doesNotStronglyRetainDelegate() async throws { - weak var weakDelegate: MockUploadDelegate? + @Test("retains the processor for the server's lifetime, and releases it on stop") + func retainsProcessorForServerLifetime() async throws { + weak var weakProcessor: PassthroughProcessor? + var server: MediaUploadServer? + do { + let processor = PassthroughProcessor() + weakProcessor = processor + server = try await MediaUploadServer.start(processor: processor, internalClient: MockInternalMediaClient()) + } + + // The server (via UploadContext) holds the processor *strongly*, matching both the + // editor's own ownership (see `EditorMediaHandlerOwnershipTests`) and Android's + // plain `val`. Holding it weakly here bought no leak protection — a host object + // that retains the editor already cycles through the editor's own strong + // `mediaProcessor` — and only risked the reference vanishing mid-request. + #expect(weakProcessor != nil, "the server must own its processor while it runs") + + // …and releases it when the server goes away, so nothing outlives the editor. + // `stop()` cancels the NWListener, whose handlers are torn down asynchronously, + // so the final release lands a beat after `server = nil` — poll rather than + // assert instantly. + server?.stop() + server = nil + for _ in 0..<200 where weakProcessor != nil { + try await Task.sleep(for: .milliseconds(10)) + } + #expect(weakProcessor == nil, "releasing the server must release the processor") + } + + @Test("delivers through an uploader the host releases mid-request") + func uploaderReleasedMidRequestStillDelivers() async throws { + let mockClient = MockInternalMediaClient() + let gate = UnsafeMutableSendablePointer(false) + let didUpload = UnsafeMutableSendablePointer(false) + let processor = GatedProcessor(gate: gate) + + // `holder` stands in for the *host's* own reference to its uploader, which the + // docs say it may drop after assigning. It's built in a nested scope so the + // `start` call's existential temporary dies with that scope; left in the test's + // own frame, that temporary keeps the uploader alive whatever the server does, + // and the test would pass vacuously. The uploader's record of having run lives in + // `didUpload`, off the object, so it survives the release either way. + let holder = UnsafeMutableSendablePointer<(any MediaUploader)?>(nil) let server: MediaUploadServer do { - let delegate = MockUploadDelegate() - weakDelegate = delegate - server = try await MediaUploadServer.start(uploadDelegate: delegate) + let uploader = ReleasableUploader(didUpload: didUpload) + holder.value = uploader + server = try await MediaUploadServer.start( + processor: processor, uploader: uploader, internalClient: mockClient + ) + } + defer { server.stop() } + + let boundary = UUID().uuidString + let body = buildMultipartBody( + boundary: boundary, filename: "photo.jpg", mimeType: "image/jpeg", + data: Data("fake image data".utf8) + ) + let url = URL(string: "http://127.0.0.1:\(server.port)/upload")! + var request = URLRequest(url: url) + request.httpMethod = "POST" + request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") + request.httpBody = body + + let requestTask = Task { try await URLSession.shared.data(for: request) } + + // Park the request inside `processFile` — past the gate that saw the uploader, + // before the delivery step. This is the window a real host sits in while a + // transcode runs. + while !processor.processFileStarted { + try await Task.sleep(for: .milliseconds(5)) } + + // The host drops its reference mid-upload. While `UploadContext` held the uploader + // weakly this dropped the last one: delivery then read nil and silently uploaded + // through the internal client instead — breaking the "GutenbergKit out of the + // network entirely" contract and creating an attachment the host never learns + // about. The server owns the uploader now, so the request delivers as promised. + holder.value = nil + + gate.value = true + let (_, response) = try await requestTask.value + + let httpResponse = try #require(response as? HTTPURLResponse) + #expect(httpResponse.statusCode == 201) + #expect(didUpload.value, "the released uploader must still own delivery") + #expect(!mockClient.uploadCalled, "GutenbergKit must not upload behind an uploader") + #expect(!mockClient.passthroughUploadCalled) + } + + @Test("keeps a filename-bearing part out of the fields handed to an uploader") + func filenameBearingPartsNeverReachFields() async throws { + // This pins the invariant that makes the UTF-8 decode in `formFields` lossless. + // A browser FormData can only carry arbitrary bytes as a Blob, and a Blob always + // gets a filename, so `handleUpload`'s partition on `filename == nil` is what keeps + // binary out of `fields`. Change that partition and the decode silently starts + // substituting U+FFFD — differently on each platform. + // + // Deliberately *not* asserted: what becomes of the second filename-bearing part. + // It is currently dropped rather than relayed, which is a separate open question; + // this test is about what must never reach `fields`. + let uploader = MockUploader() + let mockClient = MockInternalMediaClient() + let server = try await MediaUploadServer.start(uploader: uploader, internalClient: mockClient) defer { server.stop() } - // UploadContext holds the delegate weakly, so releasing the host's strong - // reference deallocates it. A strong reference here would reintroduce the - // EditorViewController → uploadServer → … → delegate → EditorViewController - // cycle, so deinit would never fire and the server would never stop. - #expect(weakDelegate == nil) + let boundary = UUID().uuidString + var body = Data() + // A plain field — no filename, so it belongs in `fields`. + body.append("--\(boundary)\r\n") + body.append("Content-Disposition: form-data; name=\"post\"\r\n\r\n") + body.append("123") + body.append("\r\n") + // The file. + body.append("--\(boundary)\r\n") + body.append("Content-Disposition: form-data; name=\"file\"; filename=\"photo.jpg\"\r\n") + body.append("Content-Type: image/jpeg\r\n\r\n") + body.append(Data("fake image data".utf8)) + body.append("\r\n") + // A Blob-shaped sidecar: has a filename, and carries bytes that are not valid + // UTF-8. If the partition ever admitted this to `fields`, the lone 0xFF would + // become U+FFFD and the value would be silently corrupted. + body.append("--\(boundary)\r\n") + body.append("Content-Disposition: form-data; name=\"sidecar\"; filename=\"blob\"\r\n") + body.append("Content-Type: application/octet-stream\r\n\r\n") + body.append(Data([0x61, 0xFF, 0x62])) + body.append("\r\n--\(boundary)--\r\n") + + let url = URL(string: "http://127.0.0.1:\(server.port)/upload")! + var request = URLRequest(url: url) + request.httpMethod = "POST" + request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") + request.httpBody = body + + _ = try await URLSession.shared.data(for: request) + + #expect(uploader.uploadCalled) + #expect(uploader.lastFields.map(\.name) == ["post"], "only filename-less parts belong in fields") + #expect(!uploader.lastFields.contains { $0.value.contains("\u{FFFD}") }, "no field value should have been lossily decoded") } - private func buildMultipartBody(boundary: String, filename: String, mimeType: String, data: Data) -> Data { + private func buildMultipartBody(boundary: String, filename: String, mimeType: String, data: Data, fields: [MediaUploadField] = []) -> Data { var body = Data() + for field in fields { + body.append("--\(boundary)\r\n") + body.append("Content-Disposition: form-data; name=\"\(field.name)\"\r\n\r\n") + body.append(field.value) + body.append("\r\n") + } body.append("--\(boundary)\r\n") body.append("Content-Disposition: form-data; name=\"file\"; filename=\"\(filename)\"\r\n") body.append("Content-Type: \(mimeType)\r\n\r\n") @@ -390,7 +664,7 @@ struct MediaUploadServerTests { // MARK: - Streaming Multipart Body Tests -@Suite("DefaultMediaUploader streaming multipart body") +@Suite("InternalMediaClient streaming multipart body") struct MultipartBodyStreamTests { @Test("streaming output matches in-memory multipart format") @@ -413,7 +687,7 @@ struct MultipartBodyStreamTests { expected.append(Data("\r\n--\(boundary)--\r\n".utf8)) // Build streaming output. - let (stream, contentLength) = try DefaultMediaUploader.multipartBodyStream( + let (stream, contentLength) = try InternalMediaClient.multipartBodyStream( fileURL: tempFile, boundary: boundary, filename: filename, mimeType: mimeType, extraFields: [] ) #expect(contentLength == expected.count) @@ -430,7 +704,7 @@ struct MultipartBodyStreamTests { // Craft a filename, field name, and MIME type that each try to smuggle a CRLF // and a fake header into the body relayed to WordPress. - let (stream, _) = try DefaultMediaUploader.multipartBodyStream( + let (stream, _) = try InternalMediaClient.multipartBodyStream( fileURL: tempFile, boundary: "boundary", filename: "evil\"\r\nX-Injected-File: 1.jpg", @@ -465,7 +739,7 @@ struct MultipartBodyStreamTests { expected.append(fileContent) expected.append(Data("\r\n--\(boundary)--\r\n".utf8)) - let (stream, contentLength) = try DefaultMediaUploader.multipartBodyStream( + let (stream, contentLength) = try InternalMediaClient.multipartBodyStream( fileURL: tempFile, boundary: boundary, filename: filename, mimeType: mimeType, extraFields: [("post", Data("123".utf8))] ) @@ -497,7 +771,7 @@ struct MultipartBodyStreamTests { expected.append(fileContent) expected.append(Data("\r\n--\(boundary)--\r\n".utf8)) - let (stream, contentLength) = try DefaultMediaUploader.multipartBodyStream( + let (stream, contentLength) = try InternalMediaClient.multipartBodyStream( fileURL: tempFile, boundary: boundary, filename: filename, mimeType: mimeType, extraFields: [("blob", binaryValue)] ) @@ -513,7 +787,7 @@ struct MultipartBodyStreamTests { try fileContent.write(to: tempFile) defer { try? FileManager.default.removeItem(at: tempFile) } - let (stream, contentLength) = try DefaultMediaUploader.multipartBodyStream( + let (stream, contentLength) = try InternalMediaClient.multipartBodyStream( fileURL: tempFile, boundary: "boundary", filename: "big.bin", mimeType: "application/octet-stream", extraFields: [] ) @@ -537,7 +811,7 @@ struct MultipartBodyStreamTests { let preamble = Data("PREAMBLE".utf8) let epilogue = Data("EPILOGUE".utf8) - let ok = DefaultMediaUploader.writeMultipartBody( + let ok = InternalMediaClient.writeMultipartBody( fileHandle: fileHandle, fileSize: fileContent.count, preamble: preamble, epilogue: epilogue, to: output ) @@ -564,7 +838,7 @@ struct MultipartBodyStreamTests { let preamble = Data("PREAMBLE".utf8) let epilogue = Data("EPILOGUE".utf8) // Claim the file is larger than it is, as if it shrank after being measured. - let ok = DefaultMediaUploader.writeMultipartBody( + let ok = InternalMediaClient.writeMultipartBody( fileHandle: fileHandle, fileSize: fileContent.count + 100, preamble: preamble, epilogue: epilogue, to: output ) @@ -577,17 +851,17 @@ struct MultipartBodyStreamTests { } } -// MARK: - DefaultMediaUploader Relay Tests +// MARK: - InternalMediaClient Relay Tests -@Suite("DefaultMediaUploader relay") -struct DefaultMediaUploaderRelayTests { +@Suite("InternalMediaClient relay") +struct InternalMediaClientRelayTests { @Test("relays a non-2xx WordPress response instead of throwing") func relaysErrorResponseVerbatim() async throws { // A WordPress REST error body, returned with a non-2xx status. let errorBody = Data(#"{"code":"rest_cannot_create","message":"Sorry, you are not allowed to upload this file type."}"#.utf8) let client = RelayStubHTTPClient(statusCode: 403, body: errorBody) - let uploader = DefaultMediaUploader(httpClient: client, siteApiRoot: URL(string: "https://example.com/wp-json/")!) + let uploader = InternalMediaClient(httpClient: client, siteApiRoot: URL(string: "https://example.com/wp-json/")!) let tempFile = FileManager.default.temporaryDirectory.appendingPathComponent("relay-\(UUID().uuidString).jpg") try Data("fake image".utf8).write(to: tempFile) @@ -604,10 +878,74 @@ struct DefaultMediaUploaderRelayTests { #expect(response.body == errorBody) } + @Test("relays the upload attachment ID header from a failed upload") + func relaysUploadAttachmentIdHeader() async throws { + // WordPress sets this header on an upload whose attachment row was created + // before metadata generation fataled. The editor reads it to retry + // post-process and clean up the orphan, so it must survive the relay. + let client = RelayStubHTTPClient( + statusCode: 500, + body: Data(#"{"code":"rest_upload_error"}"#.utf8), + headerFields: ["x-wp-upload-attachment-id": "4242"] + ) + let uploader = InternalMediaClient( + httpClient: client, siteApiRoot: URL(string: "https://example.com/wp-json/")!) + + let tempFile = FileManager.default.temporaryDirectory.appendingPathComponent( + "header-\(UUID().uuidString).jpg") + try Data("fake image".utf8).write(to: tempFile) + defer { try? FileManager.default.removeItem(at: tempFile) } + + let response = try await uploader.upload( + fileURL: tempFile, mimeType: "image/jpeg", filename: "photo.jpg", extraParts: [], query: "" + ) + + #expect(response.statusCode == 500) + #expect(response.headers["x-wp-upload-attachment-id"] == "4242") + } + + @Test("omits unrelated upstream headers from the relay") + func omitsUnrelatedHeaders() async throws { + let client = RelayStubHTTPClient( + statusCode: 201, + body: Data("{}".utf8), + headerFields: ["X-Powered-By": "PHP/8.2", "Set-Cookie": "session=secret"] + ) + let uploader = InternalMediaClient( + httpClient: client, siteApiRoot: URL(string: "https://example.com/wp-json/")!) + + let tempFile = FileManager.default.temporaryDirectory.appendingPathComponent( + "unrelated-\(UUID().uuidString).jpg") + try Data("fake image".utf8).write(to: tempFile) + defer { try? FileManager.default.removeItem(at: tempFile) } + + let response = try await uploader.upload( + fileURL: tempFile, mimeType: "image/jpeg", filename: "photo.jpg", extraParts: [], query: "" + ) + + #expect(response.headers.isEmpty) + } + + @Test("deletes an attachment, carrying the namespace and force query") + func deletesAttachment() async throws { + let client = URLCapturingHTTPClient() + let uploader = InternalMediaClient( + httpClient: client, + siteApiRoot: URL(string: "https://example.com/wp-json")!, + siteApiNamespace: ["sites/123"] + ) + + _ = try await uploader.deleteMedia(attachmentId: "42", query: "?force=true") + + let url = try #require(client.lastURL) + #expect( + url.absoluteString == "https://example.com/wp-json/wp/v2/sites/123/media/42?force=true") + } + @Test("carries the namespace and request query through to the media endpoint") func forwardsNamespaceAndQuery() async throws { let client = URLCapturingHTTPClient() - let uploader = DefaultMediaUploader( + let uploader = InternalMediaClient( httpClient: client, siteApiRoot: URL(string: "https://example.com/wp-json")!, siteApiNamespace: ["sites/123"] @@ -630,14 +968,16 @@ struct DefaultMediaUploaderRelayTests { /// An HTTP client whose `performRaw` relays a canned response without validating /// status, while `perform` throws on a non-2xx — mirroring the real -/// `EditorHTTPClient`. Lets a test prove `DefaultMediaUploader` routes uploads +/// `EditorHTTPClient`. Lets a test prove `InternalMediaClient` routes uploads /// through `performRaw` (relay) rather than `perform` (throw). private struct RelayStubHTTPClient: EditorHTTPClientProtocol { let statusCode: Int let body: Data + var headerFields: [String: String]? func perform(_ urlRequest: URLRequest) async throws -> (Data, HTTPURLResponse) { - let response = HTTPURLResponse(url: urlRequest.url!, statusCode: statusCode, httpVersion: nil, headerFields: nil)! + let response = HTTPURLResponse( + url: urlRequest.url!, statusCode: statusCode, httpVersion: nil, headerFields: headerFields)! guard (200...299).contains(statusCode) else { throw NSError(domain: "RelayStubHTTPClient", code: statusCode) } @@ -645,7 +985,8 @@ private struct RelayStubHTTPClient: EditorHTTPClientProtocol { } func performRaw(_ urlRequest: URLRequest) async throws -> (Data, HTTPURLResponse) { - let response = HTTPURLResponse(url: urlRequest.url!, statusCode: statusCode, httpVersion: nil, headerFields: nil)! + let response = HTTPURLResponse( + url: urlRequest.url!, statusCode: statusCode, httpVersion: nil, headerFields: headerFields)! return (body, response) } @@ -698,37 +1039,90 @@ private func readAllFromStream(_ stream: InputStream) -> Data { // MARK: - Mocks -private final class MockUploadDelegate: MediaUploadDelegate, @unchecked Sendable { +/// A host uploader: it performs the upload on its own stack. `upload` returns the +/// finished attachment JSON (or throws). +private final class MockUploader: MediaUploader, @unchecked Sendable { private let lock = NSLock() - private var _processFileCalled = false - private var _uploadFileCalled = false + private var _uploadCalled = false private var _lastMimeType: String? private var _lastFilename: String? + private var _lastFields: [MediaUploadField] = [] + private var _lastQuery: String? + private var _lastFileData: Data? + private let uploadBody: Data + private let error: (any Error)? - var processFileCalled: Bool { lock.withLock { _processFileCalled } } - var uploadFileCalled: Bool { lock.withLock { _uploadFileCalled } } + var uploadCalled: Bool { lock.withLock { _uploadCalled } } var lastMimeType: String? { lock.withLock { _lastMimeType } } var lastFilename: String? { lock.withLock { _lastFilename } } + var lastFields: [MediaUploadField] { lock.withLock { _lastFields } } + var lastQuery: String? { lock.withLock { _lastQuery } } + var lastFileData: Data? { lock.withLock { _lastFileData } } + + init( + uploadBody: Data = Data(#"{"id":42,"source_url":"https://example.com/photo.jpg","media_type":"image"}"#.utf8), + error: (any Error)? = nil + ) { + self.uploadBody = uploadBody + self.error = error + } - func processFile(at url: URL, mimeType: String, filename: String) async throws -> ProcessedProxyFile { + func upload(_ upload: MediaUpload) async throws -> Data { + // Read the file the server handed us so a test can assert its contents. + let fileData = try? Data(contentsOf: upload.fileURL) lock.withLock { - _processFileCalled = true - _lastMimeType = mimeType + _uploadCalled = true + _lastMimeType = upload.mimeType + _lastFilename = upload.filename + _lastFields = upload.fields + _lastQuery = upload.query + _lastFileData = fileData } - return .original + if let error { throw error } + return uploadBody } +} - func uploadFile(at url: URL, mimeType: String, filename: String) async throws -> MediaUploadResponse? { - lock.withLock { - _uploadFileCalled = true - _lastFilename = filename +/// Holds a request open inside `processFile` until the test opens `gate`, so the test +/// can act while the request is parked between the admission gate and delivery. +private final class GatedProcessor: MediaProcessor, @unchecked Sendable { + private let lock = NSLock() + private var _processFileStarted = false + private let gate: UnsafeMutableSendablePointer + + var processFileStarted: Bool { lock.withLock { _processFileStarted } } + + init(gate: UnsafeMutableSendablePointer) { + self.gate = gate + } + + func handlesFile(ofType mimeType: String, named filename: String) -> Bool { true } + + func processFile(at url: URL, mimeType: String, filename: String) async throws -> ProcessedProxyFile { + lock.withLock { _processFileStarted = true } + while !gate.value { + try await Task.sleep(for: .milliseconds(5)) } - let json = #"{"id":42,"source_url":"https://example.com/photo.jpg","media_type":"image"}"# - return MediaUploadResponse(statusCode: 201, body: Data(json.utf8)) + return .original } } -private final class ProcessOnlyDelegate: MediaUploadDelegate, @unchecked Sendable { +/// An uploader whose record of having run lives *outside* the object, so a test can +/// release its last strong reference mid-request and still assert that it delivered. +private final class ReleasableUploader: MediaUploader, @unchecked Sendable { + private let didUpload: UnsafeMutableSendablePointer + + init(didUpload: UnsafeMutableSendablePointer) { + self.didUpload = didUpload + } + + func upload(_ upload: MediaUpload) async throws -> Data { + didUpload.value = true + return Data(#"{"id":42,"source_url":"https://example.com/photo.jpg","media_type":"image"}"#.utf8) + } +} + +private final class PassthroughProcessor: MediaProcessor, @unchecked Sendable { private let lock = NSLock() private var _processFileCalled = false @@ -740,10 +1134,10 @@ private final class ProcessOnlyDelegate: MediaUploadDelegate, @unchecked Sendabl } } -/// A delegate that declines every file by metadata via `handlesFile`, so the +/// A processor that declines every file by metadata via `handlesFile`, so the /// server must pass through without ever materializing the file or calling /// `processFile`. -private final class DeclineByMetadataDelegate: MediaUploadDelegate, @unchecked Sendable { +private final class DecliningProcessor: MediaProcessor, @unchecked Sendable { private let lock = NSLock() private var _processFileCalled = false @@ -757,12 +1151,12 @@ private final class DeclineByMetadataDelegate: MediaUploadDelegate, @unchecked S } } -/// A delegate that produces a new file with changed metadata (e.g. a transcode). -private final class ResizingDelegate: MediaUploadDelegate, @unchecked Sendable { +/// A processor that produces a new file with changed metadata (e.g. a transcode). +private final class ResizingProcessor: MediaProcessor, @unchecked Sendable { private let lock = NSLock() private var _producedURL: URL? - /// The URL of the processed file this delegate wrote, for cleanup assertions. + /// The URL of the processed file this processor wrote, for cleanup assertions. var producedURL: URL? { lock.withLock { _producedURL } } func processFile(at url: URL, mimeType: String, filename: String) async throws -> ProcessedProxyFile { @@ -773,21 +1167,29 @@ private final class ResizingDelegate: MediaUploadDelegate, @unchecked Sendable { } } -private final class MockDefaultUploader: DefaultMediaUploader, @unchecked Sendable { +private final class MockInternalMediaClient: InternalMediaClient, @unchecked Sendable { private let lock = NSLock() private var _uploadCalled = false private var _passthroughUploadCalled = false private var _lastUploadMimeType: String? private var _lastUploadFilename: String? private var _lastQuery: String? + private var _deleteMediaCalled = false + private var _deletedAttachmentId: String? + + /// The response `upload`/`passthroughUpload` return. `nil` uses a 201 default. + private let uploadResponse: MediaUploadResponse? var uploadCalled: Bool { lock.withLock { _uploadCalled } } var passthroughUploadCalled: Bool { lock.withLock { _passthroughUploadCalled } } var lastUploadMimeType: String? { lock.withLock { _lastUploadMimeType } } var lastUploadFilename: String? { lock.withLock { _lastUploadFilename } } var lastQuery: String? { lock.withLock { _lastQuery } } + var deleteMediaCalled: Bool { lock.withLock { _deleteMediaCalled } } + var deletedAttachmentId: String? { lock.withLock { _deletedAttachmentId } } - init() { + init(uploadResponse: MediaUploadResponse? = nil) { + self.uploadResponse = uploadResponse super.init(httpClient: MockHTTPClient(), siteApiRoot: URL(string: "https://example.com/wp-json/")!) } @@ -798,7 +1200,7 @@ private final class MockDefaultUploader: DefaultMediaUploader, @unchecked Sendab _lastUploadFilename = filename _lastQuery = query } - return mockResponse() + return uploadResponse ?? Self.defaultResponse } override func passthroughUpload(body: RequestBody, contentType: String, query: String) async throws -> MediaUploadResponse { @@ -806,13 +1208,20 @@ private final class MockDefaultUploader: DefaultMediaUploader, @unchecked Sendab _passthroughUploadCalled = true _lastQuery = query } - return mockResponse() + return uploadResponse ?? Self.defaultResponse } - private func mockResponse() -> MediaUploadResponse { - let json = #"{"id":99,"source_url":"https://example.com/doc.pdf","media_type":"file"}"# - return MediaUploadResponse(statusCode: 201, body: Data(json.utf8)) + override func deleteMedia(attachmentId: String, query: String) async throws -> MediaUploadResponse { + lock.withLock { + _deleteMediaCalled = true + _deletedAttachmentId = attachmentId + } + return MediaUploadResponse(statusCode: 200, body: Data(#"{"deleted":true}"#.utf8)) } + + private static let defaultResponse = MediaUploadResponse( + statusCode: 201, + body: Data(#"{"id":99,"source_url":"https://example.com/doc.pdf","media_type":"file"}"#.utf8)) } private struct MockHTTPClient: EditorHTTPClientProtocol { diff --git a/src/utils/api-fetch-post-process.test.js b/src/utils/api-fetch-post-process.test.js new file mode 100644 index 000000000..2311f9fa7 --- /dev/null +++ b/src/utils/api-fetch-post-process.test.js @@ -0,0 +1,288 @@ +/** + * External dependencies + */ +import { + describe, + it, + expect, + beforeAll, + beforeEach, + afterEach, + vi, +} from 'vitest'; + +/** + * WordPress dependencies + */ +import apiFetch from '@wordpress/api-fetch'; + +/** + * Internal dependencies + */ +import { configureApiFetch } from './api-fetch'; +import * as bridge from './bridge'; +import { error } from './logger'; + +vi.mock( './bridge', async ( importOriginal ) => { + const actual = await importOriginal(); + return { + ...actual, + getGBKit: vi.fn(), + }; +} ); + +vi.mock( './logger', () => ( { + info: vi.fn(), + error: vi.fn(), + warn: vi.fn(), + debug: vi.fn(), +} ) ); + +const SITE_API_ROOT = 'https://example.com/wp-json/'; + +/** + * A `Response`-like object carrying the upload attachment ID header, which is + * what core's middleware keys off to decide whether a failure is recoverable. + * + * @param {number} status The HTTP status. + * @param {?string} attachmentId The `x-wp-upload-attachment-id` header value. + * @param {Object} body The JSON body. + * @return {Response} The response. + */ +function makeResponse( status, attachmentId, body = {} ) { + const headers = { 'Content-Type': 'application/json' }; + if ( attachmentId ) { + headers[ 'x-wp-upload-attachment-id' ] = attachmentId; + } + return new Response( JSON.stringify( body ), { status, headers } ); +} + +/** A media upload request, as `@wordpress/media-utils` issues it. */ +function uploadOptions() { + const body = new FormData(); + body.append( + 'file', + new File( [ 'data' ], 'photo.jpg', { type: 'image/jpeg' } ) + ); + return { path: '/wp/v2/media', method: 'POST', body }; +} + +/** + * The path of the Nth `fetch` call, normalized to a string. + * + * @param {number} index The zero-based call index. + * @return {string} The requested URL. + */ +function fetchedPath( index ) { + return String( global.fetch.mock.calls[ index ][ 0 ] ); +} + +describe( "core's media upload post-process middleware", () => { + let originalFetch; + + beforeAll( () => { + bridge.getGBKit.mockReturnValue( { siteApiRoot: SITE_API_ROOT } ); + configureApiFetch(); + } ); + + beforeEach( () => { + vi.clearAllMocks(); + originalFetch = global.fetch; + bridge.getGBKit.mockReturnValue( { + siteApiRoot: SITE_API_ROOT, + authHeader: 'Bearer test-token', + siteApiNamespace: [], + namespaceExcludedPaths: [], + } ); + } ); + + afterEach( () => { + global.fetch = originalFetch; + } ); + + it( 'retries post-process for an upload routed through the native server', async () => { + // Regression test for middleware ordering. The native middleware handles + // the upload without calling `next`, so core's middleware only ever sees + // it by running *above* — registered after. With the two swapped, this + // upload surfaces as a permanent failure and no post-process is issued. + bridge.getGBKit.mockReturnValue( { + siteApiRoot: SITE_API_ROOT, + authHeader: 'Bearer test-token', + siteApiNamespace: [], + namespaceExcludedPaths: [], + nativeUploadPort: 8080, + nativeUploadToken: 'relay-token', + } ); + + global.fetch = vi.fn( ( url ) => { + const path = String( url ); + if ( path.includes( 'post-process' ) ) { + return Promise.resolve( makeResponse( 200, null, { id: 42 } ) ); + } + // The native server relays WordPress's 500 plus the attachment ID. + return Promise.resolve( makeResponse( 500, '42' ) ); + } ); + + await expect( apiFetch( uploadOptions() ) ).resolves.toEqual( { + id: 42, + } ); + + expect( fetchedPath( 0 ) ).toContain( 'localhost:8080/upload' ); + expect( fetchedPath( 1 ) ).toContain( '/wp/v2/media/42/post-process' ); + } ); + + it( 'retries post-process when the upload fails with an attachment ID', async () => { + global.fetch = vi.fn( ( url ) => { + if ( String( url ).includes( 'post-process' ) ) { + return Promise.resolve( makeResponse( 200, null, { id: 42 } ) ); + } + return Promise.resolve( makeResponse( 500, '42' ) ); + } ); + + // A recovered upload resolves with the post-process result rather than + // surfacing the original 5xx to the block. + await expect( apiFetch( uploadOptions() ) ).resolves.toEqual( { + id: 42, + } ); + + expect( global.fetch ).toHaveBeenCalledTimes( 2 ); + expect( fetchedPath( 1 ) ).toContain( '/wp/v2/media/42/post-process' ); + } ); + + it( 'deletes the orphaned attachment after exhausting retries', async () => { + global.fetch = vi.fn( ( url ) => { + const path = String( url ); + if ( path.includes( 'post-process' ) ) { + return Promise.resolve( makeResponse( 500, null ) ); + } + if ( path.includes( 'force=true' ) ) { + return Promise.resolve( + makeResponse( 200, null, { deleted: true } ) + ); + } + return Promise.resolve( makeResponse( 500, '42' ) ); + } ); + + await expect( apiFetch( uploadOptions() ) ).rejects.toBeDefined(); + + const paths = global.fetch.mock.calls.map( ( [ url ] ) => + String( url ) + ); + // The initial upload, five post-process attempts, then the cleanup. + expect( + paths.filter( ( p ) => p.includes( 'post-process' ) ) + ).toHaveLength( 5 ); + expect( paths.at( -1 ) ).toContain( '/wp/v2/media/42?force=true' ); + } ); + + it( 'relays the orphan cleanup through the native server', async () => { + // End-to-end shape of the `always` case on a native-upload host: the + // upload fails, five post-process attempts fail, and the resulting + // DELETE goes to the loopback server rather than direct to WordPress — + // where a cross-origin editor's tunnelled DELETE would be blocked at + // preflight, leaving the orphan behind. + bridge.getGBKit.mockReturnValue( { + siteApiRoot: SITE_API_ROOT, + authHeader: 'Bearer test-token', + siteApiNamespace: [], + namespaceExcludedPaths: [], + nativeUploadPort: 8080, + nativeUploadToken: 'relay-token', + } ); + + global.fetch = vi.fn( ( url ) => { + const path = String( url ); + if ( path.includes( 'post-process' ) ) { + return Promise.resolve( makeResponse( 500, null ) ); + } + if ( path.includes( '/media/42' ) ) { + return Promise.resolve( + makeResponse( 200, null, { deleted: true } ) + ); + } + return Promise.resolve( makeResponse( 500, '42' ) ); + } ); + + await expect( apiFetch( uploadOptions() ) ).rejects.toBeDefined(); + + const paths = global.fetch.mock.calls.map( ( [ url ] ) => + String( url ) + ); + expect( + paths.filter( ( p ) => p.includes( 'post-process' ) ) + ).toHaveLength( 5 ); + expect( paths.at( -1 ) ).toBe( + 'http://localhost:8080/media/42?force=true' + ); + } ); + + it( 'authenticates the post-process retry', async () => { + global.fetch = vi.fn( ( url ) => { + if ( String( url ).includes( 'post-process' ) ) { + return Promise.resolve( makeResponse( 200, null, { id: 42 } ) ); + } + return Promise.resolve( makeResponse( 500, '42' ) ); + } ); + + await expect( apiFetch( uploadOptions() ) ).resolves.toBeDefined(); + + // The retry is issued through `next`, so it must still pick up the auth + // header and the site API root from the middlewares below it. + const [ url, options ] = global.fetch.mock.calls[ 1 ]; + expect( String( url ) ).toContain( + `${ SITE_API_ROOT }wp/v2/media/42/post-process` + ); + expect( options.headers.Authorization ).toBe( 'Bearer test-token' ); + } ); + + it( 'does not retry when the failure carries no attachment ID', async () => { + global.fetch = vi.fn( () => + Promise.resolve( makeResponse( 500, null ) ) + ); + + await expect( apiFetch( uploadOptions() ) ).rejects.toBeDefined(); + + expect( global.fetch ).toHaveBeenCalledTimes( 1 ); + } ); + + it( 'leaves a successful upload untouched', async () => { + global.fetch = vi.fn( () => + Promise.resolve( + makeResponse( 201, null, { id: 42, source_url: 'x' } ) + ) + ); + + await expect( apiFetch( uploadOptions() ) ).resolves.toEqual( { + id: 42, + source_url: 'x', + } ); + + expect( global.fetch ).toHaveBeenCalledTimes( 1 ); + } ); + + it( 'does not log an error for an upload that recovers', async () => { + // The initial 5xx is a handoff to core's post-process retry, not a + // failure. A silently-recovered upload must surface no error to the host. + bridge.getGBKit.mockReturnValue( { + siteApiRoot: SITE_API_ROOT, + authHeader: 'Bearer test-token', + siteApiNamespace: [], + namespaceExcludedPaths: [], + nativeUploadPort: 8080, + nativeUploadToken: 'relay-token', + } ); + + global.fetch = vi.fn( ( url ) => { + if ( String( url ).includes( 'post-process' ) ) { + return Promise.resolve( makeResponse( 200, null, { id: 42 } ) ); + } + return Promise.resolve( makeResponse( 500, '42' ) ); + } ); + + await expect( apiFetch( uploadOptions() ) ).resolves.toEqual( { + id: 42, + } ); + + expect( error ).not.toHaveBeenCalled(); + } ); +} ); diff --git a/src/utils/api-fetch-upload-middleware.test.js b/src/utils/api-fetch-upload-middleware.test.js index 21a75a14e..288e70daa 100644 --- a/src/utils/api-fetch-upload-middleware.test.js +++ b/src/utils/api-fetch-upload-middleware.test.js @@ -625,6 +625,66 @@ describe( 'nativeMediaUploadMiddleware', () => { expect( next ).not.toHaveBeenCalled(); } ); + // MARK: - Raw response passthrough + + describe( 'parse: false', () => { + beforeEach( () => { + getGBKit.mockReturnValue( { + nativeUploadPort: 8080, + nativeUploadToken: 'token', + } ); + } ); + + it( 'rejects with the Response so the attachment ID stays readable', async () => { + // Core's media upload middleware needs the `Response` itself to read + // `x-wp-upload-attachment-id` and retry post-process. Parsing the + // body here would hide the header. + const response = new Response( '{"code":"rest_upload_error"}', { + status: 500, + headers: { 'x-wp-upload-attachment-id': '4242' }, + } ); + global.fetch = vi.fn( () => Promise.resolve( response ) ); + + const options = makePostMediaOptions( makeFile() ); + options.parse = false; + + const thrown = await nativeMediaUploadMiddleware( + options, + makeNext() + ).catch( ( error ) => error ); + + expect( thrown ).toBe( response ); + expect( thrown.headers.get( 'x-wp-upload-attachment-id' ) ).toBe( + '4242' + ); + } ); + + it( 'resolves with the Response on success', async () => { + const response = new Response( '{"id":1}', { status: 201 } ); + global.fetch = vi.fn( () => Promise.resolve( response ) ); + + const options = makePostMediaOptions( makeFile() ); + options.parse = false; + + await expect( + nativeMediaUploadMiddleware( options, makeNext() ) + ).resolves.toBe( response ); + } ); + + it( 'still parses the body when parse is not disabled', async () => { + global.fetch = vi.fn( () => + Promise.resolve( new Response( '{"id":1}', { status: 201 } ) ) + ); + + await expect( + nativeMediaUploadMiddleware( + makePostMediaOptions( makeFile() ), + makeNext() + ) + ).resolves.toEqual( { id: 1 } ); + } ); + } ); + // MARK: - Signal forwarding it( 'forwards abort signal to fetch', async () => { @@ -657,4 +717,103 @@ describe( 'nativeMediaUploadMiddleware', () => { controller.signal ); } ); + + // MARK: - Media deletion + + describe( 'media deletion', () => { + beforeEach( () => { + getGBKit.mockReturnValue( { + nativeUploadPort: 8080, + nativeUploadToken: 'token', + } ); + } ); + + it( 'routes an attachment deletion through the native server', async () => { + // api-fetch tunnels DELETE as POST + `X-HTTP-Method-Override`, which + // core's CORS allow-list omits, so a cross-origin editor cannot make + // this request directly. Relaying it keeps the orphan cleanup working. + global.fetch = vi.fn( () => + Promise.resolve( + new Response( '{"deleted":true}', { status: 200 } ) + ) + ); + const next = makeNext(); + + await expect( + nativeMediaUploadMiddleware( + { method: 'DELETE', path: '/wp/v2/media/42?force=true' }, + next + ) + ).resolves.toEqual( { deleted: true } ); + + expect( next ).not.toHaveBeenCalled(); + const [ url, options ] = global.fetch.mock.calls[ 0 ]; + expect( String( url ) ).toBe( + 'http://localhost:8080/media/42?force=true' + ); + expect( options.method ).toBe( 'DELETE' ); + expect( options.headers[ 'Relay-Authorization' ] ).toBe( + 'Bearer token' + ); + } ); + + it( 'passes through a deletion of a non-media path', async () => { + global.fetch = vi.fn(); + const next = makeNext(); + + nativeMediaUploadMiddleware( + { method: 'DELETE', path: '/wp/v2/posts/42' }, + next + ); + + expect( next ).toHaveBeenCalled(); + expect( global.fetch ).not.toHaveBeenCalled(); + } ); + + it( 'passes through the media collection path', async () => { + // `/wp/v2/media` has no attachment ID, so it is not a deletion this + // server relays. + global.fetch = vi.fn(); + const next = makeNext(); + + nativeMediaUploadMiddleware( + { method: 'DELETE', path: '/wp/v2/media' }, + next + ); + + expect( next ).toHaveBeenCalled(); + expect( global.fetch ).not.toHaveBeenCalled(); + } ); + + it( 'passes through when the native server is not configured', async () => { + getGBKit.mockReturnValue( {} ); + global.fetch = vi.fn(); + const next = makeNext(); + + nativeMediaUploadMiddleware( + { method: 'DELETE', path: '/wp/v2/media/42?force=true' }, + next + ); + + expect( next ).toHaveBeenCalled(); + expect( global.fetch ).not.toHaveBeenCalled(); + } ); + + it( 'rejects with the parsed error body on failure', async () => { + global.fetch = vi.fn( () => + Promise.resolve( + new Response( '{"code":"rest_cannot_delete"}', { + status: 403, + } ) + ) + ); + + const thrown = await nativeMediaUploadMiddleware( + { method: 'DELETE', path: '/wp/v2/media/42?force=true' }, + makeNext() + ).catch( ( error ) => error ); + + expect( thrown ).toEqual( { code: 'rest_cannot_delete' } ); + } ); + } ); } ); diff --git a/src/utils/api-fetch.js b/src/utils/api-fetch.js index 5f8885e4d..43c7907e5 100644 --- a/src/utils/api-fetch.js +++ b/src/utils/api-fetch.js @@ -18,6 +18,9 @@ import { info, error as logError } from './logger'; /** Matches `POST /wp/v2/media` but not sub-paths like `/wp/v2/media/123`. */ const MEDIA_UPLOAD_PATH = /^\/wp\/v2\/media(\?|$)/; +/** Matches `/wp/v2/media/`, capturing the attachment ID. */ +const MEDIA_ATTACHMENT_PATH = /^\/wp\/v2\/media\/(\d+)(\?|$)/; + /** * Initializes the API fetch configuration and middleware. * @@ -31,8 +34,32 @@ export function configureApiFetch() { apiFetch.use( apiPathModifierMiddleware ); apiFetch.use( tokenAuthMiddleware ); apiFetch.use( filterEndpointsMiddleware ); + // `apiFetch.use` unshifts, so the last registration runs first. Core's media + // upload middleware is registered after the native one so that it runs + // before it, wrapping it: the native middleware handles an upload without + // calling `next`, so core's would never run at all from below. Both stay + // above auth, namespacing, and the root URL, so the `post-process` requests + // core issues through `next` are still authenticated and correctly + // addressed. + // + // Recovery needs `x-wp-upload-attachment-id`, readable only same-origin or + // when the server exposes it via CORS: + // + // - Through the native upload server, which exposes it: works on both + // platforms. This is the only path that recovers everywhere. + // - Direct to WordPress, which does not expose it (see + // `rest_send_cors_headers()`): never recovers on iOS, which loads from + // `file://`. On Android it recovers only when the editor is genuinely + // same-origin — `GutenbergView` derives the asset domain from the site's + // host, which drops the port, so a site on a non-default port (any local + // dev setup) is cross-origin and does not recover. + // + // There is no fallback: core sends the header before + // `wp_generate_attachment_metadata()`, so the fatal leaves no body to parse + // the ID from. apiFetch.use( nativeMediaUploadMiddleware ); - apiFetch.use( mediaUploadMiddleware ); + apiFetch.use( apiFetch.mediaUploadMiddleware ); + apiFetch.use( stripDraftPostIdMiddleware ); apiFetch.use( transformOEmbedApiResponse ); apiFetch.use( apiFetch.createPreloadingMiddleware( preloadData ?? defaultPreloadData ) @@ -67,16 +94,19 @@ function corsMiddleware( options, next ) { */ function apiPathModifierMiddleware( options, next ) { const { siteApiNamespace, namespaceExcludedPaths } = getGBKit(); - const namespaceRegex = new RegExp( `(${ siteApiNamespace.join( '|' ) })` ); const isEligiblePath = options.path && + siteApiNamespace.length > 0 && ! namespaceExcludedPaths.some( ( path ) => options.path.startsWith( path ) ); + // Escape the namespaces so each is matched literally rather than as a + // pattern. const alreadyHasSiteNamespace = - namespaceRegex.test( options.path ) || - /\/sites\/[^/]+\//.test( options.path ); + new RegExp( + `(${ siteApiNamespace.map( escapeRegExp ).join( '|' ) })` + ).test( options.path ) || /\/sites\/[^/]+\//.test( options.path ); if ( isEligiblePath && ! alreadyHasSiteNamespace ) { // Insert the API namespace after the first two path segments. @@ -89,6 +119,16 @@ function apiPathModifierMiddleware( options, next ) { return next( options ); } +/** + * Escapes a string for literal use inside a regular expression. + * + * @param {string} value The string to escape. + * @return {string} The escaped string. + */ +function escapeRegExp( value ) { + return value.replace( /[.*+?^${}()|[\]\\]/g, '\\$&' ); +} + /** * Middleware that handles token-based authentication. * @@ -153,17 +193,12 @@ function filterEndpointsMiddleware( options, next ) { } /** - * Middleware that routes media uploads through the native host's local HTTP - * server for processing (e.g. image resizing) before uploading to WordPress. + * Middleware that routes media requests through the native host's local HTTP + * server: uploads for processing (e.g. image resizing) before they reach + * WordPress, and attachment deletions for the editor's orphan cleanup. * * Exported for testing only. * - * When `nativeUploadPort` is configured in GBKit, this middleware intercepts - * `POST /wp/v2/media` requests, forwards the file to the native server, and - * returns the response in WordPress REST API attachment format so the existing - * Gutenberg upload pipeline (blob previews, save locking, entity caching) - * works unchanged. - * * When the native server is not configured, requests pass through unmodified. * * Note: Ideally, media uploads would be handled via the `mediaUpload` editor @@ -183,16 +218,44 @@ function filterEndpointsMiddleware( options, next ) { export function nativeMediaUploadMiddleware( options, next ) { const { nativeUploadPort, nativeUploadToken } = getGBKit(); + if ( ! nativeUploadPort || ! nativeUploadToken ) { + return next( options ); + } + + // Each helper returns `null` when the request is not its concern, so an + // unhandled request falls through to the default path. + return ( + nativeMediaDelete( options, nativeUploadPort, nativeUploadToken ) ?? + nativeMediaUpload( options, nativeUploadPort, nativeUploadToken ) ?? + next( options ) + ); +} + +/** + * Routes a media upload through the native upload server. + * + * Returns `null` when the request is not a media upload, so the caller falls + * through to its normal handling. + * + * Intercepts `POST /wp/v2/media`, forwards the file to the native server, and + * returns the response in WordPress REST API attachment format so the existing + * Gutenberg upload pipeline (blob previews, save locking, entity caching) works + * unchanged. + * + * @param {Object} options The api-fetch options. + * @param {number} port The native upload server port. + * @param {string} token The native upload server bearer token. + * @return {?Promise} The relayed upload, or `null` if not applicable. + */ +function nativeMediaUpload( options, port, token ) { if ( - ! nativeUploadPort || - ! nativeUploadToken || ! options.method || options.method.toUpperCase() !== 'POST' || ! options.path || ! MEDIA_UPLOAD_PATH.test( options.path ) || ! ( options.body instanceof FormData ) ) { - return next( options ); + return null; } // Only intercept a genuine file upload. `FormData.get('file')` returns a @@ -202,11 +265,11 @@ export function nativeMediaUploadMiddleware( options, next ) { // through to the default path — and guarantees `file.name` below is safe. const file = options.body.get( 'file' ); if ( ! ( file instanceof File ) ) { - return next( options ); + return null; } info( - `Routing upload of ${ file.name } through native server on port ${ nativeUploadPort }` + `Routing upload of ${ file.name } through native server on port ${ port }` ); // Forward the original request body — the file plus every sibling field @@ -218,15 +281,34 @@ export function nativeMediaUploadMiddleware( options, next ) { // Use the two-argument form of `.then()` so the rejection handler catches // *only* a connection-level failure of the `fetch()` itself — not errors // thrown while handling a response (those must surface as real failures). - return fetch( `http://localhost:${ nativeUploadPort }/upload${ query }`, { + return fetch( `http://localhost:${ port }/upload${ query }`, { method: 'POST', headers: { - 'Relay-Authorization': `Bearer ${ nativeUploadToken }`, + 'Relay-Authorization': `Bearer ${ token }`, }, body: options.body, signal: options.signal, } ).then( ( response ) => { + // `parse: false` asks for raw `Response` semantics. Core's media + // upload middleware runs above this one and makes exactly that + // request so it can read `x-wp-upload-attachment-id` off a failed + // upload and retry `post-process`. Honor it by resolving or + // rejecting with the `Response` itself, leaving the parsing (and + // the recovery decision) to that middleware — parsing here would + // hide the header and turn a recoverable upload into a permanent + // failure. + if ( options.parse === false ) { + if ( ! response.ok ) { + // A handoff to core's post-process retry, not an outcome — + // core reads `x-wp-upload-attachment-id` off this response and + // may still recover. Stay silent (as `nativeMediaDelete` does) + // rather than reporting a failure that hasn't happened yet. + return Promise.reject( response ); + } + return response; + } + // The native server relays WordPress's response verbatim. On a // non-2xx, mirror @wordpress/api-fetch: reject with the parsed WP // error body ({ code, message, data }) so @wordpress/media-utils @@ -322,6 +404,99 @@ export function nativeMediaUploadMiddleware( options, next ) { ); } +/** + * Routes a media attachment deletion through the native upload server. + * + * Returns `null` when the request is not a media deletion, so the caller falls + * through to its normal handling. + * + * Core's media upload middleware deletes the orphaned attachment when every + * `post-process` retry fails. That request cannot be made directly from a + * cross-origin editor: `@wordpress/api-fetch` tunnels `DELETE` as a `POST` + * carrying `X-HTTP-Method-Override`, and core's `rest_allowed_cors_headers` + * does not list that header, so the browser blocks it at preflight and the + * orphan survives. Relaying through the loopback server — which sets its own + * CORS policy — is what lets the cleanup complete. + * + * This runs before `X-HTTP-Method-Override` exists: api-fetch's `httpV1` + * middleware adds it further down the chain, so the method here is still a + * plain `DELETE`. + * + * @param {Object} options The api-fetch options. + * @param {number} port The native upload server port. + * @param {string} token The native upload server bearer token. + * @return {?Promise} The relayed deletion, or `null` if not applicable. + */ +function nativeMediaDelete( options, port, token ) { + if ( options.method?.toUpperCase() !== 'DELETE' || ! options.path ) { + return null; + } + + const match = MEDIA_ATTACHMENT_PATH.exec( options.path ); + if ( ! match ) { + return null; + } + + const attachmentId = match[ 1 ]; + const query = requestQuery( options.path ); + + info( + `Routing deletion of attachment ${ attachmentId } through native server` + ); + + return fetch( + `http://localhost:${ port }/media/${ attachmentId }${ query }`, + { + method: 'DELETE', + headers: { 'Relay-Authorization': `Bearer ${ token }` }, + signal: options.signal, + } + ).then( + ( response ) => { + if ( options.parse === false ) { + if ( ! response.ok ) { + return Promise.reject( response ); + } + return response; + } + + if ( ! response.ok ) { + return response + .json() + .catch( () => invalidUploadResponseError() ) + .then( ( body ) => { + logError( 'Native media deletion failed', body ); + throw body; + } ); + } + + return response.json().catch( () => { + const error = invalidUploadResponseError(); + logError( + 'Native media deletion returned an invalid response', + error + ); + throw error; + } ); + }, + ( connectionError ) => { + if ( options.signal?.aborted ) { + throw uploadAbortError( options.signal ); + } + logError( + 'Native media deletion failed at the transport layer', + connectionError + ); + throw { + code: 'fetch_error', + message: __( + 'Could not get a valid response from the server.' + ), + }; + } + ); +} + /** * The query component of a request path, including the leading `?`, or an empty * string when there is no query. @@ -379,14 +554,14 @@ function uploadAbortError( signal ) { } /** - * Middleware to modify media upload requests. + * Middleware that strips the placeholder post ID from media upload requests. * * This middleware intercepts requests to the media endpoint and conditionally * removes the 'post' field if its value is '-1', which is used for draft posts. * * @type {APIFetchMiddleware} */ -function mediaUploadMiddleware( options, next ) { +function stripDraftPostIdMiddleware( options, next ) { if ( options.path && MEDIA_UPLOAD_PATH.test( options.path ) && diff --git a/src/utils/api-fetch.test.js b/src/utils/api-fetch.test.js index f340f9277..63cac028b 100644 --- a/src/utils/api-fetch.test.js +++ b/src/utils/api-fetch.test.js @@ -194,6 +194,69 @@ describe( 'api-fetch credentials handling', () => { } ); } ); + describe( 'apiPathModifierMiddleware', () => { + /** The URL of the first `fetch` call. */ + function requestedUrl() { + return String( global.fetch.mock.calls[ 0 ][ 0 ] ); + } + + it( 'inserts the namespace when the path lacks it', async () => { + bridge.getGBKit.mockReturnValue( { + siteApiRoot: 'https://example.com/wp-json/', + siteApiNamespace: [ 'sites/123/' ], + namespaceExcludedPaths: [], + } ); + + await apiFetch( { path: '/wp/v2/posts' } ).catch( () => {} ); + + expect( requestedUrl() ).toContain( '/wp/v2/sites/123/posts' ); + } ); + + it( 'leaves the path alone when it already carries the namespace', async () => { + bridge.getGBKit.mockReturnValue( { + siteApiRoot: 'https://example.com/wp-json/', + siteApiNamespace: [ 'sites/123/' ], + namespaceExcludedPaths: [], + } ); + + await apiFetch( { path: '/wp/v2/sites/123/posts' } ).catch( + () => {} + ); + + expect( requestedUrl() ).toContain( '/wp/v2/sites/123/posts' ); + expect( requestedUrl() ).not.toContain( 'sites/123/sites/123' ); + } ); + + it( 'leaves the path alone when no namespace is configured', async () => { + // An empty namespace previously built `new RegExp('()')`, which + // matches every string. Nothing should be inserted either way, but + // the guard must reach that outcome by reporting no namespace. + bridge.getGBKit.mockReturnValue( { + siteApiRoot: 'https://example.com/wp-json/', + siteApiNamespace: [], + namespaceExcludedPaths: [], + } ); + + await apiFetch( { path: '/wp/v2/posts' } ).catch( () => {} ); + + expect( requestedUrl() ).toContain( '/wp/v2/posts' ); + expect( requestedUrl() ).not.toContain( 'undefined' ); + } ); + + it( 'skips paths excluded from namespacing', async () => { + bridge.getGBKit.mockReturnValue( { + siteApiRoot: 'https://example.com/wp-json/', + siteApiNamespace: [ 'sites/123/' ], + namespaceExcludedPaths: [ '/oembed' ], + } ); + + await apiFetch( { path: '/oembed/1.0/proxy' } ).catch( () => {} ); + + expect( requestedUrl() ).toContain( '/oembed/1.0/proxy' ); + expect( requestedUrl() ).not.toContain( 'sites/123' ); + } ); + } ); + it( 'should preserve other headers when adding Authorization', async () => { bridge.getGBKit.mockReturnValue( { siteApiRoot: 'https://example.com/wp-json/', diff --git a/wp-env/mu-plugins/gutenbergkit-cors.php b/wp-env/mu-plugins/gutenbergkit-cors.php index 666b745f2..ef64eefab 100644 --- a/wp-env/mu-plugins/gutenbergkit-cors.php +++ b/wp-env/mu-plugins/gutenbergkit-cors.php @@ -9,12 +9,13 @@ */ function gutenbergkit_cors_allowed_origins() { return array( - 'http://localhost:5173', // Vite dev server - 'http://localhost:4173', // Vite preview server - 'http://10.0.2.2:5173', // Vite dev server (Android emulator) - 'http://10.0.2.2:4173', // Vite preview server (Android emulator) - 'https://appassets.androidplatform.net', // Android production build (HTTPS site) - 'http://appassets.androidplatform.net', // Android production build (HTTP site) + 'http://localhost:5173', // Vite dev server + 'http://localhost:4173', // Vite preview server + 'http://10.0.2.2', // wp-env site origin (Android emulator) + 'http://10.0.2.2:5173', // Vite dev server (Android emulator) + 'http://10.0.2.2:4173', // Vite preview server (Android emulator) + 'https://appassets.androidplatform.net', // Android production build (HTTPS site) + 'http://appassets.androidplatform.net', // Android production build (HTTP site) ); } diff --git a/wp-env/mu-plugins/gutenbergkit-media-failure.php b/wp-env/mu-plugins/gutenbergkit-media-failure.php new file mode 100644 index 000000000..d82b39668 --- /dev/null +++ b/wp-env/mu-plugins/gutenbergkit-media-failure.php @@ -0,0 +1,177 @@ +/post-process`. + * + * Adapted from https://github.com/WordPress/gutenberg/pull/17858#issuecomment-540114688, + * with the random failure rate replaced by an explicit mode so both outcomes + * are reproducible: + * + * recover Fail once per attachment, then succeed. The retry recovers the + * upload and the attachment completes. + * always Fail every time. Exhausts all five retries, after which the editor + * DELETEs the orphaned attachment. + * off Normal uploads (the default). + * + * The mode is stored as an option rather than read per-request, because the + * upload and each post-process retry are separate requests — and an upload + * routed through the native upload server is relayed by URLSession/OkHttp, + * which carries no browser cookie. Set it over the REST API: + * + * curl -u "$USER:$APP_PASSWORD" -X POST \ + * 'http://localhost:8888/wp-json/gutenbergkit/v1/media-failure?mode=recover' + * + * Or with WP-CLI: + * + * npx wp-env run cli wp option update gbk_media_failure_mode recover + */ + +const GBK_MEDIA_FAILURE_OPTION = 'gbk_media_failure_mode'; +const GBK_MEDIA_FAILURE_MODES = array( 'off', 'recover', 'always' ); + +/** + * Registers a route for flipping the failure mode without WP-CLI. + */ +function gbk_media_failure_register_route() { + register_rest_route( + 'gutenbergkit/v1', + '/media-failure', + array( + 'methods' => array( 'GET', 'POST' ), + 'permission_callback' => function () { + return current_user_can( 'manage_options' ); + }, + 'args' => array( + 'mode' => array( + 'type' => 'string', + 'enum' => GBK_MEDIA_FAILURE_MODES, + 'default' => null, + ), + ), + 'callback' => function ( $request ) { + $mode = $request->get_param( 'mode' ); + + if ( null !== $mode ) { + update_option( GBK_MEDIA_FAILURE_OPTION, $mode ); + // A stale per-attachment counter would make `recover` skip + // its failure on an attachment that already burned one. + gbk_media_failure_reset_counters(); + } + + return array( 'mode' => gbk_media_failure_mode() ); + }, + ) + ); +} +add_action( 'rest_api_init', 'gbk_media_failure_register_route' ); + +/** + * Clears the per-attachment failure counters. + */ +function gbk_media_failure_reset_counters() { + global $wpdb; + $wpdb->delete( $wpdb->postmeta, array( 'meta_key' => '_gbk_media_failure_count' ) ); +} + +/** + * Returns the active failure mode. + */ +function gbk_media_failure_mode() { + $mode = get_option( GBK_MEDIA_FAILURE_OPTION, 'off' ); + + return in_array( $mode, GBK_MEDIA_FAILURE_MODES, true ) ? $mode : 'off'; +} + +/** + * Whether this sub-size generation pass should fatal. + * + * In `recover` mode the count is tracked per attachment, so a second upload + * fails once too rather than succeeding immediately because an earlier upload + * already burned the failure. + * + * @param int $attachment_id The attachment being processed. + */ +function gbk_media_failure_should_fail( $attachment_id ) { + switch ( gbk_media_failure_mode() ) { + case 'always': + return true; + + case 'recover': + $key = '_gbk_media_failure_count'; + // `get_post_meta` returns '' when unset, which casts to 0. + $attempts = (int) get_post_meta( $attachment_id, $key, true ); + update_post_meta( $attachment_id, $key, $attempts + 1 ); + return 0 === $attempts; + + default: + return false; + } +} + +/** + * Whether the current request is one this simulator should be able to fail. + * + * Only the upload and its `post-process` retries. A `DELETE` must be left alone + * even in `always` mode: core's `force=true` delete path also runs sub-size + * handling, so failing there fatals the editor's orphan cleanup — the very + * request that proves the retry gave up correctly. Worse, a fatal aborts the + * request before `rest_pre_serve_request` adds CORS headers, so the editor sees + * a CORS error rather than the 500, which reads like a bug in the editor rather + * than in this plugin. + */ +function gbk_media_failure_is_failable_request() { + $method = isset( $_SERVER['REQUEST_METHOD'] ) + ? strtoupper( sanitize_text_field( wp_unslash( $_SERVER['REQUEST_METHOD'] ) ) ) + : ''; + + // api-fetch tunnels DELETE through POST with an override header, so the + // bare method is not enough to identify a delete. + $override = isset( $_SERVER['HTTP_X_HTTP_METHOD_OVERRIDE'] ) + ? strtoupper( sanitize_text_field( wp_unslash( $_SERVER['HTTP_X_HTTP_METHOD_OVERRIDE'] ) ) ) + : ''; + + return 'DELETE' !== $method && 'DELETE' !== $override; +} + +/** + * Fatals during sub-size generation when the active mode calls for it. + * + * Hooks both filters core runs while building sub-sizes: the initial upload + * goes through `intermediate_image_sizes_advanced`, and the `post-process` + * retry goes through `wp_get_missing_image_subsizes`. + * + * @param array $sizes The sizes to generate. + * @param mixed $metadata Image metadata (unused). + * @param int $attachment_id The attachment being processed. + */ +function gbk_media_failure_maybe_fail( $sizes, $metadata = null, $attachment_id = 0 ) { + if ( ! gbk_media_failure_is_failable_request() ) { + return $sizes; + } + + if ( gbk_media_failure_should_fail( $attachment_id ) ) { + // Send the 500 explicitly before dying. A real PHP fatal under FPM + // surfaces as a 500, but the Playground runtime wp-env uses returns 200, + // which the editor's retry (`status >= 500`) would ignore. The status + // has to be set here because `X-WP-Upload-Attachment-ID` was already + // sent above this filter, so PHP has committed the response. + if ( ! headers_sent() ) { + http_response_code( 500 ); + } + + // Terminate the way a memory_limit or max_execution_time fatal does, + // after the attachment row exists and its ID header has been sent. + trigger_error( 'GutenbergKit simulated media failure', E_USER_ERROR ); + exit; + } + + return $sizes; +} +add_filter( 'intermediate_image_sizes_advanced', 'gbk_media_failure_maybe_fail', 10, 3 ); +add_filter( 'wp_get_missing_image_subsizes', 'gbk_media_failure_maybe_fail', 10, 3 );