Skip to content

Commit d205721

Browse files
authored
fix: Sync-flush gzip so flushed stream data reaches the client (#134)
GzipCoding.encode returned a GZIPOutputStream without syncFlush, so flush() only flushed the underlying socket and left everything written so far inside the deflater. A streamed response (e.g. application/x-ndjson) that flushed an event delivered only the 10-byte gzip header to a client sending Accept-Encoding: gzip, until the deflater's buffer filled or the stream closed. Progressive and long-poll streams stalled. Create the GZIPOutputStream with syncFlush so a flush writes everything so far in a decodable form. Whole bodies are coded without a flush, so they pay nothing for it. Document the flush contract on ContentCoding.encode and in the README. Add GzipCodingTest covering flush output and a closed-stream round trip, and a GzipIT test that a flushed event of a gzipped stream reaches the client before the stream ends.
1 parent 3ecefdb commit d205721

5 files changed

Lines changed: 106 additions & 2 deletions

File tree

‎README.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,8 @@ shared caches keep the two forms apart. It is merged into any `Vary` your handle
523523
A handler that sets its own `Content-Encoding` is left alone, and so is a payload the coding
524524
fails to shrink. Statuses that carry no content never get a coding.
525525

526-
Streamed responses (`Response.stream(...)`) are coded as they are written. A length declared by
526+
Streamed responses (`Response.stream(...)`) are coded as they are written, and a handler's
527+
`flush()` sends the client everything written so far, coded. A length declared by
527528
the sized overload describes the uncoded body, so a coded stream goes out chunked; a stream of
528529
unknown length is coded regardless of the threshold, since measuring it would defeat streaming it.
529530
For the same reason a `HEAD` whose `GET` would be compressed omits `Content-Length` rather than

‎src/main/java/com/retailsvc/http/ContentCoding.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ default Set<String> aliases() {
4949
/**
5050
* Wraps a response stream so that everything written to the returned stream reaches {@code sink}
5151
* coded. Closing the returned stream must finish the coded payload and close {@code sink}.
52+
*
53+
* <p>Flushing the returned stream must write everything written so far to {@code sink} in a form
54+
* the client can already decode, then flush {@code sink}. A coder that holds it back stalls
55+
* streaming responses. For deflate-based codings this means a sync flush, which compressors
56+
* usually leave off by default.
5257
*/
5358
OutputStream encode(OutputStream sink) throws IOException;
5459
}

‎src/main/java/com/retailsvc/http/internal/GzipCoding.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,12 @@ public InputStream decode(InputStream coded) throws IOException {
2929
return new GZIPInputStream(coded, BUFFER_SIZE);
3030
}
3131

32+
/**
33+
* Sync-flushes, so a flushed streamed response reaches the client then rather than when the
34+
* deflater's buffer fills. Whole bodies are coded without a flush, so they pay nothing for it.
35+
*/
3236
@Override
3337
public OutputStream encode(OutputStream sink) throws IOException {
34-
return new GZIPOutputStream(sink);
38+
return new GZIPOutputStream(sink, true);
3539
}
3640
}

‎src/test/java/com/retailsvc/http/GzipIT.java‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import static java.net.HttpURLConnection.HTTP_UNSUPPORTED_TYPE;
77
import static java.nio.charset.StandardCharsets.UTF_8;
88
import static org.assertj.core.api.Assertions.assertThat;
9+
import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
910

1011
import com.retailsvc.http.start.TextEchoHandler;
1112
import java.io.ByteArrayInputStream;
@@ -17,7 +18,10 @@
1718
import java.net.http.HttpRequest.BodyPublishers;
1819
import java.net.http.HttpResponse;
1920
import java.net.http.HttpResponse.BodyHandlers;
21+
import java.time.Duration;
2022
import java.util.Map;
23+
import java.util.concurrent.CountDownLatch;
24+
import java.util.concurrent.TimeUnit;
2125
import java.util.function.UnaryOperator;
2226
import java.util.zip.GZIPInputStream;
2327
import java.util.zip.GZIPOutputStream;
@@ -240,6 +244,52 @@ void headKeepsContentLengthWithoutAcceptEncoding() throws Exception {
240244
}
241245
}
242246

