From 36992081c25bf3443fb6057cea4a40b7b7bf6494 Mon Sep 17 00:00:00 2001 From: Jakob Vogel Date: Sun, 6 Jun 2021 15:02:43 +0200 Subject: [PATCH 1/7] =?UTF-8?q?Revises=20test=20case=20to=20reuse=20declar?= =?UTF-8?q?ed=20variables=20=F0=9F=A7=BC?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/test/java/BaseAWSSpec.groovy | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/test/java/BaseAWSSpec.groovy b/src/test/java/BaseAWSSpec.groovy index 3141009..1d4efb7 100644 --- a/src/test/java/BaseAWSSpec.groovy +++ b/src/test/java/BaseAWSSpec.groovy @@ -177,25 +177,28 @@ abstract class BaseAWSSpec extends BaseSpecification { def "MultipartUpload and then GET work as expected"() { when: + def bucketName = "test" + def key = "test" + def client = getClient() def transfer = TransferManagerBuilder.standard(). - withS3Client(getClient()). + withS3Client(client). withMultipartUploadThreshold(1). withMinimumUploadPartSize(1).build() def meta = new ObjectMetadata() def message = "Test".getBytes(Charsets.UTF_8) and: - if (!getClient().doesBucketExist("test")) { - getClient().createBucket("test") + if (!client.doesBucketExist(bucketName)) { + client.createBucket(bucketName) } and: meta.setContentLength(message.length) meta.addUserMetadata("userdata", "test123") - def upload = transfer.upload("test", "test", new ByteArrayInputStream("Test".getBytes(Charsets.UTF_8)), meta) + def upload = transfer.upload(bucketName, key, new ByteArrayInputStream(message), meta) upload.waitForUploadResult() def content = new String( - ByteStreams.toByteArray(client.getObject("test", "test").getObjectContent()), + ByteStreams.toByteArray(client.getObject(bucketName, key).getObjectContent()), Charsets.UTF_8) - def userdata = client.getObjectMetadata("test", "test").getUserMetaDataOf("userdata") + def userdata = client.getObjectMetadata(bucketName, key).getUserMetaDataOf("userdata") then: content == "Test" userdata == "test123" @@ -203,6 +206,8 @@ abstract class BaseAWSSpec extends BaseSpecification { def "MultipartUpload and then DELETE work as expected"() { when: + def bucketName = "test" + def key = "test" def client = getClient() def transfer = TransferManagerBuilder.standard(). withS3Client(client). @@ -211,15 +216,15 @@ abstract class BaseAWSSpec extends BaseSpecification { def meta = new ObjectMetadata() def message = "Test".getBytes(Charsets.UTF_8) and: - if (!getClient().doesBucketExist("test")) { - getClient().createBucket("test") + if (!client.doesBucketExist(bucketName)) { + client.createBucket(bucketName) } and: meta.setContentLength(message.length) - def upload = transfer.upload("test", "test", new ByteArrayInputStream("Test".getBytes(Charsets.UTF_8)), meta) + def upload = transfer.upload(bucketName, key, new ByteArrayInputStream(message), meta) upload.waitForUploadResult() - client.deleteBucket("test") - client.getObject("test", "test") + client.deleteBucket(bucketName) + client.getObject(bucketName, key) then: AmazonS3Exception e = thrown() e.statusCode == 404 From 958f6f053f1f33b34606fc188bc75e6165cac3f2 Mon Sep 17 00:00:00 2001 From: Jakob Vogel Date: Sun, 6 Jun 2021 15:06:48 +0200 Subject: [PATCH 2/7] =?UTF-8?q?Uses=20keys=20with=20slashes=20=F0=9F=A4=BA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reproduces the error as reported by @lampslave (modulo line numbers). --- src/test/java/BaseAWSSpec.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/BaseAWSSpec.groovy b/src/test/java/BaseAWSSpec.groovy index 1d4efb7..116b83f 100644 --- a/src/test/java/BaseAWSSpec.groovy +++ b/src/test/java/BaseAWSSpec.groovy @@ -178,7 +178,7 @@ abstract class BaseAWSSpec extends BaseSpecification { def "MultipartUpload and then GET work as expected"() { when: def bucketName = "test" - def key = "test" + def key = "key/with/slashes" def client = getClient() def transfer = TransferManagerBuilder.standard(). withS3Client(client). @@ -207,7 +207,7 @@ abstract class BaseAWSSpec extends BaseSpecification { def "MultipartUpload and then DELETE work as expected"() { when: def bucketName = "test" - def key = "test" + def key = "key/with/slashes" def client = getClient() def transfer = TransferManagerBuilder.standard(). withS3Client(client). From 5dd98ac6dfdfd7584f745faf28fcdd7c7bb1a57d Mon Sep 17 00:00:00 2001 From: Jakob Vogel Date: Sun, 6 Jun 2021 15:09:18 +0200 Subject: [PATCH 3/7] =?UTF-8?q?Properly=20encodes=20file=20name=20when=20c?= =?UTF-8?q?ombining=20multi-part=20upload=20=F0=9F=91=AE=E2=80=8D=E2=99=82?= =?UTF-8?q?=EF=B8=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/main/java/ninja/S3Dispatcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/ninja/S3Dispatcher.java b/src/main/java/ninja/S3Dispatcher.java index 05149dc..75dd3eb 100644 --- a/src/main/java/ninja/S3Dispatcher.java +++ b/src/main/java/ninja/S3Dispatcher.java @@ -994,7 +994,7 @@ private File getUploadDir(String uploadId) { } private File combineParts(String id, String uploadId, List parts) { - File file = new File(getUploadDir(uploadId), id); + File file = new File(getUploadDir(uploadId), StoredObject.encodeKey(id)); try { if (!file.createNewFile()) { From 170ea974a5568a2f8418f3acad219ae957990aed Mon Sep 17 00:00:00 2001 From: Jakob Vogel Date: Sun, 6 Jun 2021 15:25:30 +0200 Subject: [PATCH 4/7] =?UTF-8?q?Revises=20tests=20to=20define=20bucket=20na?= =?UTF-8?q?me=20and=20key=20centrally=20=F0=9F=8E=AF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/test/java/BaseAWSSpec.groovy | 146 ++++++++++++++++++------------- 1 file changed, 85 insertions(+), 61 deletions(-) diff --git a/src/test/java/BaseAWSSpec.groovy b/src/test/java/BaseAWSSpec.groovy index 116b83f..7b1df8f 100644 --- a/src/test/java/BaseAWSSpec.groovy +++ b/src/test/java/BaseAWSSpec.groovy @@ -23,66 +23,77 @@ import java.time.temporal.ChronoUnit abstract class BaseAWSSpec extends BaseSpecification { + def DEFAULT_BUCKET_NAME = "test" + + def DEFAULT_KEY = "key/with/slashes" + abstract AmazonS3Client getClient() def "HEAD of non-existing bucket as expected"() { given: + def bucketName = "does-not-exist" def client = getClient() when: - if (client.doesBucketExist("does-not-exist")) { - client.deleteBucket("does-not-exist") + if (client.doesBucketExist(bucketName)) { + client.deleteBucket(bucketName) } then: - !client.doesBucketExist("does-not-exist") - !client.doesBucketExistV2("does-not-exist") + !client.doesBucketExist(bucketName) + !client.doesBucketExistV2(bucketName) } def "PUT and then HEAD bucket as expected"() { given: + def bucketName = DEFAULT_BUCKET_NAME def client = getClient() when: - if (client.doesBucketExist("test")) { - client.deleteBucket("test") + if (client.doesBucketExist(bucketName)) { + client.deleteBucket(bucketName) } - client.createBucket("test") + client.createBucket(bucketName) then: - client.doesBucketExist("test") - client.doesBucketExistV2("test") + client.doesBucketExist(bucketName) + client.doesBucketExistV2(bucketName) } def "DELETE of non-existing bucket as expected"() { given: + def bucketName = "does-not-exist" def client = getClient() when: - if (client.doesBucketExist("does-not-exist")) { - client.deleteBucket("does-not-exist") + if (client.doesBucketExist(bucketName)) { + client.deleteBucket(bucketName) } and: - client.deleteBucket("does-not-exist") + client.deleteBucket(bucketName) then: AmazonS3Exception e = thrown() e.statusCode == 404 - !client.doesBucketExist("does-not-exist") - !client.doesBucketExistV2("does-not-exist") + !client.doesBucketExist(bucketName) + !client.doesBucketExistV2(bucketName) } def "PUT and then DELETE bucket as expected"() { given: + def bucketName = DEFAULT_BUCKET_NAME def client = getClient() when: - if (!client.doesBucketExist("test")) { - client.createBucket("test") + if (!client.doesBucketExist(bucketName)) { + client.createBucket(bucketName) } - client.deleteBucket("test") + client.deleteBucket(bucketName) then: - !client.doesBucketExist("test") + !client.doesBucketExist(bucketName) } def "PUT and then GET file work using TransferManager"() { - when: + given: + def bucketName = DEFAULT_BUCKET_NAME + def key = DEFAULT_KEY def client = getClient() - if (!client.doesBucketExist("test")) { - client.createBucket("test") + when: + if (!client.doesBucketExist(bucketName)) { + client.createBucket(bucketName) } and: File file = File.createTempFile("test", "") @@ -92,33 +103,35 @@ abstract class BaseAWSSpec extends BaseSpecification { } and: def tm = TransferManagerBuilder.standard().withS3Client(client).build() - tm.upload("test", "test", file).waitForUploadResult() + tm.upload(bucketName, key, file).waitForUploadResult() and: File download = File.createTempFile("s3-test", "") download.deleteOnExit() - tm.download("test", "test", download).waitForCompletion() + tm.download(bucketName, key, download).waitForCompletion() then: Files.toString(file, Charsets.UTF_8) == Files.toString(download, Charsets.UTF_8) } def "PUT and then GET work as expected"() { given: + def bucketName = DEFAULT_BUCKET_NAME + def key = DEFAULT_KEY def client = getClient() when: - if (!client.doesBucketExist("test")) { - client.createBucket("test") + if (!client.doesBucketExist(bucketName)) { + client.createBucket(bucketName) } and: client.putObject( - "test", - "test", + bucketName, + key, new ByteArrayInputStream("Test".getBytes(Charsets.UTF_8)), new ObjectMetadata()) def content = new String( - ByteStreams.toByteArray(client.getObject("test", "test").getObjectContent()), + ByteStreams.toByteArray(client.getObject(bucketName, key).getObjectContent()), Charsets.UTF_8) and: - GeneratePresignedUrlRequest request = new GeneratePresignedUrlRequest("test", "test") + GeneratePresignedUrlRequest request = new GeneratePresignedUrlRequest(bucketName, key) URLConnection c = new URL(getClient().generatePresignedUrl(request).toString()).openConnection() and: String downloadedData = new String(ByteStreams.toByteArray(c.getInputStream()), Charsets.UTF_8) @@ -130,56 +143,62 @@ abstract class BaseAWSSpec extends BaseSpecification { def "PUT and then LIST work as expected"() { given: + def bucketName = DEFAULT_BUCKET_NAME + def key1 = DEFAULT_KEY + "/Eins" + def key2 = DEFAULT_KEY + "/Zwei" def client = getClient() when: - if (client.doesBucketExist("test")) { - client.deleteBucket("test") + if (client.doesBucketExist(bucketName)) { + client.deleteBucket(bucketName) } - client.createBucket("test") + client.createBucket(bucketName) and: client.putObject( - "test", - "Eins", + bucketName, + key1, new ByteArrayInputStream("Eins".getBytes(Charsets.UTF_8)), new ObjectMetadata()) client.putObject( - "test", - "Zwei", + bucketName, + key2, new ByteArrayInputStream("Zwei".getBytes(Charsets.UTF_8)), new ObjectMetadata()) then: - def listing = client.listObjects("test") + def listing = client.listObjects(bucketName) def summaries = listing.getObjectSummaries() summaries.size() == 2 - summaries.get(0).getKey() == "Eins" - summaries.get(1).getKey() == "Zwei" + summaries.get(0).getKey() == key1 + summaries.get(1).getKey() == key2 } def "PUT and then DELETE work as expected"() { given: + def bucketName = DEFAULT_BUCKET_NAME + def key = DEFAULT_KEY def client = getClient() when: - if (!client.doesBucketExist("test")) { - client.createBucket("test") + if (!client.doesBucketExist(bucketName)) { + client.createBucket(bucketName) } and: client.putObject( - "test", - "test", + bucketName, + key, new ByteArrayInputStream("Test".getBytes(Charsets.UTF_8)), new ObjectMetadata()) - client.deleteBucket("test") - client.getObject("test", "test") + client.deleteBucket(bucketName) + client.getObject(bucketName, key) then: AmazonS3Exception e = thrown() e.statusCode == 404 } def "MultipartUpload and then GET work as expected"() { - when: - def bucketName = "test" - def key = "key/with/slashes" + given: + def bucketName = DEFAULT_BUCKET_NAME + def key = DEFAULT_KEY def client = getClient() + when: def transfer = TransferManagerBuilder.standard(). withS3Client(client). withMultipartUploadThreshold(1). @@ -205,10 +224,11 @@ abstract class BaseAWSSpec extends BaseSpecification { } def "MultipartUpload and then DELETE work as expected"() { - when: - def bucketName = "test" - def key = "key/with/slashes" + given: + def bucketName = DEFAULT_BUCKET_NAME + def key = DEFAULT_KEY def client = getClient() + when: def transfer = TransferManagerBuilder.standard(). withS3Client(client). withMultipartUploadThreshold(1). @@ -232,15 +252,17 @@ abstract class BaseAWSSpec extends BaseSpecification { def "PUT on presigned URL without signed chunks works as expected"() { given: + def bucketName = DEFAULT_BUCKET_NAME + def key = DEFAULT_KEY def client = getClient() when: - if (!client.doesBucketExist("test")) { - client.createBucket("test") + if (!client.doesBucketExist(bucketName)) { + client.createBucket(bucketName) } and: def content = "NotSigned" and: - GeneratePresignedUrlRequest putRequest = new GeneratePresignedUrlRequest("test", "test", HttpMethod.PUT) + GeneratePresignedUrlRequest putRequest = new GeneratePresignedUrlRequest(bucketName, key, HttpMethod.PUT) HttpURLConnection hc = new URL(getClient().generatePresignedUrl(putRequest).toString()).openConnection() hc.setDoOutput(true) hc.setRequestMethod("PUT") @@ -252,7 +274,7 @@ abstract class BaseAWSSpec extends BaseSpecification { } hc.getResponseCode() and: - GeneratePresignedUrlRequest request = new GeneratePresignedUrlRequest("test", "test") + GeneratePresignedUrlRequest request = new GeneratePresignedUrlRequest(bucketName, key) URLConnection c = new URL(getClient().generatePresignedUrl(request).toString()).openConnection() and: String downloadedData = new String(ByteStreams.toByteArray(c.getInputStream()), Charsets.UTF_8) @@ -263,22 +285,24 @@ abstract class BaseAWSSpec extends BaseSpecification { // reported in https://github.com/scireum/s3ninja/issues/153 def "PUT and then GET on presigned URL with ResponseHeaderOverrides works as expected"() { given: + def bucketName = DEFAULT_BUCKET_NAME + def key = DEFAULT_KEY def client = getClient() when: - if (!client.doesBucketExist("test")) { - client.createBucket("test") + if (!client.doesBucketExist(bucketName)) { + client.createBucket(bucketName) } and: client.putObject( - "test", - "test", + bucketName, + key, new ByteArrayInputStream("Test".getBytes(Charsets.UTF_8)), new ObjectMetadata()) def content = new String( - ByteStreams.toByteArray(client.getObject("test", "test").getObjectContent()), + ByteStreams.toByteArray(client.getObject(bucketName, key).getObjectContent()), Charsets.UTF_8) and: - GeneratePresignedUrlRequest request = new GeneratePresignedUrlRequest("test", "test") + GeneratePresignedUrlRequest request = new GeneratePresignedUrlRequest(bucketName, key) .withExpiration(Date.from(Instant.now().plus(1, ChronoUnit.HOURS))) .withResponseHeaders( new ResponseHeaderOverrides() From a95aec1ed1c17cebb0d83471a2d1fe3437e1858a Mon Sep 17 00:00:00 2001 From: Jakob Vogel Date: Sun, 6 Jun 2021 15:56:55 +0200 Subject: [PATCH 5/7] =?UTF-8?q?Uses=20more=20exotic=20keys=20for=20testing?= =?UTF-8?q?=20=F0=9F=A7=AA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/test/java/BaseAWSSpec.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/BaseAWSSpec.groovy b/src/test/java/BaseAWSSpec.groovy index 7b1df8f..2ea2bde 100644 --- a/src/test/java/BaseAWSSpec.groovy +++ b/src/test/java/BaseAWSSpec.groovy @@ -25,7 +25,7 @@ abstract class BaseAWSSpec extends BaseSpecification { def DEFAULT_BUCKET_NAME = "test" - def DEFAULT_KEY = "key/with/slashes" + def DEFAULT_KEY = "key/with/slashes and spaces 😇" abstract AmazonS3Client getClient() From 3d7206e63ecb5aa92b0bf9e0c55a76a62794dd6c Mon Sep 17 00:00:00 2001 From: Jakob Vogel Date: Sun, 6 Jun 2021 15:58:11 +0200 Subject: [PATCH 6/7] =?UTF-8?q?Revises=20documentation=20comment=20?= =?UTF-8?q?=E2=9C=8D=EF=B8=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/main/java/ninja/AwsHashCalculator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/ninja/AwsHashCalculator.java b/src/main/java/ninja/AwsHashCalculator.java index 015f3ba..81baeab 100644 --- a/src/main/java/ninja/AwsHashCalculator.java +++ b/src/main/java/ninja/AwsHashCalculator.java @@ -19,8 +19,8 @@ /** * Class in charge of generating the appropriate hash for the given request and path prefix by * delegating the computation to either {@link Aws4HashCalculator} or {@link - * AwsLegacyHashCalculator} depending of whether or not Aws4HashCalculator supports the request - * or not + * AwsLegacyHashCalculator} depending of whether or not {@code Aws4HashCalculator} supports the + * request or not. */ @Register(classes = AwsHashCalculator.class) public class AwsHashCalculator { From 3d8626c25f6d5d594fad2497f6d5b8d35a1b56e4 Mon Sep 17 00:00:00 2001 From: Jakob Vogel Date: Sun, 6 Jun 2021 15:58:43 +0200 Subject: [PATCH 7/7] =?UTF-8?q?Fixes=20URI=20extraction=20for=20signature?= =?UTF-8?q?=20computation=20=F0=9F=91=A8=E2=80=8D=E2=9A=95=EF=B8=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/main/java/ninja/S3Dispatcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/ninja/S3Dispatcher.java b/src/main/java/ninja/S3Dispatcher.java index 75dd3eb..070dbd1 100644 --- a/src/main/java/ninja/S3Dispatcher.java +++ b/src/main/java/ninja/S3Dispatcher.java @@ -270,7 +270,7 @@ public static String getEffectiveURI(WebContext webContext) { uri = uri.substring(1); } - return uri; + return Strings.urlEncode(uri).replace("+", "%20").replace("%2F", "/"); } /**