github-advanced-security[bot] commented on code in PR #20475:
URL: https://github.com/apache/druid/pull/20475#discussion_r4165228366
##########
processing/src/test/java/org/apache/druid/java/util/http/client/io/AppendableByteArrayInputStreamTest.java:
##########
@@ -211,6 +211,55 @@
Assertions.assertEquals(-1, in.read());
}
+ @Test
+ public void testAvailableSaturatesInsteadOfOverflowing() throws Exception
+ {
+ final AppendableByteArrayInputStream in = new
AppendableByteArrayInputStream();
+
+ // The same array is queued repeatedly: the stream retains references, so
this accounts for 2 GiB of queued
+ // bytes while allocating only one chunk.
+ final byte[] oneMebibyte = new byte[1024 * 1024];
+ for (int i = 0; i < 2048; i++) {
+ in.add(oneMebibyte);
+ }
+
+ Assertions.assertEquals(Integer.MAX_VALUE, in.available());
+
+ in.read(new byte[oneMebibyte.length]);
+
+ Assertions.assertEquals((2048L - 1) * oneMebibyte.length, (long)
in.available());
+ }
+
+ @Test
+ public void testExceptionCaughtReleasesQueuedBytes() throws IOException
+ {
+ final AppendableByteArrayInputStream in = new
AppendableByteArrayInputStream();
+ in.add(new byte[10]);
+ in.add(new byte[8192]);
+ Assertions.assertEquals(5, in.read(new byte[5]));
+
+ in.exceptionCaught(new IOException("connection reset"));
+
+ Assertions.assertEquals(0, in.available());
+ Assertions.assertEquals(5, in.read(new byte[5]), "the chunk being read is
still handed out");
+ Assertions.assertEquals(0, in.available());
+ Assertions.assertThrows(IOException.class, () -> in.read(new byte[8192]));
Review Comment:
## CodeQL / Ignored error status of call
Method execute ignores exceptional return value of InputStream.read.
[Show more
details](https://github.com/apache/druid/security/code-scanning/12044)
##########
processing/src/test/java/org/apache/druid/java/util/http/client/FriendlyServersTest.java:
##########
@@ -753,4 +760,198 @@
lifecycle.stop();
}
}
+
+ /**
+ * An abort that arrives once the response is complete must not close the
channel: it is back in the pool, and the
+ * next request may already be on it.
+ */
+ @Test
+ public void testAbortAfterCompletionLeavesThePooledChannelOpen() throws
Exception
+ {
+ final ExecutorService exec = Executors.newSingleThreadExecutor();
+ final ServerSocket serverSocket = new ServerSocket(0);
+ final BlockingQueue<String> wire = new LinkedBlockingQueue<>();
+ serveChunkedFromWire(exec, serverSocket, wire);
+
+ final Lifecycle lifecycle = new Lifecycle();
+ try {
+ final HttpClient client = HttpClientInit.createClient(
+ HttpClientConfig.builder().withNumConnections(1).build(),
+ lifecycle
+ );
+ final Request request = new Request(
+ HttpMethod.GET,
+ 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/12045)
##########
processing/src/test/java/org/apache/druid/java/util/http/client/FriendlyServersTest.java:
##########
@@ -753,4 +760,198 @@
lifecycle.stop();
}
}
+
+ /**
+ * An abort that arrives once the response is complete must not close the
channel: it is back in the pool, and the
+ * next request may already be on it.
+ */
+ @Test
+ public void testAbortAfterCompletionLeavesThePooledChannelOpen() throws
Exception
+ {
+ final ExecutorService exec = Executors.newSingleThreadExecutor();
+ final ServerSocket serverSocket = new ServerSocket(0);
+ final BlockingQueue<String> wire = new LinkedBlockingQueue<>();
+ serveChunkedFromWire(exec, serverSocket, wire);
+
+ final Lifecycle lifecycle = new Lifecycle();
+ try {
+ final HttpClient client = HttpClientInit.createClient(
+ HttpClientConfig.builder().withNumConnections(1).build(),
+ lifecycle
+ );
+ final Request request = new Request(
+ HttpMethod.GET,
+ new URL(StringUtils.format("http://localhost:%d/";,
serverSocket.getLocalPort()))
+ );
+
+ final CapturingTrafficCop<StringFullResponseHolder,
StringFullResponseHolder> first =
+ new CapturingTrafficCop<>(new
StringFullResponseHandler(StandardCharsets.UTF_8));
+ final ListenableFuture<StringFullResponseHolder> firstResponse =
client.go(request, first);
+ wire.put(CHUNKED_OK + chunk("a") + LAST_CHUNK);
+ Assertions.assertEquals("a", firstResponse.get(30,
TimeUnit.SECONDS).getContent());
+
+ first.trafficCop.get().abort();
+
+ // The server answers on its only connection, so the second request
succeeds only if it reuses the first's.
+ final ListenableFuture<StringFullResponseHolder> secondResponse =
+ client.go(request, new
StringFullResponseHandler(StandardCharsets.UTF_8));
+ wire.put(CHUNKED_OK + chunk("b") + LAST_CHUNK);
+ Assertions.assertEquals("b", secondResponse.get(30,
TimeUnit.SECONDS).getContent());
+ }
+ finally {
+ exec.shutdownNow();
+ serverSocket.close();
+ lifecycle.stop();
+ }
+ }
+
+ /**
+ * A resume that arrives once the response is complete must not touch the
channel: it is back in the pool, and the
+ * next request on it may have suspended reads.
+ */
+ @Test
+ public void testResumeAfterCompletionLeavesTheNextRequestSuspended() throws
Exception
+ {
+ final ExecutorService exec = Executors.newSingleThreadExecutor();
+ final ServerSocket serverSocket = new ServerSocket(0);
+ final BlockingQueue<String> wire = new LinkedBlockingQueue<>();
+ serveChunkedFromWire(exec, serverSocket, wire);
+
+ final Lifecycle lifecycle = new Lifecycle();
+ try {
+ final HttpClient client = HttpClientInit.createClient(
+ HttpClientConfig.builder().withNumConnections(1).build(),
+ lifecycle
+ );
+ final Request request = new Request(
+ HttpMethod.GET,
+ 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/12046)
##########
processing/src/test/java/org/apache/druid/java/util/http/client/io/AppendableByteArrayInputStreamTest.java:
##########
@@ -211,6 +211,55 @@
Assertions.assertEquals(-1, in.read());
}
+ @Test
+ public void testAvailableSaturatesInsteadOfOverflowing() throws Exception
+ {
+ final AppendableByteArrayInputStream in = new
AppendableByteArrayInputStream();
+
+ // The same array is queued repeatedly: the stream retains references, so
this accounts for 2 GiB of queued
+ // bytes while allocating only one chunk.
+ final byte[] oneMebibyte = new byte[1024 * 1024];
+ for (int i = 0; i < 2048; i++) {
+ in.add(oneMebibyte);
+ }
+
+ Assertions.assertEquals(Integer.MAX_VALUE, in.available());
+
+ in.read(new byte[oneMebibyte.length]);
Review Comment:
## CodeQL / Ignored error status of call
Method testAvailableSaturatesInsteadOfOverflowing ignores exceptional return
value of InputStream.read.
[Show more
details](https://github.com/apache/druid/security/code-scanning/12043)
--
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]
