Skip to content

Commit

Permalink
wip(#300): @NathanEckert is correct
Browse files Browse the repository at this point in the history
The modern S3EC and SDK V2 treat reading 0 bytes
from a stream differently in at least one case:
If the Stream has no more content.
  • Loading branch information
texastony committed Jun 24, 2024
1 parent 674df60 commit 56e28c1
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 14 deletions.
49 changes: 49 additions & 0 deletions .github/workflows/ghi_300.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
name: ghi_300.yml
on:
push:
branches:
- tony/refactor-tests
- 'ghi-300/**'

jobs:
Build:
runs-on: ubuntu-latest
permissions:
id-token: write
contents: read

steps:
- name: Configure AWS Credentials
uses: aws-actions/configure-aws-credentials@v2
with:
role-to-assume: arn:aws:iam::${{ secrets.CI_AWS_ACCOUNT_ID }}:role/service-role/${{ vars.CI_AWS_ROLE }}
role-session-name: S3EC-Github-CI-Tests
aws-region: ${{ vars.CI_AWS_REGION }}

- name: Checkout Code
uses: actions/checkout@v3

# TODO: Add OpenJDK
# OpenJDK would require a different action than setup-java, so setup is more involved.

- name: Setup JDK
uses: actions/setup-java@v3
with:
distribution: corretto
java-version: 8
cache: 'maven'

- name: Compile
run: |
mvn --batch-mode -no-transfer-progress clean compile
mvn --batch-mode -no-transfer-progress test-compile
shell: bash

- name: Test
run: |
export AWS_S3EC_TEST_BUCKET=${{ vars.CI_S3_BUCKET }}
export AWS_S3EC_TEST_KMS_KEY_ID=arn:aws:kms:${{ vars.CI_AWS_REGION }}:${{ secrets.CI_AWS_ACCOUNT_ID }}:key/${{ vars.CI_KMS_KEY_ID }}
export AWS_S3EC_TEST_KMS_KEY_ALIAS=arn:aws:kms:${{ vars.CI_AWS_REGION }}:${{ secrets.CI_AWS_ACCOUNT_ID }}:alias/${{ vars.CI_KMS_KEY_ALIAS }}
export AWS_REGION=${{ vars.CI_AWS_REGION }}
mvn -B -ntp -DskipCompile -Dtest=software.amazon.encryption.s3.examples.TestEndOfStreamBehavior test
shell: bash
Original file line number Diff line number Diff line change
Expand Up @@ -85,24 +85,15 @@ void testEndOfStreamBehavior(final S3Client client) throws Exception {
while (byteRead != END_OF_STREAM) {
int lenToRead = capacity - startPosition;
System.out.println("Start position: " + startPosition + " Length to read: " + lenToRead);
// @NathanEckert , about https://github.com/aws/amazon-s3-encryption-client-java/issues/300
// Crypto Tools SOMETIMES got an Assertion Error from
// https://github.com/aws/aws-sdk-java-v2/blob/2.20.38/utils/src/main/java/software/amazon/awssdk/utils/async/InputStreamSubscriber.java#L110
// when using the Encryption Client.
// If we bump our Java SDK dependencies to the latest, which today is 2.26.7,
// than we never get the Assertion Error.
// Here is the PR that changes InputStreamSubscriber b/w 2.20.38 and 2.26.7:
// https://github.com/aws/aws-sdk-java-v2/pull/5201
// This makes us suspect that something else is going wrong.
// Otherwise, we cannot detect a difference in behavior between
// the S3EC V3 Client and the S3 V2 Client with respect to this code.
byteRead = stream.read(underlyingBuffer, startPosition, lenToRead);
System.out.println("Read " + byteRead + " bytes");
startPosition += byteRead;
if (byteRead == 0) {
// Crypto Tools cannot get this case to ever occur.
System.out.println("Looping indefinitely with an encryption client, as startPosition is not increasing");
break;
// Now we always get this error; we probably were always getting this error, but the log was not writing.
throw new AssertionError(
String.format("Looping indefinitely with an encryption client, as startPosition is not increasing." +
"\n lenToRead: %s \t byteRead: %s \t startPosition: %s",
lenToRead, byteRead, startPosition));
}
}
}
Expand Down

0 comments on commit 56e28c1

Please sign in to comment.