You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by "abh1sar (via GitHub)" <gi...@apache.org> on 2026/08/04 16:03:26 UTC
[PR] Fix panic when serializing UUID-typed request parameters [cloudstack-go]
abh1sar opened a new pull request, #160:
URL: https://github.com/apache/cloudstack-go/pull/160
## Summary
`generateConvertCode` in the generator treats the generated `UUID` Go type the same as `string` when building `toURLValues()`, emitting `v.(string)` for a value that is actually stored as `cloudstack.UUID`. Since Go type assertions require an exact dynamic type match, this panics at runtime for any parameter whose Go type is `UUID`.
Currently `managementserverid` is the only parameter routed through the `UUID` type (via the `longToStringConvertedParams` map), and it's used on `listAsyncJobs`, `listHosts`, `listHostsMetrics`, `triggerShutdown`, `cancelShutdown`, `prepareForShutdown`, `listWebhookDeliveries`, and `deleteWebhookDelivery`. This has been dormant so far because `managementserverid` is optional on all of these commands, so the setter is never exercised by the standard generated tests, but it panics on every real call that actually sets it.
Fix: assert the value as `UUID` and convert to `string`, instead of asserting it as `string`:
```go
case "string":
pn("u.Set(\"%s\", v.(string))", name)
case "UUID":
pn("u.Set(\"%s\", string(v.(UUID)))", name)
```
Regenerated the affected service files (`AsyncjobService.go`, `HostService.go`, `ManagementService.go`, `WebhookService.go`); no other diff.
## Manual testing
- `go build ./...`: clean.
- `go test ./test/... ./examples/...`: all 151 existing tests pass, no regressions.
- `make mocks`: zero diff, mock signatures unaffected.
- Added a temporary local test directly exercising `TriggerShutdownParams.toURLValues()` with a `UUID`-typed `managementserverid` value: panicked before this fix, passed and returned the correct string after. Not committed, since it required an unexported-method test in the `cloudstack` package outside the generated pattern.
## Test plan
- [ ] CI passes
- [ ] Reviewer confirms no other parameters currently route through the `UUID` type
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix panic when serializing UUID-typed request parameters [cloudstack-go]
Posted by "abh1sar (via GitHub)" <gi...@apache.org>.
abh1sar merged PR #160:
URL: https://github.com/apache/cloudstack-go/pull/160
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix panic when serializing UUID-typed request parameters [cloudstack-go]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #160:
URL: https://github.com/apache/cloudstack-go/pull/160#discussion_r3714295484
##########
cloudstack/ManagementService.go:
##########
@@ -838,7 +838,7 @@ func (p *ReadyForShutdownParams) toURLValues() url.Values {
return u
}
if v, found := p.p["managementserverid"]; found {
- u.Set("managementserverid", v.(string))
+ u.Set("managementserverid", string(v.(UUID)))
Review Comment:
Consider using `v.(UUID).String()` (if `UUID` implements `fmt.Stringer`) instead of `string(v.(UUID))`. It’s more idiomatic and safer if the underlying `UUID` representation changes away from being a `string` alias. If `UUID` is intentionally a `type UUID string`, current code is fine, but this couples serialization format to the underlying type.
##########
generate/generate.go:
##########
@@ -1355,8 +1355,10 @@ func (s *service) generateConvertCode(cmd, name, typ string) {
pn := s.pn
switch typ {
- case "string", "UUID":
+ case "string":
pn("u.Set(\"%s\", v.(string))", name)
+ case "UUID":
+ pn("u.Set(\"%s\", string(v.(UUID)))", name)
Review Comment:
The generator is now encoding knowledge of `UUID` as a special-case string-like type. If there are (or will be) more string-backed custom types, this can become a growing list of ad-hoc cases. Consider centralizing \"string-like\" conversions (e.g., via a helper that maps Go types to serialization expressions) so future additions don’t require touching switch logic directly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org