Skip to content

Conversation

@Azanul
Copy link
Owner

@Azanul Azanul commented Aug 14, 2025

Fixes issue #

Proposed Changes

Signed-off-by: Azanul <azanulhaque@gmail.com>
@gooroo-dev
Copy link

gooroo-dev bot commented Aug 14, 2025

Please double check the following review of the pull request:

🐞Mistake 🤪Typo 🚨Security 🚀Performance 💪Best Practices 📖Readability ❓Others
0 0 0 0 0 1 0

Changes in the diff

  • 📖 Added input validation to randomBytesInHex function to check if count is greater than 0 and return an error otherwise.

Identified Issues

ID Type Details Severity Confidence
001 💪Best Practices The error message returned when count is less than or equal to zero could be more descriptive and consistent with Go error formatting conventions. (server/internal/auth/auth.go:28-30) 🟡 Low 🟡 Low

Issue 001

Explanation

In the new code added to randomBytesInHex function (server/internal/auth/auth.go lines 28-30), the error message is:

fmt.Errorf("expected count to be greater than 0, got %d", count)

While this is clear, Go error messages conventionally start with a lowercase letter and avoid punctuation at the end unless necessary. Also, the message could be more idiomatic by stating "count must be greater than zero" rather than "expected count to be greater than 0".

Suggested fix

if count <= 0 {
    return "", fmt.Errorf("count must be greater than zero, got %d", count)
}

Explanation of the fix

This change improves readability and aligns the error message with Go error message conventions, making it clearer and more idiomatic.

Missing tests for the incoming changes

Add tests for the randomBytesInHex function to verify:

  • It returns an error when count is zero or negative.
  • It returns a hexadecimal string of the expected length when count is positive.
  • It returns no error in the normal case.

Example test code:

func TestRandomBytesInHex(t *testing.T) {
    // Test invalid counts
    for _, count := range []int{0, -1, -10} {
        _, err := randomBytesInHex(count)
        if err == nil {
            t.Errorf("expected error for count %d, got nil", count)
        }
    }

    // Test valid count
    count := 16
    hexStr, err := randomBytesInHex(count)
    if err != nil {
        t.Fatalf("unexpected error: %v", err)
    }
    expectedLen := count * 2 // hex encoding doubles length
    if len(hexStr) != expectedLen {
        t.Errorf("expected length %d, got %d", expectedLen, len(hexStr))
    }
}

Summon me to re-review when updated! Yours, Gooroo.dev
I'd appreciate your feedback! React or reply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants