-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: avoid panic in StructToFlatMap #3248
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codex Review: Here are some suggestions.
Reply with @codex fix comments
to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
// If the pointer is to a struct, flatten it as well | ||
if field.Elem().Kind() == reflect.Struct { | ||
nestedMap := StructToFlatMap(field.Elem().Interface()) | ||
for k, v := range nestedMap { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] Guard against cycles when flattening pointer fields
The new pointer-handling block recurses into struct pointers by calling StructToFlatMap(field.Elem().Interface())
without tracking visited nodes. If the input struct contains cyclic references (for example a self-referencing linked list or a child struct pointing back to its parent), this will now recurse forever and overflow the stack. The prior version simply stored the pointer value and did not recurse, so cyclic data could be processed; the change therefore introduces a crash on cyclic inputs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a panic in the StructToFlatMap
function that occurred when processing array fields and improves handling of pointer fields by adding proper flattening support.
- Removes array type from the nil check condition to prevent panic on array fields
- Adds dedicated pointer field handling with struct flattening capability
- Includes comprehensive test coverage for array fields, pointer structs, and nil pointers
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
pkg/conv/conv.go | Refactored pointer and array handling logic to prevent panics and add pointer struct flattening |
pkg/conv/conv_test.go | Added test cases covering array fields, pointer structs, and nil pointer scenarios |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Summary
Testing
go test ./...
(fails: pattern public/: no matching files found)*go test ./pkg/conv -count=1
https://chatgpt.com/codex/tasks/task_e_68b30016307c8325a0f748cf6002d175