From fdf29cd6e354524f81f2749117ce2423bf6b7388 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 20 Jul 2026 15:05:57 -0400 Subject: [PATCH 1/5] feat(http): add path and query accessors to parsed requests The request target carries both the path and the query string, so callers that want to route on the path have to split it themselves. Expose `path` and `query` on both platforms' request types instead. A bare trailing "?" yields an empty query on both platforms, so the value can be appended to an upstream URL unconditionally. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../org/wordpress/gutenberg/HttpServer.kt | 17 +++++++ .../wordpress/gutenberg/HttpRequestTest.kt | 47 +++++++++++++++++++ .../GutenbergKitHTTP/ParsedHTTPRequest.swift | 23 +++++++++ .../ParsedHTTPRequestTests.swift | 41 ++++++++++++++++ 4 files changed, 128 insertions(+) create mode 100644 android/Gutenberg/src/test/java/org/wordpress/gutenberg/HttpRequestTest.kt 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 abfc9c7d4..8d0885539 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/HttpServer.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/HttpServer.kt @@ -45,6 +45,23 @@ data class HttpRequest( * for building an appropriate error response. */ val serverError: org.wordpress.gutenberg.http.HTTPRequestParseError? = null ) { + /** + * The path portion of [target], without the query component + * (e.g., "/wp/v2/posts" for "/wp/v2/posts?per_page=10"). + * + * Use this for routing — matching against [target] fails as soon as a + * client appends a query string. + */ + val path: String + get() = target.substringBefore('?') + + /** + * The query component of [target], including the leading "?" + * (e.g., "?per_page=10"), or an empty string when there is no query. + */ + val query: String + get() = target.substringAfter('?', "").let { if (it.isEmpty()) "" else "?$it" } + /** * Returns the value of the first header matching the given name (case-insensitive). */ diff --git a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/HttpRequestTest.kt b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/HttpRequestTest.kt new file mode 100644 index 000000000..6aae33ebd --- /dev/null +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/HttpRequestTest.kt @@ -0,0 +1,47 @@ +package org.wordpress.gutenberg + +import org.junit.Assert.assertEquals +import org.junit.Test + +class HttpRequestTest { + + private fun request(target: String) = HttpRequest( + method = "POST", + target = target, + headers = emptyMap() + ) + + @Test + fun `path is the whole target when there is no query`() { + assertEquals("/upload", request("/upload").path) + assertEquals("", request("/upload").query) + } + + @Test + fun `path and query split on the first question mark`() { + val parsed = request("/upload?_embed=wp:featuredmedia") + assertEquals("/upload", parsed.path) + assertEquals("?_embed=wp:featuredmedia", parsed.query) + } + + @Test + fun `a trailing question mark yields an empty query`() { + val parsed = request("/upload?") + assertEquals("/upload", parsed.path) + assertEquals("", parsed.query) + } + + @Test + fun `later question marks belong to the query`() { + val parsed = request("/search?q=a?b") + assertEquals("/search", parsed.path) + assertEquals("?q=a?b", parsed.query) + } + + @Test + fun `multiple query parameters are preserved`() { + val parsed = request("/wp/v2/posts?per_page=10&page=2") + assertEquals("/wp/v2/posts", parsed.path) + assertEquals("?per_page=10&page=2", parsed.query) + } +} diff --git a/ios/Sources/GutenbergKitHTTP/ParsedHTTPRequest.swift b/ios/Sources/GutenbergKitHTTP/ParsedHTTPRequest.swift index d68774921..eb52cc849 100644 --- a/ios/Sources/GutenbergKitHTTP/ParsedHTTPRequest.swift +++ b/ios/Sources/GutenbergKitHTTP/ParsedHTTPRequest.swift @@ -32,6 +32,29 @@ extension ParsedHTTPRequest { } } + /// The path portion of ``target``, without the query component + /// (e.g., "/wp/v2/posts" for "/wp/v2/posts?per_page=10"). + /// + /// Use this for routing — matching against ``target`` fails as soon as a + /// client appends a query string. + public var path: String { + let target = target + guard let separator = target.firstIndex(of: "?") else { return target } + return String(target[target.startIndex.. Date: Mon, 20 Jul 2026 15:06:08 -0400 Subject: [PATCH 2/5] fix: route the upload server on the path, not the full target Media uploads fail with a 404. `@wordpress/media-utils` uploads to `/wp/v2/media?_embed=wp:featuredmedia`, and the middleware now forwards that query on to the native server so it can be relayed to WordPress. The route guard still compared the full request target against "/upload", so a query string made it miss and return 404 before the upload handler ever ran. Match on `path` instead, and take the relayed query from `query` rather than re-deriving it in the handler. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../wordpress/gutenberg/MediaUploadServer.kt | 9 ++--- .../gutenberg/MediaUploadServerTest.kt | 30 ++++++++++++++++ .../Sources/Media/MediaUploadServer.swift | 10 +++--- .../Media/MediaUploadServerTests.swift | 34 ++++++++++++++++++- 4 files changed, 73 insertions(+), 10 deletions(-) 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 b1d60c868..c289866a8 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt @@ -175,8 +175,10 @@ internal class MediaUploadServer( } // Route: only POST /upload is handled. (OPTIONS preflight is answered by - // the HTTP library under its permissive CORS policy.) - if (request.method.uppercase() != "POST" || request.target != "/upload") { + // 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") } @@ -192,8 +194,7 @@ internal class MediaUploadServer( // The non-file parts (post, additionalData) and the original query // (e.g. ?_embed) must reach WordPress too — relay them alongside the file. val extraParts = parts.filter { it.filename == null } - val queryValue = request.target.substringAfter('?', "") - val query = if (queryValue.isEmpty()) "" else "?$queryValue" + val query = request.query val tempFile = writePartToTempFile(filePart) ?: return errorResponse(500, "Failed to save file") 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 78276226b..15bb1993b 100644 --- a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/MediaUploadServerTest.kt +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/MediaUploadServerTest.kt @@ -101,6 +101,33 @@ class MediaUploadServerTest { assertTrue(response.statusLine.contains("404")) } + @Test + fun `routes upload with a query string and relays the query`() { + val delegate = ProcessOnlyDelegate() + val mockUploader = MockDefaultUploader() + server.stop() + server = MediaUploadServer(uploadDelegate = delegate, defaultUploader = 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 + // match on the path alone, and the query must reach WordPress unchanged. + val boundary = "test-boundary-query" + val body = buildMultipartBody(boundary, "photo.jpg", "image/jpeg", "fake image data".toByteArray()) + + val response = sendRawRequest( + method = "POST", + path = "/upload?_embed=wp:featuredmedia", + 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")) + assertEquals("?_embed=wp:featuredmedia", mockUploader.lastQuery) + } + // MARK: - Upload with delegate @Test @@ -566,6 +593,7 @@ class MediaUploadServerTest { @Volatile var passthroughUploadCalled = false @Volatile var lastUploadMimeType: String? = null @Volatile var lastUploadFilename: String? = null + @Volatile var lastQuery: String? = null override suspend fun upload( file: File, mimeType: String, filename: String, @@ -574,6 +602,7 @@ class MediaUploadServerTest { uploadCalled = true lastUploadMimeType = mimeType lastUploadFilename = filename + lastQuery = query return mockResponse() } @@ -583,6 +612,7 @@ class MediaUploadServerTest { query: String ): MediaUploadResponse { passthroughUploadCalled = true + lastQuery = query return mockResponse() } diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift index d2396f274..53795d79f 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift @@ -79,8 +79,10 @@ final class MediaUploadServer: Sendable { } // Route: only POST /upload is handled. (OPTIONS preflight is answered by - // the HTTP library under its permissive CORS policy.) - guard parsed.method.uppercased() == "POST", parsed.target == "/upload" else { + // 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") } @@ -104,9 +106,7 @@ final class MediaUploadServer: Sendable { // 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.target.firstIndex(of: "?").map { - String(request.parsed.target[$0...]) - } ?? "" + let query = request.parsed.query // Write part body to a dedicated temp file for the delegate. // diff --git a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift index ddb5d0cee..d872fcc56 100644 --- a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift +++ b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift @@ -100,6 +100,32 @@ struct MediaUploadServerTests { #expect(httpResponse.statusCode == 404) } + @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) + defer { server.stop() } + + // `@wordpress/media-utils` uploads to `/wp/v2/media?_embed=wp:featuredmedia`, + // so the middleware forwards that query on to the native server. Routing must + // match on the path alone, and the query must reach WordPress unchanged. + 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?_embed=wp:featuredmedia")! + 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) + let httpResponse = try #require(response as? HTTPURLResponse) + #expect(httpResponse.statusCode == 201) + #expect(mockUploader.lastQuery == "?_embed=wp:featuredmedia") + } + @Test("calls delegate and returns upload result") func delegateProcessAndUpload() async throws { let delegate = MockUploadDelegate() @@ -599,11 +625,13 @@ private final class MockDefaultUploader: DefaultMediaUploader, @unchecked Sendab private var _passthroughUploadCalled = false private var _lastUploadMimeType: String? private var _lastUploadFilename: String? + private var _lastQuery: String? 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 } } init() { super.init(httpClient: MockHTTPClient(), siteApiRoot: URL(string: "https://example.com/wp-json/")!) @@ -614,12 +642,16 @@ private final class MockDefaultUploader: DefaultMediaUploader, @unchecked Sendab _uploadCalled = true _lastUploadMimeType = mimeType _lastUploadFilename = filename + _lastQuery = query } return mockResponse() } override func passthroughUpload(body: RequestBody, contentType: String, query: String) async throws -> MediaUploadResponse { - lock.withLock { _passthroughUploadCalled = true } + lock.withLock { + _passthroughUploadCalled = true + _lastQuery = query + } return mockResponse() } From 3455213ac36a408f00f826db7260b6d1b351a365 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Tue, 21 Jul 2026 11:05:33 -0400 Subject: [PATCH 3/5] fix(upload): normalize a bare trailing "?" to an empty query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The native `query` accessors treat a bare trailing "?" as carrying no parameters and yield an empty string, so the value can be appended to an upstream URL unconditionally. The middleware that produces the target still derived the query with `indexOf`/`slice`, which keeps the "?" — `/wp/v2/media?` became `POST /upload?`, a target both platforms document as impossible. Extract `requestQuery` so the rule is stated once on this side and named against its native counterparts. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/utils/api-fetch-upload-middleware.test.js | 31 +++++++++++++++++++ src/utils/api-fetch.js | 25 +++++++++++++-- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/utils/api-fetch-upload-middleware.test.js b/src/utils/api-fetch-upload-middleware.test.js index 586cf9602..07bac0062 100644 --- a/src/utils/api-fetch-upload-middleware.test.js +++ b/src/utils/api-fetch-upload-middleware.test.js @@ -238,6 +238,37 @@ describe( 'nativeMediaUploadMiddleware', () => { expect( fetchOptions.body.get( 'post' ) ).toBe( '123' ); } ); + it.each( [ + // A bare trailing `?` carries no parameters. The native `query` + // accessors normalize it away, so this side must too — otherwise the + // upload server receives a `?` the two platforms agree cannot exist. + [ '/wp/v2/media?', 'http://localhost:12345/upload' ], + [ '/wp/v2/media', 'http://localhost:12345/upload' ], + ] )( 'normalizes the query of %s to %s', async ( path, expectedUrl ) => { + getGBKit.mockReturnValue( { + nativeUploadPort: 12345, + nativeUploadToken: 'test-token', + } ); + + global.fetch = vi.fn( () => + Promise.resolve( { + ok: true, + json: () => Promise.resolve( { id: 1 } ), + } ) + ); + + const body = new FormData(); + body.append( 'file', makeFile(), 'photo.jpg' ); + + await nativeMediaUploadMiddleware( + { method: 'POST', path, body }, + makeNext() + ); + + const [ url ] = global.fetch.mock.calls[ 0 ]; + expect( url ).toBe( expectedUrl ); + } ); + it( 'returns the relayed WordPress attachment unchanged', async () => { getGBKit.mockReturnValue( { nativeUploadPort: 8080, diff --git a/src/utils/api-fetch.js b/src/utils/api-fetch.js index 7cf5673ff..e08d03220 100644 --- a/src/utils/api-fetch.js +++ b/src/utils/api-fetch.js @@ -206,8 +206,7 @@ export function nativeMediaUploadMiddleware( options, next ) { // (`post`, additionalData) — and the original query string (e.g. `?_embed`) // so the native server can relay them to WordPress unchanged. Rebuilding the // body with only `file` would drop the post association and additionalData. - const queryIndex = options.path.indexOf( '?' ); - const query = queryIndex === -1 ? '' : options.path.slice( queryIndex ); + const query = requestQuery( options.path ); // Use the two-argument form of `.then()` so the rejection handler catches // *only* a connection-level failure of the `fetch()` itself — not errors @@ -278,6 +277,28 @@ export function nativeMediaUploadMiddleware( options, next ) { ); } +/** + * The query component of a request path, including the leading `?`, or an empty + * string when there is no query. + * + * Mirrors the `query` accessors on the native request types (`HttpRequest` on + * Android, `ParsedHTTPRequest` on iOS): the split is on the first `?`, and a + * bare trailing `?` carries no parameters so it yields an empty string. Keeping + * the three in agreement means the value can be appended to an upstream URL + * unconditionally, whichever side derived it. + * + * @param {string} path The request path, e.g. `/wp/v2/media?_embed`. + * @return {string} The query, e.g. `?_embed`, or `''`. + */ +function requestQuery( path ) { + const separator = path.indexOf( '?' ); + if ( separator === -1 ) { + return ''; + } + const value = path.slice( separator + 1 ); + return value ? `?${ value }` : ''; +} + /** * The error rejected when the upload server's response body can't be parsed as * JSON. Shaped like a WordPress REST error so `@wordpress/media-utils` surfaces From 0aca0f9487bc09fc72960a44959631b442499632 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Tue, 21 Jul 2026 11:05:41 -0400 Subject: [PATCH 4/5] test(upload): pin which upload branch relays the query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both `upload` and `passthroughUpload` record `lastQuery` on the mock, so asserting the query alone passes whichever branch ran — a regression that collapsed routing onto one path would still go green. Assert the passthrough branch explicitly, matching the sibling tests. Forcing `processFile` to return `.processed` now fails these tests; the query assertion alone did not. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../java/org/wordpress/gutenberg/MediaUploadServerTest.kt | 5 +++++ .../GutenbergKitTests/Media/MediaUploadServerTests.swift | 5 +++++ 2 files changed, 10 insertions(+) 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 15bb1993b..7ce925f0f 100644 --- a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/MediaUploadServerTest.kt +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/MediaUploadServerTest.kt @@ -125,6 +125,11 @@ class MediaUploadServerTest { ) assertTrue("Expected 201 but got: ${response.statusLine}", response.statusLine.contains("201")) + // The delegate 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) + assertFalse(mockUploader.uploadCalled) assertEquals("?_embed=wp:featuredmedia", mockUploader.lastQuery) } diff --git a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift index d872fcc56..1d29e4ddc 100644 --- a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift +++ b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift @@ -123,6 +123,11 @@ 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. + // 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) + #expect(!mockUploader.uploadCalled) #expect(mockUploader.lastQuery == "?_embed=wp:featuredmedia") } From b1a776bc2b7bd2e2dd452366bf2eff5ccc9ce22f Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Tue, 21 Jul 2026 11:12:20 -0400 Subject: [PATCH 5/5] refactor(http): express `path` with prefix(upTo:) The explicit `startIndex.. --- ios/Sources/GutenbergKitHTTP/ParsedHTTPRequest.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ios/Sources/GutenbergKitHTTP/ParsedHTTPRequest.swift b/ios/Sources/GutenbergKitHTTP/ParsedHTTPRequest.swift index eb52cc849..95222d1c0 100644 --- a/ios/Sources/GutenbergKitHTTP/ParsedHTTPRequest.swift +++ b/ios/Sources/GutenbergKitHTTP/ParsedHTTPRequest.swift @@ -40,7 +40,7 @@ extension ParsedHTTPRequest { public var path: String { let target = target guard let separator = target.firstIndex(of: "?") else { return target } - return String(target[target.startIndex..