microcks / microcks/microcks-cli
pkg/util/rand helpers panic on invalid input instead of returning errors
Nessuno ha ancora preso questa issue.
- Lingua principale
- Go
- Stelle
- 52
- Fork
- 68
- Merge medio
- 6h 54m
- PR unite (30g)
- 10
Descrizione
### 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.
Guida per i contributori
Apri la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Direzione di ricerca
Inizia da pkg/util/rand/rand.go, concentrandoti su StringFromCharset e sul wrapper String, quindi esamina i casi di input non valido proposti in questa issue. Aggiungi pkg/util/rand/rand_test.go con test basati su tabelle per lunghezze negative, set di caratteri vuoti, lunghezza zero e input validi. Il lavoro è completato quando gli input non validi restituiscono errori descrittivi senza causare un panic e il comportamento esistente per gli input validi rimane invariato.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Valutazione
- Stack tecnologico
- go
- Ambito
- cli
- Tipo di issue
- Bug
- Difficoltà
- 2/5
- Tempo stimato
- 1-3 ore
- Stato di attività
- Tranquilla
- Chiarezza
- Specificata chiaramente
- Idoneità per principianti
- 78/100