fix: json ShapeDeserializer panic on union nested directly inside a union#672
Closed
ryunosuke121 wants to merge 1 commit into
Closed
fix: json ShapeDeserializer panic on union nested directly inside a union#672ryunosuke121 wants to merge 1 commit into
ryunosuke121 wants to merge 1 commit into
Conversation
…nion
ReadUnion decided whether it was resuming an in-progress union read by
checking whether the top of the context stack was ctxUnion. When a union
member's value is itself a union, the inner ReadUnion call saw the
parent's ctxUnion, wrongly resumed, skipped the inner '{' and passed the
1-byte token to memberFromToken, panicking with
"slice bounds out of range [1:0]".
Disambiguate by peeking: at a value position the next token can only be
'{' (or null), while on resume it can only be a member name or '}' --
disjoint sets given the parser's grammar state machine.
Also:
- memberFromToken: validate the token shape instead of panicking.
- smithy.ReadUnion helper: stop reading when the first ReadUnion call
returns no member (union value was null/empty/all-null members);
reading again consumed tokens past the union.
Fixes aws#671
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
thanks for the patch but closing in favor of #673 which solves this a little more directly |
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
2 participants
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 #671
Problem
json.(*ShapeDeserializer).ReadUnionpanics withslice bounds out of range [1:0]when a union member's value is itself a union (e.g.bedrockagentcorecontrolGetGatewayTarget'sTargetConfiguration→mcp→McpTargetConfiguration). This currently crashes terraform-provider-aws v6.48.0 on every refresh ofaws_bedrockagentcore_gateway_target.ReadUniondetected "resume after reading a member value" by checking whether the top of the context stack isctxUnion. For a union nested directly inside a union, the inner call sees the parent'sctxUnion, wrongly resumes, skips the inner'{', and passes that 1-byte token tomemberFromToken→ panic.Fix
ReadUnion: when the top of the stack isctxUnion, disambiguate by peeking. At a value position the next token can only be'{'(ornull); on resume it can only be a member name ('"') or'}'. The parser's grammar state machine guarantees these sets are disjoint, so the peek is a sound discriminator. Behavior is unchanged for all non-nested cases.memberFromToken: validate the token shape and return an error instead of panicking (defense in depth).smithy.ReadUnionhelper (serde.go): when the firstd.ReadUnionreturns no member (union value wasnull,{}, or contained only null members), the union has already been fully consumed — callingd.ReadUnionagain read tokens past the union and corrupted the stream. Return early instead.Testing
shape_deserializer_union_test.go:nullvalue (read past the union before this change)go test ./...passes across the repository (includes the JSONTestSuite corpus and fuzz seed corpora).By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Co-Authored-By: Claude Opus 4.8 (1M context)