Repository navigation
refactor: simplify the BER writer and the length reader - #55
Merged
Merged
Conversation
- AppendOID reads each arc left to right and fails at its first bad octet: a value crossing 2^32-1 before a non-digit is out of range, the reverse is invalid; signs, underscores, hex prefixes and spaces are invalid, leading zeros are not; an arc error comes before the sub-identifier count check, the first and second arc's range before their other rules. - Length's long form at the size of int: the largest length whose header still fits, length octets whose header overflows, length octets past the int range, length octets cut short, and more length octets than an int holds, which wrap silently. The cases are built from strconv.IntSize, so they hold on 32-bit platforms too.
- AppendOID splits arcs with strings.Cut and parses each with strconv.ParseUint(arc, 10, 32) instead of its own digit loop. ParseUint reads left to right and fails at the same octet: a non-digit is a syntax error, the digit crossing 2^32-1 a range error, mapped to ErrInvalidOID and ErrOIDOutOfRange. - End writes a long-form length with AppendLength instead of its own copy of the length octets. - appendBase128 takes a uint64 and sizes its groups with bits.Len64. - Length returns from each case: the short-input and short-form cases had nothing to add to the shared tail, and the long form checks its octet count before reading them; the errors are the same.
strconv.ParseUint over arcs split with strings.Cut made AppendOID about three times slower (32 to 90 ns for a 9-arc OID; encoding a 10-varbind request 545 to 1488 ns): its general-purpose parsing and a second scan of each arc cost more than the digit loop it replaced. nextArc parses one arc up to the next dot, with the same left-to-right errors, in a single pass: 27 ns.
- A colon, the octet after '9', is not a digit of an OID arc. - Length octets that pass the int range and then wrap back to a small value are rejected where they first pass it, not accepted with the wrapped length. Both pass on the code before the BER refactor.
ilyam8
added this pull request to stack #53
October 13, 2026 09:51
ilyam8
changed the title
ilyam8
marked this pull request as ready for review
October 13, 2026 09:51
stelfrag
approved these changes
Oct 13, 2026
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
2 participants
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
Phase 6 (guardrails), BER writer and reader in
internal/ber. No behavior change, no exported API change.AppendOID's nested index loops become a loop over the rest of the OID that skips emptysub-identifiers and takes each arc with
nextArc, which parses up to the next dot in one pass with the sameleft-to-right errors.
strconv.ParseUintover arcs split withstrings.Cutwas tried first and madeAppendOIDalmost three times slower (two scans per arc and its general-purpose parsing);
nextArc's doc records why.Endwrites a long-form length withAppendLengthinstead of its own copy of the lengthoctets.
appendBase128takes auint64and sizes its groups withbits.Len64.Length. Each case returns on its own; the long form checks its octet count before reading them. Same resultsand errors.
Validation
AppendOID's error precedence (out of range before anon-digit in one arc and the reverse, arc errors before the count check, the first and second arcs out of range,
signs, underscores, hex prefixes, spaces, leading zeros), and
Length's long form at the size ofint(built fromstrconv.IntSize, so the linux/386 job checks the 32-bit edges).pass the int range and wrap back); 2 cannot change any output (
nextArcleaving the dot to the caller's empty-arcskip, and
End's short-form bound at exactly 127).AppendOID-15.5% (32.6 to 27.5 ns), encoding -5% to-11%, decoding unchanged, allocations unchanged.
go test -race, FIPS 140-only, golangci-lint, cross-builds, govulncheck: pass locally.