aws-smithy-http-server: don't ignore empty path segments when routing
#1029
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
In #996 [0], we realized that we were ignoring empty URI path segments when doing label extraction. It turns out we are also ignoring them when doing routing, as the TypeScript sSDK currently does [1], from which the initial implementation was copied. However, a discussion with the Smithy team in smithy-lang/smithy#1024 [2] revealed that we _must not_ ignore empty URI path segments when routing or doing label extraction, since empty strings (`""`) should be assigned to the labels in those segments. This commit fixes the behavior so that we don't ignore empty path segments _when doing routing_. #996 will take care of fixing the behavior when doing label extraction. [0]: #996 (comment) [1]: https://github.com/awslabs/smithy-typescript/blob/d263078b81485a6a2013d243639c0c680343ff47/smithy-typescript-ssdk-libs/server-common/src/httpbinding/mux.ts#L78 [2]: smithy-lang/smithy#1024
|
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
|
|
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
|
|
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
|
|
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
|
david-perez
added a commit
that referenced
this pull request
Jan 13, 2022
…ce test
The previous commit introduced a failing test because CI doesn't have
Rust tests as a blocking step and auto-merge was enabled.
With the two operations:
1. `ListBuckets` with URI pattern `/`; and
2. `ListObjects` with URI pattern `/{bucket}`,
the test wants to route the request `/` to the first operation. However,
in #1029 we started allowing empty path segments. Since `ListBuckets`
has more weight than `ListObjects`, the request matches against it,
binding `""` to the `bucket` label.
david-perez
added a commit
that referenced
this pull request
Jan 13, 2022
…ce test (#1070) The previous commit introduced a failing test because CI doesn't have Rust tests as a blocking step and auto-merge was enabled. With the two operations: 1. `ListBuckets` with URI pattern `/`; and 2. `ListObjects` with URI pattern `/{bucket}`, the test wants to route the request `/` to the first operation. However, in #1029 we started allowing empty path segments. Since `ListBuckets` has more weight than `ListObjects`, the request matches against it, binding `""` to the `bucket` label.
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.
In #996 0, we realized that we were ignoring empty URI path segments
when doing label extraction. It turns out we are also ignoring them when
doing routing, as the TypeScript sSDK currently does 1, from which the
initial implementation was copied.
However, a discussion with the Smithy team in smithy-lang/smithy#1024 2
revealed that we must not ignore empty URI path segments when routing
or doing label extraction, since empty strings (
"") should be assignedto the labels in those segments.
This commit fixes the behavior so that we don't ignore empty path
segments when doing routing. #996 will take care of fixing the behavior
when doing label extraction.
Testing
Test suite has been expanded.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.