FrankChen021 commented on code in PR #19454:
URL: https://github.com/apache/druid/pull/19454#discussion_r3226689056
##########
server/src/main/java/org/apache/druid/server/QueryResultPusher.java:
##########
@@ -385,6 +387,36 @@ public interface Writer extends Closeable
void writeResponseEnd() throws IOException;
}
+ /**
+ * Picks the best compression encoding the client accepts. Prefers zstd over
x-lz4.
+ * Returns null if no supported encoding is advertised.
+ */
+ @Nullable
+ private static String negotiateContentEncoding(@Nullable String
acceptEncoding)
+ {
+ if (acceptEncoding == null) {
+ return null;
+ }
+ if (acceptEncoding.contains("zstd")) {
Review Comment:
[P2] Honor q=0 when negotiating encodings
This substring check treats headers like `gzip, zstd;q=0` or `x-lz4;q=0` as
accepting the encoding, then sends a compressed response that the client
explicitly marked unacceptable. QueryResultPusher handles user-facing query
responses too, so this can break standards-compliant clients. Parse
Accept-Encoding as tokens with q-values and only select zstd/x-lz4 when q is
absent or greater than zero.
##########
server/src/main/java/org/apache/druid/client/DirectDruidClient.java:
##########
@@ -516,7 +521,7 @@ public JsonParserIterator<T> make()
{
return new JsonParserIterator<>(
queryResultType,
- future,
+ wrapFutureWithDecompressor(future,
responseContentEncoding.get()),
Review Comment:
[P1] Decompressor choice races the response headers
The decompressor is selected from responseContentEncoding.get() when the
Sequence iterator is created, which can happen immediately after run() returns
and before the async HTTP handler has seen the response headers. In that common
path this passes an empty encoding and returns the original future, so a later
zstd/x-lz4 response is handed to JsonParserIterator still compressed and
parsing fails. Defer the encoding lookup until the future completes, e.g.
always transform with a function that reads the AtomicReference after
handleResponse has set it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
