Throw on truncated VLQs in strict decode mode

[gunjanjaswal]

Hi @szuend — small correctness fix I ran into while reading through the VLQ decoder.

The bug

TokenIterator.nextUnsignedVLQ() loops on the continuation bit with no bounds check:

do {
  const charCode = this.nextCharCode();
  digit = BASE64_CODES[charCode];
  result += (digit & VLQ_BASE_MASK) << shift;
  shift += VLQ_BASE_SHIFT;
} while (digit & VLQ_CONTINUATION_MASK);

If a scopes string is truncated so that its final VLQ has the continuation bit set, the loop reads one character past the end. charCodeAt returns NaN, BASE64_CODES[NaN] is undefined, and undefined & VLQ_BASE_MASK coerces to 0 — so the continuation check also sees 0 and decoding just stops. The result is a silently corrupt decode. Since DecodeMode.STRICT is documented to throw on malformed input, that case slips through the contract. The same thing happens for a continuation that runs into a character outside the base64 alphabet.

The fix

TokenIterator takes a strict flag. When a digit comes back undefined mid-VLQ (either EOF on a truncated string, or a char outside the alphabet), we now throw in strict mode and keep the existing best-effort behaviour in lax mode — lax stops and returns what it decoded so far, exactly as before. The decoder passes the flag through from its DecodeMode.

This is intentionally narrow: a , still terminates a VLQ the way it always has (it maps to 0, same as A), so the item-separator semantics and the existing lax tests are untouched.

Tests

• Unit tests in vlq.test.ts for the truncated and out-of-alphabet cases in strict mode.
• Decode-level tests in decode.test.ts for a scopes string that ends mid-VLQ, asserting strict throws and lax tolerates it.

The VLQ code is adapted from Chrome DevTools, so I kept the change minimal and in the same style as the surrounding strict/lax handling.

[szuend]

Thanks, I was debating whether we should split TokenIterator into TokenIterator and SafeTokenIterator so we can skip the if branches in the lax case (the loop is very hot).

On the other hand, this makes calls to nextUnsignedVLQ no longer monomorphic, which might hurt worse then the if.

As such, I'd be in favor of landing the CL as-is, thanks! (Just documenting that we thought about this :))

[gunjanjaswal]

Thanks for merging, and for thinking it through on the hot-loop side. The monomorphism worry is real: splitting into a SafeTokenIterator could easily cost more than the branch it removes, especially since the strict path is the rare one. If it ever shows up in a profile, the branch could always hoist out to the call site instead. Appreciate you documenting the tradeoff either way.