Fix panic binding numeric values into non-byte slice destinations - #156
Merged
Conversation
Closes: oapi-codegen#155 The reflect.Slice case handled the base64 `format: byte` path and then fell through, with a comment claiming it landed in the default error case. It did not: the next case is the integer one, so a non-byte slice destination reached v.OverflowInt on a slice reflect.Value and panicked. Only sources that parse as an integer got that far — ParseInt failed first for anything else, masking the bug behind a plausible-looking error. This was reachable from generated server code. A nullable slice query parameter using the default form/explode serialization binds through the primitive path, and the nullable wrapper then binds the raw value into a fresh slice, so `?p=123` panicked while `?p=abc` returned a binding error. Replace the fallthrough with the explicit unhandled-type error, which restores the pre-v1.2.0 behavior (the fallthrough came in with 224825a, first released in v1.2.0) and makes the comment true. A []byte destination without Format "byte" took the same panicking path and now reports the same error; that path never bound successfully, so nothing could have depended on it. Regression tests pin both the numeric and non-numeric sources against []string, []int and []byte, plus the nullable.Nullable[[]string] exploded-query route through both BindQueryParameter and BindRawQueryParameter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Closes: #155
The reflect.Slice case handled the base64
format: bytepath and then fell through, with a comment claiming it landed in the default error case. It did not: the next case is the integer one, so a non-byte slice destination reached v.OverflowInt on a slice reflect.Value and panicked. Only sources that parse as an integer got that far — ParseInt failed first for anything else, masking the bug behind a plausible-looking error.This was reachable from generated server code. A nullable slice query parameter using the default form/explode serialization binds through the primitive path, and the nullable wrapper then binds the raw value into a fresh slice, so
?p=123panicked while?p=abcreturned a binding error.Replace the fallthrough with the explicit unhandled-type error, which restores the pre-v1.2.0 behavior (the fallthrough came in with 224825a, first released in v1.2.0) and makes the comment true. A []byte destination without Format "byte" took the same panicking path and now reports the same error; that path never bound successfully, so nothing could have depended on it.
Regression tests pin both the numeric and non-numeric sources against []string, []int and []byte, plus the nullable.Nullable[[]string] exploded-query route through both BindQueryParameter and BindRawQueryParameter.