fix(rand): return errors for invalid random string inputs#360
Open
hazelmayank wants to merge 1 commit into
Open
fix(rand): return errors for invalid random string inputs#360hazelmayank wants to merge 1 commit into
hazelmayank wants to merge 1 commit into
Conversation
Signed-off-by: hazelmayank <mayankjeefinal@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes two panic cases in
pkg/util/rand. The helpersStringandStringFromCharsetboth declare(string, error)returns, but two invalid inputs caused the process to panic instead of returning an error:StringFromCharset(1, "")panicked becausecrypto/rand.Intwas called withmax = 0.String(-1)panicked becausemake([]byte, -1)rejected the negative length.This PR adds input validation before allocation and random-index generation, so invalid inputs now return descriptive errors instead of crashing the host process.
Why
errorreturn; the panic paths were unreachable for callers who expected to handle them as errors.cmd/login.gois the only current caller and is unaffected, because it always passes positivenand a non-empty charset.What changed
StringFromCharset:n < 0returns"rand: requested length %d is negative".n > 0withlen(charset) == 0returns"rand: cannot generate %d-character string from empty charset".n == 0continues to return("", nil)regardless of charset, preserving the existing no-op behavior.StringFromCharsetto spell out the new error contract.pkg/util/rand/rand_test.gowith table-driven cases for both valid and invalid inputs. No statistical or distribution assertions — only deterministic length and charset-membership checks.Behavior preserved for valid inputs
Public signatures are unchanged.
Test plan
gofmt -l pkg/util/rand/rand.go pkg/util/rand/rand_test.go— no outputgit diff --check -- pkg/util/rand/rand.go pkg/util/rand/rand_test.go— cleango test -v ./pkg/util/rand/...— all cases passgo vet ./...— cleango test ./...— full sweep passesmake build-local— builds successfullyFiles changed
pkg/util/rand/rand.go— two guard clauses + expanded doc comment.pkg/util/rand/rand_test.go— new file; first tests in this package.Notes
go test -coverbecause my local Go toolchain reportsgo: no such tool "covdata". The PR is scoped only topkg/util/randand its tests, so the missing coverage report does not affect the change itself; CI will produce a coverage report on its own toolchain if the project requires one.Fixes #359