github-advanced-security[bot] commented on code in PR #18891:
URL: https://github.com/apache/druid/pull/18891#discussion_r2675095472
##########
extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3DataSegmentPullerTest.java:
##########
@@ -335,48 +323,53 @@
outputStream.write(value);
}
- ListObjectsV2Result list = EasyMock.createMock(ListObjectsV2Result.class);
- S3ObjectSummary objectSummary1 =
EasyMock.createMock(S3ObjectSummary.class);
-
EasyMock.expect(objectSummary1.getBucketName()).andReturn(bucket).anyTimes();
- EasyMock.expect(objectSummary1.getKey()).andReturn(keyPrefix +
"meta.smoosh").anyTimes();
- S3ObjectSummary objectSummary2 =
EasyMock.createMock(S3ObjectSummary.class);
-
EasyMock.expect(objectSummary2.getBucketName()).andReturn(bucket).anyTimes();
- EasyMock.expect(objectSummary2.getKey()).andReturn(keyPrefix +
"00000.smoosh").anyTimes();
-
EasyMock.expect(list.getObjectSummaries()).andReturn(List.of(objectSummary1,
objectSummary2)).once();
-
EasyMock.expect(s3Client.listObjectsV2(EasyMock.anyObject())).andReturn(list).once();
-
- final S3Object object1 = new S3Object();
- object1.setBucketName(bucket);
- object1.setKey(keyPrefix + "meta.smoosh");
- object1.getObjectMetadata().setLastModified(new Date(0));
- object1.setObjectContent(new FileInputStream(tmpFile));
-
- final S3Object object2 = new S3Object();
- object2.setBucketName(bucket);
- object2.setKey(keyPrefix + "00000.smoosh");
- object2.getObjectMetadata().setLastModified(new Date(0));
- object2.setObjectContent(new FileInputStream(tmpFile));
+ S3Object objectSummary1 = S3Object.builder()
+ .key(keyPrefix + "meta.smoosh")
+ .build();
+ S3Object objectSummary2 = S3Object.builder()
+ .key(keyPrefix + "00000.smoosh")
+ .build();
+
+ ListObjectsV2Response listResponse = ListObjectsV2Response.builder()
+ .contents(List.of(objectSummary1, objectSummary2))
+ .build();
+
EasyMock.expect(s3Client.listObjectsV2(EasyMock.anyObject())).andReturn(listResponse).once();
+
+ final GetObjectResponse getObjectResponse1 = GetObjectResponse.builder()
+ .lastModified(Instant.ofEpochMilli(0))
+ .build();
+ final ResponseInputStream<GetObjectResponse> responseInputStream1 = new
ResponseInputStream<>(
+ getObjectResponse1,
+ AbortableInputStream.create(new FileInputStream(tmpFile))
Review Comment:
## Potential input resource leak
This FileInputStream is not always closed on method exit.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10641)
##########
extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3DataSegmentPullerTest.java:
##########
@@ -335,48 +323,53 @@
outputStream.write(value);
}
- ListObjectsV2Result list = EasyMock.createMock(ListObjectsV2Result.class);
- S3ObjectSummary objectSummary1 =
EasyMock.createMock(S3ObjectSummary.class);
-
EasyMock.expect(objectSummary1.getBucketName()).andReturn(bucket).anyTimes();
- EasyMock.expect(objectSummary1.getKey()).andReturn(keyPrefix +
"meta.smoosh").anyTimes();
- S3ObjectSummary objectSummary2 =
EasyMock.createMock(S3ObjectSummary.class);
-
EasyMock.expect(objectSummary2.getBucketName()).andReturn(bucket).anyTimes();
- EasyMock.expect(objectSummary2.getKey()).andReturn(keyPrefix +
"00000.smoosh").anyTimes();
-
EasyMock.expect(list.getObjectSummaries()).andReturn(List.of(objectSummary1,
objectSummary2)).once();
-
EasyMock.expect(s3Client.listObjectsV2(EasyMock.anyObject())).andReturn(list).once();
-
- final S3Object object1 = new S3Object();
- object1.setBucketName(bucket);
- object1.setKey(keyPrefix + "meta.smoosh");
- object1.getObjectMetadata().setLastModified(new Date(0));
- object1.setObjectContent(new FileInputStream(tmpFile));
-
- final S3Object object2 = new S3Object();
- object2.setBucketName(bucket);
- object2.setKey(keyPrefix + "00000.smoosh");
- object2.getObjectMetadata().setLastModified(new Date(0));
- object2.setObjectContent(new FileInputStream(tmpFile));
+ S3Object objectSummary1 = S3Object.builder()
+ .key(keyPrefix + "meta.smoosh")
+ .build();
+ S3Object objectSummary2 = S3Object.builder()
+ .key(keyPrefix + "00000.smoosh")
+ .build();
+
+ ListObjectsV2Response listResponse = ListObjectsV2Response.builder()
+ .contents(List.of(objectSummary1, objectSummary2))
+ .build();
+
EasyMock.expect(s3Client.listObjectsV2(EasyMock.anyObject())).andReturn(listResponse).once();
+
+ final GetObjectResponse getObjectResponse1 = GetObjectResponse.builder()
+ .lastModified(Instant.ofEpochMilli(0))
+ .build();
+ final ResponseInputStream<GetObjectResponse> responseInputStream1 = new
ResponseInputStream<>(
+ getObjectResponse1,
+ AbortableInputStream.create(new FileInputStream(tmpFile))
+ );
+ final GetObjectResponse getObjectResponse2 = GetObjectResponse.builder()
+ .lastModified(Instant.ofEpochMilli(0))
+ .build();
+ final ResponseInputStream<GetObjectResponse> responseInputStream2 = new
ResponseInputStream<>(
+ getObjectResponse2,
+ AbortableInputStream.create(new FileInputStream(tmpFile2))
Review Comment:
## Potential input resource leak
This FileInputStream is not always closed on method exit.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10643)
##########
extensions-core/s3-extensions/src/test/java/org/apache/druid/data/input/s3/S3InputSourceTest.java:
##########
@@ -1402,13 +1406,15 @@
final String s3Bucket = uri.getAuthority();
final String key = S3Utils.extractS3Key(uri);
- S3Object someObject = new S3Object();
- someObject.setBucketName(s3Bucket);
- someObject.setKey(key);
ByteArrayOutputStream gzipped = new ByteArrayOutputStream();
CompressionUtils.gzip(new ByteArrayInputStream(CONTENT), gzipped);
- someObject.setObjectContent(new
ByteArrayInputStream(gzipped.toByteArray()));
-
EasyMock.expect(S3_CLIENT.getObject(EasyMock.anyObject(GetObjectRequest.class))).andReturn(someObject).once();
+
+ GetObjectResponse response = GetObjectResponse.builder().build();
+ ResponseInputStream<GetObjectResponse> responseInputStream = new
ResponseInputStream<>(
+ response,
+ AbortableInputStream.create(new
ByteArrayInputStream(gzipped.toByteArray()))
Review Comment:
## Potential input resource leak
This ResponseInputStream<GetObjectResponse> is not always closed on method
exit.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10646)
##########
extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3DataSegmentPullerTest.java:
##########
@@ -204,43 +196,40 @@
outputStream.write(value);
}
- S3Object object0 = new S3Object();
-
- object0.setBucketName(bucket);
- object0.setKey(keyPrefix + "/renames-0.gz");
- object0.getObjectMetadata().setLastModified(new Date(0));
- object0.setObjectContent(new FileInputStream(tmpFile));
-
- final S3ObjectSummary objectSummary = new S3ObjectSummary();
- objectSummary.setBucketName(bucket);
- objectSummary.setKey(keyPrefix + "/renames-0.gz");
- objectSummary.setLastModified(new Date(0));
-
- final ListObjectsV2Result listObjectsResult = new ListObjectsV2Result();
- listObjectsResult.setKeyCount(1);
- listObjectsResult.getObjectSummaries().add(objectSummary);
+ final GetObjectResponse getObjectResponse = GetObjectResponse.builder()
+ .lastModified(Instant.ofEpochMilli(0))
+ .build();
+ final ResponseInputStream<GetObjectResponse> responseInputStream = new
ResponseInputStream<>(
+ getObjectResponse,
+ AbortableInputStream.create(new FileInputStream(tmpFile))
+ );
Review Comment:
## Potential input resource leak
This ResponseInputStream<GetObjectResponse> is not always closed on method
exit.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10636)
##########
extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3DataSegmentPullerTest.java:
##########
@@ -265,18 +255,20 @@
outputStream.write(value);
}
- final S3Object object0 = new S3Object();
- object0.setBucketName(bucket);
- object0.setKey(keyPrefix + "/test-object");
- object0.getObjectMetadata().setLastModified(new Date(0));
- object0.setObjectContent(new FileInputStream(tmpFile));
+ final GetObjectResponse getObjectResponse = GetObjectResponse.builder()
+ .lastModified(Instant.ofEpochMilli(0))
+ .build();
+ final ResponseInputStream<GetObjectResponse> responseInputStream = new
ResponseInputStream<>(
+ getObjectResponse,
+ AbortableInputStream.create(new FileInputStream(tmpFile))
+ );
Review Comment:
## Potential input resource leak
This ResponseInputStream<GetObjectResponse> is not always closed on method
exit.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10638)
##########
extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/ServerSideEncryptingAmazonS3Test.java:
##########
@@ -19,130 +19,391 @@
package org.apache.druid.storage.s3;
-import com.amazonaws.services.s3.AmazonS3;
-import com.amazonaws.services.s3.model.PutObjectRequest;
-import com.amazonaws.services.s3.model.PutObjectResult;
-import com.amazonaws.services.s3.transfer.TransferManager;
-import com.amazonaws.services.s3.transfer.Upload;
+import org.easymock.Capture;
import org.easymock.EasyMock;
import org.junit.Assert;
import org.junit.Before;
+import org.junit.Rule;
import org.junit.Test;
-
-
+import org.junit.rules.TemporaryFolder;
+import software.amazon.awssdk.core.sync.RequestBody;
+import software.amazon.awssdk.services.s3.S3AsyncClient;
+import software.amazon.awssdk.services.s3.S3Client;
+import software.amazon.awssdk.services.s3.model.Grant;
+import software.amazon.awssdk.services.s3.model.Grantee;
+import software.amazon.awssdk.services.s3.model.PutObjectRequest;
+import software.amazon.awssdk.services.s3.model.PutObjectResponse;
+import software.amazon.awssdk.transfer.s3.S3TransferManager;
+import software.amazon.awssdk.transfer.s3.model.CompletedFileUpload;
+import software.amazon.awssdk.transfer.s3.model.FileUpload;
+import software.amazon.awssdk.transfer.s3.model.UploadFileRequest;
+
+import java.io.File;
+import java.io.IOException;
import java.lang.reflect.Field;
-
+import java.util.concurrent.CompletableFuture;
public class ServerSideEncryptingAmazonS3Test
{
- private AmazonS3 mockAmazonS3;
+ @Rule
+ public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+ private S3Client mockS3Client;
private ServerSideEncryption mockServerSideEncryption;
private S3TransferConfig mockTransferConfig;
- private TransferManager mockTransferManager;
@Before
public void setup()
{
- mockAmazonS3 = EasyMock.createMock(AmazonS3.class);
+ mockS3Client = EasyMock.createMock(S3Client.class);
mockServerSideEncryption = EasyMock.createMock(ServerSideEncryption.class);
mockTransferConfig = EasyMock.createMock(S3TransferConfig.class);
- mockTransferManager = EasyMock.createMock(TransferManager.class);
}
@Test
- public void testConstructor_WithTransferManager() throws
NoSuchFieldException, IllegalAccessException
+ public void testConstructor_TransferManagerNullWithoutAsyncClient() throws
NoSuchFieldException, IllegalAccessException
{
- EasyMock.expect(mockTransferConfig.isUseTransferManager()).andReturn(true);
-
EasyMock.expect(mockTransferConfig.getMinimumUploadPartSize()).andReturn(5L);
-
EasyMock.expect(mockTransferConfig.getMultipartUploadThreshold()).andReturn(10L);
+ // When no async client is provided, transferManager should be null
+ // isUseTransferManager is called in constructor, so we need to set up
expectation
+
EasyMock.expect(mockTransferConfig.isUseTransferManager()).andReturn(true).anyTimes();
EasyMock.replay(mockTransferConfig);
- ServerSideEncryptingAmazonS3 s3 = new
ServerSideEncryptingAmazonS3(mockAmazonS3, mockServerSideEncryption,
mockTransferConfig);
+ ServerSideEncryptingAmazonS3 s3 = new ServerSideEncryptingAmazonS3(
+ mockS3Client,
+ null, // no async client
+ mockServerSideEncryption,
+ mockTransferConfig
+ );
Field transferManagerField =
ServerSideEncryptingAmazonS3.class.getDeclaredField("transferManager");
transferManagerField.setAccessible(true);
Object transferManager = transferManagerField.get(s3);
- Assert.assertNotNull("TransferManager should be initialized",
transferManager);
+ Assert.assertNull("TransferManager should be null when no async client
provided", transferManager);
Assert.assertNotNull(s3);
- EasyMock.verify(mockTransferConfig);
+ Assert.assertEquals(mockS3Client, s3.getS3Client());
+ }
+
+ @Test
+ public void testUpload() throws IOException
+ {
+ File testFile = temporaryFolder.newFile("test-upload.txt");
+
+ PutObjectResponse mockResponse = PutObjectResponse.builder().build();
+
+ // Set up transfer config to return false for useTransferManager
+
EasyMock.expect(mockTransferConfig.isUseTransferManager()).andReturn(false).anyTimes();
+ EasyMock.replay(mockTransferConfig);
+
+ // decorate method takes a Builder and returns a Builder
+
EasyMock.expect(mockServerSideEncryption.decorate(EasyMock.anyObject(PutObjectRequest.Builder.class)))
+ .andAnswer(() -> (PutObjectRequest.Builder)
EasyMock.getCurrentArguments()[0]);
+ EasyMock.replay(mockServerSideEncryption);
+
+ // The actual call is putObject(PutObjectRequest, RequestBody)
+
EasyMock.expect(mockS3Client.putObject(EasyMock.anyObject(PutObjectRequest.class),
EasyMock.anyObject(RequestBody.class)))
+ .andReturn(mockResponse).once();
+ EasyMock.replay(mockS3Client);
+
+ ServerSideEncryptingAmazonS3 s3 = new ServerSideEncryptingAmazonS3(
+ mockS3Client,
+ null,
+ mockServerSideEncryption,
+ mockTransferConfig
+ );
+ s3.upload("bucket", "key", testFile, null);
+
+ EasyMock.verify(mockServerSideEncryption);
+ EasyMock.verify(mockS3Client);
+ }
+
+ @Test
+ public void testUpload_WithGrantFullControlHeaderFormatted() throws
IOException
+ {
+ File testFile = temporaryFolder.newFile("test-upload-acl.txt");
+
+ PutObjectResponse mockResponse = PutObjectResponse.builder().build();
+
+ // Set up transfer config to return false for useTransferManager
+
EasyMock.expect(mockTransferConfig.isUseTransferManager()).andReturn(false).anyTimes();
Review Comment:
## Unread local variable
Variable 'File testFile' is never read.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10654)
##########
extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3DataSegmentPullerTest.java:
##########
@@ -335,48 +323,53 @@
outputStream.write(value);
}
- ListObjectsV2Result list = EasyMock.createMock(ListObjectsV2Result.class);
- S3ObjectSummary objectSummary1 =
EasyMock.createMock(S3ObjectSummary.class);
-
EasyMock.expect(objectSummary1.getBucketName()).andReturn(bucket).anyTimes();
- EasyMock.expect(objectSummary1.getKey()).andReturn(keyPrefix +
"meta.smoosh").anyTimes();
- S3ObjectSummary objectSummary2 =
EasyMock.createMock(S3ObjectSummary.class);
-
EasyMock.expect(objectSummary2.getBucketName()).andReturn(bucket).anyTimes();
- EasyMock.expect(objectSummary2.getKey()).andReturn(keyPrefix +
"00000.smoosh").anyTimes();
-
EasyMock.expect(list.getObjectSummaries()).andReturn(List.of(objectSummary1,
objectSummary2)).once();
-
EasyMock.expect(s3Client.listObjectsV2(EasyMock.anyObject())).andReturn(list).once();
-
- final S3Object object1 = new S3Object();
- object1.setBucketName(bucket);
- object1.setKey(keyPrefix + "meta.smoosh");
- object1.getObjectMetadata().setLastModified(new Date(0));
- object1.setObjectContent(new FileInputStream(tmpFile));
-
- final S3Object object2 = new S3Object();
- object2.setBucketName(bucket);
- object2.setKey(keyPrefix + "00000.smoosh");
- object2.getObjectMetadata().setLastModified(new Date(0));
- object2.setObjectContent(new FileInputStream(tmpFile));
+ S3Object objectSummary1 = S3Object.builder()
+ .key(keyPrefix + "meta.smoosh")
+ .build();
+ S3Object objectSummary2 = S3Object.builder()
+ .key(keyPrefix + "00000.smoosh")
+ .build();
+
+ ListObjectsV2Response listResponse = ListObjectsV2Response.builder()
+ .contents(List.of(objectSummary1, objectSummary2))
+ .build();
+
EasyMock.expect(s3Client.listObjectsV2(EasyMock.anyObject())).andReturn(listResponse).once();
+
+ final GetObjectResponse getObjectResponse1 = GetObjectResponse.builder()
+ .lastModified(Instant.ofEpochMilli(0))
+ .build();
+ final ResponseInputStream<GetObjectResponse> responseInputStream1 = new
ResponseInputStream<>(
+ getObjectResponse1,
+ AbortableInputStream.create(new FileInputStream(tmpFile))
+ );
Review Comment:
## Potential input resource leak
This ResponseInputStream<GetObjectResponse> is not always closed on method
exit.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10640)
##########
extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentMover.java:
##########
@@ -166,15 +167,15 @@
selfCheckingMove(s3Bucket, targetS3Bucket, s3Path, targetS3Path,
copyMsg);
return null;
}
- catch (AmazonServiceException | IOException |
SegmentLoadingException e) {
+ catch (S3Exception | IOException | SegmentLoadingException e) {
log.info(e, "Error while trying to move " + copyMsg);
throw e;
}
}
);
}
catch (Exception e) {
- Throwables.propagateIfInstanceOf(e, AmazonServiceException.class);
+ Throwables.propagateIfInstanceOf(e, S3Exception.class);
Review Comment:
## Deprecated method or constructor invocation
Invoking [Throwables.propagateIfInstanceOf](1) should be avoided because it
has been deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10655)
##########
extensions-core/s3-extensions/src/test/java/org/apache/druid/data/input/s3/S3InputSourceTest.java:
##########
@@ -1379,21 +1383,21 @@
final String s3Bucket = uri.getAuthority();
final String key = S3Utils.extractS3Key(uri);
- S3ObjectInputStream someInputStream =
EasyMock.createMock(S3ObjectInputStream.class);
- EasyMock.expect(someInputStream.read(EasyMock.anyObject(),
EasyMock.anyInt(), EasyMock.anyInt()))
- .andThrow(new SdkClientException("Data read has a different length
than the expected")).anyTimes();
- someInputStream.close();
+ InputStream mockInputStream = EasyMock.createMock(InputStream.class);
+ EasyMock.expect(mockInputStream.read(EasyMock.anyObject(),
EasyMock.anyInt(), EasyMock.anyInt()))
+ .andThrow(SdkClientException.builder().message("Data read has a
different length than the expected").build()).anyTimes();
+ mockInputStream.close();
expectLastCall().andVoid().anyTimes();
- S3Object someObject = EasyMock.createMock(S3Object.class);
- EasyMock.expect(someObject.getBucketName()).andReturn(s3Bucket).anyTimes();
- EasyMock.expect(someObject.getKey()).andReturn(key).anyTimes();
-
EasyMock.expect(someObject.getObjectContent()).andReturn(someInputStream).anyTimes();
+ GetObjectResponse response = GetObjectResponse.builder().build();
+ ResponseInputStream<GetObjectResponse> responseInputStream = new
ResponseInputStream<>(
+ response,
+ AbortableInputStream.create(mockInputStream)
Review Comment:
## Potential input resource leak
This ResponseInputStream<GetObjectResponse> is not always closed on method
exit.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10645)
##########
extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3DataSegmentPullerTest.java:
##########
@@ -335,48 +323,53 @@
outputStream.write(value);
}
- ListObjectsV2Result list = EasyMock.createMock(ListObjectsV2Result.class);
- S3ObjectSummary objectSummary1 =
EasyMock.createMock(S3ObjectSummary.class);
-
EasyMock.expect(objectSummary1.getBucketName()).andReturn(bucket).anyTimes();
- EasyMock.expect(objectSummary1.getKey()).andReturn(keyPrefix +
"meta.smoosh").anyTimes();
- S3ObjectSummary objectSummary2 =
EasyMock.createMock(S3ObjectSummary.class);
-
EasyMock.expect(objectSummary2.getBucketName()).andReturn(bucket).anyTimes();
- EasyMock.expect(objectSummary2.getKey()).andReturn(keyPrefix +
"00000.smoosh").anyTimes();
-
EasyMock.expect(list.getObjectSummaries()).andReturn(List.of(objectSummary1,
objectSummary2)).once();
-
EasyMock.expect(s3Client.listObjectsV2(EasyMock.anyObject())).andReturn(list).once();
-
- final S3Object object1 = new S3Object();
- object1.setBucketName(bucket);
- object1.setKey(keyPrefix + "meta.smoosh");
- object1.getObjectMetadata().setLastModified(new Date(0));
- object1.setObjectContent(new FileInputStream(tmpFile));
-
- final S3Object object2 = new S3Object();
- object2.setBucketName(bucket);
- object2.setKey(keyPrefix + "00000.smoosh");
- object2.getObjectMetadata().setLastModified(new Date(0));
- object2.setObjectContent(new FileInputStream(tmpFile));
+ S3Object objectSummary1 = S3Object.builder()
+ .key(keyPrefix + "meta.smoosh")
+ .build();
+ S3Object objectSummary2 = S3Object.builder()
+ .key(keyPrefix + "00000.smoosh")
+ .build();
+
+ ListObjectsV2Response listResponse = ListObjectsV2Response.builder()
+ .contents(List.of(objectSummary1, objectSummary2))
+ .build();
+
EasyMock.expect(s3Client.listObjectsV2(EasyMock.anyObject())).andReturn(listResponse).once();
+
+ final GetObjectResponse getObjectResponse1 = GetObjectResponse.builder()
+ .lastModified(Instant.ofEpochMilli(0))
+ .build();
+ final ResponseInputStream<GetObjectResponse> responseInputStream1 = new
ResponseInputStream<>(
+ getObjectResponse1,
+ AbortableInputStream.create(new FileInputStream(tmpFile))
+ );
+ final GetObjectResponse getObjectResponse2 = GetObjectResponse.builder()
+ .lastModified(Instant.ofEpochMilli(0))
+ .build();
+ final ResponseInputStream<GetObjectResponse> responseInputStream2 = new
ResponseInputStream<>(
+ getObjectResponse2,
+ AbortableInputStream.create(new FileInputStream(tmpFile2))
+ );
Review Comment:
## Potential input resource leak
This ResponseInputStream<GetObjectResponse> is not always closed on method
exit.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10642)
##########
extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3DataSegmentPullerTest.java:
##########
@@ -95,32 +99,29 @@
outputStream.write(value);
}
- final S3Object object0 = new S3Object();
- object0.setBucketName(bucket);
- object0.setKey(keyPrefix + "/renames-0.gz");
- object0.getObjectMetadata().setLastModified(new Date(0));
- object0.setObjectContent(new FileInputStream(tmpFile));
-
- final S3ObjectSummary objectSummary = new S3ObjectSummary();
- objectSummary.setBucketName(bucket);
- objectSummary.setKey(keyPrefix + "/renames-0.gz");
- objectSummary.setLastModified(new Date(0));
+ final GetObjectResponse getObjectResponse = GetObjectResponse.builder()
+ .lastModified(Instant.ofEpochMilli(0))
+ .build();
+ final ResponseInputStream<GetObjectResponse> responseInputStream = new
ResponseInputStream<>(
+ getObjectResponse,
+ AbortableInputStream.create(new FileInputStream(tmpFile))
+ );
Review Comment:
## Potential input resource leak
This ResponseInputStream<GetObjectResponse> is not always closed on method
exit.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10634)
##########
extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3DataSegmentPullerTest.java:
##########
@@ -95,32 +99,29 @@
outputStream.write(value);
}
- final S3Object object0 = new S3Object();
- object0.setBucketName(bucket);
- object0.setKey(keyPrefix + "/renames-0.gz");
- object0.getObjectMetadata().setLastModified(new Date(0));
- object0.setObjectContent(new FileInputStream(tmpFile));
-
- final S3ObjectSummary objectSummary = new S3ObjectSummary();
- objectSummary.setBucketName(bucket);
- objectSummary.setKey(keyPrefix + "/renames-0.gz");
- objectSummary.setLastModified(new Date(0));
+ final GetObjectResponse getObjectResponse = GetObjectResponse.builder()
+ .lastModified(Instant.ofEpochMilli(0))
+ .build();
+ final ResponseInputStream<GetObjectResponse> responseInputStream = new
ResponseInputStream<>(
+ getObjectResponse,
+ AbortableInputStream.create(new FileInputStream(tmpFile))
Review Comment:
## Potential input resource leak
This FileInputStream is not always closed on method exit.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10635)
##########
extensions-core/s3-extensions/src/test/java/org/apache/druid/data/input/s3/S3InputSourceTest.java:
##########
@@ -1365,11 +1368,12 @@
final String s3Bucket = uri.getAuthority();
final String key = S3Utils.extractS3Key(uri);
- S3Object someObject = new S3Object();
- someObject.setBucketName(s3Bucket);
- someObject.setKey(key);
- someObject.setObjectContent(new ByteArrayInputStream(CONTENT));
-
EasyMock.expect(S3_CLIENT.getObject(EasyMock.anyObject(GetObjectRequest.class))).andReturn(someObject).once();
+ GetObjectResponse response = GetObjectResponse.builder().build();
+ ResponseInputStream<GetObjectResponse> responseInputStream = new
ResponseInputStream<>(
+ response,
+ AbortableInputStream.create(new ByteArrayInputStream(CONTENT))
Review Comment:
## Potential input resource leak
This ResponseInputStream<GetObjectResponse> is not always closed on method
exit.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10644)
##########
extensions-core/kinesis-indexing-service/src/test/java/org/apache/druid/indexing/kinesis/KinesisRecordSupplierTest.java:
##########
@@ -320,50 +320,59 @@
@Test
public void testPollWithKinesisInternalFailure() throws InterruptedException
{
- EasyMock.expect(kinesis.getShardIterator(
- EasyMock.anyObject(),
- EasyMock.eq(SHARD_ID0),
- EasyMock.anyString(),
- EasyMock.anyString()
- )).andReturn(
- getShardIteratorResult0).anyTimes();
-
- EasyMock.expect(kinesis.getShardIterator(
- EasyMock.anyObject(),
- EasyMock.eq(SHARD_ID1),
- EasyMock.anyString(),
- EasyMock.anyString()
- )).andReturn(
- getShardIteratorResult1).anyTimes();
-
-
EasyMock.expect(getShardIteratorResult0.getShardIterator()).andReturn(SHARD0_ITERATOR).anyTimes();
-
EasyMock.expect(getShardIteratorResult1.getShardIterator()).andReturn(SHARD1_ITERATOR).anyTimes();
- EasyMock.expect(kinesis.getRecords(generateGetRecordsReq(SHARD0_ITERATOR)))
- .andReturn(getRecordsResult0)
- .anyTimes();
- EasyMock.expect(kinesis.getRecords(generateGetRecordsReq(SHARD1_ITERATOR)))
- .andReturn(getRecordsResult1)
- .anyTimes();
- AmazonServiceException getException = new
AmazonServiceException("InternalFailure");
- getException.setErrorCode("InternalFailure");
- getException.setStatusCode(500);
- getException.setServiceName("AmazonKinesis");
-
EasyMock.expect(getRecordsResult0.getRecords()).andThrow(getException).once();
-
EasyMock.expect(getRecordsResult0.getRecords()).andReturn(SHARD0_RECORDS).once();
- AmazonServiceException getException2 = new
AmazonServiceException("InternalFailure");
- getException2.setErrorCode("InternalFailure");
- getException2.setStatusCode(503);
- getException2.setServiceName("AmazonKinesis");
-
EasyMock.expect(getRecordsResult1.getRecords()).andThrow(getException2).once();
-
EasyMock.expect(getRecordsResult1.getRecords()).andReturn(SHARD1_RECORDS).once();
-
EasyMock.expect(getRecordsResult0.getNextShardIterator()).andReturn(null).anyTimes();
-
EasyMock.expect(getRecordsResult1.getNextShardIterator()).andReturn(null).anyTimes();
-
EasyMock.expect(getRecordsResult0.getMillisBehindLatest()).andReturn(SHARD0_LAG_MILLIS).once();
-
EasyMock.expect(getRecordsResult0.getMillisBehindLatest()).andReturn(SHARD0_LAG_MILLIS).once();
-
EasyMock.expect(getRecordsResult1.getMillisBehindLatest()).andReturn(SHARD1_LAG_MILLIS).once();
-
EasyMock.expect(getRecordsResult1.getMillisBehindLatest()).andReturn(SHARD1_LAG_MILLIS).once();
-
- replayAll();
+ // Setup get shard iterator responses
+ GetShardIteratorResponse getShardIteratorResult0 =
GetShardIteratorResponse.builder()
+ .shardIterator(SHARD0_ITERATOR)
+ .build();
+ GetShardIteratorResponse getShardIteratorResult1 =
GetShardIteratorResponse.builder()
+ .shardIterator(SHARD1_ITERATOR)
+ .build();
+
+
EasyMock.expect(kinesis.getShardIterator(EasyMock.anyObject(GetShardIteratorRequest.class)))
+ .andAnswer(() -> {
+ GetShardIteratorRequest req = (GetShardIteratorRequest)
EasyMock.getCurrentArguments()[0];
+ if (SHARD_ID0.equals(req.shardId())) {
+ return getShardIteratorResult0;
+ } else {
+ return getShardIteratorResult1;
+ }
+ }).anyTimes();
+
+ // Setup get records responses - first call throws exception, second
succeeds
+ GetRecordsResponse getRecordsResult0Success = GetRecordsResponse.builder()
+ .records(SHARD0_RECORDS_V2)
+ .nextShardIterator(null)
+ .millisBehindLatest(SHARD0_LAG_MILLIS)
+ .build();
+
+ GetRecordsResponse getRecordsResult1Success = GetRecordsResponse.builder()
+ .records(SHARD1_RECORDS_V2)
+ .nextShardIterator(null)
+ .millisBehindLatest(SHARD1_LAG_MILLIS)
+ .build();
+
+ KinesisException getException = (KinesisException)
KinesisException.builder()
+ .message("InternalFailure")
+ .statusCode(500)
+ .build();
+
+ KinesisException getException2 = (KinesisException)
KinesisException.builder()
+ .message("InternalFailure")
+ .statusCode(503)
Review Comment:
## Unread local variable
Variable 'KinesisException getException2' is never read.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10652)
##########
extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3DataSegmentPullerTest.java:
##########
@@ -204,43 +196,40 @@
outputStream.write(value);
}
- S3Object object0 = new S3Object();
-
- object0.setBucketName(bucket);
- object0.setKey(keyPrefix + "/renames-0.gz");
- object0.getObjectMetadata().setLastModified(new Date(0));
- object0.setObjectContent(new FileInputStream(tmpFile));
-
- final S3ObjectSummary objectSummary = new S3ObjectSummary();
- objectSummary.setBucketName(bucket);
- objectSummary.setKey(keyPrefix + "/renames-0.gz");
- objectSummary.setLastModified(new Date(0));
-
- final ListObjectsV2Result listObjectsResult = new ListObjectsV2Result();
- listObjectsResult.setKeyCount(1);
- listObjectsResult.getObjectSummaries().add(objectSummary);
+ final GetObjectResponse getObjectResponse = GetObjectResponse.builder()
+ .lastModified(Instant.ofEpochMilli(0))
+ .build();
+ final ResponseInputStream<GetObjectResponse> responseInputStream = new
ResponseInputStream<>(
+ getObjectResponse,
+ AbortableInputStream.create(new FileInputStream(tmpFile))
Review Comment:
## Potential input resource leak
This FileInputStream is not always closed on method exit.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10637)
##########
extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3TestUtils.java:
##########
@@ -164,17 +168,28 @@
return null;
}
- public static S3ObjectSummary newS3ObjectSummary(
+ public static S3Object newS3Object(
+ String bucket,
Review Comment:
## Useless parameter
The parameter 'bucket' is never used.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10647)
##########
extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3DataSegmentPullerTest.java:
##########
@@ -265,18 +255,20 @@
outputStream.write(value);
}
- final S3Object object0 = new S3Object();
- object0.setBucketName(bucket);
- object0.setKey(keyPrefix + "/test-object");
- object0.getObjectMetadata().setLastModified(new Date(0));
- object0.setObjectContent(new FileInputStream(tmpFile));
+ final GetObjectResponse getObjectResponse = GetObjectResponse.builder()
+ .lastModified(Instant.ofEpochMilli(0))
+ .build();
+ final ResponseInputStream<GetObjectResponse> responseInputStream = new
ResponseInputStream<>(
+ getObjectResponse,
+ AbortableInputStream.create(new FileInputStream(tmpFile))
Review Comment:
## Potential input resource leak
This FileInputStream is not always closed on method exit.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10639)
##########
extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/output/S3StorageConnector.java:
##########
@@ -138,13 +141,20 @@
}
@Override
- public InputStream open(GetObjectRequest object, long offset)
+ public InputStream open(GetObjectRequest.Builder object, long offset)
{
- if (object.getRange() != null) {
- long[] oldRange = object.getRange();
- object.setRange(oldRange[0] + offset, oldRange[1]);
+ // For SDK v2, we need to modify the range. Since builders are mutable,
+ // we need to handle the offset by rebuilding the range.
+ // Get the current request to inspect its range
+ GetObjectRequest currentRequest = object.build();
+ String currentRange = currentRequest.range();
+ if (currentRange != null && currentRange.startsWith("bytes=")) {
+ String[] parts = currentRange.substring(6).split("-");
+ long oldStart = Long.parseLong(parts[0]);
+ long oldEnd = parts.length > 1 && !parts[1].isEmpty() ?
Long.parseLong(parts[1]) : Long.MAX_VALUE;
Review Comment:
## Missing catch of NumberFormatException
Potential uncaught 'java.lang.NumberFormatException'.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10650)
##########
extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/output/S3StorageConnector.java:
##########
@@ -138,13 +141,20 @@
}
@Override
- public InputStream open(GetObjectRequest object, long offset)
+ public InputStream open(GetObjectRequest.Builder object, long offset)
{
- if (object.getRange() != null) {
- long[] oldRange = object.getRange();
- object.setRange(oldRange[0] + offset, oldRange[1]);
+ // For SDK v2, we need to modify the range. Since builders are mutable,
+ // we need to handle the offset by rebuilding the range.
+ // Get the current request to inspect its range
+ GetObjectRequest currentRequest = object.build();
+ String currentRange = currentRequest.range();
+ if (currentRange != null && currentRange.startsWith("bytes=")) {
+ String[] parts = currentRange.substring(6).split("-");
+ long oldStart = Long.parseLong(parts[0]);
Review Comment:
## Missing catch of NumberFormatException
Potential uncaught 'java.lang.NumberFormatException'.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10649)
##########
extensions-core/kinesis-indexing-service/src/test/java/org/apache/druid/indexing/kinesis/KinesisRecordSupplierTest.java:
##########
@@ -320,50 +320,59 @@
@Test
public void testPollWithKinesisInternalFailure() throws InterruptedException
{
- EasyMock.expect(kinesis.getShardIterator(
- EasyMock.anyObject(),
- EasyMock.eq(SHARD_ID0),
- EasyMock.anyString(),
- EasyMock.anyString()
- )).andReturn(
- getShardIteratorResult0).anyTimes();
-
- EasyMock.expect(kinesis.getShardIterator(
- EasyMock.anyObject(),
- EasyMock.eq(SHARD_ID1),
- EasyMock.anyString(),
- EasyMock.anyString()
- )).andReturn(
- getShardIteratorResult1).anyTimes();
-
-
EasyMock.expect(getShardIteratorResult0.getShardIterator()).andReturn(SHARD0_ITERATOR).anyTimes();
-
EasyMock.expect(getShardIteratorResult1.getShardIterator()).andReturn(SHARD1_ITERATOR).anyTimes();
- EasyMock.expect(kinesis.getRecords(generateGetRecordsReq(SHARD0_ITERATOR)))
- .andReturn(getRecordsResult0)
- .anyTimes();
- EasyMock.expect(kinesis.getRecords(generateGetRecordsReq(SHARD1_ITERATOR)))
- .andReturn(getRecordsResult1)
- .anyTimes();
- AmazonServiceException getException = new
AmazonServiceException("InternalFailure");
- getException.setErrorCode("InternalFailure");
- getException.setStatusCode(500);
- getException.setServiceName("AmazonKinesis");
-
EasyMock.expect(getRecordsResult0.getRecords()).andThrow(getException).once();
-
EasyMock.expect(getRecordsResult0.getRecords()).andReturn(SHARD0_RECORDS).once();
- AmazonServiceException getException2 = new
AmazonServiceException("InternalFailure");
- getException2.setErrorCode("InternalFailure");
- getException2.setStatusCode(503);
- getException2.setServiceName("AmazonKinesis");
-
EasyMock.expect(getRecordsResult1.getRecords()).andThrow(getException2).once();
-
EasyMock.expect(getRecordsResult1.getRecords()).andReturn(SHARD1_RECORDS).once();
-
EasyMock.expect(getRecordsResult0.getNextShardIterator()).andReturn(null).anyTimes();
-
EasyMock.expect(getRecordsResult1.getNextShardIterator()).andReturn(null).anyTimes();
-
EasyMock.expect(getRecordsResult0.getMillisBehindLatest()).andReturn(SHARD0_LAG_MILLIS).once();
-
EasyMock.expect(getRecordsResult0.getMillisBehindLatest()).andReturn(SHARD0_LAG_MILLIS).once();
-
EasyMock.expect(getRecordsResult1.getMillisBehindLatest()).andReturn(SHARD1_LAG_MILLIS).once();
-
EasyMock.expect(getRecordsResult1.getMillisBehindLatest()).andReturn(SHARD1_LAG_MILLIS).once();
-
- replayAll();
+ // Setup get shard iterator responses
+ GetShardIteratorResponse getShardIteratorResult0 =
GetShardIteratorResponse.builder()
+ .shardIterator(SHARD0_ITERATOR)
+ .build();
+ GetShardIteratorResponse getShardIteratorResult1 =
GetShardIteratorResponse.builder()
+ .shardIterator(SHARD1_ITERATOR)
+ .build();
+
+
EasyMock.expect(kinesis.getShardIterator(EasyMock.anyObject(GetShardIteratorRequest.class)))
+ .andAnswer(() -> {
+ GetShardIteratorRequest req = (GetShardIteratorRequest)
EasyMock.getCurrentArguments()[0];
+ if (SHARD_ID0.equals(req.shardId())) {
+ return getShardIteratorResult0;
+ } else {
+ return getShardIteratorResult1;
+ }
+ }).anyTimes();
+
+ // Setup get records responses - first call throws exception, second
succeeds
+ GetRecordsResponse getRecordsResult0Success = GetRecordsResponse.builder()
+ .records(SHARD0_RECORDS_V2)
+ .nextShardIterator(null)
+ .millisBehindLatest(SHARD0_LAG_MILLIS)
+ .build();
+
+ GetRecordsResponse getRecordsResult1Success = GetRecordsResponse.builder()
+ .records(SHARD1_RECORDS_V2)
+ .nextShardIterator(null)
+ .millisBehindLatest(SHARD1_LAG_MILLIS)
+ .build();
+
+ KinesisException getException = (KinesisException)
KinesisException.builder()
+ .message("InternalFailure")
+ .statusCode(500)
Review Comment:
## Unread local variable
Variable 'KinesisException getException' is never read.
[Show more
details](https://github.com/apache/druid/security/code-scanning/10651)
--
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]
