fix: upper bound on CertId#161
Conversation
Allow a CertId up 64 bytes / 512 bits (allowing for a SHA512 hash in the future, though SHA256 is used).
There was a problem hiding this comment.
Pull request overview
This PR tightens validation for CertId (Subject Key Identifier) by enforcing both minimum and maximum byte-length bounds, preventing overly large identifiers while keeping room for future hash sizes (e.g., SHA-512).
Changes:
- Introduces a
MAX_LENGTHbound (64 bytes) alongside the existing minimum length. - Enforces the
[MIN_LENGTH, MAX_LENGTH]range in bothTryFrom<&[u8]>andTryFrom<Vec<u8>>implementations. - Adds a proptest property test to verify both
TryFromimplementations accept/reject inputs consistently based on length.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
More details
The PR correctly adds an upper bound of 64 bytes to CertId validation, complementing the existing 16-byte minimum. The SmallVec buffer properly handles the expanded range via heap allocation for values >20 bytes, and the property test validates both TryFrom implementations consistently enforce the [16, 64] invariant. No behavioral regressions or memory safety issues detected.
🤖 Datadog Autotest · Commit 4c82dad · What is Autotest? · Any feedback? Reach out in #autotest
Bound the maximum as well as the minimum
CertId.fix: upper bound on CertId (4c82dad)