Skip to content

fix(druid): sniff HTML error body before JSON parse (second leg) #20152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
shoemoney wants to merge 3 commits into from
Closed

fix(druid): sniff HTML error body before JSON parse (second leg) #20152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
72e0a0e
fix(druid): sniff HTML error body before JSON parse (second leg)
shoemoney Aug 25, 2026
a166687
fix(druid): propagate handler exceptions through the future, restrict…
shoemoney Aug 27, 2026
8b737a3
fix(druid): also restrict 429 capacity mapping to HTML bodies
shoemoney Sep 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion processing/src/main/java/org/apache/druid/java/util/http/client/NettyHttpClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,12 @@ public void abort()
catch (Exception ex) {
log.warn(ex, "[%s] Exception thrown while processing message, closing channel.", requestDesc);

// Complete the future with the exception itself rather than null: a handler (e.g. handleResponse)
// may throw a specific, meaningful exception (query capacity exceeded, interrupted, etc.) and
// completing with null discards it, leaving callers with a successful-looking null result instead
// of the real failure.
if (!retVal.isDone()) {
retVal.set(null);
retVal.setException(ex);
}
channel.close();
channelResourceContainer.returnResource();
Expand Down
66 changes: 66 additions & 0 deletions server/src/main/java/org/apache/druid/client/DirectDruidClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.apache.druid.java.util.http.client.response.StatusResponseHolder;
import org.apache.druid.query.Queries;
import org.apache.druid.query.Query;
import org.apache.druid.query.QueryCapacityExceededException;
import org.apache.druid.query.QueryContext;
import org.apache.druid.query.QueryMetrics;
import org.apache.druid.query.QueryPlus;
Expand Down Expand Up @@ -243,6 +244,71 @@ public ClientResponse<InputStream> handleResponse(HttpResponse response, Traffic
{
trafficCopRef.set(trafficCop);
checkQueryTimeout();
// Handle 429/503 HTML before JSON parse to avoid JsonParseException 0x3c ('<')
final int statusCode = response.getStatus().getCode();
final String contentType = response.headers().get(HttpHeaders.Names.CONTENT_TYPE);
final ChannelBuffer contentBuffer = response.getContent();
boolean isHtmlContentType = contentType != null && StringUtils.toLowerCase(contentType).contains("text/html");
boolean isHtmlBody = false;
if (contentBuffer.readableBytes() > 0) {

@FrankChen021 FrankChen021 Aug 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Chunked HTML can bypass prefix sniffing

handleResponse inspects only the initial response buffer, but later chunked bytes reach handleChunk without persistent classification. A chunked 503 HTML response whose first buffer is empty or lacks the identifying prefix can therefore reach JsonParserIterator and fail as a JSON parse error instead of producing the intended capacity error. Carry the sniff state across chunks or buffer and classify the prefix before handing the stream to the parser.

@FrankChen021 FrankChen021 Sep 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Chunked HTML responses can bypass classification

When a chunked response has an empty initial buffer, this check leaves isHtmlBody false. Subsequent handleChunk calls only enqueue bytes and do not retain or inspect the response prefix, so a later chunk beginning with HTML is fed to JsonParserIterator and becomes a generic JSON parse/interruption error. Persist the classification across chunks or buffer the prefix before enqueueing, and add an empty-initial-chunk test.

int readerIndex = contentBuffer.readerIndex();
int readable = contentBuffer.readableBytes();
for (int i = 0; i < readable; i++) {
byte b = contentBuffer.getByte(readerIndex + i);
if (b == ' ' || b == '\n' || b == '\r' || b == '\t') {
continue;
}
if (b == '<') {
isHtmlBody = true;
} else if (b != '{' && b != '[') {
// Not JSON start, but only treat '<' as HTML indicator
}
break;
}
}
// A 429/503 is only treated as capacity-exceeded here when the body is confirmed HTML/non-JSON; a
// 429/503 carrying a proper JSON error body (e.g. a genuine QueryCapacityExceededException from a data
// server) falls through to the normal JSON error handling below, which preserves the real error message.
if ((statusCode == 429 || statusCode == 503) && (isHtmlContentType || isHtmlBody)) {
String msg = StringUtils.format(
"Query[%s] url[%s] failed with status[%s] [%s]",
query.getId(),
url,
statusCode,
response.getStatus().getReasonPhrase()
);
if (contentBuffer.readableBytes() > 0) {
int len = Math.min(contentBuffer.readableBytes(), 512);
byte[] previewBytes = new byte[len];
contentBuffer.getBytes(contentBuffer.readerIndex(), previewBytes);
String preview = StringUtils.fromUtf8(previewBytes);
preview = preview.substring(0, Math.min(preview.length(), 256));
msg = StringUtils.format("%s: %s", msg, preview);
}
throw QueryCapacityExceededException.withErrorMessageAndResolvedHost(msg);

@FrankChen021 FrankChen021 Aug 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Handler exceptions complete the Netty future with null

With production NettyHttpClient, this throw occurs before the handler response is assigned. Netty then completes the future successfully with null and closes the channel, so JsonParserIterator treats the result as a possible scatter-gather limit and raises ResourceLimitExceededException. The intended capacity/interruption error is therefore lost. Propagate the exception through the future or return an error-bearing response, and test with the real Netty client.

}
if (isHtmlContentType || isHtmlBody) {
int len = Math.min(contentBuffer.readableBytes(), 512);
byte[] previewBytes = new byte[len];
if (len > 0) {
contentBuffer.getBytes(contentBuffer.readerIndex(), previewBytes);
}
String preview = len > 0 ? StringUtils.fromUtf8(previewBytes) : "";
preview = preview.substring(0, Math.min(preview.length(), 256));
throw new org.apache.druid.query.QueryInterruptedException(
org.apache.druid.query.QueryException.UNKNOWN_EXCEPTION_ERROR_CODE,
StringUtils.format(
"Query[%s] url[%s] returned HTML response instead of JSON with status[%s] contentType[%s] preview[%s]",
query.getId(),
url,
statusCode,
contentType,
preview
),
org.apache.druid.query.QueryInterruptedException.class.getName(),
host
);
}
checkTotalBytesLimit(response.getContent().readableBytes());

log.debug("Initial response from url[%s] for queryId[%s]", url, query.getId());
Expand Down
Loading