microcks / microcks/microcks-cli

pkg/util/rand helpers panic on invalid input instead of returning errors

Aperta Adatta ai principianti
#359 2 commenti 0 reazioni 0 assegnatari Vedi su GitHub

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

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. 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

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.