Fix GetObject tests to use SigV4 constructor (#1116)

- Fixed com.amazonaws.services.s3.enforceV4 not being respected
- Replace deprecated no-arg AmazonS3Client in other tests
- Remove unit tests that assert unsupported behavior
- Re-enabled S3IntegrationTests, without CN region tests that were known bad
- Scoped down ACL of bucket created in S3IntegrationTests

Author: palpatim

CHANGELOG.md

@@ -1,5 +1,14 @@
 # Change Log - AWS SDK for Android
 
+## [Release 2.15.0](https://github.com/aws/aws-sdk-android/releases/tag/release_v2.15.0)
+
+### Mis. Updates
+
+- **Breaking Changes**
+  - Removed deprecated SDKGlobalConfiguration options:
+    - `ENABLE_S3_SIGV4_SYSTEM_PROPERTY`
+    - `ENFORCE_S3_SIGV4_SYSTEM_PROPERTY`
+
 ## [Release 2.14.2](https://github.com/aws/aws-sdk-android/releases/tag/release_v2.14.2)
 
 ### New Features

README.md

@@ -134,7 +134,7 @@ private class S3Example extends AsyncTask<Void,Void,Void>{
                 Regions.SELECT_YOUR_REGION // Region enum
             );
 
-            AmazonS3Client s3Client = new AmazonS3Client(credentialsProvider);
+            AmazonS3Client s3Client = new AmazonS3Client(credentialsProvider, Regions.getRegion(Regions.SELECT_YOUR_REGION));
             File fileToUpload = YOUR_FILE;
             //(Replace "MY-BUCKET" with your S3 bucket name, and "MY-OBJECT-KEY" with whatever you would like to name the file in S3)
             PutObjectRequest putRequest = new PutObjectRequest("MY-BUCKET", "MY-OBJECT-KEY",

aws-android-sdk-core/src/main/java/com/amazonaws/SDKGlobalConfiguration.java

@@ -88,36 +88,6 @@ public class SDKGlobalConfiguration {
     public static final String DISABLE_REMOTE_REGIONS_FILE_SYSTEM_PROPERTY =
             "com.amazonaws.regions.RegionUtils.disableRemote";
 
-    /**
-     * By default, the AmazonS3Client will continue to use the legacy S3Signer
-     * to authenticate requests it makes to S3 in regions that support the older
-     * protocol. Setting this property to anything other than null will cause
-     * the client to upgrade to Signature Version 4 whenever it has been
-     * configured with an explicit region (which is a required parameter for
-     * Signature Version 4). The client will continue to use the older signature
-     * protocol when not configured with a region to avoid breaking existing
-     * applications.
-     * <p>
-     * Signature Version 4 is more secure than the legacy S3Signer, but requires
-     * calculating a SHA-256 hash of the entire request body which can be
-     * expensive for large upload requests.
-     */
-    @Deprecated
-    public static final String ENABLE_S3_SIGV4_SYSTEM_PROPERTY =
-            "com.amazonaws.services.s3.enableV4";
-
-    /**
-     * Like {@link #ENABLE_S3_SIGV4_SYSTEM_PROPERTY}, but causes the client to
-     * always use Signature Version 4, assuming a region of
-     * &quot;us-east-1&quot; if no explicit region has been configured. This
-     * guarantees that the more secure authentication protocol will be used, but
-     * will cause authentication failures in code that accesses buckets in
-     * regions other than US Standard without explicitly configuring a region.
-     */
-    @Deprecated
-    public static final String ENFORCE_S3_SIGV4_SYSTEM_PROPERTY =
-            "com.amazonaws.services.s3.enforceV4";
-
     /**
      * The default size of the buffer when uploading data from a stream. A
      * buffer of this size will be created and filled with the first bytes from

aws-android-sdk-ddb-mapper-test/src/androidTest/java/com/amazonaws/mobileconnectors/dynamodbv2/dynamodbmapper/DynamoDBS3IntegrationTestBase.java

@@ -42,10 +42,8 @@ public abstract class DynamoDBS3IntegrationTestBase extends DynamoDBIntegrationT
     @BeforeClass
     public static void setUp() throws Exception {
         DynamoDBIntegrationTestBase.setUp();
-        s3East = new AmazonS3Client(credentials);
-        s3East.setRegion(Region.getRegion(Regions.US_EAST_1));
-        s3West = new AmazonS3Client(credentials);
-        s3West.setRegion(Region.getRegion(Regions.US_WEST_2));
+        s3East = new AmazonS3Client(credentials, Region.getRegion(Regions.US_EAST_1));
+        s3West = new AmazonS3Client(credentials, Region.getRegion(Regions.US_WEST_2));
 
         createBucket(s3East, EAST_BUCKET);
         createBucket(s3West, WEST_BUCKET);

aws-android-sdk-ddb-mapper-test/src/androidTest/java/com/amazonaws/mobileconnectors/dynamodbv2/dynamodbmapper/S3ClientCacheIntegrationTest.java

@@ -24,6 +24,7 @@
 import com.amazonaws.auth.AWSCredentials;
 import com.amazonaws.auth.BasicAWSCredentials;
 import com.amazonaws.mobileconnectors.s3.transfermanager.TransferManager;
+import com.amazonaws.regions.Regions;
 import com.amazonaws.services.s3.AmazonS3Client;
 import com.amazonaws.services.s3.model.Region;
 
@@ -57,11 +58,17 @@ public void testClientReuse() {
     @Test
     public void testUserProvidedClients() {
         S3ClientCache s3cc = new S3ClientCache(credentials);
-        AmazonS3Client s3East1 = new AmazonS3Client(credentials);
+        AmazonS3Client s3East1 = new AmazonS3Client(credentials,
+                com.amazonaws.regions.Region.getRegion(Regions.US_EAST_1));
+
         s3East1.setRegion(Region.US_Standard.toAWSRegion());
-        AmazonS3Client s3West1 = new AmazonS3Client(credentials);
+        AmazonS3Client s3West1 = new AmazonS3Client(credentials,
+                com.amazonaws.regions.Region.getRegion(Regions.US_WEST_1));
+
         s3West1.setRegion(Region.US_West.toAWSRegion());
-        AmazonS3Client s3West2 = new AmazonS3Client(credentials);
+        AmazonS3Client s3West2 = new AmazonS3Client(credentials,
+                com.amazonaws.regions.Region.getRegion(Regions.US_WEST_2));
+
         s3West2.setRegion(Region.US_West_2.toAWSRegion());
 
         s3cc.useClient(s3East1);
@@ -88,7 +95,8 @@ public void testReplaceClient() {
         TransferManager tmEast1 = s3cc.getTransferManager(Region.US_Standard);
         assertNotNull(tmEast1);
 
-        AmazonS3Client newS3East = new AmazonS3Client(credentials);
+        AmazonS3Client newS3East = new AmazonS3Client(credentials,
+                com.amazonaws.regions.Region.getRegion(Regions.US_EAST_1));
         newS3East.setRegion(Region.US_Standard.toAWSRegion());
         s3cc.useClient(newS3East); // should remove old TM
 
@@ -101,7 +109,8 @@ public void testReplaceClient() {
     @Test
     public void testBadClientCache() {
         S3ClientCache s3cc = new S3ClientCache(credentials);
-        AmazonS3Client notAnAWSEndpoint = new AmazonS3Client(credentials);
+        AmazonS3Client notAnAWSEndpoint = new AmazonS3Client(credentials,
+                com.amazonaws.regions.Region.getRegion(Regions.US_EAST_1));
         notAnAWSEndpoint.setEndpoint("i.am.an.invalid.aws.endpoint.com");
         try {
             s3cc.useClient(notAnAWSEndpoint);
@@ -114,9 +123,10 @@ public void testBadClientCache() {
     }
 
     @Test
-    public void testNonExistantRegion() {
+    public void testNonExistentRegion() {
         S3ClientCache s3cc = new S3ClientCache(credentials);
-        AmazonS3Client notAnAWSEndpoint = new AmazonS3Client(credentials);
+        AmazonS3Client notAnAWSEndpoint = new AmazonS3Client(credentials,
+                com.amazonaws.regions.Region.getRegion(Regions.US_EAST_1));
         notAnAWSEndpoint.setEndpoint("s3-mordor.amazonaws.com");
         try {
             s3cc.useClient(notAnAWSEndpoint);

aws-android-sdk-ddb-mapper/src/test/java/com/amazonaws/mobileconnectors/dynamodbv2/dynamodbmapper/S3LinkTest.java

@@ -27,6 +27,7 @@
 import com.amazonaws.internal.StaticCredentialsProvider;
 import com.amazonaws.metrics.RequestMetricCollector;
 import com.amazonaws.mobileconnectors.s3.transfermanager.TransferManager;
+import com.amazonaws.regions.Regions;
 import com.amazonaws.services.dynamodbv2.AmazonDynamoDB;
 import com.amazonaws.services.s3.AmazonS3Client;
 import com.amazonaws.services.s3.model.AccessControlList;
@@ -63,9 +64,11 @@ public class S3LinkTest
     @Before
     public void setUp() {
         AWSCredentials credentials = new BasicAWSCredentials("mock", "mock");
+        StaticCredentialsProvider s3CredentialProvider = new StaticCredentialsProvider(credentials);
+
         mockDDB = EasyMock.createMock(AmazonDynamoDB.class);
         mockS3 = EasyMock.createMock(AmazonS3Client.class);
-        mapper = new DynamoDBMapper(mockDDB, new StaticCredentialsProvider(credentials));
+        mapper = new DynamoDBMapper(mockDDB, s3CredentialProvider);
     }
 
     @Test(expected = IllegalArgumentException.class)
@@ -268,9 +271,13 @@ public void testUploadFromFile() throws IOException {
     @Test
     public void testGetURL() {
 
-        AmazonS3Client realS3 = new AmazonS3Client();
+        AWSCredentials credentials = new BasicAWSCredentials("mock", "mock");
+        StaticCredentialsProvider s3CredentialProvider = new StaticCredentialsProvider(credentials);
+        AmazonS3Client realS3 = new AmazonS3Client(s3CredentialProvider,
+                com.amazonaws.regions.Region.getRegion(Regions.US_WEST_2));
+
         mapper.getS3ClientCache().useClient(realS3);
-        S3Link link = mapper.createS3Link(bucket, key);
+        S3Link link = mapper.createS3Link(Region.US_West_2, bucket, key);
 
         URL createdURL = realS3.getUrl(bucket, key);
         URL retrievedURL = link.getUrl();
@@ -280,9 +287,13 @@ public void testGetURL() {
 
     @Test
     public void testGetTransferManager() {
-        AmazonS3Client realS3 = new AmazonS3Client();
+        AWSCredentials credentials = new BasicAWSCredentials("mock", "mock");
+        StaticCredentialsProvider s3CredentialProvider = new StaticCredentialsProvider(credentials);
+        AmazonS3Client realS3 = new AmazonS3Client(s3CredentialProvider,
+                com.amazonaws.regions.Region.getRegion(Regions.US_WEST_2));
+
         mapper.getS3ClientCache().useClient(realS3);
-        S3Link link = mapper.createS3Link(bucket, key);
+        S3Link link = mapper.createS3Link(Region.US_West_2, bucket, key);
 
         TransferManager tm = link.getTransferManager();
 

aws-android-sdk-s3-test/src/androidTest/java/com/amazonaws/services/s3/BucketNameValidationIntegrationTest.java

@@ -19,6 +19,8 @@
 
 import com.amazonaws.auth.AWSCredentials;
 import com.amazonaws.auth.BasicAWSCredentials;
+import com.amazonaws.regions.Region;
+import com.amazonaws.regions.Regions;
 
 import org.junit.Test;
 
@@ -37,7 +39,8 @@ public class BucketNameValidationIntegrationTest {
 
     private final AWSCredentials fakeCredentials =
             new BasicAWSCredentials("fakeAccessKey", "fakeSecretKey");
-    private final AmazonS3 s3 = new AmazonS3Client(fakeCredentials);
+    private final AmazonS3 s3 =
+            new AmazonS3Client(fakeCredentials, Region.getRegion(Regions.DEFAULT_REGION));
 
     /**
      * Tests that the bucket names that don't follow S3's recommended

aws-android-sdk-s3-test/src/androidTest/java/com/amazonaws/services/s3/CleanupBucketIntegrationTests.java

@@ -23,10 +23,9 @@ public class CleanupBucketIntegrationTests extends AWSTestBase {
     private AmazonS3 s3;
 
     @Before
-    public void setup() throws FileNotFoundException, IOException {
+    public void setup() {
         setUpCredentials();
-        s3 = new AmazonS3Client(credentials);
-        s3.setRegion(Region.getRegion(Regions.US_WEST_1));
+        s3 = new AmazonS3Client(credentials, Region.getRegion(Regions.US_WEST_1));
     }
 
     @Test

aws-android-sdk-s3-test/src/androidTest/java/com/amazonaws/services/s3/GetObjectIntegrationTest.java

@@ -449,7 +449,7 @@ public void testServerSideEncryptionBadAlgorithm() {
     @Ignore("causes a dns exception in some cases")
     @Test
     public void testNonUsRegionBucketNamesWithPeriods() {
-        final AmazonS3 regionalClient = new AmazonS3Client(credentials);
+        final AmazonS3 regionalClient = new AmazonS3Client(credentials, Region.getRegion(Regions.SA_EAST_1));
         regionalClient.setEndpoint("s3-sa-east-1.amazonaws.com");
         final String regionalBucketName = bucketName + ".with.periods.regional";
         try {
@@ -517,7 +517,8 @@ private long drainStream(InputStream inputStream) throws IOException {
     public void testGetObjectWithClientSetToNullBeforeReadingFromInputStream()
             throws Exception {
 
-        AmazonS3 s3Local = new AmazonS3Client(credentials);
+        // Matches the default client set up in S3IntegrationTestBase
+        AmazonS3 s3Local = new AmazonS3Client(credentials, Region.getRegion(Regions.US_WEST_1));
 
         final S3Object obj = s3Local.getObject(bucketName, key);
 
@@ -559,9 +560,11 @@ private void doGarbageCollection() {
     @Test
     public void testRequesterPays() throws InterruptedException {
 
-        final AmazonS3Client requester = new AmazonS3Client(
-                new PropertiesFileCredentialsProvider(
-                        requesterPaysCredentialsFilePath));
+        PropertiesFileCredentialsProvider requesterPaysCredentialsProvider =
+                new PropertiesFileCredentialsProvider(requesterPaysCredentialsFilePath);
+
+        final AmazonS3Client requester = new AmazonS3Client(requesterPaysCredentialsProvider,
+                Region.getRegion(Regions.DEFAULT_REGION));
 
         final Owner owner = requester.getS3AccountOwner();
         final String id = owner.getId();

aws-android-sdk-s3-test/src/androidTest/java/com/amazonaws/services/s3/PresignedUrlIntegrationTest.java

@@ -23,6 +23,8 @@
 import com.amazonaws.HttpMethod;
 import com.amazonaws.SDKGlobalConfiguration;
 import com.amazonaws.auth.STSSessionCredentialsProvider;
+import com.amazonaws.regions.Region;
+import com.amazonaws.regions.Regions;
 import com.amazonaws.services.s3.internal.MD5DigestCalculatingInputStream;
 import com.amazonaws.services.s3.model.GeneratePresignedUrlRequest;
 import com.amazonaws.services.s3.model.ResponseHeaderOverrides;
@@ -101,17 +103,6 @@ public static void setUp() throws Exception {
      */
     @Test
     public void testSpecialKeys() throws Exception {
-        // SigV2
-        System.clearProperty(SDKGlobalConfiguration.ENFORCE_S3_SIGV4_SYSTEM_PROPERTY);
-        goTestSpecialKeys();
-
-        // SigV4
-        System.setProperty(SDKGlobalConfiguration.ENFORCE_S3_SIGV4_SYSTEM_PROPERTY, "true");
-        goTestSpecialKeys();
-        System.clearProperty(SDKGlobalConfiguration.ENFORCE_S3_SIGV4_SYSTEM_PROPERTY);
-    }
-
-    private void goTestSpecialKeys() throws Exception {
         List<String> keys = SpecialObjectKeyNameGenerator.initAllSpecialKeyNames();
 
         s3.setS3ClientOptions(new S3ClientOptions().withPathStyleAccess(false));
@@ -353,17 +344,12 @@ public void testPresignUrlWithSessionTokenCredential() throws Exception {
         final String key = "presign-url-sts-test";
 
         STSSessionCredentialsProvider provider = new STSSessionCredentialsProvider(credentials);
-        AmazonS3Client s3WithSts = new AmazonS3Client(provider);
+        AmazonS3Client s3WithSts = new AmazonS3Client(provider,
+                Region.getRegion(Regions.US_WEST_2));
         s3WithSts.setEndpoint(S3_REGIONAL_ENDPOINT);
         s3WithSts.putObject(virtHostBucketName, key, file);
 
-        // SigV2
-        testGetUrl(s3WithSts, virtHostBucketName, key);
-
-        // SigV4
-        System.setProperty(SDKGlobalConfiguration.ENFORCE_S3_SIGV4_SYSTEM_PROPERTY, "true");
         testGetUrl(s3WithSts, virtHostBucketName, key);
-        System.clearProperty(SDKGlobalConfiguration.ENFORCE_S3_SIGV4_SYSTEM_PROPERTY);
     }
 
     /*