Guard Get<Entity>ID helpers against count/empty-list panic (#103)#152
Open
jmsperu wants to merge 1 commit into
Open
Guard Get<Entity>ID helpers against count/empty-list panic (#103)#152jmsperu wants to merge 1 commit into
jmsperu wants to merge 1 commit into
Conversation
The generated Get<Entity>ID and Get<Entity>ByID helpers index the result slice at [0] as soon as Count == 1, without checking the slice is non-empty. When the API reports count >= 1 but returns an empty list (a count/slice mismatch seen e.g. on the metrics APIs), this panics with 'index out of range [0] with length 0' (issue apache#103). Fix the generator template (generate/generate.go) to emit a length guard before the [0] access, returning the same 'No match found' error already used for the count == 0 case, and apply it to the generated helpers (additive only). Adds a regression test that panics on the unpatched code and passes with the guard. Signed-off-by: James Peru <james@xcobean.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
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 #103.
Problem
The generated
Get<Entity>ID/Get<Entity>ByIDhelpers index the result slice at[0]as soon asCount == 1, without checking that the slice is actually non-empty:When the backend reports
count >= 1but returns an empty list — a count/slice mismatch that occurs e.g. on the metrics APIs — this panics:This is the panic reported in #103 (it surfaced via CAPC calling
GetVirtualMachinesMetricID). As @rajeshvenkata suggested in the issue, the helper should check the slice before accessing it.Fix
The helpers are generated, so the root-cause fix is in the generator template (
generate/generate.go, both theGet<Entity>IDandGet<Entity>ByIDemitters). A length guard is emitted before the[0]access, returning the sameNo match founderror already used for thecount == 0case. The guard is then applied to the generated helpers — additive only (212 guards across 64 files; no other lines change), so the diff stays reviewable and a future regeneration reproduces it.Test
Adds
test/GetMetricIDRegression_test.go, a self-contained regression test that serves{"count":1,"virtualmachine":[]}and assertsGetVirtualMachinesMetricIDreturns an error rather than panicking. It panics on the unpatched code (reproducing #103) and passes with the guard.Verification
go build ./...— cleango test ./test/ -run TestGetVirtualMachinesMetricIDEmptyListWithNonZeroCount— passes (and fails with the exactindex out of range [0]panic when the guard is reverted)Note
The count/empty-list mismatch this guards against is the same family as the metrics JSON-tag issues (#135/#136, PR #137): when a response key does not match the struct tag,
countparses but the slice isnil. This guard makes the helpers robust to that regardless of root cause.