Skip to content

fix: reject out-of-range timestamps in isValid#133

Open
spokodev wants to merge 1 commit into
ulid:masterfrom
spokodev:fix/isvalid-timestamp-bound
Open

fix: reject out-of-range timestamps in isValid#133
spokodev wants to merge 1 commit into
ulid:masterfrom
spokodev:fix/isvalid-timestamp-bound

Conversation

@spokodev

Copy link
Copy Markdown

Problem

isValid only checks the length (26) and that every character is in the Crockford base32 alphabet. It does not constrain the leading character, so it accepts strings whose timestamp component exceeds the 48-bit maximum (TIME_MAX). The rest of the library rejects those — decodeTime throws and ULID_REGEX (/^[0-7]…/) does not match — so isValid disagrees with decodeTime:

isValid("Z".repeat(26));      // true
decodeTime("Z".repeat(26));   // throws: Malformed ULID: timestamp too large

isValid("8" + "0".repeat(25)) // true, but decodeTime throws

Per the spec the largest valid ULID is 7ZZZZZZZZZZZZZZZZZZZZZZZZZ — a leading character above 7 overflows the timestamp. Over a fuzz of random 26-char Crockford strings, the current isValid disagrees with "does decodeTime succeed?" on ~74% of inputs.

Fix

Validate against the existing ULID_REGEX (already exported from constants and used by ulidToUUID), which additionally constrains the leading character to 0-7. After the change isValid(x) agrees with decodeTime(x) succeeding on 100% of fuzzed inputs, and the regex's character class preserves the existing case-insensitive behaviour.

Tests

isValid had no test coverage; added a describe("isValid", …) block covering a generated ULID (both cases), wrong length / out-of-alphabet rejection, the 48-bit timestamp bound, and agreement with decodeTime. The two bound/agreement assertions fail before the change and pass after. Full suite green (25/25), prettier --check clean.

isValid only checked the length and the Crockford alphabet, so it accepted
ULID strings whose leading character is above 7 — i.e. a timestamp beyond
the 48-bit maximum. decodeTime and ULID_REGEX both reject those, so isValid
disagreed with the rest of the library:

  isValid("Z".repeat(26))    // true
  decodeTime("Z".repeat(26)) // throws "Malformed ULID: timestamp too large"

Validate against the existing ULID_REGEX, which also constrains the leading
character to 0-7.
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.

1 participant