From d8be8c28bda03e0064dea7943714f68b8a2c8496 Mon Sep 17 00:00:00 2001 From: Mrinmoy Matilal Date: Sat, 25 Jul 2026 17:46:31 +0000 Subject: [PATCH 1/2] fix: handle empty charset in StringFromCharset and Added Unit test for rand.go Signed-off-by: Mrinmoy Matilal --- pkg/util/rand/rand.go | 4 +- pkg/util/rand/rand_test.go | 114 +++++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 pkg/util/rand/rand_test.go diff --git a/pkg/util/rand/rand.go b/pkg/util/rand/rand.go index 1e748bf9..a3a092e5 100644 --- a/pkg/util/rand/rand.go +++ b/pkg/util/rand/rand.go @@ -15,6 +15,9 @@ func String(n int) (string, error) { // StringFromCharset generates, from a given charset, a cryptographically-secure pseudo-random string of a given length. func StringFromCharset(n int, charset string) (string, error) { + if n > 0 && len(charset) == 0 { + return "", fmt.Errorf("charset must not be empty when n > 0") + } b := make([]byte, n) maxIdx := big.NewInt(int64(len(charset))) for i := 0; i < n; i++ { @@ -22,7 +25,6 @@ func StringFromCharset(n int, charset string) (string, error) { if err != nil { return "", fmt.Errorf("failed to generate random string: %w", err) } - // randIdx is necessarily safe to convert to int, because the max came from an int. randIdxInt := int(randIdx.Int64()) b[i] = charset[randIdxInt] } diff --git a/pkg/util/rand/rand_test.go b/pkg/util/rand/rand_test.go new file mode 100644 index 00000000..e9dfcd4d --- /dev/null +++ b/pkg/util/rand/rand_test.go @@ -0,0 +1,114 @@ +package rand + +import ( + "strings" + "testing" +) + +func TestString(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + length int + }{ + {name: "zero length", length: 0}, + {name: "single character", length: 1}, + {name: "short string", length: 8}, + {name: "longer string", length: 32}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, err := String(tt.length) + if err != nil { + t.Fatalf("String(%d) returned error: %v", tt.length, err) + } + if len(got) != tt.length { + t.Fatalf("String(%d) length = %d, want %d", tt.length, len(got), tt.length) + } + for i := 0; i < len(got); i++ { + if !strings.Contains(letterBytes, string(got[i])) { + t.Fatalf("String(%d) contains invalid character %q at index %d", tt.length, got[i], i) + } + } + }) + } +} + +func TestStringProducesDifferentValues(t *testing.T) { + t.Parallel() + + const length = 32 + + first, err := String(length) + if err != nil { + t.Fatalf("first String(%d) returned error: %v", length, err) + } + + second, err := String(length) + if err != nil { + t.Fatalf("second String(%d) returned error: %v", length, err) + } + + if first == second { + t.Fatalf("expected two generated strings to differ, both were %q", first) + } +} + +func TestStringFromCharset(t *testing.T) { + t.Parallel() + + const charset = "01" + + got, err := StringFromCharset(16, charset) + if err != nil { + t.Fatalf("StringFromCharset returned error: %v", err) + } + if len(got) != 16 { + t.Fatalf("StringFromCharset length = %d, want 16", len(got)) + } + for i := 0; i < len(got); i++ { + if !strings.Contains(charset, string(got[i])) { + t.Fatalf("StringFromCharset contains invalid character %q at index %d", got[i], i) + } + } +} + +func TestStringFromCharsetZeroLength(t *testing.T) { + t.Parallel() + + got, err := StringFromCharset(0, "abc") + if err != nil { + t.Fatalf("StringFromCharset(0) returned error: %v", err) + } + if got != "" { + t.Fatalf("StringFromCharset(0) = %q, want empty string", got) + } +} + +func TestStringFromCharsetSingleCharacter(t *testing.T) { + t.Parallel() + + got, err := StringFromCharset(10, "x") + if err != nil { + t.Fatalf("StringFromCharset returned error: %v", err) + } + if got != "xxxxxxxxxx" { + t.Fatalf("StringFromCharset with single-character charset = %q, want %q", got, "xxxxxxxxxx") + } +} + +func TestStringFromCharsetEmptyCharset(t *testing.T) { + t.Parallel() + + got, err := StringFromCharset(5, "") + if err == nil { + t.Fatal("StringFromCharset with empty charset expected error, got nil") + } + if got != "" { + t.Fatalf("StringFromCharset with empty charset = %q, want empty string on error", got) + } +} From 8ac1cf346e4ec38a62a3ba52ba1a204309c22ac0 Mon Sep 17 00:00:00 2001 From: Mrinmoy Matilal Date: Sat, 25 Jul 2026 18:28:05 +0000 Subject: [PATCH 2/2] fix: address review feedback Signed-off-by: Mrinmoy Matilal --- pkg/util/rand/rand.go | 3 +++ pkg/util/rand/rand_test.go | 28 ++++++++++++++-------------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/pkg/util/rand/rand.go b/pkg/util/rand/rand.go index a3a092e5..297979f0 100644 --- a/pkg/util/rand/rand.go +++ b/pkg/util/rand/rand.go @@ -15,6 +15,9 @@ func String(n int) (string, error) { // StringFromCharset generates, from a given charset, a cryptographically-secure pseudo-random string of a given length. func StringFromCharset(n int, charset string) (string, error) { + if n<0{ + return "", fmt.Errorf("n must be greater than 0") + } if n > 0 && len(charset) == 0 { return "", fmt.Errorf("charset must not be empty when n > 0") } diff --git a/pkg/util/rand/rand_test.go b/pkg/util/rand/rand_test.go index e9dfcd4d..3b33e0ff 100644 --- a/pkg/util/rand/rand_test.go +++ b/pkg/util/rand/rand_test.go @@ -38,23 +38,23 @@ func TestString(t *testing.T) { } } -func TestStringProducesDifferentValues(t *testing.T) { +func TestStringRepeatedCalls(t *testing.T) { t.Parallel() - const length = 32 + + const ( + length = 32 + attempts=5 + ) - first, err := String(length) - if err != nil { - t.Fatalf("first String(%d) returned error: %v", length, err) - } - - second, err := String(length) - if err != nil { - t.Fatalf("second String(%d) returned error: %v", length, err) - } - - if first == second { - t.Fatalf("expected two generated strings to differ, both were %q", first) + for i := 0; i < attempts; i++ { + got, err := String(length) + if err != nil { + t.Fatalf("String(%d) returned error on attempt %d: %v", length, i, err) + } + if len(got) != length { + t.Fatalf("String(%d) length on attempt %d = %d, want %d", length, i, len(got), length) + } } }