fix(apiquery): reject complex elements in comma arrays - #118
fix(apiquery): reject complex elements in comma arrays#118sylvesterkaczmarek 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: 0dbb1d8b19
ℹ️ 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".
| } | ||
| for _, pair := range innerPairs { | ||
| elements = append(elements, pair.value) | ||
| if len(innerPairs) != 1 || innerPairs[0].key != "" { |
There was a problem hiding this comment.
When an element is itself an array or slice, the recursive call returns exactly one unkeyed pair, so this condition accepts it; for example, [][]string{{"a", "b"}} is still flattened to a,b, losing the nested-array boundary instead of producing the promised complex-element error. Check the element's dereferenced kind before recursion, and add nested-array regression coverage alongside the map case.
AGENTS.md reference: AGENTS.md:L65-L68
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f50b438. Comma-array encoding now rejects dereferenced array, slice, map, and struct elements before recursive encoding. Added nested-array regression coverage; focused apiquery tests pass.
There was a problem hiding this comment.
Fixed in f50b438. Comma-form encoding now rejects nested slices/arrays before recursive encoding, so their boundaries cannot be flattened accidentally. Added regression coverage for nested arrays while retaining the existing primitive-array behavior. go test ./internal/apiquery passes.
|
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
Reject complex/keyed elements in comma-form query arrays instead of silently discarding their keys and flattening only their values.
Problem
internal/apiquery.encodeArraycurrently handlesArrayQueryFormatCommaby recursively encoding each array element and appending every returned pair's value to the comma-delimited list. For map/object elements, that means the nested query keys are lost.For example, a value shaped like:
can be reduced to a query equivalent to
filter=alice, which no longer represents the original object structure and gives the caller no indication that serialization changed its meaning.Fix
Require each comma-form element to encode to exactly one unkeyed scalar pair. If recursive encoding produces keyed or multiple pairs, return a clear
apiqueryerror instead of emitting a corrupted query parameter.Primitive comma arrays keep their existing behavior.
Regression coverage
Added focused tests proving that:
[]stringstill encodes asalice,bob.Validation
The branch is based directly on current upstream
main(ee92673a416c0851a5fe8907a2453db7bd450633) and contains one commit touching onlyinternal/apiquery/encoder.goplus focused regression coverage. Full Go test execution is left to repository CI.Risk
Low. The newly rejected inputs are shapes whose key structure cannot be represented by the comma-array encoding. Existing primitive comma arrays and the repeat/indices/brackets array formats are unchanged.