S3 bucket slashes #1587
S3 bucket slashes #1587
Conversation
…an the s3 client it is called on
…rds compatibility with sigv2.
Codecov Report
@@ Coverage Diff @@
## master #1587 +/- ##
==========================================
+ Coverage 95.56% 95.56% +<.01%
==========================================
Files 183 183
Lines 6469 6478 +9
Branches 1325 1329 +4
==========================================
+ Hits 6182 6191 +9
Misses 287 287
Continue to review full report at Codecov.
|
| req.params.Key = prefix + '/' + key; | ||
| req.params.Bucket = bucket.substr(0, slashIndex); | ||
| } else { | ||
| var msg = 'Bucket names cannot contain forward slashes. Bucket: ' + bucket; |
There was a problem hiding this comment.
Won't this run into the same BC issue as throwing whenever someone uses a bucket + prefix instead of just a bucket name? Do we currently allow users to provide a bucket name of foo/bar and no key when they want to access the key bar in the bucket foo?
There was a problem hiding this comment.
If a user did not supply a key at all, then they would receive a missing required param error. That said, it is possible to turn off param validation so maybe I should just make sure Key exists as a possible parameter.
There was a problem hiding this comment.
That sounds like a fairly extreme edge case. I'd be fine with leaving as is.
| Scenario: Sigv2 Bucket with trailing slashes with Key | ||
| Given I use signatureVersion "v2" | ||
| When I put "" to the key "foo" with bucket suffix "/a/b/" | ||
| Then the object "a/b//foo" should exist |
There was a problem hiding this comment.
We can't change it, but this behavior does not seem completely intuitive. Bash at least will treat one or more slashes in a sequence as a single directory separator. Just something to keep in mind for any future high-level interfaces, especially those that allow/encourage users to interact with S3 as if it were a filesystem.
There was a problem hiding this comment.
Agreed, and I can't imagine this would be desired behavior. I wonder if other SDKs collapse multiple slashes into one.
test/services/s3.spec.js
Outdated
| @@ -205,11 +205,62 @@ | |||
| done(); | |||
| }); | |||
| }); | |||
| it('using sigv4 and the "Bucket" parameter contains forward slashes and "Key" exists', function(done) { | |||
There was a problem hiding this comment.
Copy paste error, I'll fix that.
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread. |
Updates
s3.uploadto use the same signature version as the client calling it.Also allows forward slashes in S3 bucket names when an operation supports a Bucket and Key parameter. This is to maintain backwards compatibility with code written when signature version 2 was the default.
/cc @jeskew