247+
@Test
248+
void flushedEventOfGzippedStreamReachesClientBeforeStreamEnds() throws Exception {
249+
byte[] first = "{\"event\":1}\n".getBytes(UTF_8);
250+
byte[] second = "{\"event\":2}\n".getBytes(UTF_8);
251+
CountDownLatch clientSawFirst = new CountDownLatch(1);
252+
RequestHandler events =
253+
req ->
254+
Response.stream(
255+
HTTP_OK,
256+
"application/x-ndjson",
257+
out -> {
258+
out.write(first);
259+
out.flush();
260+
try {
261+
clientSawFirst.await(10, TimeUnit.SECONDS);
262+
} catch (InterruptedException e) {
263+
Thread.currentThread().interrupt();
264+
throw new IOException(e);
265+
}
266+
out.write(second);
267+
});
268+
try (var s = serverWith(Map.of(), builder -> builder.extraRoute("/events", events));
269+
var client = httpClient()) {
270+
var request =
271+
HttpRequest.newBuilder()
272+
.uri(URI.create("http://localhost:%d/events".formatted(s.listenPort())))
273+
.header(ACCEPT_ENCODING, "gzip")
274+
.GET()
275+
.build();
276+
277+
HttpResponse<InputStream> response = client.send(request, BodyHandlers.ofInputStream());
278+
279+
assertThat(response.headers().firstValue(CONTENT_ENCODING)).contains("gzip");
280+
try (var in = new GZIPInputStream(response.body())) {
281+
try {
282+
byte[] got =
283+
assertTimeoutPreemptively(Duration.ofSeconds(5), () -> in.readNBytes(first.length));
284+
assertThat(got).isEqualTo(first);
285+
} finally {
286+
clientSawFirst.countDown();
287+
}
288+
assertThat(in.readAllBytes()).isEqualTo(second);
289+
}
290+
}
291+
}
292+
243293
// -- fixtures --
244294

245295
private OpenApiServer echoServer() {
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
package com.retailsvc.http.internal;
2+
3+
import static java.nio.charset.StandardCharsets.UTF_8;
4+
import static org.assertj.core.api.Assertions.assertThat;
5+
6+
import java.io.ByteArrayInputStream;
7+
import java.io.ByteArrayOutputStream;
8+
import java.io.IOException;
9+
import java.util.zip.GZIPInputStream;
10+
import org.junit.jupiter.api.Test;
11+
12+
class GzipCodingTest {
13+
14+
private final GzipCoding coding = new GzipCoding();
15+
16+
@Test
17+
void flushEmitsEverythingWrittenSoFarInDecodableForm() throws IOException {
18+
byte[] event = "{\"event\":1}\n".getBytes(UTF_8);
19+
var sink = new ByteArrayOutputStream();
20+
21+
var out = coding.encode(sink);
22+
out.write(event);
23+
out.flush();
24+
25+
try (var in = new GZIPInputStream(new ByteArrayInputStream(sink.toByteArray()))) {
26+
assertThat(in.readNBytes(event.length)).isEqualTo(event);
27+
}
28+
}
29+
30+
@Test
31+
void closedStreamRoundTrips() throws IOException {
32+
byte[] body = "{\"event\":1}\n{\"event\":2}\n".getBytes(UTF_8);
33+
var sink = new ByteArrayOutputStream();
34+
35+
try (var out = coding.encode(sink)) {
36+
out.write(body);
37+
out.flush();
38+
}
39+
40+
try (var in = coding.decode(new ByteArrayInputStream(sink.toByteArray()))) {
41+
assertThat(in.readAllBytes()).isEqualTo(body);
42+
}
43+
}
44+
}

0 commit comments

Comments
 (0)