microcks / microcks/microcks-cli
pkg/util/rand helpers panic on invalid input instead of returning errors
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 52
- Forks
- 68
- Avg merge
- 6h 54m
- Merged PRs (30d)
- 10
Description
### Describe the bug
`pkg/util/rand` provides two helpers — `String` and `StringFromCharset` — that both return `(string, error)`. The signature implies invalid inputs should surface as a Go `error`, but some invalid inputs currently cause the process to panic instead:
- `StringFromCharset(1, "")` panics in `crypto/rand.Int` because the upper bound is `<= 0`.
- `String(-1)` panics in `make([]byte, -1)` because the slice length is negative.
Since both functions already declare an `error` return, callers naturally expect that path to be used. Today they cannot recover — the helpers crash the host process before any caller code can react.
---
### Expected behavior
Invalid inputs should return a clear error instead of panicking:
```text
StringFromCharset(1, "") -> "", error("rand: cannot generate ... from empty charset")
String(-1) -> "", error("rand: requested length -1 is negative")
```
`n == 0` should keep returning `("", nil)` regardless of the charset, since that's the natural no-op result for both helpers.
---
### Actual behavior
`StringFromCharset(1, "")` panics with:
```text
panic: crypto/rand: argument to Int is <= 0
```
`String(-1)` panics with:
```text
panic: runtime error: makeslice: len out of range
```
In both cases the process crashes; the `error` return value is never set.
---
### How to Reproduce?
Reproduced locally on the latest `master`.
**Reproducer for empty charset:**
```go
package main
import (
"fmt"
r "github.com/microcks/microcks-cli/pkg/util/rand"
)
func main() {
fmt.Println("Testing empty charset...")
value, err := r.StringFromCharset(1, "")
fmt.Printf("value=%q err=%v\n", value, err)
}
```
Output:
```text
Testing empty charset...
panic: crypto/rand: argument to Int is <= 0
```
**Reproducer for negative length:**
```go
package main
import (
"fmt"
r "github.com/microcks/microcks-cli/pkg/util/rand"
)
func main() {
fmt.Println("Testing negative length...")
value, err := r.String(-1)
fmt.Printf("value=%q err=%v\n", value, err)
}
```
Output:
```text
Testing negative length...
panic: runtime error: makeslice: len out of range
```
In both cases the program exits via panic before the `err=` line can print.
---
### Microcks version or git rev
_No response_
---
### Install method (`docker-compose`, `helm chart`, `operator`, `docker-desktop extension`,...)
_No response_
---
### Additional information
**Root cause.** `StringFromCharset` in `pkg/util/rand/rand.go` does not validate `n` or `charset` before allocating the output slice and calling `crypto/rand.Int`:
```go
func StringFromCharset(n int, charset string) (string, error) {
b := make([]byte, n)
maxIdx := big.NewInt(int64(len(charset)))
for i := 0; i < n; i++ {
randIdx, err := rand.Int(rand.Reader, maxIdx)
...
}
...
}
```
- `n < 0` → `make([]byte, n)` panics with `makeslice: len out of range`.
- `n > 0` with `charset == ""` → `rand.Int(rand.Reader, big.NewInt(0))` panics with `argument to Int is <= 0`.
- `n == 0` → already works correctly (loop doesn't execute, returns `("", nil)`).
`String` calls `StringFromCharset` and inherits the negative-length panic.
**Proposed fix.** Add two guard clauses at the top of `StringFromCharset`:
```go
if n < 0 {
return "", fmt.Errorf("rand: requested length %d is negative", n)
}
if n > 0 && len(charset) == 0 {
return "", fmt.Errorf("rand: cannot generate %d-character string from empty charset", n)
}
```
This preserves every currently-working input (including `n == 0`) and turns both panic paths into descriptive errors. The public signatures stay the same, so no callers (including the SSO PKCE flow in `cmd/login.go`) need to change.
A small test file `pkg/util/rand/rand_test.go` would lock this in with table-driven cases for both valid and invalid inputs — the package currently has no test coverage.
I would like to send a small PR for this if the maintainers agree on the approach.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in pkg/util/rand/rand.go, focusing on StringFromCharset and the String wrapper, then review the proposed invalid-input cases in this issue. Add pkg/util/rand/rand_test.go with table-driven tests for negative lengths, empty charsets, zero length, and valid inputs. Done means invalid inputs return descriptive errors without panicking and existing valid behavior remains unchanged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- cli
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100