Skip to content

Commit 1a00d3e

Browse files
kessplaskessplas
authored
fix: unwrap CompletionException in default client, rethrow as S3Encry… (#162)
* fix: unwrap CompletionException in default client, rethrow as S3EncryptionClientException
1 parent 0fed900 commit 1a00d3e

File tree

2 files changed

+188
-25
lines changed

2 files changed

+188
-25
lines changed

‎src/main/java/software/amazon/encryption/s3/S3EncryptionClient.java

Lines changed: 64 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,15 @@ public PutObjectResponse putObject(PutObjectRequest putObjectRequest, RequestBod
187187
.secureRandom(_secureRandom)
188188
.build();
189189

190-
CompletableFuture<PutObjectResponse> futurePut = pipeline.putObject(putObjectRequest, AsyncRequestBody.fromInputStream(requestBody.contentStreamProvider().newStream(), requestBody.optionalContentLength().orElse(-1L), Executors.newSingleThreadExecutor()));
191-
return futurePut.join();
190+
try {
191+
CompletableFuture<PutObjectResponse> futurePut = pipeline.putObject(putObjectRequest, AsyncRequestBody.fromInputStream(requestBody.contentStreamProvider().newStream(), requestBody.optionalContentLength().orElse(-1L), Executors.newSingleThreadExecutor()));
192+
return futurePut.join();
193+
} catch (CompletionException completionException) {
194+
throw new S3EncryptionClientException(completionException.getMessage(), completionException.getCause());
195+
} catch (Exception exception) {
196+
throw new S3EncryptionClientException(exception.getMessage(), exception);
197+
}
198+
192199
}
193200

194201
/**
@@ -311,7 +318,7 @@ private CompleteMultipartUploadResponse multipartPutObject(PutObjectRequest requ
311318

312319
private <T extends Throwable> T onAbort(UploadObjectObserver observer, T t) {
313320
observer.onAbort();
314-
return t;
321+
throw new S3EncryptionClientException(t.getMessage(), t);
315322
}
316323

317324
/**
@@ -329,15 +336,23 @@ public DeleteObjectResponse deleteObject(DeleteObjectRequest deleteObjectRequest
329336
DeleteObjectRequest actualRequest = deleteObjectRequest.toBuilder()
330337
.overrideConfiguration(API_NAME_INTERCEPTOR)
331338
.build();
332-
// Delete the object
333-
DeleteObjectResponse deleteObjectResponse = _wrappedAsyncClient.deleteObject(actualRequest).join();
334-
// If Instruction file exists, delete the instruction file as well.
335-
String instructionObjectKey = deleteObjectRequest.key() + INSTRUCTION_FILE_SUFFIX;
336-
_wrappedAsyncClient.deleteObject(builder -> builder
337-
.overrideConfiguration(API_NAME_INTERCEPTOR)
338-
.bucket(deleteObjectRequest.bucket())
339-
.key(instructionObjectKey)).join();
340-
return deleteObjectResponse;
339+
340+
try {
341+
// Delete the object
342+
DeleteObjectResponse deleteObjectResponse = _wrappedAsyncClient.deleteObject(actualRequest).join();
343+
// If Instruction file exists, delete the instruction file as well.
344+
String instructionObjectKey = deleteObjectRequest.key() + INSTRUCTION_FILE_SUFFIX;
345+
_wrappedAsyncClient.deleteObject(builder -> builder
346+
.overrideConfiguration(API_NAME_INTERCEPTOR)
347+
.bucket(deleteObjectRequest.bucket())
348+
.key(instructionObjectKey)).join();
349+
// Return original deletion
350+
return deleteObjectResponse;
351+
} catch (CompletionException e) {
352+
throw new S3EncryptionClientException(e.getCause().getMessage(), e.getCause());
353+
} catch (Exception e) {
354+
throw new S3EncryptionClientException("Unable to delete object.", e);
355+
}
341356
}
342357

343358
/**
@@ -355,16 +370,22 @@ public DeleteObjectsResponse deleteObjects(DeleteObjectsRequest deleteObjectsReq
355370
DeleteObjectsRequest actualRequest = deleteObjectsRequest.toBuilder()
356371
.overrideConfiguration(API_NAME_INTERCEPTOR)
357372
.build();
358-
// Delete the objects
359-
DeleteObjectsResponse deleteObjectsResponse = _wrappedAsyncClient.deleteObjects(actualRequest).join();
360-
// If Instruction files exists, delete the instruction files as well.
361-
List<ObjectIdentifier> deleteObjects = instructionFileKeysToDelete(deleteObjectsRequest);
362-
_wrappedAsyncClient.deleteObjects(DeleteObjectsRequest.builder()
363-
.overrideConfiguration(API_NAME_INTERCEPTOR)
364-
.bucket(deleteObjectsRequest.bucket())
365-
.delete(builder -> builder.objects(deleteObjects))
366-
.build()).join();
367-
return deleteObjectsResponse;
373+
try {
374+
// Delete the objects
375+
DeleteObjectsResponse deleteObjectsResponse = _wrappedAsyncClient.deleteObjects(actualRequest).join();
376+
// If Instruction files exists, delete the instruction files as well.
377+
List<ObjectIdentifier> deleteObjects = instructionFileKeysToDelete(deleteObjectsRequest);
378+
_wrappedAsyncClient.deleteObjects(DeleteObjectsRequest.builder()
379+
.overrideConfiguration(API_NAME_INTERCEPTOR)
380+
.bucket(deleteObjectsRequest.bucket())
381+
.delete(builder -> builder.objects(deleteObjects))
382+
.build()).join();
383+
return deleteObjectsResponse;
384+
} catch (CompletionException e) {
385+
throw new S3EncryptionClientException(e.getCause().getMessage(), e.getCause());
386+
} catch (Exception e) {
387+
throw new S3EncryptionClientException("Unable to delete objects.", e);
388+
}
368389
}
369390

370391
/**
@@ -379,7 +400,13 @@ public DeleteObjectsResponse deleteObjects(DeleteObjectsRequest deleteObjectsReq
379400
*/
380401
@Override
381402
public CreateMultipartUploadResponse createMultipartUpload(CreateMultipartUploadRequest request) {
382-
return _multipartPipeline.createMultipartUpload(request);
403+
try {
404+
return _multipartPipeline.createMultipartUpload(request);
405+
} catch (CompletionException e) {
406+
throw new S3EncryptionClientException(e.getCause().getMessage(), e.getCause());
407+
} catch (Exception e) {
408+
throw new S3EncryptionClientException("Unable to create Multipart upload.", e);
409+
}
383410
}
384411

385412
/**
@@ -396,7 +423,13 @@ public CreateMultipartUploadResponse createMultipartUpload(CreateMultipartUpload
396423
@Override
397424
public UploadPartResponse uploadPart(UploadPartRequest request, RequestBody requestBody)
398425
throws AwsServiceException, SdkClientException {
399-
return _multipartPipeline.uploadPart(request, requestBody);
426+
try {
427+
return _multipartPipeline.uploadPart(request, requestBody);
428+
} catch (CompletionException e) {
429+
throw new S3EncryptionClientException(e.getCause().getMessage(), e.getCause());
430+
} catch (Exception e) {
431+
throw new S3EncryptionClientException("Unable to upload part.", e);
432+
}
400433
}
401434

402435
/**
@@ -407,7 +440,13 @@ public UploadPartResponse uploadPart(UploadPartRequest request, RequestBody requ
407440
@Override
408441
public CompleteMultipartUploadResponse completeMultipartUpload(CompleteMultipartUploadRequest request)
409442
throws AwsServiceException, SdkClientException {
410-
return _multipartPipeline.completeMultipartUpload(request);
443+
try {
444+
return _multipartPipeline.completeMultipartUpload(request);
445+
} catch (CompletionException e) {
446+
throw new S3EncryptionClientException(e.getCause().getMessage(), e.getCause());
447+
} catch (Exception e) {
448+
throw new S3EncryptionClientException("Unable to complete Multipart upload.", e);
449+
}
411450
}
412451

413452
/**

‎src/test/java/software/amazon/encryption/s3/S3EncryptionClientTest.java

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,17 @@
1717
import software.amazon.awssdk.core.sync.RequestBody;
1818
import software.amazon.awssdk.services.s3.S3AsyncClient;
1919
import software.amazon.awssdk.services.s3.S3Client;
20+
import software.amazon.awssdk.services.s3.model.CreateMultipartUploadResponse;
2021
import software.amazon.awssdk.services.s3.model.GetObjectResponse;
22+
import software.amazon.awssdk.services.s3.model.NoSuchBucketException;
23+
import software.amazon.awssdk.services.s3.model.NoSuchKeyException;
2124
import software.amazon.awssdk.services.s3.model.ObjectIdentifier;
2225
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
2326
import software.amazon.awssdk.services.s3.model.S3Exception;
2427
import software.amazon.encryption.s3.materials.CryptographicMaterialsManager;
2528
import software.amazon.encryption.s3.materials.DefaultCryptoMaterialsManager;
2629
import software.amazon.encryption.s3.materials.KmsKeyring;
30+
import software.amazon.encryption.s3.utils.BoundedInputStream;
2731

2832
import javax.crypto.KeyGenerator;
2933
import javax.crypto.SecretKey;
@@ -41,6 +45,8 @@
4145
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
4246
import static org.junit.jupiter.api.Assertions.assertEquals;
4347
import static org.junit.jupiter.api.Assertions.assertThrows;
48+
import static org.junit.jupiter.api.Assertions.assertTrue;
49+
import static org.junit.jupiter.api.Assertions.fail;
4450
import static org.mockito.ArgumentMatchers.any;
4551
import static org.mockito.Mockito.mock;
4652
import static org.mockito.Mockito.never;
@@ -208,6 +214,65 @@ public void deleteObjectWithWrongObjectKeySuccess() {
208214
v3Client.close();
209215
}
210216

217+
@Test
218+
public void deleteObjectWithWrongBucketFailure() {
219+
// V3 Client
220+
S3Client v3Client = S3EncryptionClient.builder()
221+
.aesKey(AES_KEY)
222+
.build();
223+
try {
224+
v3Client.deleteObject(builder -> builder.bucket("NotMyBukkit").key("InvalidKey"));
225+
} catch (S3EncryptionClientException exception) {
226+
// Verify inner exception
227+
assertTrue(exception.getCause() instanceof NoSuchBucketException);
228+
}
229+
230+
v3Client.close();
231+
}
232+
233+
@Test
234+
public void deleteObjectsWithWrongBucketFailure() {
235+
// V3 Client
236+
S3Client v3Client = S3EncryptionClient.builder()
237+
.aesKey(AES_KEY)
238+
.build();
239+
List<ObjectIdentifier> objects = new ArrayList<>();
240+
objects.add(ObjectIdentifier.builder().key("InvalidKey").build());
241+
try {
242+
v3Client.deleteObjects(builder -> builder.bucket("NotMyBukkit").delete(builder1 -> builder1.objects(objects)));
243+
} catch (S3EncryptionClientException exception) {
244+
// Verify inner exception
245+
assertTrue(exception.getCause() instanceof NoSuchBucketException);
246+
}
247+
v3Client.close();
248+
}
249+
250+
@Test
251+
public void getNonExistentObject() {
252+
final String objectKey = appendTestSuffix("this-is-not-an-object-key");
253+
S3Client v3Client = S3EncryptionClient.builder()
254+
.kmsKeyId(KMS_KEY_ALIAS)
255+
.build();
256+
257+
// Ensure the object does not exist
258+
deleteObject(BUCKET, objectKey, v3Client);
259+
260+
try {
261+
v3Client.getObjectAsBytes(builder -> builder
262+
.bucket(BUCKET)
263+
.key(objectKey)
264+
.build());
265+
} catch (S3EncryptionClientException exception) {
266+
// Depending on the permissions of the calling principal,
267+
// this could be NoSuchKeyException
268+
// or S3Exception (access denied)
269+
assertTrue(exception.getCause() instanceof S3Exception);
270+
}
271+
272+
// Cleanup
273+
v3Client.close();
274+
}
275+
211276
@Test
212277
public void s3EncryptionClientWithMultipleKeyringsFails() {
213278
assertThrows(S3EncryptionClientException.class, () -> S3EncryptionClient.builder()
@@ -544,6 +609,65 @@ public void attemptToDecryptPlaintext() {
544609
v3Client.close();
545610
}
546611

612+
@Test
613+
public void createMultipartUploadFailure() {
614+
// V3 Client
615+
S3Client v3Client = S3EncryptionClient.builder()
616+
.aesKey(AES_KEY)
617+
.build();
618+
try {
619+
v3Client.createMultipartUpload(builder -> builder.bucket("NotMyBukkit").key("InvalidKey").build());
620+
} catch (S3EncryptionClientException exception) {
621+
// Verify inner exception
622+
assertTrue(exception.getCause() instanceof NoSuchBucketException);
623+
}
624+
625+
v3Client.close();
626+
}
627+
628+
@Test
629+
public void uploadPartFailure() {
630+
final String objectKey = appendTestSuffix("upload-part-failure");
631+
// V3 Client
632+
S3Client v3Client = S3EncryptionClient.builder()
633+
.aesKey(AES_KEY)
634+
.build();
635+
636+
// To get a server-side failure from uploadPart,
637+
// a valid MPU request must be created
638+
CreateMultipartUploadResponse initiateResult = v3Client.createMultipartUpload(builder ->
639+
builder.bucket(BUCKET).key(objectKey));
640+
641+
try {
642+
v3Client.uploadPart(builder -> builder.partNumber(1).bucket("NotMyBukkit").key("InvalidKey").uploadId(initiateResult.uploadId()).build(),
643+
RequestBody.fromInputStream(new BoundedInputStream(16), 16));
644+
} catch (S3EncryptionClientException exception) {
645+
// Verify inner exception
646+
assertTrue(exception.getCause() instanceof NoSuchBucketException);
647+
}
648+
649+
// MPU was not completed, but delete to be safe
650+
deleteObject(BUCKET, objectKey, v3Client);
651+
v3Client.close();
652+
}
653+
654+
655+
@Test
656+
public void completeMultipartUploadFailure() {
657+
// V3 Client
658+
S3Client v3Client = S3EncryptionClient.builder()
659+
.aesKey(AES_KEY)
660+
.build();
661+
try {
662+
v3Client.completeMultipartUpload(builder -> builder.bucket("NotMyBukkit").key("InvalidKey").uploadId("Invalid").build());
663+
} catch (S3EncryptionClientException exception) {
664+
// Verify inner exception
665+
assertTrue(exception.getCause() instanceof NoSuchBucketException);
666+
}
667+
668+
v3Client.close();
669+
}
670+
547671
/**
548672
* A simple, reusable round-trip (encryption + decryption) using a given
549673
* S3Client. Useful for testing client configuration.

0 commit comments

Comments
 (0)