From 59bc00d46e2bcfa8b6a86f4a700db721e7392af0 Mon Sep 17 00:00:00 2001 From: Vamsi Vaddavalli Date: Wed, 12 Aug 2026 23:13:18 -0500 Subject: [PATCH 1/3] Don't close the stream when gzipping a request body --- .../okhttp3/internal/http/GzipRequestBody.kt | 10 ++++- .../kotlin/okhttp3/MultipartBodyTest.kt | 45 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/http/GzipRequestBody.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/http/GzipRequestBody.kt index 0804780ed239..e06612f54131 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/http/GzipRequestBody.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/http/GzipRequestBody.kt @@ -17,6 +17,7 @@ package okhttp3.internal.http import okhttp3.RequestBody import okio.BufferedSink +import okio.ForwardingSink import okio.GzipSink import okio.buffer @@ -29,7 +30,14 @@ internal class GzipRequestBody( override fun contentLength() = -1L override fun writeTo(sink: BufferedSink) { - GzipSink(sink).buffer().use(delegate::writeTo) + GzipSink( + object : ForwardingSink(sink) { + override fun close() { + // GzipSink.close() writes the gzip footer, then closes whatever it wraps. + // Swallow that close so the caller's sink stays open (MultipartBody writes more). + } + }, + ).buffer().use(delegate::writeTo) } override fun isOneShot() = delegate.isOneShot() diff --git a/okhttp/src/jvmTest/kotlin/okhttp3/MultipartBodyTest.kt b/okhttp/src/jvmTest/kotlin/okhttp3/MultipartBodyTest.kt index a2b69d015ec0..27c4974a999c 100644 --- a/okhttp/src/jvmTest/kotlin/okhttp3/MultipartBodyTest.kt +++ b/okhttp/src/jvmTest/kotlin/okhttp3/MultipartBodyTest.kt @@ -17,6 +17,7 @@ package okhttp3 import assertk.assertThat import assertk.assertions.isEqualTo +import assertk.assertions.isNull import java.io.IOException import java.nio.charset.StandardCharsets import kotlin.test.assertFailsWith @@ -24,8 +25,12 @@ import okhttp3.Headers.Companion.headersOf import okhttp3.MediaType.Companion.toMediaType import okhttp3.MediaType.Companion.toMediaTypeOrNull import okhttp3.RequestBody.Companion.toRequestBody +import okhttp3.internal.http.GzipRequestBody import okio.Buffer import okio.BufferedSink +import okio.ForwardingSink +import okio.GzipSource +import okio.buffer import okio.utf8Size import org.junit.jupiter.api.Test @@ -254,6 +259,46 @@ class MultipartBodyTest { assertThat(buffer.readUtf8()).isEqualTo(expected) } + @Test + fun gzippedParts() { + val body = + MultipartBody + .Builder("123") + .addPart(GzipRequestBody("part1".toRequestBody(null))) + .addPart(GzipRequestBody("part2".toRequestBody(null))) + .build() + assertThat(body.boundary).isEqualTo("123") + assertThat(body.type).isEqualTo(MultipartBody.MIXED) + assertThat(body.contentType().toString()) + .isEqualTo("multipart/mixed; boundary=123") + assertThat(body.parts.size).isEqualTo(2) + assertThat(body.contentLength()).isEqualTo(-1) + + val buffer = Buffer() + // Don't write to Buffer directly: Buffer.close() is a no-op. Wrap it so close() + // actually closes the sink, like a real HTTP write. + val sink = object : ForwardingSink(buffer) {}.buffer() + body.writeTo(sink) + sink.close() + + MultipartReader(boundary = "123", source = buffer).use { reader -> + val part1 = reader.nextPart()!! + val part1Body = + GzipSource(part1.body).use { + it.buffer().readUtf8() + } + assertThat(part1Body).isEqualTo("part1") + + val part2 = reader.nextPart()!! + val part2Body = + GzipSource(part2.body).use { + it.buffer().readUtf8() + } + assertThat(part2Body).isEqualTo("part2") + assertThat(reader.nextPart()).isNull() + } + } + @Test fun contentTypeHeaderIsForbidden() { val multipart = MultipartBody.Builder() From e08d3ccf2c3702e4b782d42a3d166ab7102bf15b Mon Sep 17 00:00:00 2001 From: Vamsi Vaddavalli Date: Thu, 13 Aug 2026 18:48:46 -0500 Subject: [PATCH 2/3] Don't close the multipart sink when a part closes its stream --- .../kotlin/okhttp3/MultipartBody.kt | 9 +++- .../kotlin/okhttp3/RequestBody.kt | 2 +- .../okhttp3/internal/http/GzipRequestBody.kt | 10 +--- .../kotlin/okhttp3/MultipartBodyTest.kt | 46 +++++++++++++++++++ 4 files changed, 56 insertions(+), 11 deletions(-) diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/MultipartBody.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/MultipartBody.kt index 6407ce29ea6b..db06b5c8ece8 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/MultipartBody.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/MultipartBody.kt @@ -23,6 +23,8 @@ import okio.Buffer import okio.BufferedSink import okio.ByteString import okio.ByteString.Companion.encodeUtf8 +import okio.ForwardingSink +import okio.buffer /** * An [RFC 2387][rfc_2387]-compliant request body. @@ -160,7 +162,12 @@ class MultipartBody internal constructor( if (countBytes) { byteCount += contentLength } else { - body.writeTo(sink) + object : ForwardingSink(sink) { + override fun close() { + // GzipSink.close() writes the gzip footer, then closes whatever it wraps. + // Swallow that close so we can write the trailing CRLF and later parts. + } + }.buffer().use(body::writeTo) } sink.write(CRLF) diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/RequestBody.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/RequestBody.kt index be595678aa92..3dca9c725d65 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/RequestBody.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/RequestBody.kt @@ -41,7 +41,7 @@ abstract class RequestBody { @Throws(IOException::class) open fun contentLength(): Long = -1L - /** Writes the content of this request to [sink]. */ + /** Writes the content of this request to [sink]. This must not close [sink]. */ @Throws(IOException::class) abstract fun writeTo(sink: BufferedSink) diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/http/GzipRequestBody.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/http/GzipRequestBody.kt index e06612f54131..0804780ed239 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/http/GzipRequestBody.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/http/GzipRequestBody.kt @@ -17,7 +17,6 @@ package okhttp3.internal.http import okhttp3.RequestBody import okio.BufferedSink -import okio.ForwardingSink import okio.GzipSink import okio.buffer @@ -30,14 +29,7 @@ internal class GzipRequestBody( override fun contentLength() = -1L override fun writeTo(sink: BufferedSink) { - GzipSink( - object : ForwardingSink(sink) { - override fun close() { - // GzipSink.close() writes the gzip footer, then closes whatever it wraps. - // Swallow that close so the caller's sink stays open (MultipartBody writes more). - } - }, - ).buffer().use(delegate::writeTo) + GzipSink(sink).buffer().use(delegate::writeTo) } override fun isOneShot() = delegate.isOneShot() diff --git a/okhttp/src/jvmTest/kotlin/okhttp3/MultipartBodyTest.kt b/okhttp/src/jvmTest/kotlin/okhttp3/MultipartBodyTest.kt index 27c4974a999c..19a9bcdb395a 100644 --- a/okhttp/src/jvmTest/kotlin/okhttp3/MultipartBodyTest.kt +++ b/okhttp/src/jvmTest/kotlin/okhttp3/MultipartBodyTest.kt @@ -259,6 +259,52 @@ class MultipartBodyTest { assertThat(buffer.readUtf8()).isEqualTo(expected) } + @Test + fun partThatClosesTheSink() { + class ClosingBody( + private val body: String, + ) : RequestBody() { + override fun contentType(): MediaType? = null + + @Throws(IOException::class) + override fun writeTo(sink: BufferedSink) { + sink.writeUtf8(body) + sink.close() + } + } + + val expected = + """ + |--123 + | + |hello + |--123 + | + |world + |--123-- + | + """.trimMargin().replace("\n", "\r\n") + val body = + MultipartBody + .Builder("123") + .addPart(ClosingBody("hello")) + .addPart("world".toRequestBody(null)) + .build() + assertThat(body.boundary).isEqualTo("123") + assertThat(body.type).isEqualTo(MultipartBody.MIXED) + assertThat(body.contentType().toString()) + .isEqualTo("multipart/mixed; boundary=123") + assertThat(body.parts.size).isEqualTo(2) + assertThat(body.contentLength()).isEqualTo(-1) + val buffer = Buffer() + // Don't write to Buffer directly: Buffer.close() is a no-op. Wrap it so close() + // actually closes the sink, like a real HTTP write. + val sink = object : ForwardingSink(buffer) {}.buffer() + body.writeTo(sink) + sink.close() + assertThat(buffer.readUtf8()).isEqualTo(expected) + } + @Test fun gzippedParts() { val body = From 6075f070cd02fd654cea574617994e4f7fee62ac Mon Sep 17 00:00:00 2001 From: Vamsi Vaddavalli Date: Fri, 14 Aug 2026 01:00:05 -0500 Subject: [PATCH 3/3] Extract NoCloseSink and simplify the multipart tests --- .../kotlin/okhttp3/MultipartBody.kt | 14 ++++--- .../kotlin/okhttp3/RequestBody.kt | 2 +- .../kotlin/okhttp3/MultipartBodyTest.kt | 39 ++++--------------- 3 files changed, 17 insertions(+), 38 deletions(-) diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/MultipartBody.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/MultipartBody.kt index db06b5c8ece8..abd8cc48f8ca 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/MultipartBody.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/MultipartBody.kt @@ -24,6 +24,7 @@ import okio.BufferedSink import okio.ByteString import okio.ByteString.Companion.encodeUtf8 import okio.ForwardingSink +import okio.Sink import okio.buffer /** @@ -162,12 +163,7 @@ class MultipartBody internal constructor( if (countBytes) { byteCount += contentLength } else { - object : ForwardingSink(sink) { - override fun close() { - // GzipSink.close() writes the gzip footer, then closes whatever it wraps. - // Swallow that close so we can write the trailing CRLF and later parts. - } - }.buffer().use(body::writeTo) + NoCloseSink(sink).buffer().use(body::writeTo) } sink.write(CRLF) @@ -384,4 +380,10 @@ class MultipartBody internal constructor( append('"') } } + + private class NoCloseSink( + sink: Sink, + ) : ForwardingSink(sink) { + override fun close() {} + } } diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/RequestBody.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/RequestBody.kt index 3dca9c725d65..22d4995fbf3b 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/RequestBody.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/RequestBody.kt @@ -41,7 +41,7 @@ abstract class RequestBody { @Throws(IOException::class) open fun contentLength(): Long = -1L - /** Writes the content of this request to [sink]. This must not close [sink]. */ + /** Writes the content of this request to [sink]. This should not close [sink]. */ @Throws(IOException::class) abstract fun writeTo(sink: BufferedSink) diff --git a/okhttp/src/jvmTest/kotlin/okhttp3/MultipartBodyTest.kt b/okhttp/src/jvmTest/kotlin/okhttp3/MultipartBodyTest.kt index 19a9bcdb395a..2df3855b96bb 100644 --- a/okhttp/src/jvmTest/kotlin/okhttp3/MultipartBodyTest.kt +++ b/okhttp/src/jvmTest/kotlin/okhttp3/MultipartBodyTest.kt @@ -17,7 +17,6 @@ package okhttp3 import assertk.assertThat import assertk.assertions.isEqualTo -import assertk.assertions.isNull import java.io.IOException import java.nio.charset.StandardCharsets import kotlin.test.assertFailsWith @@ -29,7 +28,6 @@ import okhttp3.internal.http.GzipRequestBody import okio.Buffer import okio.BufferedSink import okio.ForwardingSink -import okio.GzipSource import okio.buffer import okio.utf8Size import org.junit.jupiter.api.Test @@ -290,12 +288,6 @@ class MultipartBodyTest { .addPart(ClosingBody("hello")) .addPart("world".toRequestBody(null)) .build() - assertThat(body.boundary).isEqualTo("123") - assertThat(body.type).isEqualTo(MultipartBody.MIXED) - assertThat(body.contentType().toString()) - .isEqualTo("multipart/mixed; boundary=123") - assertThat(body.parts.size).isEqualTo(2) - assertThat(body.contentLength()).isEqualTo(-1) val buffer = Buffer() // Don't write to Buffer directly: Buffer.close() is a no-op. Wrap it so close() // actually closes the sink, like a real HTTP write. @@ -313,12 +305,13 @@ class MultipartBodyTest { .addPart(GzipRequestBody("part1".toRequestBody(null))) .addPart(GzipRequestBody("part2".toRequestBody(null))) .build() - assertThat(body.boundary).isEqualTo("123") - assertThat(body.type).isEqualTo(MultipartBody.MIXED) - assertThat(body.contentType().toString()) - .isEqualTo("multipart/mixed; boundary=123") - assertThat(body.parts.size).isEqualTo(2) - assertThat(body.contentLength()).isEqualTo(-1) + + val expected = Buffer() + expected.writeUtf8("--123\r\n\r\n") + GzipRequestBody("part1".toRequestBody(null)).writeTo(expected) + expected.writeUtf8("\r\n--123\r\n\r\n") + GzipRequestBody("part2".toRequestBody(null)).writeTo(expected) + expected.writeUtf8("\r\n--123--\r\n") val buffer = Buffer() // Don't write to Buffer directly: Buffer.close() is a no-op. Wrap it so close() @@ -326,23 +319,7 @@ class MultipartBodyTest { val sink = object : ForwardingSink(buffer) {}.buffer() body.writeTo(sink) sink.close() - - MultipartReader(boundary = "123", source = buffer).use { reader -> - val part1 = reader.nextPart()!! - val part1Body = - GzipSource(part1.body).use { - it.buffer().readUtf8() - } - assertThat(part1Body).isEqualTo("part1") - - val part2 = reader.nextPart()!! - val part2Body = - GzipSource(part2.body).use { - it.buffer().readUtf8() - } - assertThat(part2Body).isEqualTo("part2") - assertThat(reader.nextPart()).isNull() - } + assertThat(buffer.readUtf8()).isEqualTo(expected.readUtf8()) } @Test