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 685db033c..05cfb9f4d 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt @@ -603,6 +603,22 @@ internal class MediaUploadServer( * * A list rather than a map so repeated names (e.g. a `field[]` array) survive * verbatim, in the order the editor sent them. + * + * This decode can't mangle anything, but only because of who is on the other end — + * nothing in the code enforces it. Three things have to stay true: + * + * 1. Only the editor's own web page can reach this server. It listens on loopback, + * and every request has to carry a per-session token. + * 2. Text the editor puts in a form field is already valid Unicode. The browser + * guarantees that when the value is set, so it cannot hand us bad bytes. + * 3. The only way a browser can put *raw* bytes in a form is a file or a Blob, and + * those always arrive with a filename. Anything with a filename is handled as the + * file, never as a field — so raw bytes never reach this decode. + * + * If one of those stops being true, bad bytes quietly turn into replacement + * characters, and the platforms don't even agree on how many: ED A0 80 becomes one + * of them here and three on iOS. There is no single behavior worth documenting, so + * the tests pin rule 3 instead. */ private fun formFields(parts: List): List = parts.map { MediaUploadField(it.name, String(it.body.readBytes(), Charsets.UTF_8)) } @@ -697,10 +713,10 @@ internal open class InternalMediaClient( ): MediaUploadResponse { val mediaType = mimeType.toMediaType() val builder = okhttp3.MultipartBody.Builder().setType(okhttp3.MultipartBody.FORM) - // Preserve the non-file parts (post, additionalData) through the re-encode. - // Append each field's raw bytes (not via String) so a non-UTF-8 value is - // forwarded verbatim rather than coerced. filename=null makes it a plain - // field, matching okhttp's String overload byte-for-byte. + // Non-file parts (post, additionalData) have to survive the re-encode unchanged: + // appending raw bytes keeps them byte-for-byte identical to the plain passthrough, + // and filename=null makes each a plain field, exactly what okhttp's String overload + // would emit. (Bad bytes can't get here; see formFields.) for (part in extraParts) { builder.addFormDataPart(part.name, null, part.body.readBytes().toRequestBody()) } 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 60af5c723..ff22f8c1f 100644 --- a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/MediaUploadServerTest.kt +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/MediaUploadServerTest.kt @@ -274,6 +274,85 @@ class MediaUploadServerTest { assertFalse(client.uploadCalled) } + @Test + fun `keeps a binary Blob part out of an uploader's fields`() { + // Pin rule 3: a Blob always has a filename, so it's dropped before the decode. + val uploader = RecordingUploader() + server.stop() + server = MediaUploadServer( + processor = null, internalClient = MockInternalMediaClient(), uploader = uploader, + cacheDir = tempFolder.root + ) + + val boundary = "test-boundary-blob" + val body = java.io.ByteArrayOutputStream().apply { + // Ordered as uploadToServer emits it: the file first, then additionalData. + write("--$boundary\r\n".toByteArray()) + write("Content-Disposition: form-data; name=\"file\"; filename=\"photo.jpg\"\r\n".toByteArray()) + write("Content-Type: image/jpeg\r\n\r\n".toByteArray()) + write("fake image data".toByteArray()) + write("\r\n--$boundary\r\n".toByteArray()) + write("Content-Disposition: form-data; name=\"post\"\r\n\r\n".toByteArray()) + write("42\r\n".toByteArray()) + // A Blob-shaped part: it has a filename, and its bytes are not valid UTF-8. + write("--$boundary\r\n".toByteArray()) + write("Content-Disposition: form-data; name=\"blob\"; filename=\"blob\"\r\n".toByteArray()) + write("Content-Type: application/octet-stream\r\n\r\n".toByteArray()) + write(byteArrayOf(0xED.toByte(), 0xA0.toByte(), 0x80.toByte())) + write("\r\n--$boundary--\r\n".toByteArray()) + }.toByteArray() + + sendRawRequest( + method = "POST", + path = "/upload", + headers = mapOf( + "Relay-Authorization" to "Bearer ${server.token}", + "Content-Type" to "multipart/form-data; boundary=$boundary" + ), + body = body + ) + + // The Blob is dropped rather than decoded, and file is still the file. + assertEquals("photo.jpg", uploader.received?.filename) + assertEquals(listOf(MediaUploadField("post", "42")), uploader.received?.fields) + } + + @Test + fun `round-trips a non-Latin field value exactly`() { + // The other half: valid UTF-8 round-trips, so real captions and titles survive. + val uploader = RecordingUploader() + server.stop() + server = MediaUploadServer( + processor = null, internalClient = MockInternalMediaClient(), uploader = uploader, + cacheDir = tempFolder.root + ) + + val caption = "Grüße 🎉 日本語" + val boundary = "test-boundary-utf8" + val body = java.io.ByteArrayOutputStream().apply { + write("--$boundary\r\n".toByteArray()) + write("Content-Disposition: form-data; name=\"caption\"\r\n\r\n".toByteArray()) + write("$caption\r\n".toByteArray()) + write("--$boundary\r\n".toByteArray()) + write("Content-Disposition: form-data; name=\"file\"; filename=\"photo.jpg\"\r\n".toByteArray()) + write("Content-Type: image/jpeg\r\n\r\n".toByteArray()) + write("fake image data".toByteArray()) + write("\r\n--$boundary--\r\n".toByteArray()) + }.toByteArray() + + sendRawRequest( + method = "POST", + path = "/upload", + headers = mapOf( + "Relay-Authorization" to "Bearer ${server.token}", + "Content-Type" to "multipart/form-data; boundary=$boundary" + ), + body = body + ) + + assertEquals(listOf(MediaUploadField("caption", caption)), uploader.received?.fields) + } + @Test fun `an uploader sees a file the processor's metadata gate would have declined`() { // The gate exists to skip a temp copy for a file the processor won't touch. An diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift index 5e206c466..ef17e707b 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift @@ -462,6 +462,22 @@ final class MediaUploadServer: Sendable { /// /// A list rather than a dictionary so repeated names (e.g. a `field[]` array) /// survive verbatim, in the order the editor sent them. + /// + /// This decode can't mangle anything, but only because of who is on the other end — + /// nothing in the code enforces it. Three things have to stay true: + /// + /// 1. Only the editor's own web page can reach this server. It listens on loopback, + /// and every request has to carry a per-session token. + /// 2. Text the editor puts in a form field is already valid Unicode. The browser + /// guarantees that when the value is set, so it cannot hand us bad bytes. + /// 3. The only way a browser can put *raw* bytes in a form is a file or a Blob, and + /// those always arrive with a filename. Anything with a filename is handled as + /// the file, never as a field — so raw bytes never reach this decode. + /// + /// If one of those stops being true, bad bytes quietly turn into replacement + /// characters, and the platforms don't even agree on how many: `ED A0 80` becomes + /// three of them here and one on Android. There is no single behavior worth + /// documenting, so the tests pin rule 3 instead. private static func formFields(from parts: [MultipartPart]) async throws -> [MediaUploadField] { var fields: [MediaUploadField] = [] for part in parts { @@ -750,11 +766,12 @@ class InternalMediaClient: @unchecked Sendable { mimeType: String, extraFields: [(name: String, value: Data)] ) throws -> (InputStream, Int) { - // Serialize the non-file parts (post, additionalData) into the preamble - // ahead of the streamed file. They are small, so keeping them in memory is - // fine; `contentLength` counts them via `preamble.count`. Field values are - // appended as raw bytes (not through String) so a non-UTF-8 value is - // forwarded verbatim rather than coerced to empty. + // The non-file parts (post, additionalData) go into the preamble ahead of the streamed + // file; they're small, and `contentLength` counts them via `preamble.count`. Their + // values are appended as raw bytes rather than through `String(data:encoding:)`, which + // returns nil on bad UTF-8 — and the `?? ""` you'd reach for behind it would quietly + // drop a whole field. Raw bytes also keep this re-encode byte-for-byte identical to + // the plain passthrough it replaces. (Bad bytes can't get here; see `formFields`.) var preamble = Data() for field in extraFields { preamble.append(Data("--\(boundary)\r\n".utf8)) diff --git a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift index 841e53285..76d44b47a 100644 --- a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift +++ b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift @@ -491,6 +491,76 @@ struct MediaUploadServerTests { #expect(received.query == "?_embed=wp:featuredmedia") } + @Test("keeps a binary Blob part out of an uploader's fields") + func binaryPartExcludedFromFields() async throws { + // Pin rule 3: a Blob always has a filename, so it's dropped before the decode. + let uploader = RecordingUploader() + let server = try await MediaUploadServer.start(uploader: uploader, internalClient: MockInternalMediaClient()) + defer { server.stop() } + + let boundary = UUID().uuidString + var body = Data() + // Ordered as `uploadToServer` emits it: the file first, then additionalData. + body.append("--\(boundary)\r\n") + body.append("Content-Disposition: form-data; name=\"file\"; filename=\"photo.jpg\"\r\n") + body.append("Content-Type: image/jpeg\r\n\r\n") + body.append(Data("fake image data".utf8)) + body.append("\r\n--\(boundary)\r\n") + body.append("Content-Disposition: form-data; name=\"post\"\r\n\r\n") + body.append("42\r\n") + // A Blob-shaped part: it has a filename, and its bytes are not valid UTF-8. + body.append("--\(boundary)\r\n") + body.append("Content-Disposition: form-data; name=\"blob\"; filename=\"blob\"\r\n") + body.append("Content-Type: application/octet-stream\r\n\r\n") + body.append(Data([0xED, 0xA0, 0x80])) + body.append("\r\n--\(boundary)--\r\n") + + 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 + + _ = try await URLSession.shared.data(for: request) + + // The Blob is dropped rather than decoded, and `file` is still the file. + let received = try #require(uploader.received) + #expect(received.filename == "photo.jpg") + #expect(received.fields == [MediaUploadField(name: "post", value: "42")]) + } + + @Test("round-trips a non-Latin field value exactly") + func nonLatinFieldRoundTrips() async throws { + // The other half: valid UTF-8 round-trips, so real captions and titles survive. + let uploader = RecordingUploader() + let server = try await MediaUploadServer.start(uploader: uploader, internalClient: MockInternalMediaClient()) + defer { server.stop() } + + let caption = "Grüße 🎉 日本語" + let boundary = UUID().uuidString + var body = Data() + body.append("--\(boundary)\r\n") + body.append("Content-Disposition: form-data; name=\"caption\"\r\n\r\n") + body.append("\(caption)\r\n") + body.append("--\(boundary)\r\n") + body.append("Content-Disposition: form-data; name=\"file\"; filename=\"photo.jpg\"\r\n") + body.append("Content-Type: image/jpeg\r\n\r\n") + body.append(Data("fake image data".utf8)) + body.append("\r\n--\(boundary)--\r\n") + + 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 + + _ = try await URLSession.shared.data(for: request) + + #expect(uploader.received?.fields == [MediaUploadField(name: "caption", value: caption)]) + } + @Test("a processor still processes the file an uploader delivers") func processorRunsForUploader() async throws { let processor = ProcessOnlyProcessor()