From e9193d6a3751bfa6ac6c6de83283cf16b750bf9a Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Thu, 2 Apr 2026 14:36:30 -0400 Subject: [PATCH 01/13] fix: Avoid "native" term in user-facing errors messages The term "native" is likely unfamiliar to users, provides no tangible value, and may cause confusion. --- src/utils/api-fetch.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/api-fetch.js b/src/utils/api-fetch.js index 2c55447cd..4f1b57d84 100644 --- a/src/utils/api-fetch.js +++ b/src/utils/api-fetch.js @@ -202,7 +202,7 @@ export function nativeMediaUploadMiddleware( options, next ) { if ( ! response.ok ) { return response.text().then( ( body ) => { const error = new Error( - `Native upload failed (${ response.status }): ${ + `Upload failed (${ response.status }): ${ body || response.statusText }` ); From 32aa39ac2175b005fa12710ce2d834c3efc78a4a Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Thu, 2 Apr 2026 14:38:51 -0400 Subject: [PATCH 02/13] fix: drain oversized request body before sending 413 response MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Content-Length exceeds maxBodySize, the server now reads and discards the full request body before responding with 413. This ensures the client (WebView fetch) receives the error response cleanly instead of a connection reset (RFC 9110 §15.5.14). Adds a `.draining` parser state that tracks consumed bytes without buffering them, keeping memory and disk usage at zero for rejected uploads. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../org/wordpress/gutenberg/HttpServer.kt | 13 ++++ .../gutenberg/http/HTTPRequestParser.kt | 21 +++++- .../gutenberg/http/HTTPRequestParserTests.kt | 65 +++++++++++++++++++ .../GutenbergKitHTTP/HTTPRequestParser.swift | 27 ++++++-- ios/Sources/GutenbergKitHTTP/HTTPServer.swift | 6 ++ .../HTTPRequestParserTests.swift | 42 +++++++++++- 6 files changed, 164 insertions(+), 10 deletions(-) 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 fcc98d655..0011915d2 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/HttpServer.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/HttpServer.kt @@ -286,6 +286,19 @@ class HttpServer( parser.append(buffer.copyOfRange(0, bytesRead)) } + // Drain oversized body before throwing so the + // client receives the 413 (RFC 9110 §15.5.14). + if (parser.state == HTTPRequestParser.State.DRAINING) { + while (!parser.state.isComplete) { + if (System.nanoTime() > deadlineNanos) { + throw SocketTimeoutException("Read deadline exceeded") + } + val bytesRead = input.read(buffer) + if (bytesRead == -1) break + parser.append(buffer.copyOfRange(0, bytesRead)) + } + } + // Validate headers (triggers full RFC validation). val partial = try { parser.parseRequest() diff --git a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/http/HTTPRequestParser.kt b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/http/HTTPRequestParser.kt index 20e46a758..370d7d59f 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/http/HTTPRequestParser.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/http/HTTPRequestParser.kt @@ -37,12 +37,18 @@ class HTTPRequestParser( NEEDS_MORE_DATA, /** Headers have been fully received but the body is still incomplete. */ HEADERS_COMPLETE, + /** + * The request body exceeds the maximum allowed size and is being + * drained (read and discarded) so the server can send a clean 413 + * response. No body bytes are buffered in this state. + */ + DRAINING, /** All data has been received (headers and body). */ COMPLETE; /** Whether headers have been fully received. */ val hasHeaders: Boolean - get() = this == HEADERS_COMPLETE || this == COMPLETE + get() = this == HEADERS_COMPLETE || this == DRAINING || this == COMPLETE /** Whether all data has been received. */ val isComplete: Boolean @@ -107,6 +113,17 @@ class HTTPRequestParser( fun append(data: ByteArray): Unit = synchronized(lock) { if (_state == State.COMPLETE) return + // In drain mode, discard bytes without buffering and check + // whether the full Content-Length has been consumed. + if (_state == State.DRAINING) { + bytesWritten += data.size.toLong() + val offset = headerEndOffset + if (offset != null && bytesWritten - offset >= expectedContentLength) { + _state = State.COMPLETE + } + return + } + val accepted: Boolean try { accepted = buffer.append(data) @@ -166,7 +183,7 @@ class HTTPRequestParser( if (expectedContentLength > maxBodySize) { parseError = HTTPRequestParseError.PAYLOAD_TOO_LARGE - _state = State.COMPLETE + _state = State.DRAINING return } } diff --git a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/http/HTTPRequestParserTests.kt b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/http/HTTPRequestParserTests.kt index 93bdefa35..d6b3f49b5 100644 --- a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/http/HTTPRequestParserTests.kt +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/http/HTTPRequestParserTests.kt @@ -4,6 +4,7 @@ import org.junit.Assert.assertArrayEquals import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNull +import org.junit.Assert.assertThrows import org.junit.Assert.assertTrue import org.junit.Test @@ -97,6 +98,70 @@ class HTTPRequestParserTests { assertArrayEquals(body.toByteArray(), request.body?.readBytes()) } + @Test + fun `drains oversized body before reporting payloadTooLarge`() { + val parser = HTTPRequestParser(maxBodySize = 100) + parser.append("POST /upload HTTP/1.1\r\nHost: localhost\r\nContent-Length: 101\r\n\r\n".toByteArray()) + + // Parser enters drain mode — not yet complete. + assertEquals(HTTPRequestParser.State.DRAINING, parser.state) + + // Feed the remaining body bytes to complete the drain. + parser.append(ByteArray(101) { 0x41 }) + assertTrue(parser.state.isComplete) + assertThrows(HTTPRequestParseException::class.java) { + parser.parseRequest() + }.also { + assertEquals(HTTPRequestParseError.PAYLOAD_TOO_LARGE, it.error) + } + } + + @Test + fun `enters drain mode for oversized Content-Length even when body has not arrived`() { + val parser = HTTPRequestParser(maxBodySize = 50) + parser.append("POST /upload HTTP/1.1\r\nHost: localhost\r\nContent-Length: 999999\r\n\r\n".toByteArray()) + + // Parser enters drain mode — headers are available but not yet complete. + assertEquals(HTTPRequestParser.State.DRAINING, parser.state) + assertTrue(parser.state.hasHeaders) + assertFalse(parser.state.isComplete) + + // Feed body bytes in chunks to complete the drain. + val chunkSize = 8192 + var remaining = 999999 + while (remaining > 0) { + val size = minOf(chunkSize, remaining) + parser.append(ByteArray(size) { 0x42 }) + remaining -= size + } + + assertTrue(parser.state.isComplete) + assertThrows(HTTPRequestParseException::class.java) { + parser.parseRequest() + }.also { + assertEquals(HTTPRequestParseError.PAYLOAD_TOO_LARGE, it.error) + } + } + + @Test + fun `drain mode does not buffer body bytes`() { + val parser = HTTPRequestParser(maxBodySize = 10) + parser.append("POST /upload HTTP/1.1\r\nHost: localhost\r\nContent-Length: 1000\r\n\r\n".toByteArray()) + assertEquals(HTTPRequestParser.State.DRAINING, parser.state) + + // Feed 1000 bytes of body data. + parser.append(ByteArray(1000) { 0x43 }) + assertTrue(parser.state.isComplete) + + // The parser should still report the error, confirming the bytes + // were consumed without being stored. + assertThrows(HTTPRequestParseException::class.java) { + parser.parseRequest() + }.also { + assertEquals(HTTPRequestParseError.PAYLOAD_TOO_LARGE, it.error) + } + } + // MARK: - Error HTTP Status Mapping @Test diff --git a/ios/Sources/GutenbergKitHTTP/HTTPRequestParser.swift b/ios/Sources/GutenbergKitHTTP/HTTPRequestParser.swift index 293c92773..50ebb781d 100644 --- a/ios/Sources/GutenbergKitHTTP/HTTPRequestParser.swift +++ b/ios/Sources/GutenbergKitHTTP/HTTPRequestParser.swift @@ -27,6 +27,10 @@ public final class HTTPRequestParser: @unchecked Sendable { case needsMoreData /// Headers have been fully received but the body is still incomplete. case headersComplete + /// The request body exceeds the maximum allowed size and is being + /// drained (read and discarded) so the server can send a clean 413 + /// response. No body bytes are buffered in this state. + case draining /// All data has been received (headers and body). case complete } @@ -179,6 +183,17 @@ public final class HTTPRequestParser: @unchecked Sendable { lock.withLock { guard !_state.isComplete else { return } + // In drain mode, discard bytes without buffering and check + // whether the full Content-Length has been consumed. + if case .draining = _state { + bytesWritten += data.count + if let offset = headerEndOffset, + Int64(bytesWritten - offset) >= expectedContentLength { + _state = .complete + } + return + } + let accepted: Bool do { accepted = try buffer.append(data) @@ -236,7 +251,7 @@ public final class HTTPRequestParser: @unchecked Sendable { if expectedContentLength > maxBodySize { _parseError = .payloadTooLarge - _state = .complete + _state = .draining return } } @@ -403,14 +418,16 @@ extension HTTPRequestParser.State { /// Whether all data has been received (headers and body). public var isComplete: Bool { - if case .complete = self { return true } - return false + switch self { + case .complete: return true + case .needsMoreData, .headersComplete, .draining: return false + } } - /// Whether headers have been fully received (true for both `.headersComplete` and `.complete`). + /// Whether headers have been fully received (true for `.headersComplete`, `.draining`, and `.complete`). public var hasHeaders: Bool { switch self { - case .headersComplete, .complete: return true + case .headersComplete, .draining, .complete: return true case .needsMoreData: return false } } diff --git a/ios/Sources/GutenbergKitHTTP/HTTPServer.swift b/ios/Sources/GutenbergKitHTTP/HTTPServer.swift index 17cfeb1c7..86dbc176f 100644 --- a/ios/Sources/GutenbergKitHTTP/HTTPServer.swift +++ b/ios/Sources/GutenbergKitHTTP/HTTPServer.swift @@ -270,6 +270,12 @@ public final class HTTPServer: Sendable { // Phase 1: receive headers only. try await Self.receiveUntil(\.hasHeaders, parser: parser, on: connection, idleTimeout: idleTimeout) + // Drain oversized body before throwing so the + // client receives the 413 (RFC 9110 §15.5.14). + if parser.state == .draining { + try await Self.receiveUntil(\.isComplete, parser: parser, on: connection, idleTimeout: idleTimeout) + } + // Validate headers (triggers full RFC validation). guard let partial = try parser.parseRequest() else { throw HTTPServerError.connectionClosed diff --git a/ios/Tests/GutenbergKitHTTPTests/HTTPRequestParserTests.swift b/ios/Tests/GutenbergKitHTTPTests/HTTPRequestParserTests.swift index f4df6bd08..7da3e0d62 100644 --- a/ios/Tests/GutenbergKitHTTPTests/HTTPRequestParserTests.swift +++ b/ios/Tests/GutenbergKitHTTPTests/HTTPRequestParserTests.swift @@ -389,11 +389,16 @@ struct HTTPRequestParserTests { // MARK: - Max Body Size - @Test("rejects request when Content-Length exceeds maxBodySize") + @Test("drains oversized body before reporting payloadTooLarge") func rejectsOversizedContentLength() { let parser = HTTPRequestParser(maxBodySize: 100) parser.append(Data("POST /upload HTTP/1.1\r\nHost: localhost\r\nContent-Length: 101\r\n\r\n".utf8)) + // Parser enters drain mode — not yet complete. + #expect(parser.state == .draining) + + // Feed the remaining body bytes to complete the drain. + parser.append(Data(repeating: 0x41, count: 101)) #expect(parser.state.isComplete) #expect(throws: HTTPRequestParseError.payloadTooLarge) { try parser.parseRequest() @@ -424,18 +429,49 @@ struct HTTPRequestParserTests { #expect(try readAll(requestBody) == Data(body.utf8)) } - @Test("rejects oversized Content-Length even when body data hasn't arrived") + @Test("enters drain mode for oversized Content-Length even when body hasn't arrived") func rejectsOversizedBeforeBodyArrives() { let parser = HTTPRequestParser(maxBodySize: 50) parser.append(Data("POST /upload HTTP/1.1\r\nHost: localhost\r\nContent-Length: 999999\r\n\r\n".utf8)) - // Parser should mark complete immediately without waiting for body bytes + // Parser enters drain mode — headers are available but not yet complete. + #expect(parser.state == .draining) + #expect(parser.state.hasHeaders) + #expect(!parser.state.isComplete) + + // Feed body bytes in chunks to complete the drain. + let chunkSize = 8192 + var remaining = 999999 + while remaining > 0 { + let size = min(chunkSize, remaining) + parser.append(Data(repeating: 0x42, count: size)) + remaining -= size + } + #expect(parser.state.isComplete) #expect(throws: HTTPRequestParseError.payloadTooLarge) { try parser.parseRequest() } } + @Test("drain mode does not buffer body bytes") + func drainDoesNotBuffer() { + let parser = HTTPRequestParser(maxBodySize: 10) + let headers = "POST /upload HTTP/1.1\r\nHost: localhost\r\nContent-Length: 1000\r\n\r\n" + parser.append(Data(headers.utf8)) + #expect(parser.state == .draining) + + // Feed 1000 bytes of body data. + parser.append(Data(repeating: 0x43, count: 1000)) + #expect(parser.state.isComplete) + + // The parser should still report the error, confirming the bytes + // were consumed without being stored. + #expect(throws: HTTPRequestParseError.payloadTooLarge) { + try parser.parseRequest() + } + } + @Test("rejects headers that exceed maxHeaderSize without terminator") func rejectsOversizedHeaders() { let parser = HTTPRequestParser() From 0b6c67be77c1b9b5adb0eda8e5c37d1dca46c0d8 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Thu, 2 Apr 2026 14:42:39 -0400 Subject: [PATCH 03/13] fix: include CORS headers on server-generated error responses Add an errorResponseHeaders parameter to HTTPServer (iOS) and HttpServer (Android) so that callers can specify headers to include on all server-generated error responses (413, 407, 408, etc.). MediaUploadServer passes its CORS headers through this parameter so the browser does not block error responses due to missing Access-Control-Allow-Origin. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../java/org/wordpress/gutenberg/HttpServer.kt | 11 ++++++++++- .../wordpress/gutenberg/MediaUploadServer.kt | 11 ++++++----- .../Sources/Media/MediaUploadServer.swift | 1 + ios/Sources/GutenbergKitHTTP/HTTPServer.swift | 17 ++++++++++++----- 4 files changed, 29 insertions(+), 11 deletions(-) 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 0011915d2..967f3980b 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/HttpServer.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/HttpServer.kt @@ -150,6 +150,7 @@ class HttpServer( private val readTimeoutMs: Int = DEFAULT_READ_TIMEOUT_MS, private val idleTimeoutMs: Int = DEFAULT_IDLE_TIMEOUT_MS, private val cacheDir: File? = null, + private val errorResponseHeaders: Map = emptyMap(), private val handler: suspend (HttpRequest) -> HttpResponse ) { @Volatile @@ -250,6 +251,7 @@ class HttpServer( try { sendResponse(sock, HttpResponse( status = 408, + headers = errorResponseHeaders + ("Content-Type" to "text/plain"), body = "Request Timeout".toByteArray() )) } catch (_: Exception) { @@ -306,12 +308,14 @@ class HttpServer( val statusText = STATUS_TEXT[e.error.httpStatus] ?: "Bad Request" sendResponse(socket, HttpResponse( status = e.error.httpStatus, + headers = errorResponseHeaders + ("Content-Type" to "text/plain"), body = statusText.toByteArray() )) return } catch (_: java.io.IOException) { sendResponse(socket, HttpResponse( status = 500, + headers = errorResponseHeaders + ("Content-Type" to "text/plain"), body = "Internal Server Error".toByteArray() )) return @@ -320,6 +324,7 @@ class HttpServer( if (partial == null) { sendResponse(socket, HttpResponse( status = 400, + headers = errorResponseHeaders + ("Content-Type" to "text/plain"), body = "Bad Request".toByteArray() )) return @@ -335,7 +340,7 @@ class HttpServer( if (!authenticate(proxyAuth, token)) { sendResponse(socket, HttpResponse( status = 407, - headers = mapOf("Content-Type" to "text/plain", "Proxy-Authenticate" to "Bearer") + headers = errorResponseHeaders + mapOf("Content-Type" to "text/plain", "Proxy-Authenticate" to "Bearer") )) return } @@ -348,6 +353,7 @@ class HttpServer( if (upperMethod in listOf("POST", "PUT", "PATCH") && partial.header("Content-Length") == null) { sendResponse(socket, HttpResponse( status = 411, + headers = errorResponseHeaders + ("Content-Type" to "text/plain"), body = "Length Required".toByteArray() )) return @@ -370,12 +376,14 @@ class HttpServer( val statusText = STATUS_TEXT[e.error.httpStatus] ?: "Bad Request" sendResponse(socket, HttpResponse( status = e.error.httpStatus, + headers = errorResponseHeaders + ("Content-Type" to "text/plain"), body = statusText.toByteArray() )) return } catch (_: java.io.IOException) { sendResponse(socket, HttpResponse( status = 500, + headers = errorResponseHeaders + ("Content-Type" to "text/plain"), body = "Internal Server Error".toByteArray() )) return @@ -387,6 +395,7 @@ class HttpServer( parsed?.body?.fileOwner?.close() sendResponse(socket, HttpResponse( status = 400, + headers = errorResponseHeaders + ("Content-Type" to "text/plain"), body = "Bad Request".toByteArray() )) return 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 badb4d803..a7eb0b7a0 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt @@ -78,6 +78,7 @@ internal class MediaUploadServer( externallyAccessible = false, requiresAuthentication = true, cacheDir = cacheDir, + errorResponseHeaders = corsHeaders, handler = { request -> handleRequest(request) } ) server.start() @@ -190,11 +191,6 @@ internal class MediaUploadServer( // MARK: - Response Building - private val corsHeaders: Map = mapOf( - "Access-Control-Allow-Origin" to "*", - "Access-Control-Allow-Headers" to "Relay-Authorization, Content-Type" - ) - private fun corsPreflightResponse(): HttpResponse = HttpResponse( status = 204, headers = corsHeaders + mapOf( @@ -240,6 +236,11 @@ internal class MediaUploadServer( companion object { private const val TAG = "MediaUploadServer" + + private val corsHeaders: Map = mapOf( + "Access-Control-Allow-Origin" to "*", + "Access-Control-Allow-Headers" to "Relay-Authorization, Content-Type" + ) } } diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift index c4ef144b8..bdcc1ea53 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift @@ -36,6 +36,7 @@ final class MediaUploadServer: Sendable { let server = try await HTTPServer.start( name: "media-upload", requiresAuthentication: true, + errorResponseHeaders: corsHeaders, handler: { request in await Self.handleRequest(request, context: context) } diff --git a/ios/Sources/GutenbergKitHTTP/HTTPServer.swift b/ios/Sources/GutenbergKitHTTP/HTTPServer.swift index 86dbc176f..b068ce702 100644 --- a/ios/Sources/GutenbergKitHTTP/HTTPServer.swift +++ b/ios/Sources/GutenbergKitHTTP/HTTPServer.swift @@ -134,6 +134,9 @@ public final class HTTPServer: Sendable { /// the connection. Defaults to 30 seconds. /// - idleTimeout: The maximum time to wait between consecutive reads before closing /// the connection. Prevents slow-loris attacks. Defaults to 5 seconds. + /// - errorResponseHeaders: Headers to include on all server-generated error responses + /// (e.g. 408 timeout, 413 payload too large). Useful for CORS headers that the + /// browser requires even on error responses. /// - handler: A closure invoked for each fully-parsed request. Return an ``HTTPResponse`` /// to send back to the client. /// - Returns: A running ``HTTPServer`` instance. @@ -147,6 +150,7 @@ public final class HTTPServer: Sendable { maxConnections: Int = HTTPServer.defaultMaxConnections, readTimeout: Duration = HTTPServer.defaultReadTimeout, idleTimeout: Duration = HTTPServer.defaultIdleTimeout, + errorResponseHeaders: [(String, String)] = [], handler: @escaping @Sendable (HTTPServer.Request) async -> HTTPResponse ) async throws -> HTTPServer { // Sanitize to prevent path traversal — only allow safe filename characters. @@ -187,7 +191,8 @@ public final class HTTPServer: Sendable { requiresAuthentication: requiresAuth, maxRequestBodySize: maxRequestBodySize, readTimeout: readTimeout, idleTimeout: idleTimeout, tempDirectory: tempDirectory, - connectionCounter: connectionCounter, connectionTasks: connectionTasks, handler: handler + connectionCounter: connectionCounter, connectionTasks: connectionTasks, + errorResponseHeaders: errorResponseHeaders, handler: handler ) } @@ -251,6 +256,7 @@ public final class HTTPServer: Sendable { tempDirectory: URL, connectionCounter: ConnectionCounter, connectionTasks: ConnectionTasks, + errorResponseHeaders: [(String, String)], handler: @escaping @Sendable (HTTPServer.Request) async -> HTTPResponse ) { connection.start(queue: queue) @@ -325,27 +331,28 @@ public final class HTTPServer: Sendable { let ms = Double(sec) * 1000.0 + Double(atto) / 1_000_000_000_000_000.0 Logger.httpServer.debug("\(request.method) \(request.target) → \(response.status) (\(String(format: "%.1f", ms))ms)") } catch HTTPServerError.authenticationFailed { - await send(HTTPResponse(status: 407, headers: [("Content-Type", "text/plain"), ("Proxy-Authenticate", "Bearer")]), on: connection) + await send(HTTPResponse(status: 407, headers: errorResponseHeaders + [("Content-Type", "text/plain"), ("Proxy-Authenticate", "Bearer")]), on: connection) } catch HTTPServerError.lengthRequired { - await send(HTTPResponse(status: 411, statusText: "Length Required", body: Data("Length Required".utf8)), on: connection) + await send(HTTPResponse(status: 411, statusText: "Length Required", headers: errorResponseHeaders, body: Data("Length Required".utf8)), on: connection) } catch is CancellationError { Logger.httpServer.debug("Connection cancelled during shutdown") connection.cancel() } catch HTTPServerError.readTimeout { Logger.httpServer.warning("Read timeout, closing connection") - await send(HTTPResponse(status: 408, statusText: "Request Timeout", body: Data("Request Timeout".utf8)), on: connection) + await send(HTTPResponse(status: 408, statusText: "Request Timeout", headers: errorResponseHeaders, body: Data("Request Timeout".utf8)), on: connection) } catch let error as HTTPRequestParseError { Logger.httpServer.error("Parse error: \(error)") let statusText = String(error.httpStatusText) let response = HTTPResponse( status: error.httpStatus, statusText: statusText, + headers: errorResponseHeaders, body: Data(statusText.utf8) ) await send(response, on: connection) } catch { Logger.httpServer.error("Unexpected error: \(error)") - await send(HTTPResponse(status: 400, statusText: "Bad Request", body: Data("Malformed HTTP request".utf8)), on: connection) + await send(HTTPResponse(status: 400, statusText: "Bad Request", headers: errorResponseHeaders, body: Data("Malformed HTTP request".utf8)), on: connection) } } connectionTasks.track(taskID, task) From 0563a94e8dd30d70aeff3cdff1c708863ec39e51 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Thu, 2 Apr 2026 14:49:20 -0400 Subject: [PATCH 04/13] Revert "fix: include CORS headers on server-generated error responses" This reverts commit 0b6c67be77c1b9b5adb0eda8e5c37d1dca46c0d8. --- .../java/org/wordpress/gutenberg/HttpServer.kt | 11 +---------- .../wordpress/gutenberg/MediaUploadServer.kt | 11 +++++------ .../Sources/Media/MediaUploadServer.swift | 1 - ios/Sources/GutenbergKitHTTP/HTTPServer.swift | 17 +++++------------ 4 files changed, 11 insertions(+), 29 deletions(-) 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 967f3980b..0011915d2 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/HttpServer.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/HttpServer.kt @@ -150,7 +150,6 @@ class HttpServer( private val readTimeoutMs: Int = DEFAULT_READ_TIMEOUT_MS, private val idleTimeoutMs: Int = DEFAULT_IDLE_TIMEOUT_MS, private val cacheDir: File? = null, - private val errorResponseHeaders: Map = emptyMap(), private val handler: suspend (HttpRequest) -> HttpResponse ) { @Volatile @@ -251,7 +250,6 @@ class HttpServer( try { sendResponse(sock, HttpResponse( status = 408, - headers = errorResponseHeaders + ("Content-Type" to "text/plain"), body = "Request Timeout".toByteArray() )) } catch (_: Exception) { @@ -308,14 +306,12 @@ class HttpServer( val statusText = STATUS_TEXT[e.error.httpStatus] ?: "Bad Request" sendResponse(socket, HttpResponse( status = e.error.httpStatus, - headers = errorResponseHeaders + ("Content-Type" to "text/plain"), body = statusText.toByteArray() )) return } catch (_: java.io.IOException) { sendResponse(socket, HttpResponse( status = 500, - headers = errorResponseHeaders + ("Content-Type" to "text/plain"), body = "Internal Server Error".toByteArray() )) return @@ -324,7 +320,6 @@ class HttpServer( if (partial == null) { sendResponse(socket, HttpResponse( status = 400, - headers = errorResponseHeaders + ("Content-Type" to "text/plain"), body = "Bad Request".toByteArray() )) return @@ -340,7 +335,7 @@ class HttpServer( if (!authenticate(proxyAuth, token)) { sendResponse(socket, HttpResponse( status = 407, - headers = errorResponseHeaders + mapOf("Content-Type" to "text/plain", "Proxy-Authenticate" to "Bearer") + headers = mapOf("Content-Type" to "text/plain", "Proxy-Authenticate" to "Bearer") )) return } @@ -353,7 +348,6 @@ class HttpServer( if (upperMethod in listOf("POST", "PUT", "PATCH") && partial.header("Content-Length") == null) { sendResponse(socket, HttpResponse( status = 411, - headers = errorResponseHeaders + ("Content-Type" to "text/plain"), body = "Length Required".toByteArray() )) return @@ -376,14 +370,12 @@ class HttpServer( val statusText = STATUS_TEXT[e.error.httpStatus] ?: "Bad Request" sendResponse(socket, HttpResponse( status = e.error.httpStatus, - headers = errorResponseHeaders + ("Content-Type" to "text/plain"), body = statusText.toByteArray() )) return } catch (_: java.io.IOException) { sendResponse(socket, HttpResponse( status = 500, - headers = errorResponseHeaders + ("Content-Type" to "text/plain"), body = "Internal Server Error".toByteArray() )) return @@ -395,7 +387,6 @@ class HttpServer( parsed?.body?.fileOwner?.close() sendResponse(socket, HttpResponse( status = 400, - headers = errorResponseHeaders + ("Content-Type" to "text/plain"), body = "Bad Request".toByteArray() )) return 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 a7eb0b7a0..badb4d803 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt @@ -78,7 +78,6 @@ internal class MediaUploadServer( externallyAccessible = false, requiresAuthentication = true, cacheDir = cacheDir, - errorResponseHeaders = corsHeaders, handler = { request -> handleRequest(request) } ) server.start() @@ -191,6 +190,11 @@ internal class MediaUploadServer( // MARK: - Response Building + private val corsHeaders: Map = mapOf( + "Access-Control-Allow-Origin" to "*", + "Access-Control-Allow-Headers" to "Relay-Authorization, Content-Type" + ) + private fun corsPreflightResponse(): HttpResponse = HttpResponse( status = 204, headers = corsHeaders + mapOf( @@ -236,11 +240,6 @@ internal class MediaUploadServer( companion object { private const val TAG = "MediaUploadServer" - - private val corsHeaders: Map = mapOf( - "Access-Control-Allow-Origin" to "*", - "Access-Control-Allow-Headers" to "Relay-Authorization, Content-Type" - ) } } diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift index bdcc1ea53..c4ef144b8 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift @@ -36,7 +36,6 @@ final class MediaUploadServer: Sendable { let server = try await HTTPServer.start( name: "media-upload", requiresAuthentication: true, - errorResponseHeaders: corsHeaders, handler: { request in await Self.handleRequest(request, context: context) } diff --git a/ios/Sources/GutenbergKitHTTP/HTTPServer.swift b/ios/Sources/GutenbergKitHTTP/HTTPServer.swift index b068ce702..86dbc176f 100644 --- a/ios/Sources/GutenbergKitHTTP/HTTPServer.swift +++ b/ios/Sources/GutenbergKitHTTP/HTTPServer.swift @@ -134,9 +134,6 @@ public final class HTTPServer: Sendable { /// the connection. Defaults to 30 seconds. /// - idleTimeout: The maximum time to wait between consecutive reads before closing /// the connection. Prevents slow-loris attacks. Defaults to 5 seconds. - /// - errorResponseHeaders: Headers to include on all server-generated error responses - /// (e.g. 408 timeout, 413 payload too large). Useful for CORS headers that the - /// browser requires even on error responses. /// - handler: A closure invoked for each fully-parsed request. Return an ``HTTPResponse`` /// to send back to the client. /// - Returns: A running ``HTTPServer`` instance. @@ -150,7 +147,6 @@ public final class HTTPServer: Sendable { maxConnections: Int = HTTPServer.defaultMaxConnections, readTimeout: Duration = HTTPServer.defaultReadTimeout, idleTimeout: Duration = HTTPServer.defaultIdleTimeout, - errorResponseHeaders: [(String, String)] = [], handler: @escaping @Sendable (HTTPServer.Request) async -> HTTPResponse ) async throws -> HTTPServer { // Sanitize to prevent path traversal — only allow safe filename characters. @@ -191,8 +187,7 @@ public final class HTTPServer: Sendable { requiresAuthentication: requiresAuth, maxRequestBodySize: maxRequestBodySize, readTimeout: readTimeout, idleTimeout: idleTimeout, tempDirectory: tempDirectory, - connectionCounter: connectionCounter, connectionTasks: connectionTasks, - errorResponseHeaders: errorResponseHeaders, handler: handler + connectionCounter: connectionCounter, connectionTasks: connectionTasks, handler: handler ) } @@ -256,7 +251,6 @@ public final class HTTPServer: Sendable { tempDirectory: URL, connectionCounter: ConnectionCounter, connectionTasks: ConnectionTasks, - errorResponseHeaders: [(String, String)], handler: @escaping @Sendable (HTTPServer.Request) async -> HTTPResponse ) { connection.start(queue: queue) @@ -331,28 +325,27 @@ public final class HTTPServer: Sendable { let ms = Double(sec) * 1000.0 + Double(atto) / 1_000_000_000_000_000.0 Logger.httpServer.debug("\(request.method) \(request.target) → \(response.status) (\(String(format: "%.1f", ms))ms)") } catch HTTPServerError.authenticationFailed { - await send(HTTPResponse(status: 407, headers: errorResponseHeaders + [("Content-Type", "text/plain"), ("Proxy-Authenticate", "Bearer")]), on: connection) + await send(HTTPResponse(status: 407, headers: [("Content-Type", "text/plain"), ("Proxy-Authenticate", "Bearer")]), on: connection) } catch HTTPServerError.lengthRequired { - await send(HTTPResponse(status: 411, statusText: "Length Required", headers: errorResponseHeaders, body: Data("Length Required".utf8)), on: connection) + await send(HTTPResponse(status: 411, statusText: "Length Required", body: Data("Length Required".utf8)), on: connection) } catch is CancellationError { Logger.httpServer.debug("Connection cancelled during shutdown") connection.cancel() } catch HTTPServerError.readTimeout { Logger.httpServer.warning("Read timeout, closing connection") - await send(HTTPResponse(status: 408, statusText: "Request Timeout", headers: errorResponseHeaders, body: Data("Request Timeout".utf8)), on: connection) + await send(HTTPResponse(status: 408, statusText: "Request Timeout", body: Data("Request Timeout".utf8)), on: connection) } catch let error as HTTPRequestParseError { Logger.httpServer.error("Parse error: \(error)") let statusText = String(error.httpStatusText) let response = HTTPResponse( status: error.httpStatus, statusText: statusText, - headers: errorResponseHeaders, body: Data(statusText.utf8) ) await send(response, on: connection) } catch { Logger.httpServer.error("Unexpected error: \(error)") - await send(HTTPResponse(status: 400, statusText: "Bad Request", headers: errorResponseHeaders, body: Data("Malformed HTTP request".utf8)), on: connection) + await send(HTTPResponse(status: 400, statusText: "Bad Request", body: Data("Malformed HTTP request".utf8)), on: connection) } } connectionTasks.track(taskID, task) From 62d9aee88fb95fb7614c961f66a5412476ad101c Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Thu, 2 Apr 2026 15:30:49 -0400 Subject: [PATCH 05/13] fix: route 413 response through handler for CORS headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of the HTTP server library building 413 responses directly (which lacked CORS headers), payloadTooLarge is now treated as a non-fatal parse error. parseRequest() returns the parsed headers as a partial request and exposes the error via a new parseError property. The server passes it to the handler via a serverError field on the request, letting MediaUploadServer build the response with CORS headers — consistent with how OPTIONS preflight is handled. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../http/InstrumentedFixtureTests.kt | 13 ++++++- .../org/wordpress/gutenberg/HttpServer.kt | 31 +++++++++++++++- .../wordpress/gutenberg/MediaUploadServer.kt | 11 ++++++ .../gutenberg/http/HTTPRequestParser.kt | 21 +++++++++-- .../wordpress/gutenberg/http/FixtureTests.kt | 13 ++++++- .../gutenberg/http/HTTPRequestParserTests.kt | 36 +++++++++---------- .../Sources/Media/MediaUploadServer.swift | 10 ++++++ .../GutenbergKitHTTP/HTTPRequestParser.swift | 21 +++++++++-- ios/Sources/GutenbergKitHTTP/HTTPServer.swift | 19 +++++++++- .../GutenbergKitHTTPTests/FixtureTests.swift | 9 ++++- .../HTTPRequestParserTests.swift | 35 ++++++++++-------- 11 files changed, 176 insertions(+), 43 deletions(-) diff --git a/android/Gutenberg/src/androidTest/java/org/wordpress/gutenberg/http/InstrumentedFixtureTests.kt b/android/Gutenberg/src/androidTest/java/org/wordpress/gutenberg/http/InstrumentedFixtureTests.kt index 86f4b2a36..0c69a115f 100644 --- a/android/Gutenberg/src/androidTest/java/org/wordpress/gutenberg/http/InstrumentedFixtureTests.kt +++ b/android/Gutenberg/src/androidTest/java/org/wordpress/gutenberg/http/InstrumentedFixtureTests.kt @@ -150,7 +150,18 @@ class InstrumentedFixtureTests { try { parser.parseRequest() - fail("$description: expected error $expectedError but parsing succeeded") + // Non-fatal errors (e.g., payloadTooLarge) are exposed via + // pendingParseError instead of being thrown. + val pendingError = parser.pendingParseError + if (pendingError != null) { + assertEquals( + expectedError, + pendingError.errorId, + "$description: expected $expectedError but got ${pendingError.errorId}" + ) + } else { + fail("$description: expected error $expectedError but parsing succeeded") + } } catch (e: HTTPRequestParseException) { assertEquals( expectedError, 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 0011915d2..2f1a7643c 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/HttpServer.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/HttpServer.kt @@ -39,7 +39,11 @@ data class HttpRequest( val target: String, val headers: Map, val body: org.wordpress.gutenberg.http.RequestBody? = null, - val parseDurationMs: Double = 0.0 + val parseDurationMs: Double = 0.0, + /** A server-detected error that occurred after headers were parsed + * (e.g., payload too large). When set, the handler is responsible + * for building an appropriate error response. */ + val serverError: org.wordpress.gutenberg.http.HTTPRequestParseError? = null ) { /** * Returns the value of the first header matching the given name (case-insensitive). @@ -325,6 +329,31 @@ class HttpServer( return } + // If the parser detected a non-fatal error (e.g., payload too + // large after drain), let the handler build the response. + parser.pendingParseError?.let { error -> + val parseDurationMs = (System.nanoTime() - parseStart) / 1_000_000.0 + val request = HttpRequest( + method = partial.method, + target = partial.target, + headers = partial.headers, + parseDurationMs = parseDurationMs, + serverError = error + ) + val response = try { + handler(request) + } catch (e: Exception) { + Log.e(TAG, "Handler threw", e) + HttpResponse( + status = error.httpStatus, + body = (STATUS_TEXT[error.httpStatus] ?: "Error").toByteArray() + ) + } + sendResponse(socket, response) + Log.d(TAG, "${partial.method} ${partial.target} → ${response.status} (${"%.1f".format(parseDurationMs)}ms)") + return + } + // Check auth before consuming body to avoid buffering up to // maxBodySize for unauthenticated clients. // OPTIONS is exempt because CORS preflight requests 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 badb4d803..056a5c5fa 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt @@ -3,6 +3,7 @@ package org.wordpress.gutenberg import android.util.Log import org.wordpress.gutenberg.http.HeaderValue import org.wordpress.gutenberg.http.MultipartPart +import org.wordpress.gutenberg.http.HTTPRequestParseError import org.wordpress.gutenberg.http.MultipartParseException import java.io.File import java.io.IOException @@ -91,6 +92,16 @@ internal class MediaUploadServer( // MARK: - Request Handling private suspend fun handleRequest(request: HttpRequest): HttpResponse { + // Server-detected error (e.g., payload too large) — build the + // error response here so it includes CORS headers. + request.serverError?.let { error -> + val message = when (error) { + HTTPRequestParseError.PAYLOAD_TOO_LARGE -> "The file is too large to upload in the editor." + else -> error.errorId + } + return errorResponse(error.httpStatus, message) + } + // CORS preflight — the library exempts OPTIONS from auth, so this is // reached without a token. if (request.method.uppercase() == "OPTIONS") { diff --git a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/http/HTTPRequestParser.kt b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/http/HTTPRequestParser.kt index 370d7d59f..1c94163db 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/http/HTTPRequestParser.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/http/HTTPRequestParser.kt @@ -82,6 +82,15 @@ class HTTPRequestParser( /** The current buffering state. */ val state: State get() = synchronized(lock) { _state } + /** + * The parse error detected during buffering, if any. + * + * Non-fatal errors like [HTTPRequestParseError.PAYLOAD_TOO_LARGE] are + * exposed here instead of being thrown by [parseRequest], allowing the + * caller to still access the parsed headers. + */ + val pendingParseError: HTTPRequestParseError? get() = synchronized(lock) { parseError } + /** Creates a parser and immediately parses the given raw HTTP string. */ constructor( input: String, @@ -211,7 +220,11 @@ class HTTPRequestParser( fun parseRequest(): ParsedHTTPRequest? = synchronized(lock) { if (!_state.hasHeaders) return null - parseError?.let { throw HTTPRequestParseException(it) } + // Payload-too-large means "valid headers, rejected body" — let + // the caller access the parsed headers so the handler can build + // a response (e.g., with CORS headers). Other parse errors + // indicate genuinely malformed requests and are still thrown. + parseError?.let { if (it != HTTPRequestParseError.PAYLOAD_TOO_LARGE) throw HTTPRequestParseException(it) } if (parsedHeaders == null) { val headerData = buffer.read(0, minOf(bytesWritten, MAX_HEADER_SIZE.toLong()).toInt()) @@ -227,7 +240,11 @@ class HTTPRequestParser( val headers = parsedHeaders ?: return null - if (_state != State.COMPLETE) { + // Return partial (headers only) when the body was rejected or + // hasn't fully arrived yet. The payloadTooLarge case goes through + // drain mode which discards body bytes without buffering them, so + // there is no body to extract even though the state is COMPLETE. + if (_state != State.COMPLETE || parseError != null) { return ParsedHTTPRequest( method = headers.method, target = headers.target, diff --git a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/http/FixtureTests.kt b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/http/FixtureTests.kt index a08e4d373..012ba42a3 100644 --- a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/http/FixtureTests.kt +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/http/FixtureTests.kt @@ -149,7 +149,18 @@ class FixtureTests { try { parser.parseRequest() - fail("$description: expected error $expectedError but parsing succeeded") + // Non-fatal errors (e.g., payloadTooLarge) are exposed via + // pendingParseError instead of being thrown. + val pendingError = parser.pendingParseError + if (pendingError != null) { + assertEquals( + expectedError, + pendingError.errorId, + "$description: expected $expectedError but got ${pendingError.errorId}" + ) + } else { + fail("$description: expected error $expectedError but parsing succeeded") + } } catch (e: HTTPRequestParseException) { assertEquals( expectedError, diff --git a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/http/HTTPRequestParserTests.kt b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/http/HTTPRequestParserTests.kt index d6b3f49b5..abdc9c2e1 100644 --- a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/http/HTTPRequestParserTests.kt +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/http/HTTPRequestParserTests.kt @@ -4,7 +4,6 @@ import org.junit.Assert.assertArrayEquals import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNull -import org.junit.Assert.assertThrows import org.junit.Assert.assertTrue import org.junit.Test @@ -99,7 +98,7 @@ class HTTPRequestParserTests { } @Test - fun `drains oversized body before reporting payloadTooLarge`() { + fun `drains oversized body and returns partial with parseError`() { val parser = HTTPRequestParser(maxBodySize = 100) parser.append("POST /upload HTTP/1.1\r\nHost: localhost\r\nContent-Length: 101\r\n\r\n".toByteArray()) @@ -109,11 +108,13 @@ class HTTPRequestParserTests { // Feed the remaining body bytes to complete the drain. parser.append(ByteArray(101) { 0x41 }) assertTrue(parser.state.isComplete) - assertThrows(HTTPRequestParseException::class.java) { - parser.parseRequest() - }.also { - assertEquals(HTTPRequestParseError.PAYLOAD_TOO_LARGE, it.error) - } + + // parseRequest() returns partial headers instead of throwing. + val request = parser.parseRequest()!! + assertEquals("POST", request.method) + assertEquals("/upload", request.target) + assertFalse(request.isComplete) + assertEquals(HTTPRequestParseError.PAYLOAD_TOO_LARGE, parser.pendingParseError) } @Test @@ -136,11 +137,10 @@ class HTTPRequestParserTests { } assertTrue(parser.state.isComplete) - assertThrows(HTTPRequestParseException::class.java) { - parser.parseRequest() - }.also { - assertEquals(HTTPRequestParseError.PAYLOAD_TOO_LARGE, it.error) - } + val request = parser.parseRequest()!! + assertEquals("POST", request.method) + assertFalse(request.isComplete) + assertEquals(HTTPRequestParseError.PAYLOAD_TOO_LARGE, parser.pendingParseError) } @Test @@ -153,13 +153,11 @@ class HTTPRequestParserTests { parser.append(ByteArray(1000) { 0x43 }) assertTrue(parser.state.isComplete) - // The parser should still report the error, confirming the bytes - // were consumed without being stored. - assertThrows(HTTPRequestParseException::class.java) { - parser.parseRequest() - }.also { - assertEquals(HTTPRequestParseError.PAYLOAD_TOO_LARGE, it.error) - } + // parseRequest() returns headers; error is on pendingParseError. + val request = parser.parseRequest()!! + assertEquals("POST", request.method) + assertFalse(request.isComplete) + assertEquals(HTTPRequestParseError.PAYLOAD_TOO_LARGE, parser.pendingParseError) } // MARK: - Error HTTP Status Mapping diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift index c4ef144b8..2464a7ff5 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift @@ -60,6 +60,16 @@ final class MediaUploadServer: Sendable { private static func handleRequest(_ request: HTTPServer.Request, context: UploadContext) async -> HTTPResponse { let parsed = request.parsed + // Server-detected error (e.g., payload too large) — build the + // error response here so it includes CORS headers. + if let serverError = request.serverError { + let message: String = switch serverError { + case .payloadTooLarge: "The file is too large to upload in the editor." + default: "\(serverError.httpStatusText)" + } + return errorResponse(status: serverError.httpStatus, body: message) + } + // CORS preflight — the library exempts OPTIONS from auth, so this is // reached without a token. if parsed.method.uppercased() == "OPTIONS" { diff --git a/ios/Sources/GutenbergKitHTTP/HTTPRequestParser.swift b/ios/Sources/GutenbergKitHTTP/HTTPRequestParser.swift index 50ebb781d..0fce5c251 100644 --- a/ios/Sources/GutenbergKitHTTP/HTTPRequestParser.swift +++ b/ios/Sources/GutenbergKitHTTP/HTTPRequestParser.swift @@ -107,6 +107,15 @@ public final class HTTPRequestParser: @unchecked Sendable { lock.withLock { _state } } + /// The parse error detected during buffering, if any. + /// + /// Non-fatal errors like ``HTTPRequestParseError/payloadTooLarge`` are + /// exposed here instead of being thrown by ``parseRequest()``, allowing + /// the caller to still access the parsed headers. + public var parseError: HTTPRequestParseError? { + lock.withLock { _parseError } + } + /// The expected body length from `Content-Length`, available once headers have been received. public var expectedBodyLength: Int64? { lock.withLock { @@ -128,7 +137,11 @@ public final class HTTPRequestParser: @unchecked Sendable { try lock.withLock { guard _state.hasHeaders else { return nil } - if let error = _parseError { + // Payload-too-large means "valid headers, rejected body" — let + // the caller access the parsed headers so the handler can build + // a response (e.g., with CORS headers). Other parse errors + // indicate genuinely malformed requests and are still thrown. + if let error = _parseError, error != .payloadTooLarge { throw error } @@ -147,7 +160,11 @@ public final class HTTPRequestParser: @unchecked Sendable { guard let headers = _parsedHeaders else { return nil } - guard _state.isComplete else { + // Return partial (headers only) when the body was rejected or + // hasn't fully arrived yet. The payloadTooLarge case goes through + // drain mode which discards body bytes without buffering them, so + // there is no body to extract even though the state is .complete. + guard _state.isComplete, _parseError == nil else { return .partial( method: headers.method, target: headers.target, diff --git a/ios/Sources/GutenbergKitHTTP/HTTPServer.swift b/ios/Sources/GutenbergKitHTTP/HTTPServer.swift index 86dbc176f..b49aa4602 100644 --- a/ios/Sources/GutenbergKitHTTP/HTTPServer.swift +++ b/ios/Sources/GutenbergKitHTTP/HTTPServer.swift @@ -75,6 +75,16 @@ public final class HTTPServer: Sendable { public let parsed: ParsedHTTPRequest /// Time spent receiving and parsing the request. public let parseDuration: Duration + /// A server-detected error that occurred after headers were parsed + /// (e.g., payload too large). When set, the handler is responsible + /// for building an appropriate error response. + public let serverError: HTTPRequestParseError? + + init(parsed: ParsedHTTPRequest, parseDuration: Duration, serverError: HTTPRequestParseError? = nil) { + self.parsed = parsed + self.parseDuration = parseDuration + self.serverError = serverError + } } public typealias Response = HTTPResponse @@ -281,6 +291,13 @@ public final class HTTPServer: Sendable { throw HTTPServerError.connectionClosed } + // If the parser detected a non-fatal error (e.g., + // payload too large after drain), return the partial + // request so the handler can build the response. + if parser.parseError != nil { + return partial + } + // Check auth before consuming body to avoid buffering // up to maxRequestBodySize for unauthenticated clients. // OPTIONS is exempt because CORS preflight requests @@ -319,7 +336,7 @@ public final class HTTPServer: Sendable { } } - let response = await handler(Request(parsed: request, parseDuration: duration)) + let response = await handler(Request(parsed: request, parseDuration: duration, serverError: parser.parseError)) await send(response, on: connection) let (sec, atto) = duration.components let ms = Double(sec) * 1000.0 + Double(atto) / 1_000_000_000_000_000.0 diff --git a/ios/Tests/GutenbergKitHTTPTests/FixtureTests.swift b/ios/Tests/GutenbergKitHTTPTests/FixtureTests.swift index f361ccdb7..22feadbd0 100644 --- a/ios/Tests/GutenbergKitHTTPTests/FixtureTests.swift +++ b/ios/Tests/GutenbergKitHTTPTests/FixtureTests.swift @@ -255,7 +255,14 @@ struct RequestParsingFixtureTests { let expectedError = testCase.expected.error do { _ = try parser.parseRequest() - Issue.record("Expected error \(expectedError) but parsing succeeded — \(testCase.description)") + // Non-fatal errors (e.g., payloadTooLarge) are exposed via + // parseError instead of being thrown. + if let parseError = parser.parseError { + let errorName = String(describing: parseError) + #expect(errorName == expectedError, "\(testCase.description): expected \(expectedError) but got \(errorName)") + } else { + Issue.record("Expected error \(expectedError) but parsing succeeded — \(testCase.description)") + } } catch { let errorName = String(describing: error) #expect(errorName == expectedError, "\(testCase.description): expected \(expectedError) but got \(errorName)") diff --git a/ios/Tests/GutenbergKitHTTPTests/HTTPRequestParserTests.swift b/ios/Tests/GutenbergKitHTTPTests/HTTPRequestParserTests.swift index 7da3e0d62..c4dc5366c 100644 --- a/ios/Tests/GutenbergKitHTTPTests/HTTPRequestParserTests.swift +++ b/ios/Tests/GutenbergKitHTTPTests/HTTPRequestParserTests.swift @@ -389,8 +389,8 @@ struct HTTPRequestParserTests { // MARK: - Max Body Size - @Test("drains oversized body before reporting payloadTooLarge") - func rejectsOversizedContentLength() { + @Test("drains oversized body and returns partial with parseError") + func rejectsOversizedContentLength() throws { let parser = HTTPRequestParser(maxBodySize: 100) parser.append(Data("POST /upload HTTP/1.1\r\nHost: localhost\r\nContent-Length: 101\r\n\r\n".utf8)) @@ -400,9 +400,13 @@ struct HTTPRequestParserTests { // Feed the remaining body bytes to complete the drain. parser.append(Data(repeating: 0x41, count: 101)) #expect(parser.state.isComplete) - #expect(throws: HTTPRequestParseError.payloadTooLarge) { - try parser.parseRequest() - } + + // parseRequest() returns partial headers instead of throwing. + let request = try #require(try parser.parseRequest()) + #expect(request.method == "POST") + #expect(request.target == "/upload") + #expect(!request.isComplete) + #expect(parser.parseError == .payloadTooLarge) } @Test("accepts request when Content-Length equals maxBodySize") @@ -430,7 +434,7 @@ struct HTTPRequestParserTests { } @Test("enters drain mode for oversized Content-Length even when body hasn't arrived") - func rejectsOversizedBeforeBodyArrives() { + func rejectsOversizedBeforeBodyArrives() throws { let parser = HTTPRequestParser(maxBodySize: 50) parser.append(Data("POST /upload HTTP/1.1\r\nHost: localhost\r\nContent-Length: 999999\r\n\r\n".utf8)) @@ -449,13 +453,14 @@ struct HTTPRequestParserTests { } #expect(parser.state.isComplete) - #expect(throws: HTTPRequestParseError.payloadTooLarge) { - try parser.parseRequest() - } + let request = try #require(try parser.parseRequest()) + #expect(request.method == "POST") + #expect(!request.isComplete) + #expect(parser.parseError == .payloadTooLarge) } @Test("drain mode does not buffer body bytes") - func drainDoesNotBuffer() { + func drainDoesNotBuffer() throws { let parser = HTTPRequestParser(maxBodySize: 10) let headers = "POST /upload HTTP/1.1\r\nHost: localhost\r\nContent-Length: 1000\r\n\r\n" parser.append(Data(headers.utf8)) @@ -465,11 +470,11 @@ struct HTTPRequestParserTests { parser.append(Data(repeating: 0x43, count: 1000)) #expect(parser.state.isComplete) - // The parser should still report the error, confirming the bytes - // were consumed without being stored. - #expect(throws: HTTPRequestParseError.payloadTooLarge) { - try parser.parseRequest() - } + // parseRequest() returns headers; error is on parseError. + let request = try #require(try parser.parseRequest()) + #expect(request.method == "POST") + #expect(!request.isComplete) + #expect(parser.parseError == .payloadTooLarge) } @Test("rejects headers that exceed maxHeaderSize without terminator") From 0a4cbac766a5cef47cac7e43463eefec6bc979db Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Thu, 2 Apr 2026 17:32:59 -0400 Subject: [PATCH 06/13] perf(ios): stream multipart upload body from disk instead of memory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace Data(contentsOf:) + httpBody with a streaming InputStream via httpBodyStream in DefaultMediaUploader. The multipart body (preamble, file content, epilogue) is written through a bound stream pair on a background thread, keeping peak memory at ~65 KB regardless of file size — down from ~2x file size previously. Android already streams via OkHttp's file.asRequestBody() and needs no changes. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../Sources/Media/MediaUploadServer.swift | 104 ++++++++++++++++-- .../Media/MediaUploadServerTests.swift | 71 ++++++++++++ 2 files changed, 167 insertions(+), 8 deletions(-) diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift index 2464a7ff5..7db2ce634 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift @@ -292,15 +292,11 @@ class DefaultMediaUploader: @unchecked Sendable { } func upload(fileURL: URL, mimeType: String, filename: String) async throws -> MediaUploadResult { - let fileData = try Data(contentsOf: fileURL) let boundary = UUID().uuidString - var body = Data() - 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") - body.append(fileData) - body.append("\r\n--\(boundary)--\r\n") + let (bodyStream, contentLength) = try Self.multipartBodyStream( + fileURL: fileURL, boundary: boundary, filename: filename, mimeType: mimeType + ) // When a site API namespace is configured (e.g. "sites/12345/"), insert // it into the media endpoint path so the request reaches the correct site. @@ -313,7 +309,8 @@ class DefaultMediaUploader: @unchecked Sendable { var request = URLRequest(url: uploadURL) request.httpMethod = "POST" request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") - request.httpBody = body + request.setValue("\(contentLength)", forHTTPHeaderField: "Content-Length") + request.httpBodyStream = bodyStream let (data, response) = try await httpClient.perform(request) @@ -343,6 +340,92 @@ class DefaultMediaUploader: @unchecked Sendable { height: wpMedia.media_details?.height ) } + + // MARK: - Streaming Multipart Body + + /// Builds a multipart/form-data body as an `InputStream` that streams the + /// file from disk without loading it into memory. + /// + /// Uses a bound stream pair with a background writer thread — the same + /// pattern as `RequestBody.makePipedFileSliceStream`. + /// + /// - Returns: A tuple of the input stream and the total content length. + static func multipartBodyStream( + fileURL: URL, + boundary: String, + filename: String, + mimeType: String + ) throws -> (InputStream, Int) { + let preamble = Data( + ("--\(boundary)\r\n" + + "Content-Disposition: form-data; name=\"file\"; filename=\"\(filename)\"\r\n" + + "Content-Type: \(mimeType)\r\n\r\n").utf8 + ) + let epilogue = Data("\r\n--\(boundary)--\r\n".utf8) + + guard let fileSize = try FileManager.default.attributesOfItem(atPath: fileURL.path)[.size] as? Int else { + throw MediaUploadError.streamReadFailed + } + let contentLength = preamble.count + fileSize + epilogue.count + + let fileHandle = try FileHandle(forReadingFrom: fileURL) + + var readStream: InputStream? + var writeStream: OutputStream? + Stream.getBoundStreams(withBufferSize: 65_536, inputStream: &readStream, outputStream: &writeStream) + + guard let inputStream = readStream, let outputStream = writeStream else { + try? fileHandle.close() + throw MediaUploadError.streamReadFailed + } + + outputStream.open() + + // OutputStream is not Sendable but is safely transferred to the + // writer thread — only the thread accesses it after this point. + nonisolated(unsafe) let output = outputStream + + Thread.detachNewThread { + defer { + output.close() + try? fileHandle.close() + } + + // Write preamble (multipart headers). + guard Self.writeAll(preamble, to: output) else { return } + + // Stream file content in chunks. + var remaining = fileSize + while remaining > 0 { + let chunkSize = min(65_536, remaining) + guard let chunk = try? fileHandle.read(upToCount: chunkSize), + !chunk.isEmpty else { + break + } + guard Self.writeAll(chunk, to: output) else { return } + remaining -= chunk.count + } + + // Write epilogue (closing boundary). + _ = Self.writeAll(epilogue, to: output) + } + + return (inputStream, contentLength) + } + + /// Writes all bytes of `data` to the output stream, handling partial writes. + private static func writeAll(_ data: Data, to output: OutputStream) -> Bool { + data.withUnsafeBytes { buffer in + guard let base = buffer.baseAddress?.assumingMemoryBound(to: UInt8.self) else { return false } + var written = 0 + while written < data.count { + let result = output.write(base.advanced(by: written), maxLength: data.count - written) + if result <= 0 { return false } + written += result + } + return true + } + } } /// WordPress REST API media response (subset of fields). @@ -374,12 +457,17 @@ enum MediaUploadError: Error, LocalizedError { /// The WordPress REST API returned a non-JSON response (e.g. HTML error page). case unexpectedResponse(preview: String, underlyingError: Error) + /// Failed to read the file for streaming upload. + case streamReadFailed + var errorDescription: String? { switch self { case .uploadFailed(let statusCode, let preview): return "Upload failed (\(statusCode)): \(preview)" case .unexpectedResponse(let preview, _): return "WordPress returned an unexpected response: \(preview)" + case .streamReadFailed: + return "Failed to read file for upload" } } } diff --git a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift index 2ebb21bd4..7b78674e3 100644 --- a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift +++ b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift @@ -171,6 +171,77 @@ struct MediaUploadServerTests { } } +// MARK: - Streaming Multipart Body Tests + +@Suite("DefaultMediaUploader streaming multipart body") +struct MultipartBodyStreamTests { + + @Test("streaming output matches in-memory multipart format") + func streamMatchesInMemory() throws { + let tempFile = FileManager.default.temporaryDirectory.appendingPathComponent("stream-test-\(UUID().uuidString)") + let fileContent = Data("hello world".utf8) + try fileContent.write(to: tempFile) + defer { try? FileManager.default.removeItem(at: tempFile) } + + let boundary = "test-boundary-123" + let filename = "photo.jpg" + let mimeType = "image/jpeg" + + // Build expected output using the old in-memory approach. + var expected = Data() + expected.append(Data("--\(boundary)\r\n".utf8)) + expected.append(Data("Content-Disposition: form-data; name=\"file\"; filename=\"\(filename)\"\r\n".utf8)) + expected.append(Data("Content-Type: \(mimeType)\r\n\r\n".utf8)) + expected.append(fileContent) + expected.append(Data("\r\n--\(boundary)--\r\n".utf8)) + + // Build streaming output. + let (stream, contentLength) = try DefaultMediaUploader.multipartBodyStream( + fileURL: tempFile, boundary: boundary, filename: filename, mimeType: mimeType + ) + #expect(contentLength == expected.count) + + let result = readAllFromStream(stream) + #expect(result == expected) + } + + @Test("content length matches actual stream output for larger files") + func contentLengthAccurate() throws { + let tempFile = FileManager.default.temporaryDirectory.appendingPathComponent("stream-test-\(UUID().uuidString)") + let fileContent = Data(repeating: 0x42, count: 100_000) + try fileContent.write(to: tempFile) + defer { try? FileManager.default.removeItem(at: tempFile) } + + let (stream, contentLength) = try DefaultMediaUploader.multipartBodyStream( + fileURL: tempFile, boundary: "boundary", filename: "big.bin", mimeType: "application/octet-stream" + ) + + let result = readAllFromStream(stream) + #expect(result.count == contentLength) + } +} + +// MARK: - Helpers + +/// Reads all bytes from an InputStream using `read()` return value as +/// the sole termination signal (not `hasBytesAvailable`, which is +/// unreliable for piped/bound streams). +private func readAllFromStream(_ stream: InputStream) -> Data { + stream.open() + defer { stream.close() } + + var data = Data() + let bufferSize = 8192 + let buffer = UnsafeMutablePointer.allocate(capacity: bufferSize) + defer { buffer.deallocate() } + while true { + let read = stream.read(buffer, maxLength: bufferSize) + if read <= 0 { break } + data.append(buffer, count: read) + } + return data +} + // MARK: - Mocks private final class MockUploadDelegate: MediaUploadDelegate, @unchecked Sendable { From 692a8916eb3b185887777d90de9ab66cb5656508 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 3 Apr 2026 11:17:50 -0400 Subject: [PATCH 07/13] perf: passthrough upload when delegate does not modify the file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the delegate's processFile returns the original file unchanged (e.g., GIFs, non-images, files already within size limits), the original request body is forwarded directly to WordPress — skipping multipart re-encoding and the extra file read. Detection: after processFile, compare the returned URL/File to the input. If unchanged and uploadFile returns nil, signal passthrough back to handleUpload which streams the original body via passthroughUpload(). Also extracts shared response parsing into performUpload() on both platforms to avoid duplication between upload() and passthroughUpload(). Co-Authored-By: Claude Opus 4.6 (1M context) --- .../wordpress/gutenberg/MediaUploadServer.kt | 96 ++++++++++++++++--- .../gutenberg/MediaUploadServerTest.kt | 32 +++++-- .../Sources/Media/MediaUploadServer.swift | 81 +++++++++++++--- .../Media/MediaUploadServerTests.swift | 22 ++++- 4 files changed, 190 insertions(+), 41 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 056a5c5fa..de50120d3 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt @@ -10,6 +10,7 @@ import java.io.IOException import java.util.UUID import okhttp3.MediaType.Companion.toMediaType import okhttp3.RequestBody.Companion.asRequestBody +import okio.source /** * Result of a successful media upload to the remote WordPress server. @@ -123,7 +124,7 @@ internal class MediaUploadServer( val tempFile = writePartToTempFile(filePart) ?: return errorResponse(500, "Failed to save file") - return processAndRespond(tempFile, filePart) + return processAndRespond(request, tempFile, filePart) } private fun parseFilePart(request: HttpRequest): MultipartPart? { @@ -168,13 +169,33 @@ internal class MediaUploadServer( } } - private suspend fun processAndRespond(tempFile: File, filePart: MultipartPart): HttpResponse { + private suspend fun processAndRespond( + request: HttpRequest, tempFile: File, filePart: MultipartPart + ): HttpResponse { var processedFile: File? = null try { - val (media, processed) = processAndUpload( + val uploadResult = processAndUpload( tempFile, filePart.contentType, filePart.filename ?: "upload" ) - processedFile = processed + val media = when (uploadResult) { + is UploadResult.Uploaded -> { + processedFile = uploadResult.processedFile + Log.d(TAG, "Uploading processed file to WordPress") + uploadResult.result + } + is UploadResult.Passthrough -> { + // Delegate didn't modify the file — forward the original + // request body to WordPress without re-encoding. + Log.d(TAG, "Passthrough: forwarding original request body to WordPress") + val body = request.body + ?: throw MediaUploadException("Missing request body for passthrough") + val contentType = request.header("Content-Type") + ?: throw MediaUploadException("Missing Content-Type for passthrough") + val uploader = defaultUploader + ?: throw MediaUploadException("No default uploader configured") + uploader.passthroughUpload(body, contentType) + } + } return successResponse(media) } catch (e: MediaUploadException) { Log.e(TAG, "Upload processing failed", e) @@ -187,16 +208,30 @@ internal class MediaUploadServer( // MARK: - Delegate Pipeline + private sealed class UploadResult { + data class Uploaded(val result: MediaUploadResult, val processedFile: File) : UploadResult() + data object Passthrough : UploadResult() + } + private suspend fun processAndUpload( file: File, mimeType: String, filename: String - ): Pair { + ): UploadResult { val processedFile = uploadDelegate?.processFile(file, mimeType) ?: file - val result = uploadDelegate?.uploadFile(processedFile, mimeType, filename) - ?: defaultUploader?.upload(processedFile, mimeType, filename) - ?: error("No upload delegate or default uploader configured") + // If the delegate provided its own upload, use that. + uploadDelegate?.uploadFile(processedFile, mimeType, filename)?.let { + return UploadResult.Uploaded(it, processedFile) + } - return Pair(result, processedFile) + // If the delegate didn't modify the file, the original request + // body can be forwarded directly — skip multipart re-encoding. + if (processedFile == file) { + return UploadResult.Passthrough + } + + val result = defaultUploader?.upload(processedFile, mimeType, filename) + ?: error("No upload delegate or default uploader configured") + return UploadResult.Uploaded(result, processedFile) } // MARK: - Response Building @@ -266,6 +301,13 @@ internal open class DefaultMediaUploader( private val authHeader: String, private val siteApiNamespace: List = emptyList() ) { + /** The WordPress media endpoint URL, accounting for site API namespaces. */ + private val mediaEndpointUrl: String + get() { + val namespace = siteApiNamespace.firstOrNull() ?: "" + return "${siteApiRoot}wp/v2/${namespace}media" + } + open suspend fun upload(file: File, mimeType: String, filename: String): MediaUploadResult { val mediaType = mimeType.toMediaType() val requestBody = okhttp3.MultipartBody.Builder() @@ -273,15 +315,43 @@ internal open class DefaultMediaUploader( .addFormDataPart("file", filename, file.asRequestBody(mediaType)) .build() - // When a site API namespace is configured (e.g. "sites/12345/"), insert - // it into the media endpoint path so the request reaches the correct site. - val namespace = siteApiNamespace.firstOrNull() ?: "" val request = okhttp3.Request.Builder() - .url("${siteApiRoot}wp/v2/${namespace}media") + .url(mediaEndpointUrl) .addHeader("Authorization", authHeader) .post(requestBody) .build() + return performUpload(request) + } + + /** + * Forwards the original request body to WordPress without re-encoding. + * + * Used when the delegate's `processFile` returned the file unchanged — + * the incoming multipart body is already valid for WordPress. + */ + open suspend fun passthroughUpload( + body: org.wordpress.gutenberg.http.RequestBody, + contentType: String + ): MediaUploadResult { + val streamBody = object : okhttp3.RequestBody() { + override fun contentType() = contentType.toMediaType() + override fun contentLength() = body.size + override fun writeTo(sink: okio.BufferedSink) { + body.inputStream().use { sink.writeAll(it.source()) } + } + } + + val request = okhttp3.Request.Builder() + .url(mediaEndpointUrl) + .addHeader("Authorization", authHeader) + .post(streamBody) + .build() + + return performUpload(request) + } + + private fun performUpload(request: okhttp3.Request): MediaUploadResult { val response = httpClient.newCall(request).execute() val body = response.body?.string() 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 a7f4f16d7..86ad0c51d 100644 --- a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/MediaUploadServerTest.kt +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/MediaUploadServerTest.kt @@ -6,6 +6,7 @@ import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Rule @@ -135,7 +136,7 @@ class MediaUploadServerTest { // MARK: - Fallback to default uploader @Test - fun `falls back to default uploader when delegate returns nil for uploadFile`() { + fun `uses passthrough when delegate does not modify file`() { val delegate = ProcessOnlyDelegate() val mockUploader = MockDefaultUploader() @@ -157,7 +158,9 @@ class MediaUploadServerTest { assertTrue("Expected 200 but got: ${response.statusLine}", response.statusLine.contains("200")) assertTrue(delegate.processFileCalled) - assertTrue(mockUploader.uploadCalled) + // Passthrough: original body forwarded directly, not re-encoded. + assertTrue(mockUploader.passthroughUploadCalled) + assertFalse(mockUploader.uploadCalled) val json = JsonParser.parseString(response.body).asJsonObject assertEquals(99, json.get("id").asInt) @@ -386,17 +389,28 @@ class MediaUploadServerTest { authHeader = "Bearer mock" ) { @Volatile var uploadCalled = false + @Volatile var passthroughUploadCalled = false override suspend fun upload(file: File, mimeType: String, filename: String): MediaUploadResult { uploadCalled = true - return MediaUploadResult( - id = 99, - url = "https://example.com/doc.pdf", - title = "doc", - mime = "application/pdf", - type = "file" - ) + return mockResult() + } + + override suspend fun passthroughUpload( + body: org.wordpress.gutenberg.http.RequestBody, + contentType: String + ): MediaUploadResult { + passthroughUploadCalled = true + return mockResult() } + + private fun mockResult() = MediaUploadResult( + id = 99, + url = "https://example.com/doc.pdf", + title = "doc", + mime = "application/pdf", + type = "file" + ) } } diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift index 7db2ce634..2c4649e00 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift @@ -123,11 +123,27 @@ final class MediaUploadServer: Sendable { let result: Result var processedURL: URL? do { - let (media, processed) = try await processAndUpload( + let uploadResult = try await processAndUpload( fileURL: fileURL, mimeType: mimeType, filename: filePart.filename ?? "upload", context: context ) - processedURL = processed - result = .success(media) + switch uploadResult { + case .uploaded(let media, let processed): + processedURL = processed + Logger.uploadServer.debug("Uploading processed file to WordPress") + result = .success(media) + case .passthrough: + // Delegate didn't modify the file — forward the original + // request body to WordPress without re-encoding. + 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 { + result = .failure(UploadError.noUploader) + break + } + let media = try await defaultUploader.passthroughUpload(body: body, contentType: contentType) + result = .success(media) + } } catch { result = .failure(error) } @@ -158,9 +174,18 @@ final class MediaUploadServer: Sendable { // MARK: - Delegate Pipeline + /// Result of the delegate processing + upload pipeline. + private enum UploadResult { + /// The delegate (or default uploader) completed the upload. + case uploaded(MediaUploadResult, processedURL: URL) + /// The delegate didn't modify the file and `uploadFile` returned nil. + /// The caller should forward the original request body to WordPress. + case passthrough + } + private static func processAndUpload( fileURL: URL, mimeType: String, filename: String, context: UploadContext - ) async throws -> (MediaUploadResult, URL) { + ) async throws -> UploadResult { // Step 1: Process (resize, transcode, etc.) let processedURL: URL if let delegate = context.uploadDelegate { @@ -172,9 +197,15 @@ final class MediaUploadServer: Sendable { // Step 2: Upload to remote WordPress if let delegate = context.uploadDelegate, let result = try await delegate.uploadFile(at: processedURL, mimeType: mimeType, filename: filename) { - return (result, processedURL) + return .uploaded(result, processedURL: processedURL) } else if let defaultUploader = context.defaultUploader { - return (try await defaultUploader.upload(fileURL: processedURL, mimeType: mimeType, filename: filename), processedURL) + // If the delegate didn't modify the file, the original request + // body can be forwarded directly — skip multipart re-encoding. + if processedURL == fileURL { + return .passthrough + } + let result = try await defaultUploader.upload(fileURL: processedURL, mimeType: mimeType, filename: filename) + return .uploaded(result, processedURL: processedURL) } else { throw UploadError.noUploader } @@ -291,6 +322,16 @@ class DefaultMediaUploader: @unchecked Sendable { self.siteApiNamespace = siteApiNamespace.first } + /// The WordPress media endpoint URL, accounting for site API namespaces. + private var mediaEndpointURL: URL { + let mediaPath = if let siteApiNamespace { + "wp/v2/\(siteApiNamespace)media" + } else { + "wp/v2/media" + } + return siteApiRoot.appending(path: mediaPath) + } + func upload(fileURL: URL, mimeType: String, filename: String) async throws -> MediaUploadResult { let boundary = UUID().uuidString @@ -298,20 +339,30 @@ class DefaultMediaUploader: @unchecked Sendable { fileURL: fileURL, boundary: boundary, filename: filename, mimeType: mimeType ) - // When a site API namespace is configured (e.g. "sites/12345/"), insert - // it into the media endpoint path so the request reaches the correct site. - let mediaPath = if let siteApiNamespace { - "wp/v2/\(siteApiNamespace)media" - } else { - "wp/v2/media" - } - let uploadURL = siteApiRoot.appending(path: mediaPath) - var request = URLRequest(url: uploadURL) + var request = URLRequest(url: mediaEndpointURL) request.httpMethod = "POST" request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") request.setValue("\(contentLength)", forHTTPHeaderField: "Content-Length") request.httpBodyStream = bodyStream + return try await performUpload(request) + } + + /// Forwards the original request body to WordPress without re-encoding. + /// + /// Used when the delegate's `processFile` returned the file unchanged — + /// the incoming multipart body is already valid for WordPress. + func passthroughUpload(body: RequestBody, contentType: String) async throws -> MediaUploadResult { + var request = URLRequest(url: mediaEndpointURL) + request.httpMethod = "POST" + request.setValue(contentType, forHTTPHeaderField: "Content-Type") + request.setValue("\(body.count)", forHTTPHeaderField: "Content-Length") + request.httpBodyStream = try body.makeInputStream() + + return try await performUpload(request) + } + + private func performUpload(_ request: URLRequest) async throws -> MediaUploadResult { let (data, response) = try await httpClient.perform(request) guard (200...299).contains(response.statusCode) else { diff --git a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift index 7b78674e3..b67af18db 100644 --- a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift +++ b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift @@ -1,4 +1,5 @@ import Foundation +import GutenbergKitHTTP import Testing @testable import GutenbergKit @@ -131,8 +132,8 @@ struct MediaUploadServerTests { #expect(result.type == "image") } - @Test("falls back to default uploader when delegate returns nil") - func delegateFallbackToDefault() async throws { + @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) @@ -154,7 +155,9 @@ struct MediaUploadServerTests { #expect(httpResponse.statusCode == 200) #expect(delegate.processFileCalled) - #expect(mockUploader.uploadCalled) + // Passthrough: original body forwarded directly, not re-encoded. + #expect(mockUploader.passthroughUploadCalled) + #expect(!mockUploader.uploadCalled) let result = try JSONDecoder().decode(MediaUploadResult.self, from: data) #expect(result.id == 99) @@ -294,8 +297,10 @@ private final class ProcessOnlyDelegate: MediaUploadDelegate, @unchecked Sendabl private final class MockDefaultUploader: DefaultMediaUploader, @unchecked Sendable { private let lock = NSLock() private var _uploadCalled = false + private var _passthroughUploadCalled = false var uploadCalled: Bool { lock.withLock { _uploadCalled } } + var passthroughUploadCalled: Bool { lock.withLock { _passthroughUploadCalled } } init() { super.init(httpClient: MockHTTPClient(), siteApiRoot: URL(string: "https://example.com/wp-json/")!) @@ -303,7 +308,16 @@ private final class MockDefaultUploader: DefaultMediaUploader, @unchecked Sendab override func upload(fileURL: URL, mimeType: String, filename: String) async throws -> MediaUploadResult { lock.withLock { _uploadCalled = true } - return MediaUploadResult( + return mockResult() + } + + override func passthroughUpload(body: RequestBody, contentType: String) async throws -> MediaUploadResult { + lock.withLock { _passthroughUploadCalled = true } + return mockResult() + } + + private func mockResult() -> MediaUploadResult { + MediaUploadResult( id: 99, url: "https://example.com/doc.pdf", title: "doc", From e3d2efe49cc8caa6ab00c89749a5a8dada8d4516 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 3 Apr 2026 11:21:18 -0400 Subject: [PATCH 08/13] refactor(android): fix Detekt lint violations Extract readUntil() helper from HttpServer.handleRequest() to reduce nesting depth and throw count. Extract performPassthroughUpload() from MediaUploadServer.processAndRespond() to consolidate throw statements. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../org/wordpress/gutenberg/HttpServer.kt | 45 +++++++++---------- .../wordpress/gutenberg/MediaUploadServer.kt | 18 +++++--- 2 files changed, 32 insertions(+), 31 deletions(-) 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 2f1a7643c..190f4cde7 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/HttpServer.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/HttpServer.kt @@ -281,26 +281,12 @@ class HttpServer( val buffer = ByteArray(READ_CHUNK_SIZE) // Phase 1: receive headers only. - while (!parser.state.hasHeaders) { - if (System.nanoTime() > deadlineNanos) { - throw SocketTimeoutException("Read deadline exceeded") - } - val bytesRead = input.read(buffer) - if (bytesRead == -1) break - parser.append(buffer.copyOfRange(0, bytesRead)) - } + readUntil(parser, input, buffer, deadlineNanos) { it.hasHeaders } // Drain oversized body before throwing so the // client receives the 413 (RFC 9110 §15.5.14). if (parser.state == HTTPRequestParser.State.DRAINING) { - while (!parser.state.isComplete) { - if (System.nanoTime() > deadlineNanos) { - throw SocketTimeoutException("Read deadline exceeded") - } - val bytesRead = input.read(buffer) - if (bytesRead == -1) break - parser.append(buffer.copyOfRange(0, bytesRead)) - } + readUntil(parser, input, buffer, deadlineNanos) { it.isComplete } } // Validate headers (triggers full RFC validation). @@ -383,14 +369,7 @@ class HttpServer( } // Phase 2: receive body (skipped if already complete). - while (!parser.state.isComplete) { - if (System.nanoTime() > deadlineNanos) { - throw SocketTimeoutException("Read deadline exceeded") - } - val bytesRead = input.read(buffer) - if (bytesRead == -1) break - parser.append(buffer.copyOfRange(0, bytesRead)) - } + readUntil(parser, input, buffer, deadlineNanos) { it.isComplete } // Final parse with body. val parsed = try { @@ -448,6 +427,24 @@ class HttpServer( } } + /** Reads data into the parser until [condition] is satisfied or the connection closes. */ + private fun readUntil( + parser: HTTPRequestParser, + input: BufferedInputStream, + buffer: ByteArray, + deadlineNanos: Long, + condition: (HTTPRequestParser.State) -> Boolean + ) { + while (!condition(parser.state)) { + if (System.nanoTime() > deadlineNanos) { + throw SocketTimeoutException("Read deadline exceeded") + } + val bytesRead = input.read(buffer) + if (bytesRead == -1) break + parser.append(buffer.copyOfRange(0, bytesRead)) + } + } + private fun sendResponse(socket: Socket, response: HttpResponse) { val output = socket.getOutputStream() output.write(serializeResponse(response)) 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 de50120d3..c264644bc 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt @@ -187,13 +187,7 @@ internal class MediaUploadServer( // Delegate didn't modify the file — forward the original // request body to WordPress without re-encoding. Log.d(TAG, "Passthrough: forwarding original request body to WordPress") - val body = request.body - ?: throw MediaUploadException("Missing request body for passthrough") - val contentType = request.header("Content-Type") - ?: throw MediaUploadException("Missing Content-Type for passthrough") - val uploader = defaultUploader - ?: throw MediaUploadException("No default uploader configured") - uploader.passthroughUpload(body, contentType) + performPassthroughUpload(request) } } return successResponse(media) @@ -213,6 +207,16 @@ internal class MediaUploadServer( data object Passthrough : UploadResult() } + private suspend fun performPassthroughUpload(request: HttpRequest): MediaUploadResult { + 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") + } + return uploader.passthroughUpload(body, contentType) + } + private suspend fun processAndUpload( file: File, mimeType: String, filename: String ): UploadResult { From 5e9562d4600ac0e9996016b9e9c2b89c983abbd7 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 3 Apr 2026 11:34:17 -0400 Subject: [PATCH 09/13] fix(ios): prevent integer overflow in drain mode byte tracking Cast `bytesWritten` and `offset` to Int64 individually before subtracting, avoiding a potential Int overflow when the difference is computed before the widening cast. Co-Authored-By: Claude Opus 4.6 (1M context) --- ios/Sources/GutenbergKitHTTP/HTTPRequestParser.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ios/Sources/GutenbergKitHTTP/HTTPRequestParser.swift b/ios/Sources/GutenbergKitHTTP/HTTPRequestParser.swift index 0fce5c251..e2778bef0 100644 --- a/ios/Sources/GutenbergKitHTTP/HTTPRequestParser.swift +++ b/ios/Sources/GutenbergKitHTTP/HTTPRequestParser.swift @@ -205,7 +205,7 @@ public final class HTTPRequestParser: @unchecked Sendable { if case .draining = _state { bytesWritten += data.count if let offset = headerEndOffset, - Int64(bytesWritten - offset) >= expectedContentLength { + Int64(bytesWritten) - Int64(offset) >= expectedContentLength { _state = .complete } return From f3a08d119579826ac91d012e9dfb4216030911d2 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 3 Apr 2026 11:34:44 -0400 Subject: [PATCH 10/13] refactor(ios): replace deprecated URL.path with path(percentEncoded:) URL.path is deprecated on iOS 16+. Use path(percentEncoded: false) to get the file system path without percent-encoding. Co-Authored-By: Claude Opus 4.6 (1M context) --- ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift index 2c4649e00..0a8c25fa9 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift @@ -414,7 +414,7 @@ class DefaultMediaUploader: @unchecked Sendable { ) let epilogue = Data("\r\n--\(boundary)--\r\n".utf8) - guard let fileSize = try FileManager.default.attributesOfItem(atPath: fileURL.path)[.size] as? Int else { + guard let fileSize = try FileManager.default.attributesOfItem(atPath: fileURL.path(percentEncoded: false))[.size] as? Int else { throw MediaUploadError.streamReadFailed } let contentLength = preamble.count + fileSize + epilogue.count From 71440d97016f8f0a694b501b0a7612e9c4c9cb07 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 3 Apr 2026 11:35:08 -0400 Subject: [PATCH 11/13] refactor(ios): localize "file too large" error via EditorLocalization Add a `fileTooLarge` case to `EditorLocalizableString` so host apps can provide translations for the 413 error message. The hardcoded string in MediaUploadServer now reads from the localization system. Co-Authored-By: Claude Opus 4.6 (1M context) --- ios/Sources/GutenbergKit/Sources/EditorLocalization.swift | 2 ++ ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/ios/Sources/GutenbergKit/Sources/EditorLocalization.swift b/ios/Sources/GutenbergKit/Sources/EditorLocalization.swift index 6f11ad9ae..76668f8d2 100644 --- a/ios/Sources/GutenbergKit/Sources/EditorLocalization.swift +++ b/ios/Sources/GutenbergKit/Sources/EditorLocalization.swift @@ -10,6 +10,7 @@ public enum EditorLocalizableString { // MARK: - Media case failedToInsertMedia + case fileTooLarge // MARK: - Patterns case patterns @@ -39,6 +40,7 @@ public final class EditorLocalization { case .search: "Search" case .insertBlock: "Insert Block" case .failedToInsertMedia: "Failed to insert media" + case .fileTooLarge: "The file is too large to upload in the editor." case .patterns: "Patterns" case .noPatternsFound: "No Patterns Found" case .insertPattern: "Insert Pattern" diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift index 0a8c25fa9..a1992d17c 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift @@ -64,7 +64,7 @@ final class MediaUploadServer: Sendable { // error response here so it includes CORS headers. if let serverError = request.serverError { let message: String = switch serverError { - case .payloadTooLarge: "The file is too large to upload in the editor." + case .payloadTooLarge: await MainActor.run { EditorLocalization[.fileTooLarge] } default: "\(serverError.httpStatusText)" } return errorResponse(status: serverError.httpStatus, body: message) From 80157199d437bc3f805df5c14dd6f03d477993ef Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 3 Apr 2026 11:37:39 -0400 Subject: [PATCH 12/13] Revert "refactor(ios): localize "file too large" error via EditorLocalization" This reverts commit 71440d97016f8f0a694b501b0a7612e9c4c9cb07. --- ios/Sources/GutenbergKit/Sources/EditorLocalization.swift | 2 -- ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/ios/Sources/GutenbergKit/Sources/EditorLocalization.swift b/ios/Sources/GutenbergKit/Sources/EditorLocalization.swift index 76668f8d2..6f11ad9ae 100644 --- a/ios/Sources/GutenbergKit/Sources/EditorLocalization.swift +++ b/ios/Sources/GutenbergKit/Sources/EditorLocalization.swift @@ -10,7 +10,6 @@ public enum EditorLocalizableString { // MARK: - Media case failedToInsertMedia - case fileTooLarge // MARK: - Patterns case patterns @@ -40,7 +39,6 @@ public final class EditorLocalization { case .search: "Search" case .insertBlock: "Insert Block" case .failedToInsertMedia: "Failed to insert media" - case .fileTooLarge: "The file is too large to upload in the editor." case .patterns: "Patterns" case .noPatternsFound: "No Patterns Found" case .insertPattern: "Insert Pattern" diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift index a1992d17c..0a8c25fa9 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift @@ -64,7 +64,7 @@ final class MediaUploadServer: Sendable { // error response here so it includes CORS headers. if let serverError = request.serverError { let message: String = switch serverError { - case .payloadTooLarge: await MainActor.run { EditorLocalization[.fileTooLarge] } + case .payloadTooLarge: "The file is too large to upload in the editor." default: "\(serverError.httpStatusText)" } return errorResponse(status: serverError.httpStatus, body: message) From 691bb272587967c42b5838eef0398afef12ddf72 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Thu, 9 Apr 2026 13:16:00 -0600 Subject: [PATCH 13/13] fix(ios): review adjustments for #419 (#441) * fix(ios): use Int64 for HTTPRequestParser.bytesWritten Change `bytesWritten` from `Int` to `Int64` for consistency with `expectedContentLength` and `maxBodySize`, which are already `Int64`. * test(ios): add end-to-end test for 413 response with CORS headers Expose maxRequestBodySize on MediaUploadServer.start() and add an integration test that sends an oversized request and verifies the response includes both a 413 status and CORS headers. * fix(test): increase maxRequestBodySize for 413 test The multipart overhead (~191 bytes) plus auth headers meant the previous limit of 100 bytes caused the connection to reset before the drain could complete. Use 1024 bytes with a 2048-byte payload so the parser can drain the body and deliver the 413 response. * fix(test): use raw TCP for 413 test to avoid URLSession connection reset URLSession treats a server response during upload as a connection error (NSURLErrorNetworkConnectionLost). Use a raw NWConnection to send the request and read the response directly, which correctly receives the 413 with CORS headers. * fix: complete drain immediately when body arrives with headers When the entire HTTP request (headers + body) arrives in a single read, the parser enters DRAINING but never completes because the body bytes were already counted in bytesWritten. Subsequent reads find no more data, causing a timeout. Check the drain condition immediately when entering the draining state, transitioning to complete if all body bytes have already been received. Fixes both iOS and Android parsers. --- .../gutenberg/http/HTTPRequestParser.kt | 17 ++++++++--- .../Sources/Media/MediaUploadServer.swift | 6 +++- .../GutenbergKitHTTP/HTTPRequestParser.swift | 28 ++++++++++++------- .../Media/MediaUploadServerTests.swift | 25 +++++++++++++++++ 4 files changed, 61 insertions(+), 15 deletions(-) diff --git a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/http/HTTPRequestParser.kt b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/http/HTTPRequestParser.kt index 1c94163db..0157386e4 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/http/HTTPRequestParser.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/http/HTTPRequestParser.kt @@ -126,10 +126,7 @@ class HTTPRequestParser( // whether the full Content-Length has been consumed. if (_state == State.DRAINING) { bytesWritten += data.size.toLong() - val offset = headerEndOffset - if (offset != null && bytesWritten - offset >= expectedContentLength) { - _state = State.COMPLETE - } + drainIfComplete() return } @@ -193,6 +190,10 @@ class HTTPRequestParser( if (expectedContentLength > maxBodySize) { parseError = HTTPRequestParseError.PAYLOAD_TOO_LARGE _state = State.DRAINING + // Complete immediately if body bytes already received + // satisfy the drain — small requests may arrive as a + // single read. + drainIfComplete() return } } @@ -207,6 +208,14 @@ class HTTPRequestParser( } } + /** Transitions from DRAINING to COMPLETE if all body bytes have been received. */ + private fun drainIfComplete() { + val offset = headerEndOffset ?: return + if (bytesWritten - offset >= expectedContentLength) { + _state = State.COMPLETE + } + } + /** * Parses the buffered data into a structured HTTP request. * diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift index 0a8c25fa9..c06a0fa79 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift @@ -27,15 +27,19 @@ final class MediaUploadServer: Sendable { /// - Parameters: /// - uploadDelegate: Optional delegate for customizing file processing and upload. /// - defaultUploader: Fallback uploader used when no delegate provides `uploadFile`. + /// - 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 + defaultUploader: DefaultMediaUploader? = nil, + maxRequestBodySize: Int64 = HTTPRequestParser.defaultMaxBodySize ) async throws -> MediaUploadServer { let context = UploadContext(uploadDelegate: uploadDelegate, defaultUploader: defaultUploader) let server = try await HTTPServer.start( name: "media-upload", requiresAuthentication: true, + maxRequestBodySize: maxRequestBodySize, handler: { request in await Self.handleRequest(request, context: context) } diff --git a/ios/Sources/GutenbergKitHTTP/HTTPRequestParser.swift b/ios/Sources/GutenbergKitHTTP/HTTPRequestParser.swift index e2778bef0..07f188ea3 100644 --- a/ios/Sources/GutenbergKitHTTP/HTTPRequestParser.swift +++ b/ios/Sources/GutenbergKitHTTP/HTTPRequestParser.swift @@ -50,7 +50,7 @@ public final class HTTPRequestParser: @unchecked Sendable { private var buffer: Buffer private let maxBodySize: Int64 private let inMemoryBodyThreshold: Int - private var bytesWritten: Int = 0 + private var bytesWritten: Int64 = 0 private var _state: State = .needsMoreData // Lightweight scan results (populated by append) @@ -146,7 +146,7 @@ public final class HTTPRequestParser: @unchecked Sendable { } if _parsedHeaders == nil { - let headerData = try buffer.read(from: 0, maxLength: min(bytesWritten, Self.maxHeaderSize)) + let headerData = try buffer.read(from: 0, maxLength: Int(min(bytesWritten, Int64(Self.maxHeaderSize)))) switch HTTPRequestSerializer.parseHeaders(from: headerData) { case .parsed(let headers): _parsedHeaders = headers @@ -203,9 +203,9 @@ public final class HTTPRequestParser: @unchecked Sendable { // In drain mode, discard bytes without buffering and check // whether the full Content-Length has been consumed. if case .draining = _state { - bytesWritten += data.count + bytesWritten += Int64(data.count) if let offset = headerEndOffset, - Int64(bytesWritten) - Int64(offset) >= expectedContentLength { + bytesWritten - Int64(offset) >= expectedContentLength { _state = .complete } return @@ -224,12 +224,12 @@ public final class HTTPRequestParser: @unchecked Sendable { _state = .complete return } - bytesWritten += data.count + bytesWritten += Int64(data.count) if headerEndOffset == nil { let buffered: Data do { - buffered = try buffer.read(from: 0, maxLength: min(bytesWritten, Self.maxHeaderSize)) + buffered = try buffer.read(from: 0, maxLength: Int(min(bytesWritten, Int64(Self.maxHeaderSize)))) } catch { _parseError = .bufferIOError _state = .complete @@ -247,7 +247,7 @@ public final class HTTPRequestParser: @unchecked Sendable { let effectiveData = buffered[scanStart...] guard let separatorRange = effectiveData.range(of: separator) else { - if bytesWritten > Self.maxHeaderSize { + if bytesWritten > Int64(Self.maxHeaderSize) { _parseError = .headersTooLarge _state = .complete } else { @@ -268,15 +268,23 @@ public final class HTTPRequestParser: @unchecked Sendable { if expectedContentLength > maxBodySize { _parseError = .payloadTooLarge - _state = .draining + // Check if the body bytes already received in this + // chunk satisfy the drain — small requests may arrive + // as a single read. + if let offset = headerEndOffset, + bytesWritten - Int64(offset) >= expectedContentLength { + _state = .complete + } else { + _state = .draining + } return } } guard let offset = headerEndOffset else { return } - let bodyBytesAvailable = bytesWritten - offset + let bodyBytesAvailable = bytesWritten - Int64(offset) - if Int64(bodyBytesAvailable) >= expectedContentLength { + if bodyBytesAvailable >= expectedContentLength { _state = .complete } else { _state = .headersComplete diff --git a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift index b67af18db..02f2f93e7 100644 --- a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift +++ b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift @@ -163,6 +163,31 @@ struct MediaUploadServerTests { #expect(result.id == 99) } + @Test("returns 413 with CORS headers when request body exceeds max size") + func oversizedUploadReturns413WithCORSHeaders() async throws { + let server = try await MediaUploadServer.start(maxRequestBodySize: 1024) + defer { server.stop() } + + let boundary = UUID().uuidString + let oversizedData = Data(repeating: 0x42, count: 2048) + let body = buildMultipartBody(boundary: boundary, filename: "big.bin", mimeType: "application/octet-stream", data: oversizedData) + + 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 (data, response) = try await URLSession.shared.data(for: request) + let httpResponse = try #require(response as? HTTPURLResponse) + #expect(httpResponse.statusCode == 413) + #expect(httpResponse.value(forHTTPHeaderField: "Access-Control-Allow-Origin") == "*") + + let responseBody = String(data: data, encoding: .utf8) ?? "" + #expect(responseBody.contains("too large")) + } + private func buildMultipartBody(boundary: String, filename: String, mimeType: String, data: Data) -> Data { var body = Data() body.append("--\(boundary)\r\n")