Add restXml protocol test for http label escaping #1759
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sugmanue
reviewed
May 4, 2023
| ], | ||
| params: { | ||
| Bucket: "mybucket", | ||
| Key: "my key.txt" |
There was a problem hiding this comment.
Can we add another test with a path in the key? Something like foo/bar/my key.txt. I'd expect this to be encoded as foo/bar/my%20key.txt and not foo%2Fbar%2Fmy%20key.txt, is that the case?
There was a problem hiding this comment.
Your expectation is correct and I agree that's a good test to add, thanks for calling it out. I pushed an update including that test.
milesziemer
force-pushed
the
add-restxml-http-label-encoding-test
branch
from
2609a30 to
674628e
Compare
sugmanue
approved these changes
May 4, 2023
Adds an httpRequestTests case to restXml protocol tests to verify that httpLabel bindings in the request URI are percent-encoded. restJson1 had an existing test case for this, but was updated to verify the space character is encoded. An operation was also added to the testing S3 service model with httpRequestTests to verify the same thing, but with S3's virtual host addressing. No other protocols had tests added, because restJson1 and restXml are the only protocols that support httpLabel bindings. Changes were tested by generating and running protocol tests in aws-sdk-go-v2 and aws-sdk-js-v3, confirming the new test case is generated and runs successfully.
milesziemer
force-pushed
the
add-restxml-http-label-encoding-test
branch
from
674628e to
12eb715
Compare
kstich
approved these changes
May 22, 2023
david-perez
added a commit
to smithy-lang/smithy-rs
that referenced
this pull request
Aug 1, 2023
smithy-lang/smithy#1562 was upstreamed and then the test was tweaked in smithy-lang/smithy#1759.
syall
pushed a commit
to Xtansia/smithy
that referenced
this pull request
Aug 11, 2023
Adds an httpRequestTests case to restXml protocol tests to verify that httpLabel bindings in the request URI are percent-encoded. restJson1 had an existing test case for this, but was updated to verify the space character is encoded. An operation was also added to the testing S3 service model with httpRequestTests to verify the same thing, but with S3's virtual host addressing. No other protocols had tests added, because restJson1 and restXml are the only protocols that support httpLabel bindings. Changes were tested by generating and running protocol tests in aws-sdk-go-v2 and aws-sdk-js-v3, confirming the new test case is generated and runs successfully.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
Description of changes:
Adds an httpRequestTests case to restXml protocol tests to verify that httpLabel bindings in the request URI are percent-encoded. restJson1 had an existing test case for this, but was updated to verify the space character is encoded. An operation was also added to the testing S3 service model with httpRequestTests to verify the same thing, but with S3's virtual host addressing. No other protocols had tests added, because restJson1 and restXml are the only protocols that support httpLabel bindings.
Changes were tested by generating and running protocol tests in aws-sdk-go-v2 and aws-sdk-js-v3, confirming the new test case is generated and runs successfully.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.