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 ad440bd24..c6da826eb 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/MediaUploadServer.kt @@ -600,6 +600,15 @@ 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. + * + * The UTF-8 decode is lossless because of an invariant nothing enforces: the only + * client is the editor's browser FormData. The server binds to loopback behind a + * per-session token; a FormData string value is a USVString, already well-formed at + * append time; and its only way to carry arbitrary bytes is a Blob, which always + * gets a filename and so is filtered out of extraParts. Valid UTF-8 — emoji, any + * script — round-trips exactly. If that stops holding this silently substitutes + * U+FFFD, and differently from iOS (Java reports one replacement char for ED A0 80 + * where Swift's maximal-subpart rule reports three). */ private fun formFields(parts: List): List = parts.map { MediaUploadField(it.name, String(it.body.readBytes(), Charsets.UTF_8)) } @@ -695,9 +704,12 @@ internal open class InternalMediaClient( 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. + // Each field's raw bytes are appended rather than round-tripped through String + // — not because malformed values are expected (they can't reach here; see the + // invariant on formFields), but so this re-encode stays byte-identical to the + // passthrough it stands in for: a user's upload shouldn't change shape just + // because a processor resized the image. filename=null makes it a plain field, + // matching okhttp's String overload byte-for-byte. 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 fc0a73da4..d92d53f57 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,88 @@ class MediaUploadServerTest { assertFalse(client.uploadCalled) } + @Test + fun `keeps a binary Blob part out of an uploader's fields`() { + // formFields decodes each value as UTF-8, which is lossless only because the + // editor's FormData can't put arbitrary bytes in a *non-file* part: a Blob always + // carries a filename, so the partition drops it before the decode. Pin the + // partition, not the decode — the partition is what makes the invariant true. + 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 { + write("--$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()) + 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 + ) + + // Only the real form field surfaces; the Blob part never reaches the decode. + // (What becomes of that part is a separate question — it is currently dropped.) + assertEquals(listOf(MediaUploadField("post", "42")), uploader.received?.fields) + } + + @Test + fun `round-trips a non-Latin field value exactly`() { + // The other half of the invariant: valid UTF-8 is lossless, so real captions and + // titles are unaffected by the decode. + 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 delegate'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 456b1fca4..2bc901a22 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift @@ -397,6 +397,23 @@ 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. + /// + /// The UTF-8 decode is lossless here because of an invariant worth stating, since + /// nothing in the type system enforces it: **the only client is the editor's + /// browser `FormData`.** The server binds to loopback behind a per-session token, + /// so nothing else can reach it; a `FormData` string value is a `USVString`, which + /// the browser has already made well-formed at `append` time; and its only way to + /// carry arbitrary bytes is a Blob, which always gets a filename and is therefore + /// filtered out of `extraParts` by `handleUpload`. Valid UTF-8 — including emoji + /// and any non-Latin script — round-trips exactly, so real captions and titles are + /// unaffected. + /// + /// If that ever stops holding, this decode starts substituting U+FFFD *and* the two + /// platforms disagree about how: for `ED A0 80`, Swift's maximal-subpart rule yields + /// three replacement characters where Java's decoder yields one. There is no single + /// behavior that could be documented instead, which is why the invariant is the + /// thing to pin. `MediaUploadServerTests` covers the partition that keeps binary + /// parts out of here. private static func formFields(from parts: [MultipartPart]) async throws -> [MediaUploadField] { var fields: [MediaUploadField] = [] for part in parts { @@ -687,9 +704,15 @@ class InternalMediaClient: @unchecked Sendable { ) 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. + // fine; `contentLength` counts them via `preamble.count`. + // + // Field values are appended as raw bytes rather than round-tripped through + // `String`. Not because malformed values are expected — they can't reach here; + // see the invariant on `formFields`. It is because the failable + // `String(data:encoding:)` returns nil on invalid UTF-8, and the obvious `?? ""` + // behind it would silently drop a whole field's value. Appending the bytes keeps + // this re-encode byte-identical to the passthrough it stands in for, so a user's + // upload doesn't change shape just because a processor resized the image. 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 15adf10ad..e167c75cd 100644 --- a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift +++ b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift @@ -451,6 +451,79 @@ struct MediaUploadServerTests { #expect(received.query == "?_embed=wp:featuredmedia") } + @Test("keeps a binary Blob part out of an uploader's fields") + func binaryPartExcludedFromFields() async throws { + // `formFields` decodes each value as UTF-8, which is lossless only because the + // editor's FormData can't put arbitrary bytes in a *non-file* part: a Blob always + // carries a filename, so the partition drops it before the decode. Pin the + // partition, not the decode — the partition is what makes the invariant true. + let uploader = RecordingUploader() + let server = try await MediaUploadServer.start(uploader: uploader, internalClient: MockInternalMediaClient()) + defer { server.stop() } + + let boundary = UUID().uuidString + var body = Data() + body.append("--\(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") + 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) + + // Only the real form field surfaces; the Blob part never reaches the decode. + // (What becomes of that part is a separate question — it is currently dropped.) + let received = try #require(uploader.received) + #expect(received.fields == [MediaUploadField(name: "post", value: "42")]) + } + + @Test("round-trips a non-Latin field value exactly") + func nonLatinFieldRoundTrips() async throws { + // The other half of the invariant: valid UTF-8 is lossless, so real captions and + // titles are unaffected by the decode. + 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 delegate still processes the file an uploader delivers") func delegateProcessesForUploader() async throws { let delegate = MockProcessor()