Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ wp-env-android-reset: ## Remove the Android emulator URL remap and restart
@RESET=1 $(MAKE) wp-env-start

.PHONY: wp-env-media-failure
wp-env-media-failure: ## Report the media upload failure simulation mode (MODE=off|recover|always to set it)
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

################################################################################
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,20 @@ interface MediaUploadDelegate {
* the editor sees a complete attachment object.
*/
suspend fun uploadFile(file: File, mimeType: String, filename: String): MediaUploadResponse? = null

/**
* Delete a previously uploaded attachment.
*
* The editor deletes the attachment when an upload's server-side
* post-processing fails past recovery, so it does not leave an orphan
* behind. A delegate that uploaded the attachment itself via [uploadFile]
* owns an ID only it can resolve, so it must delete the attachment itself
* too — the default uploader would address the wrong site.
*
* Return the raw response (status code + body), which GutenbergKit relays
* to the editor unchanged, or null to use the default uploader.
*/
suspend fun deleteFile(attachmentId: String): MediaUploadResponse? = null
}

/**
Expand Down Expand Up @@ -234,7 +248,7 @@ internal class MediaUploadServer(

if (method == "DELETE") {
attachmentIdFromPath(request.path)?.let { attachmentId ->
return handleDelete(attachmentId, request.query)
return handleMediaDelete(attachmentId, request.query)
}
}

Expand All @@ -255,17 +269,22 @@ internal class MediaUploadServer(
}

/**
* Relays the editor's orphan cleanup to WordPress.
* Relays the editor's orphan cleanup.
*
* Core's media upload middleware deletes the attachment when every
* `post-process` retry fails. A cross-origin editor cannot issue that
* request directly — api-fetch tunnels `DELETE` 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 cleanup run.
*
* Offers the deletion to the delegate first, as [handleUpload] does, so a
* host that uploaded the attachment itself deletes it from the same place.
*/
private suspend fun handleDelete(attachmentId: String, query: String): HttpResponse {
val uploader = defaultUploader ?: return errorResponse(500, "No uploader configured")
private suspend fun handleMediaDelete(attachmentId: String, query: String): HttpResponse {
return try {
uploadDelegate?.deleteFile(attachmentId)?.let { return relayResponse(it) }

val uploader = defaultUploader ?: return errorResponse(500, "No uploader configured")
relayResponse(uploader.deleteMedia(attachmentId, query))
} catch (e: IOException) {
Log.e(TAG, "Media deletion failed", e)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,27 @@ class MediaUploadServerTest {
assertTrue(response.statusLine.contains("404"))
}

@Test
fun `routes a deletion to the delegate when it handles one`() {
// A host that uploaded the attachment itself owns an ID only it can
// resolve, so the default uploader must not be asked to delete it. No
// default uploader is configured, so a 200 here can only come from the
// delegate — the fallback path would fail with "no uploader".
val delegate = DeletingDelegate()
server.stop()
server = MediaUploadServer(uploadDelegate = delegate, defaultUploader = null, 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"))
assertEquals("42", delegate.deletedAttachmentId)
}

@Test
fun `routes upload with a query string and relays the query`() {
val delegate = ProcessOnlyDelegate()
Expand Down Expand Up @@ -734,6 +755,19 @@ class MediaUploadServerTest {
}
}

/**
* A delegate that handles deletions itself, as a host uploading to its own
* media service would.
*/
private class DeletingDelegate : MediaUploadDelegate {
@Volatile var deletedAttachmentId: String? = null

override suspend fun deleteFile(attachmentId: String): MediaUploadResponse? {
deletedAttachmentId = attachmentId
return MediaUploadResponse(200, """{"deleted":true}""".toByteArray())
}
}

private class ProcessOnlyDelegate : MediaUploadDelegate {
@Volatile var processFileCalled = false

Expand Down
21 changes: 0 additions & 21 deletions docs/code/local-wordpress.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,6 @@ The mode is stored server-side, so it persists across uploads and retries until

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

The mode is stored as an option rather than per-request state, because the upload and each retry are separate requests — and an upload routed through the native upload server is relayed by URLSession/OkHttp, which carries no browser cookie.

**Note:** the plugin sets the 500 status itself. A real PHP fatal under FPM surfaces as a 500, but the Playground runtime wp-env uses returns 200, which the editor's `status >= 500` check would ignore.

**Note:** a simulated fatal aborts the request before WordPress adds CORS headers, so a cross-origin editor reports these responses as CORS errors with provisional request headers rather than as a readable 500. That is expected for the upload and `post-process` responses — the editor only needs their status and the attachment ID header. It is why the plugin never fails a `DELETE`, including the `POST` + `X-Http-Method-Override: DELETE` form api-fetch actually sends: failing the orphan cleanup would make a correctly working retry look broken.

**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.
Expand Down Expand Up @@ -162,21 +156,6 @@ Another service is using port 8888. Stop the conflicting service or change the w
}
```

Under the Playground runtime the culprit is often a previous wp-env server that outlived `make wp-env-stop`, which then makes every subsequent start fail with `EADDRINUSE`:

```bash
lsof -ti:8888 # confirm what holds the port
pkill -f "wp-playground.js"
```

### Credentials rejected with HTTP 401

The Playground runtime starts from a fresh database on each start, so an existing `.wp-env.credentials.json` no longer matches the site's application password. Regenerate it:

```bash
make wp-env-start RESET=1
```

### Resetting the environment

To start fresh, destroy the environment and recreate it:
Expand Down
17 changes: 17 additions & 0 deletions ios/Sources/GutenbergKit/Sources/Media/MediaUploadDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,19 @@ public protocol MediaUploadDelegate: AnyObject, Sendable {
/// 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?

/// Delete a previously uploaded attachment.
///
/// The editor deletes the attachment when an upload's server-side
/// post-processing fails past recovery, so it does not leave an orphan
/// behind. A delegate that uploaded the attachment itself via
/// ``uploadFile(at:mimeType:filename:)`` owns an ID only it can resolve, so
/// it must delete the attachment itself too — the default uploader would
/// address the wrong site.
///
/// Return the raw response (status code + body), which GutenbergKit relays
/// to the editor unchanged, or `nil` to use the default uploader.
func deleteFile(attachmentId: String) async throws -> MediaUploadResponse?
}

/// Default implementations.
Expand All @@ -97,4 +110,8 @@ extension MediaUploadDelegate {
public func uploadFile(at url: URL, mimeType: String, filename: String) async throws -> MediaUploadResponse? {
nil
}

public func deleteFile(attachmentId: String) async throws -> MediaUploadResponse? {
nil
}
}
20 changes: 14 additions & 6 deletions ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ final class MediaUploadServer: Sendable {
}

if method == "DELETE", let attachmentId = attachmentId(fromPath: parsed.path) {
return await handleDelete(attachmentId, query: parsed.query, context: context)
return await handleMediaDelete(attachmentId, query: parsed.query, context: context)
}

return errorResponse(status: 404, message: "Not found")
Expand Down Expand Up @@ -208,20 +208,28 @@ final class MediaUploadServer: Sendable {
return id
}

/// Relays the editor's orphan cleanup to WordPress.
/// Relays the editor's orphan cleanup.
///
/// Core's media upload middleware deletes the attachment when every
/// `post-process` retry fails. A cross-origin editor cannot issue that
/// request directly — api-fetch tunnels `DELETE` 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 cleanup run.
private static func handleDelete(
///
/// Offers the deletion to the delegate first, as ``handleUpload(_:context:)``
/// does, so a host that uploaded the attachment itself deletes it from the
/// same place.
private static func handleMediaDelete(
_ attachmentId: String, query: String, context: UploadContext
) async -> HTTPResponse {
guard let defaultUploader = context.defaultUploader else {
return errorResponse(status: 500, message: UploadError.noUploader.localizedDescription)
}
do {
if let response = try await context.uploadDelegate?.deleteFile(attachmentId: attachmentId) {
return relayResponse(response)
}

guard let defaultUploader = context.defaultUploader else {
return errorResponse(status: 500, message: UploadError.noUploader.localizedDescription)
}
let response = try await defaultUploader.deleteMedia(attachmentId: attachmentId, query: query)
return relayResponse(response)
} catch {
Expand Down
36 changes: 36 additions & 0 deletions ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,28 @@ struct MediaUploadServerTests {
#expect(mockUploader.lastQuery == "?_embed=wp:featuredmedia")
}

@Test("routes a deletion to the delegate when it handles one")
func delegateHandlesDeletion() async throws {
// A host that uploaded the attachment itself owns an ID only it can
// resolve, so the default uploader must not be asked to delete it.
// No default uploader is configured, so a 200 here can only come from the
// delegate — the fallback path would fail with "no uploader".
let delegate = DeletingDelegate()
let server = try await MediaUploadServer.start(uploadDelegate: delegate)
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(delegate.deletedAttachmentId == "42")
}

@Test("calls delegate and returns upload result")
func delegateProcessAndUpload() async throws {
let delegate = MockUploadDelegate()
Expand Down Expand Up @@ -795,6 +817,20 @@ private final class MockUploadDelegate: MediaUploadDelegate, @unchecked Sendable
}
}

/// A delegate that handles deletions itself, as a host uploading to its own
/// media service would.
private final class DeletingDelegate: MediaUploadDelegate, @unchecked Sendable {
private let lock = NSLock()
private var _deletedAttachmentId: String?

var deletedAttachmentId: String? { lock.withLock { _deletedAttachmentId } }

func deleteFile(attachmentId: String) async throws -> MediaUploadResponse? {
lock.withLock { _deletedAttachmentId = attachmentId }
return MediaUploadResponse(statusCode: 200, body: Data(#"{"deleted":true}"#.utf8))
}
}

private final class ProcessOnlyDelegate: MediaUploadDelegate, @unchecked Sendable {
private let lock = NSLock()
private var _processFileCalled = false
Expand Down
69 changes: 39 additions & 30 deletions src/utils/api-fetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,6 @@ function corsMiddleware( options, next ) {
*/
function apiPathModifierMiddleware( options, next ) {
const { siteApiNamespace, namespaceExcludedPaths } = getGBKit();
// Self-hosted sites configure no namespace, so there is nothing to insert.
// This has to gate the rewrite explicitly: the namespace match below cannot
// stand in for it, and an empty namespace would otherwise interpolate
// `undefined` into the path.
const isEligiblePath =
options.path &&
siteApiNamespace.length > 0 &&
Expand All @@ -113,10 +109,11 @@ function apiPathModifierMiddleware( options, next ) {
).test( options.path ) || /\/sites\/[^/]+\//.test( options.path );

if ( isEligiblePath && ! alreadyHasSiteNamespace ) {
// Insert the API namespace after the first two path segments.
// Insert the API namespace after the first two path segments, with a
// single trailing slash.
options.path = options.path.replace(
/^(?<apiPath>\/?(?:[\w.-]+\/){2})/,
`$<apiPath>${ siteApiNamespace[ 0 ] }`
`$<apiPath>${ siteApiNamespace[ 0 ].replace( /\/+$/, '' ) }/`
);
}

Expand Down Expand Up @@ -197,17 +194,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
Expand All @@ -227,27 +219,44 @@ function filterEndpointsMiddleware( options, next ) {
export function nativeMediaUploadMiddleware( options, next ) {
const { nativeUploadPort, nativeUploadToken } = getGBKit();

if ( nativeUploadPort && nativeUploadToken ) {
const deletion = nativeMediaDelete(
options,
nativeUploadPort,
nativeUploadToken
);
if ( deletion ) {
return deletion;
}
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
Expand All @@ -257,11 +266,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
Expand All @@ -273,10 +282,10 @@ 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,
Expand Down
Loading
Loading