Allowing keys with leading slashes for S3 #1264
Conversation
|
Changelog? |
| @@ -37,7 +37,7 @@ | |||
| * so that it preserves the path structure. | |||
| */ | |||
| public static final XmlMarshaller<String> GREEDY_STRING = | |||
| new SimplePathMarshaller<>(ValueToStringConverter.FROM_STRING, PathMarshaller.GREEDY); | |||
| new SimplePathMarshaller<>(ValueToStringConverter.FROM_STRING, PathMarshaller.GREEDY_WITH_SLASHES); | |||
There was a problem hiding this comment.
What services are affected by this change? How certain are we that they wouldn't be broken by this?
There was a problem hiding this comment.
This only effects XML services of which none of them currently use the greedy path marshaller.
There was a problem hiding this comment.
Other than S3 that is.
| public class KeysWithLeadingSlashIntegrationTest extends S3IntegrationTestBase { | ||
|
|
||
| private static final String BUCKET = temporaryBucketName(KeysWithLeadingSlashIntegrationTest.class); | ||
| private static final String KEY = "/stupidkeywithillegalleadingslashthatsucks"; |
There was a problem hiding this comment.
No passive aggression here. Nope.
There was a problem hiding this comment.
Just a random key name I swear.
|
I don't like to change the protocol tests as I've done here but I think in this case it's fine. I've confirmed no other XML service uses the greedy path besides S3 and this is the correct behavior for S3. We shouldn't see any new services use XML. It could effect existing services if they add operations with a greedy path in the future. At that point we could switch this to be an S3 only customization but that would introduce a significant amount of additional handwritten S3 code and a bunch of new codegen customizations. |
Codecov Report
@@ Coverage Diff @@
## master #1264 +/- ##
============================================
+ Coverage 70.13% 70.15% +0.01%
Complexity 188 188
============================================
Files 776 776
Lines 24122 24129 +7
Branches 1799 1799
============================================
+ Hits 16919 16927 +8
Misses 6419 6419
+ Partials 784 783 -1
Continue to review full report at Codecov.
|
Dongie/pr revert
S3 allows keys to begin with slashes for unknown reasons. This change allows those keys to work with the SDK and our HTTP clients.
Testing
Added integration test
Types of changes
Checklist
mvn installsucceedsLicense