Skip to content

Commit

Permalink
Uncomment previously disabled AsyncHttpChecksumIntegrationTest (#4545)
Browse files Browse the repository at this point in the history
* Uncomment previously disabled AsyncHttpChecksumIntegrationTest

* Update comment to explain more about those tests
  • Loading branch information
gosar committed Oct 4, 2023
1 parent 6ef423f commit 55b9d34
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,7 @@ public SdkHttpFullRequest.Builder execute(SdkHttpFullRequest.Builder request, Re
return request;
}

// If SRA is enabled, we skip flexible checksum in header
// TODO(post-sra-identity-auth): we should remove this after SRA is fully released
// If SRA is enabled, skip flexible checksum in header, since it is handled by SRA signer
if (sraSigningEnabled(context)) {
return request;
}
Expand Down Expand Up @@ -136,8 +135,7 @@ private void addMd5ChecksumInHeader(SdkHttpFullRequest.Builder request) {

private boolean flexibleChecksumInTrailerRequired(RequestExecutionContext context, ChecksumSpecs checksumSpecs) {

// If SRA is enabled and it's sync client,
// skip it since flexible checksum trailer is handled in SRA signer
// If SRA is enabled and it's sync client, skip flexible checksum trailer, since it is handled in SRA signer
if (sraSigningEnabled(context) && clientType == ClientType.SYNC) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
import software.amazon.awssdk.services.s3.model.PutObjectResponse;
import software.amazon.awssdk.services.s3.utils.CaptureChecksumValidationInterceptor;
import software.amazon.awssdk.services.s3.utils.ChecksumUtils;
import software.amazon.awssdk.testutils.RandomTempFile;

public class AsyncHttpChecksumIntegrationTest extends S3IntegrationTestBase {
Expand Down Expand Up @@ -235,13 +234,15 @@ void asyncValidSignedTrailerChecksumCalculatedBySdkClient() {
}

/**
* If http is used, payload signing will be enforced, but it's not currently supported in async path
* TODO: re-enable it once it's supported
* S3 clients by default don't do payload signing. But when http is used, payload signing is expected to be enforced. But
* payload signing is not currently supported in async path (for both pre/post SRA signers).
* However, this test passes, because of https://github
* .com/aws/aws-sdk-java-v2/blob/38e221bd815af31a6c6b91557499af155103c21a/core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAwsS3V4Signer.java#L279-L285.
* Keeping this test enabled, to ensure moving to SRA Identity & Auth, does not break current behavior.
* TODO: Update this test with right asserts when payload signing is supported in async.
*/
@Disabled("Payload signing is not supported for S3 async client")
@Test
public void putObject_with_bufferCreatedFromEmptyString() {

s3HttpAsync.putObject(PutObjectRequest.builder()
.bucket(BUCKET)
.key(KEY)
Expand All @@ -262,10 +263,13 @@ public void putObject_with_bufferCreatedFromEmptyString() {
}

/**
* If http is used, payload signing will be enforced, but it's not currently supported in async path
* TODO: re-enable it once it's supported
* S3 clients by default don't do payload signing. But when http is used, payload signing is expected to be enforced. But
* payload signing is not currently supported in async path (for both pre/post SRA signers).
* However, this test passes, because of https://github
* .com/aws/aws-sdk-java-v2/blob/38e221bd815af31a6c6b91557499af155103c21a/core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAwsS3V4Signer.java#L279-L285.
* Keeping this test enabled, to ensure moving to SRA Identity & Auth, does not break current behavior.
* TODO: Update this test with right asserts when payload signing is supported in async.
*/
@Disabled("Payload signing is not supported for S3 async client")
@Test
public void putObject_with_bufferCreatedFromZeroCapacityByteBuffer() {
ByteBuffer content = ByteBuffer.allocate(0);
Expand Down

0 comments on commit 55b9d34

Please sign in to comment.