github-advanced-security[bot] commented on code in PR #19567:
URL: https://github.com/apache/druid/pull/19567#discussion_r3399842005
##########
processing/src/test/java/org/apache/druid/java/util/http/client/FriendlyServersTest.java:
##########
@@ -384,11 +389,120 @@
StatusResponseHandler.getInstance()
).get().getStatus();
- Assert.assertEquals(200, status.getCode());
+ Assert.assertEquals(200, status.code());
}
}
finally {
lifecycle.stop();
}
}
+
+ /**
+ * Regression test for the case where the same {@link Request} (or a copy of
it) is sent more
+ * than once. Netty 4's HttpObjectEncoder advances the content's reader
index and releases
+ * {@link io.netty.handler.codec.http.FullHttpRequest} after encoding; if
{@link NettyHttpClient}
+ * handed the Request's stored {@link io.netty.buffer.ByteBuf} directly to
the encoder, the
+ * second send would either drain to zero bytes or hit an {@link
io.netty.util.IllegalReferenceCountException}.
+ *
+ * KerberosHttpClient hits this path: on a 401 it calls {@code
Request.copy()} (which deep-copies
+ * the content) and re-sends. This test exercises the same shape.
+ */
+ @Test
+ public void testRequestCanBeSentMultipleTimes() throws Exception
+ {
+ final ExecutorService exec = Executors.newCachedThreadPool();
+ final ServerSocket serverSocket = new ServerSocket(0);
+ final BlockingQueue<String> receivedBodies = new LinkedBlockingQueue<>();
+ // Keep-alive HTTP/1.1 loop: each accepted connection serves consecutive
requests until the
+ // peer disconnects. Needed because the client's channel pool reuses
connections, and a
+ // single-shot server would cause a write-to-closed-channel race on the
second send.
+ exec.submit(
+ () -> {
+ while (!Thread.currentThread().isInterrupted()) {
+ final Socket clientSocket;
+ try {
+ clientSocket = serverSocket.accept();
+ }
+ catch (IOException e) {
+ return;
+ }
+ exec.submit(() -> {
+ try (
+ Socket s = clientSocket;
+ BufferedReader in = new BufferedReader(
+ new InputStreamReader(s.getInputStream(),
StandardCharsets.UTF_8)
+ );
+ OutputStream out = s.getOutputStream()
+ ) {
+ while (true) {
+ String requestLine = in.readLine();
+ if (requestLine == null) {
+ return;
+ }
+ int contentLength = 0;
+ String header;
+ while (!(header = in.readLine()).equals("")) {
Review Comment:
## CodeQL / Inefficient empty string test
Inefficient comparison to empty string, check for zero length instead.
[Show more
details](https://github.com/apache/druid/security/code-scanning/11304)
##########
processing/src/test/java/org/apache/druid/java/util/http/client/FriendlyServersTest.java:
##########
@@ -384,11 +389,120 @@
StatusResponseHandler.getInstance()
).get().getStatus();
- Assert.assertEquals(200, status.getCode());
+ Assert.assertEquals(200, status.code());
}
}
finally {
lifecycle.stop();
}
}
+
+ /**
+ * Regression test for the case where the same {@link Request} (or a copy of
it) is sent more
+ * than once. Netty 4's HttpObjectEncoder advances the content's reader
index and releases
+ * {@link io.netty.handler.codec.http.FullHttpRequest} after encoding; if
{@link NettyHttpClient}
+ * handed the Request's stored {@link io.netty.buffer.ByteBuf} directly to
the encoder, the
+ * second send would either drain to zero bytes or hit an {@link
io.netty.util.IllegalReferenceCountException}.
+ *
+ * KerberosHttpClient hits this path: on a 401 it calls {@code
Request.copy()} (which deep-copies
+ * the content) and re-sends. This test exercises the same shape.
+ */
+ @Test
+ public void testRequestCanBeSentMultipleTimes() throws Exception
+ {
+ final ExecutorService exec = Executors.newCachedThreadPool();
+ final ServerSocket serverSocket = new ServerSocket(0);
+ final BlockingQueue<String> receivedBodies = new LinkedBlockingQueue<>();
+ // Keep-alive HTTP/1.1 loop: each accepted connection serves consecutive
requests until the
+ // peer disconnects. Needed because the client's channel pool reuses
connections, and a
+ // single-shot server would cause a write-to-closed-channel race on the
second send.
+ exec.submit(
+ () -> {
+ while (!Thread.currentThread().isInterrupted()) {
+ final Socket clientSocket;
+ try {
+ clientSocket = serverSocket.accept();
+ }
+ catch (IOException e) {
+ return;
+ }
+ exec.submit(() -> {
+ try (
+ Socket s = clientSocket;
+ BufferedReader in = new BufferedReader(
+ new InputStreamReader(s.getInputStream(),
StandardCharsets.UTF_8)
+ );
+ OutputStream out = s.getOutputStream()
+ ) {
+ while (true) {
+ String requestLine = in.readLine();
+ if (requestLine == null) {
+ return;
+ }
+ int contentLength = 0;
+ String header;
+ while (!(header = in.readLine()).equals("")) {
+ if
(header.toLowerCase(Locale.ROOT).startsWith("content-length:")) {
+ contentLength =
Integer.parseInt(header.substring("content-length:".length()).trim());
+ }
+ }
+ char[] body = new char[contentLength];
+ int read = 0;
+ while (read < contentLength) {
+ int n = in.read(body, read, contentLength - read);
+ if (n < 0) {
+ return;
+ }
+ read += n;
+ }
+ receivedBodies.put(new String(body, 0, read));
+ out.write("HTTP/1.1 200 OK\r\nContent-Length:
2\r\n\r\nok".getBytes(StandardCharsets.UTF_8));
+ out.flush();
+ }
+ }
+ catch (Exception ignored) {
+ // suppress
+ }
+ });
+ }
+ }
+ );
+
+ final Lifecycle lifecycle = new Lifecycle();
+ try {
+ final HttpClient client =
HttpClientInit.createClient(HttpClientConfig.builder().build(), lifecycle);
+ final URL url = new URL(StringUtils.format("http://localhost:%d/";,
serverSocket.getLocalPort()));
Review Comment:
## CodeQL / Deprecated method or constructor invocation
Invoking [URL.URL](1) should be avoided because it has been deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/11303)
--
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]
