fix(ssdk): route on path and method only, not required query params#1917
Open
TrevorBurnham wants to merge 1 commit intosmithy-lang:mainfrom
Open
fix(ssdk): route on path and method only, not required query params#1917TrevorBurnham wants to merge 1 commit intosmithy-lang:mainfrom
TrevorBurnham wants to merge 1 commit intosmithy-lang:mainfrom
Conversation
Required @httpquery parameters were included in UriSpec query segments, causing HttpBindingMux to reject requests missing those params with UnknownOperationException instead of letting validation produce ValidationException. - UriSpec.match() now skips 'query' type segments during routing, only enforcing 'query_literal' segments for operation disambiguation - UriSpec.rank only counts query_literal segments - Codegen no longer emits required @httpquery members as query segments Fixes smithy-lang#1840
This file contains hidden or 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
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.
Fixes #1840
Description of changes:
When a Smithy operation has a
@required@httpQueryparameter, the generated SSDK throwsUnknownOperationExceptioninstead ofValidationExceptionwhen the parameter is missing from the request.Root cause
The codegen emits required
@httpQuerymembers as{ type: 'query' }segments in theUriSpecpassed toHttpBindingMux. During routing,UriSpec.match()requires all query segments (including these) to be present in the request. When a required query param is missing, the mux fails to match any operation and the handler throwsUnknownOperationException.Meanwhile, the SSDK already generates proper
RequiredValidatorchecks for these members, but that validation runs after routing, so it never executes when the query param is absent.Fix
Two complementary changes ensure required query params are validated, not routed on:
@aws-smithy/server-common(runtime)UriSpec.match()now filters query segments to only enforcequery_literaltypes (URI query literals like?action=Foo) during routing.querytype segments (from@required @httpQuerymembers) are skipped — they don't participate in operation matching. Therankcalculation was updated accordingly to only countquery_literalsegments for routing priority.This also makes the runtime backward-compatible with already-generated code: existing SSDKs that include
querysegments in their mux will now simply ignore them during routing instead of using them to reject requests.smithy-typescript-codegen(codegen)HttpBindingProtocolGenerator.generateUriSpec()no longer emits required@httpQuerymembers as query segments. Only URI query literals (fromhttpTrait.getUri().getQueryLiterals()) are included. This means newly generated code won't include the unnecessary segments at all.Before
After
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.