FrankChen021 commented on code in PR #19567:
URL: https://github.com/apache/druid/pull/19567#discussion_r3499362509
##########
processing/src/main/java/org/apache/druid/java/util/http/client/NettyHttpClient.java:
##########
@@ -131,29 +123,32 @@ public <Intermediate, Final> ListenableFuture<Final> go(
return Futures.immediateFailedFuture(
new ChannelException(
"Faulty channel in resource pool",
- channelFuture.getCause()
+ channelFuture.cause()
)
);
} else {
- channel = channelFuture.getChannel();
+ channel = channelFuture.channel();
- // In case we get a channel that never had its readability turned back
on.
- channel.setReadable(true);
+ // In case we get a channel that never had its reads turned back on.
+ channel.config().setAutoRead(true);
}
final String urlFile =
StringUtils.nullToEmptyNonDruidDataString(url.getFile());
- final HttpRequest httpRequest = new DefaultHttpRequest(
+ // retainedDuplicate so the encoder's read+release doesn't disturb the
Request's stored
+ // ByteBuf, preserving it for callers that resend or copy() the Request.
+ final FullHttpRequest httpRequest = new DefaultFullHttpRequest(
HttpVersion.HTTP_1_1,
method,
- urlFile.isEmpty() ? "/" : urlFile
+ urlFile.isEmpty() ? "/" : urlFile,
+ request.hasContent() ? request.getContent().retainedDuplicate() :
Unpooled.EMPTY_BUFFER
);
- if (!headers.containsKey(HttpHeaders.Names.HOST)) {
- httpRequest.headers().add(HttpHeaders.Names.HOST, getHost(url));
+ if (!headers.containsKey(HttpHeaderNames.HOST.toString())) {
Review Comment:
[P2] Check default headers case-insensitively
Request stores headers in a case-sensitive Multimap, but the migration now
checks only the lower-case Netty 4 names before adding defaults. A caller that
already set "Host" or "Accept-Encoding" using conventional HTTP casing is not
detected here, so the client adds a default and then copies the caller header
later, producing duplicate Host headers or multiple Accept-Encoding values.
This regresses the old Netty 3 behavior for callers overriding those headers
and can break virtual-host/proxy requests. Please make these checks
case-insensitive or normalize Request header names before adding defaults.
--
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]
