fix(apiform): support primitive pointers in comma arrays - #119
fix(apiform): support primitive pointers in comma arrays#119sylvesterkaczmarek wants to merge 2 commits into
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfc696f92e
ℹ️ 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
- 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 address that feedback".
| item = reflect.Value{} | ||
| break | ||
| } | ||
| item = item.Elem() |
There was a problem hiding this comment.
When a comma-form array contains a non-nil *float32, this new unwrapping path reaches the existing combined float case, which calls strconv.FormatFloat with a 64-bit size. Consequently, a value such as float32(1.2) is sent as 1.2000000476837158 rather than 1.2, unlike the scalar encoder's dedicated Float32 handling. Distinguish reflect.Float32 from reflect.Float64 when serializing these newly supported pointer elements.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 877e5ff. Pointer-unwrapped float32 values now use 32-bit formatting, with a regression for float32(1.2) producing 1.2. Focused apiform tests pass.
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
2 participants
Summary
Dereference non-nil pointer/interface elements when encoding multipart arrays with
FormatComma, while preserving the existing empty-string representation for nil elements.Problem
internal/apiform.encodeArrayalready special-cases nil pointer/interface elements in comma-form arrays, but non-nil pointers are never dereferenced before the primitive-type switch.As a result, a valid primitive array such as:
fails with:
even though the pointed-to values are ordinary strings and the same encoder supports primitive values directly.
Fix
Unwrap pointer and interface layers before primitive comma serialization. If a nil value is encountered at any layer, keep the existing behavior of adding an empty comma element.
Complex non-primitive elements continue to return the same error as before.
Regression coverage
Added a focused multipart regression for
[]*string{&first, nil, &second}and verify it serializes as:Validation
The branch is based directly on current upstream
main(ee92673a416c0851a5fe8907a2453db7bd450633) and contains one commit touching onlyinternal/apiform/encoder.goplus focused regression coverage. Full Go test execution is left to repository CI.Risk
Low. The change only affects pointer/interface elements inside
FormatCommaarrays. Direct primitives, nil elements, complex-element rejection, and all other multipart array formats retain their existing behavior.