github-code-scanning[bot] commented on code in PR #12032:
URL: https://github.com/apache/druid/pull/12032#discussion_r1066442932
##########
core/src/main/java/org/apache/druid/java/util/http/client/response/SequenceInputStreamResponseHandler.java:
##########
@@ -56,18 +56,16 @@
@Override
public ClientResponse<InputStream> handleResponse(HttpResponse response,
TrafficCop trafficCop)
{
- try (ChannelBufferInputStream channelStream = new
ChannelBufferInputStream(response.getContent())) {
- queue.put(channelStream);
- }
- catch (IOException e) {
- throw new RuntimeException(e);
+ try {
+ // SequenceInputStream constructor blocks if the queue is empty, however
no content will be queued until
+ // the first chunk comes in. Adding an empty initial buffer to unblock.
+ queue.put(new ByteBufInputStream(Unpooled.EMPTY_BUFFER)); // lgtm
[java/input-resource-leak]
Review Comment:
## Potential input resource leak
This ByteBufInputStream is not always closed on method exit.
[Show more
details](https://github.com/apache/druid/security/code-scanning/3638)
##########
core/src/main/java/org/apache/druid/java/util/http/client/response/SequenceInputStreamResponseHandler.java:
##########
@@ -102,21 +100,19 @@
@Override
public ClientResponse<InputStream> handleChunk(
ClientResponse<InputStream> clientResponse,
- HttpChunk chunk,
+ HttpContent chunk,
long chunkNum
)
{
- final ChannelBuffer channelBuffer = chunk.getContent();
- final int bytes = channelBuffer.readableBytes();
+ final ByteBuf byteBuf = chunk.content();
+ final int bytes = byteBuf.readableBytes();
if (bytes > 0) {
- try (ChannelBufferInputStream channelStream = new
ChannelBufferInputStream(channelBuffer)) {
- queue.put(channelStream);
+ try {
+ // input streams will be closed by the consumer as we iterate through
them in SequenceInputStream
+ queue.put(new ByteBufInputStream(byteBuf.retain(), true)); // lgtm
[java/input-resource-leak]
Review Comment:
## Potential input resource leak
This ByteBufInputStream is not always closed on method exit.
[Show more
details](https://github.com/apache/druid/security/code-scanning/3639)
##########
core/src/test/java/org/apache/druid/java/util/http/client/FriendlyServersTest.java:
##########
@@ -123,35 +118,30 @@
final ExecutorService exec = Executors.newSingleThreadExecutor();
final ServerSocket serverSocket = new ServerSocket(0);
exec.submit(
- new Runnable()
- {
- @Override
- public void run()
- {
- while (!Thread.currentThread().isInterrupted()) {
- try (
- Socket clientSocket = serverSocket.accept();
- BufferedReader in = new BufferedReader(
- new InputStreamReader(clientSocket.getInputStream(),
StandardCharsets.UTF_8)
- );
- OutputStream out = clientSocket.getOutputStream()
- ) {
- StringBuilder request = new StringBuilder();
- String line;
- while (!"".equals((line = in.readLine()))) {
- request.append(line).append("\r\n");
- }
- requestContent.set(request.toString());
- out.write("HTTP/1.1 200
OK\r\n\r\n".getBytes(StandardCharsets.UTF_8));
-
- while (!in.readLine().equals("")) {
- // skip lines
- }
- out.write("HTTP/1.1 200 OK\r\nContent-Length:
6\r\n\r\nhello!".getBytes(StandardCharsets.UTF_8));
+ () -> {
+ while (!Thread.currentThread().isInterrupted()) {
+ try (
+ Socket clientSocket = serverSocket.accept();
+ BufferedReader in = new BufferedReader(
+ new InputStreamReader(clientSocket.getInputStream(),
StandardCharsets.UTF_8)
+ );
+ OutputStream out = clientSocket.getOutputStream()
+ ) {
+ StringBuilder request = new StringBuilder();
+ String line;
+ while (!"".equals((line = in.readLine()))) {
Review Comment:
## 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/3642)
##########
core/src/main/java/org/apache/druid/java/util/http/client/response/SequenceInputStreamResponseHandler.java:
##########
@@ -136,19 +132,14 @@
try {
// An empty byte array is put at the end to give the
SequenceInputStream.close() as something to close out
// after done is set to true, regardless of the rest of the stream's
state.
- queue.put(ByteSource.empty().openStream());
+ queue.put(new ByteBufInputStream(Unpooled.EMPTY_BUFFER, true)); //
lgtm [java/input-resource-leak]
Review Comment:
## Potential input resource leak
This ByteBufInputStream is not always closed on method exit.
[Show more
details](https://github.com/apache/druid/security/code-scanning/3640)
##########
core/src/test/java/org/apache/druid/java/util/http/client/FriendlyServersTest.java:
##########
@@ -253,6 +246,39 @@
}
}
+ @Nonnull
+ private AtomicReference<String> acceptEncodingServer(ExecutorService exec,
ServerSocket serverSocket)
+ {
+ final AtomicReference<String> foundAcceptEncoding = new
AtomicReference<>(null);
+
+ exec.submit(
+ () -> {
+ while (!Thread.currentThread().isInterrupted()) {
+ try (
+ Socket clientSocket = serverSocket.accept();
+ BufferedReader in = new BufferedReader(
+ new InputStreamReader(clientSocket.getInputStream(),
StandardCharsets.UTF_8)
+ );
+ OutputStream out = clientSocket.getOutputStream()
+ ) {
+ // Read headers
+ String header;
+ while (!(header = in.readLine()).equals("")) {
Review Comment:
## 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/3644)
##########
core/src/test/java/org/apache/druid/java/util/http/client/FriendlyServersTest.java:
##########
@@ -123,35 +118,30 @@
final ExecutorService exec = Executors.newSingleThreadExecutor();
final ServerSocket serverSocket = new ServerSocket(0);
exec.submit(
- new Runnable()
- {
- @Override
- public void run()
- {
- while (!Thread.currentThread().isInterrupted()) {
- try (
- Socket clientSocket = serverSocket.accept();
- BufferedReader in = new BufferedReader(
- new InputStreamReader(clientSocket.getInputStream(),
StandardCharsets.UTF_8)
- );
- OutputStream out = clientSocket.getOutputStream()
- ) {
- StringBuilder request = new StringBuilder();
- String line;
- while (!"".equals((line = in.readLine()))) {
- request.append(line).append("\r\n");
- }
- requestContent.set(request.toString());
- out.write("HTTP/1.1 200
OK\r\n\r\n".getBytes(StandardCharsets.UTF_8));
-
- while (!in.readLine().equals("")) {
- // skip lines
- }
- out.write("HTTP/1.1 200 OK\r\nContent-Length:
6\r\n\r\nhello!".getBytes(StandardCharsets.UTF_8));
+ () -> {
+ while (!Thread.currentThread().isInterrupted()) {
+ try (
+ Socket clientSocket = serverSocket.accept();
+ BufferedReader in = new BufferedReader(
+ new InputStreamReader(clientSocket.getInputStream(),
StandardCharsets.UTF_8)
+ );
+ OutputStream out = clientSocket.getOutputStream()
+ ) {
+ StringBuilder request = new StringBuilder();
+ String line;
+ while (!"".equals((line = in.readLine()))) {
+ request.append(line).append("\r\n");
}
- catch (Exception e) {
- Assert.fail(e.toString());
+ requestContent.set(request.toString());
+ out.write("HTTP/1.1 200
OK\r\n\r\n".getBytes(StandardCharsets.UTF_8));
+
+ while (!in.readLine().equals("")) {
Review Comment:
## 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/3643)
##########
core/src/test/java/org/apache/druid/java/util/http/client/FriendlyServersTest.java:
##########
@@ -66,27 +66,22 @@
final ExecutorService exec = Executors.newSingleThreadExecutor();
final ServerSocket serverSocket = new ServerSocket(0);
exec.submit(
- new Runnable()
- {
- @Override
- public void run()
- {
- while (!Thread.currentThread().isInterrupted()) {
- try (
- Socket clientSocket = serverSocket.accept();
- BufferedReader in = new BufferedReader(
- new InputStreamReader(clientSocket.getInputStream(),
StandardCharsets.UTF_8)
- );
- OutputStream out = clientSocket.getOutputStream()
- ) {
- while (!in.readLine().equals("")) {
- // skip lines
- }
- out.write("HTTP/1.1 200 OK\r\nContent-Length:
6\r\n\r\nhello!".getBytes(StandardCharsets.UTF_8));
- }
- catch (Exception e) {
- // Suppress
+ () -> {
+ while (!Thread.currentThread().isInterrupted()) {
+ try (
+ Socket clientSocket = serverSocket.accept();
+ BufferedReader in = new BufferedReader(
+ new InputStreamReader(clientSocket.getInputStream(),
StandardCharsets.UTF_8)
+ );
+ OutputStream out = clientSocket.getOutputStream()
+ ) {
+ while (!in.readLine().equals("")) {
Review Comment:
## 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/3641)
--
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]
