Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,11 @@ data class HttpRequest(
val target: String,
val headers: Map<String, String>,
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).
Expand Down Expand Up @@ -286,6 +290,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()
Expand All @@ -312,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Comment on lines +96 to +104
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach of passing server errors to the handler was taken rather than the adding a new errorResponseHeaders configuration (added in 0b6c67b, reverted in 0563a94). The configuration approach leaks host/caller concerns into the library. A similar consideration occurred when implementing authentication bypass for preflight OPTIONS requests.

Ultimately, the 413 responses need to include the correct CORS headers, otherwise the error code and message do not arrive to the client. It instead receives a CORS error.

@jkmassel what do you think of this approach?


// CORS preflight — the library exempts OPTIONS from auth, so this is
// reached without a token.
if (request.method.uppercase() == "OPTIONS") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -76,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,
Expand Down Expand Up @@ -107,6 +122,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)
Expand Down Expand Up @@ -166,7 +192,7 @@ class HTTPRequestParser(

if (expectedContentLength > maxBodySize) {
parseError = HTTPRequestParseError.PAYLOAD_TOO_LARGE
_state = State.COMPLETE
_state = State.DRAINING
return
}
}
Expand Down Expand Up @@ -194,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())
Expand All @@ -210,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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,69 @@ class HTTPRequestParserTests {
assertArrayEquals(body.toByteArray(), request.body?.readBytes())
}

@Test
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())

// 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)

// 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
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)
val request = parser.parseRequest()!!
assertEquals("POST", request.method)
assertFalse(request.isComplete)
assertEquals(HTTPRequestParseError.PAYLOAD_TOO_LARGE, parser.pendingParseError)
}

@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)

// 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

@Test
Expand Down
Loading
Loading