diff --git a/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt b/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt index a978a8f286..714219e460 100644 --- a/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt +++ b/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt @@ -10,6 +10,7 @@ import com.google.gson.Gson import com.google.gson.GsonBuilder import com.google.gson.ToNumberPolicy import com.google.gson.reflect.TypeToken +import com.itsaky.androidide.utils.ContentTypeHeaders import com.itsaky.androidide.utils.DatabaseVersionResolver import io.pebbletemplates.pebble.PebbleEngine import io.pebbletemplates.pebble.loader.StringLoader @@ -480,6 +481,10 @@ class WebServer( } private fun handleClient(clientSocket: Socket) { + // Whether the client has already been told 200. Read by the error path, which must not put a + // second status line on a response that has already started (see the assignment below). + var responseStarted = false + if (debugEnabled) log.debug("In handleClient(), socket is {}.", clientSocket) val input = clientSocket.getInputStream() @@ -625,6 +630,8 @@ class WebServer( while (nextChunk.size == contentChunkSize) { val path2 = "$path-$fragmentNumber" val cursor2 = database.rawQuery(query2, arrayOf(path2)) + // Whether the client has already been told 200; see the assignment below. + var responseStarted = false try { if (cursor2.moveToFirst()) { nextChunk = cursor2.getBlob(0) @@ -657,8 +664,19 @@ class WebServer( dbContent = instantiatePebbleTemplate(templateId, dbContent, path, dbMimeType, compression) } + // Built before the status line goes out: everything after the first println is on the + // wire (the writer autoflushes), so a throw past that point cannot be answered with a + // fresh error response. dbMimeType is a platform type from Cursor.getString, so a NULL + // ContentTypes.value throws here rather than there. + val contentTypeHeader = ContentTypeHeaders.headerValue(dbMimeType) + writer.println("HTTP/1.1 200 OK") - writer.println("Content-Type: $dbMimeType") + // From here on the client has been told 200. A failure while writing the body -- a + // dropped connection is the common one -- must not append a second status line to that + // response; sendError writes nothing at all when told the output has started, which is + // the only honest thing left to do with a half-sent reply. + responseStarted = true + writer.println("Content-Type: $contentTypeHeader") writer.println("Content-Length: ${dbContent.size}") writer.println("Connection: close") writer.println() @@ -666,8 +684,15 @@ class WebServer( output.write(dbContent) output.flush() } catch (e: Exception) { - log.error("Error processing request: {}", e.message) - sendError(writer, output, httpInternalServerError, "Internal Server Error", e.message ?: "") + log.error("Error processing request: {}", e.message, e) + sendError( + writer, + output, + httpInternalServerError, + "Internal Server Error", + e.message ?: "", + outputStarted = responseStarted, + ) } finally { cursor.close() } diff --git a/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt b/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt index e68b2e05e4..3bbb403556 100644 --- a/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt +++ b/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt @@ -10,6 +10,7 @@ import io.mockk.mockkStatic import io.mockk.unmockkAll import io.mockk.verify import org.junit.After +import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before @@ -377,6 +378,68 @@ class WebServerTest { } } + // ADFA-5241: the helper decides the charset, but only a real response proves the header that + // reaches a client. Two thirds of the database's text rows are non-ASCII with no BOM, so an + // undeclared encoding renders them as mojibake in any client that does not assume UTF-8. + @Test + fun `a text response declares utf-8 and a binary one does not`() { + assertContentTypeHeader(storedMimeType = "text/html", expected = "text/html; charset=utf-8") + assertContentTypeHeader(storedMimeType = "image/png", expected = "image/png") + } + + private fun assertContentTypeHeader( + storedMimeType: String, + expected: String, + ) { + val port = freePort() + val db = mockk(relaxed = true) + every { SQLiteDatabase.openDatabase(any(), isNull(), any()) } returns db + // Deliberately no version stub: this row is compression = "none", so nothing decodes and no + // dictionary is consulted. Declaring one would couple the assertion to a lazy-load path it + // does not exercise. + every { + db.rawQuery(match { it.contains("FROM Content") }, any()) + } returns + mockk(relaxed = true) { + every { count } returns 1 + every { moveToFirst() } returns true + every { getBlob(0) } returns "payload".toByteArray() + every { getString(1) } returns storedMimeType + every { getString(2) } returns "none" + every { getInt(3) } returns 0 + } + + val server = WebServer(testConfig(port)) + val serverThread = Thread { server.start() }.apply { isDaemon = true } + serverThread.start() + try { + awaitPortBound(port) + val response = sendRawGetRequest(port, "/some/path") + val header = + response.lineSequence().firstOrNull { it.startsWith("Content-Type:", ignoreCase = true) } + ?: error("No Content-Type in the response:\n$response") + assertEquals("Content-Type: $expected", header.trim()) + } finally { + server.stop() + serverThread.join(2_000) + } + } + + // Same as sendRawGetRequestAndAwaitClose, but hands back what the server actually wrote. + private fun sendRawGetRequest( + port: Int, + path: String, + ): String = + Socket().use { socket -> + socket.connect(InetSocketAddress("localhost", port), 2_000) + socket.soTimeout = 2_000 + socket.getOutputStream().apply { + write("GET $path HTTP/1.1\r\n\r\n".toByteArray(Charsets.ISO_8859_1)) + flush() + } + socket.getInputStream().readBytes().toString(Charsets.ISO_8859_1) + } + // Blocks until the server closes the connection (every response sends "Connection: close"), // so by the time this returns the server has fully finished processing this one request -- // making repeated calls a reliable way to serialize several full request/response cycles. diff --git a/common/src/main/java/com/itsaky/androidide/utils/ContentTypeHeaders.kt b/common/src/main/java/com/itsaky/androidide/utils/ContentTypeHeaders.kt new file mode 100644 index 0000000000..298ebe4761 --- /dev/null +++ b/common/src/main/java/com/itsaky/androidide/utils/ContentTypeHeaders.kt @@ -0,0 +1,259 @@ +package com.itsaky.androidide.utils + +/** + * Turns a stored `ContentTypes.value` into the `Content-Type` a client should be sent. + * + * `documentation.db` stores bare MIME types -- no `ContentTypes.value` carries a charset -- so a + * text response says nothing about its encoding, and a client that does not assume UTF-8 falls back + * to a legacy single-byte encoding. 17,903 of the 29,139 text rows in the 21-Aug database -- + * 61.4%, a full census rather than a sample -- contain non-ASCII bytes with no BOM, so they render + * as mojibake wherever that guess goes wrong (ADFA-5241): a page's U+21B3 arrow arriving as the + * three Latin-1 characters its UTF-8 bytes decode to. (The rate is per-generation; an older export + * measures far lower, so quote the database when quoting the number.) + * + * This is deliberately *not* fixed by storing the parameter in the database. `ContentTypes.value` + * doubles as a lookup key matched exactly by the plugin installer + * (`ExtensionToContentTypeResolver`), by `docdb-studio`'s anchor extraction, and by three scripts in + * `OfflineDocumentationTools`; two of those would fail silently rather than loudly. + * + * Kept in `common` so both documentation transports answer the same way -- the socket server here + * and ADFA-5176's `shouldInterceptRequest` interceptor, which otherwise carries its own rule. + */ +object ContentTypeHeaders { + private const val UTF_8 = "utf-8" + private const val TEXT_PLAIN = "text/plain" + + // What a value carrying a control character becomes: renders nothing, injects nothing. + private const val OCTET_STREAM = "application/octet-stream" + + // Textual types outside text/* and the +xml family. application/x-typescript has no rows in the + // current database but costs nothing to keep; application/javascript does have rows, and is what + // ExtensionToContentTypeResolver maps ".mjs" to, so omitting it served real files undeclared. + private val TEXTUAL_APPLICATION_TYPES = + setOf( + "application/javascript", + "application/ecmascript", + "application/x-typescript", + "application/x-sh", + ) + + /** + * The charset to send with [mimeType], or null when there is none to send: the type is binary, or + * the format fixes its own encoding. + * + * `application/json` is deliberately absent: RFC 8259 defines no charset parameter for it and + * fixes the encoding as UTF-8, so declaring one is meaningless rather than helpful. XML-based + * types are included even though a document may carry its own declaration, because a + * transport-level charset takes precedence and SVG in particular usually omits it. + */ + internal fun charsetFor(mimeType: String): String? = typeAndCharset(mimeType).second + + /** + * The media type and the charset to send with it, which is what + * `WebResourceResponse(type, encoding, stream)` wants and what [headerValue] builds its header + * from. One decision, so the two documentation transports cannot disagree. + * + * The type is normalized, not passed through: the database stores a bare `text` and a + * `text/text`, neither of which is a media type (`type "/" subtype` is required), and a client + * that cannot parse the type discards the charset with it -- which would have made this whole + * change a no-op on exactly those rows. A value carrying a control character is refused outright; + * see [safeType]. + * + * An unusable `charset=` counts as declaring nothing, so the default applies. [headerValue] + * rebuilds the header rather than appending to the stored string, so that substitution reaches + * both transports instead of only this one. + */ + internal fun typeAndCharset(mimeType: String): Pair { + // Refusal has to be asked, not inferred. This used to read the answer out of safeType's return + // value -- OCTET_STREAM with no charset -- and a control character inside the charset parameter + // defeated it: safeType refused the type, declaredCharset re-parsed the *original* string and + // found a charset, so "refused" was never true and the CRLF went into the header verbatim. + // text/html; charset=x" + + val header = ContentTypeHeaders.headerValue(split) + + assertThat(header).isEqualTo("application/octet-stream") + assertThat(header).doesNotContain("\r") + assertThat(header).doesNotContain("\n") + assertThat(ContentTypeHeaders.typeAndCharset(split).second).isNull() + } + + @Test + fun `a control character in a parameter name refuses the whole value`() { + val header = ContentTypeHeaders.headerValue("text/html; x\r\nX-Injected: y=1") + + assertThat(header).isEqualTo("application/octet-stream") + } + + // parameters() strips the quotes, so appending the value raw turned one charset into a charset + // plus a second parameter the client would honour. + @Test + fun `a quoted charset cannot smuggle a second parameter`() { + val header = ContentTypeHeaders.headerValue("""text/html; charset="utf-8; x=y"""") + + // Quoted, so the recipient reads one charset whose value happens to contain a semicolon -- + // not a charset plus a second parameter. Appended raw it was the latter. + assertThat(header).isEqualTo("""text/html; charset="utf-8; x=y"""") + } + + // Refusal used to be read off the returned string, so a genuine octet-stream looked refused. + @Test + fun `a stored octet-stream keeps its own parameters`() { + val header = ContentTypeHeaders.headerValue("application/octet-stream; name=file.bin") + + assertThat(header).contains("name=file.bin") + } +}