Fix UTF-8 astral encoding, Crockford decode, and a CI-breaking analyzer info#11
Merged
Conversation
…e U+FFFF The `UTF8Encoder.convert` 3-byte branch had no upper bound, so a scalar code point in `U+10000..U+10FFFF` (a documented `source: 32` input) was force-fit into a 3-byte sequence and produced invalid UTF-8 — for example `U+1F600` emitted the byte `0xFF`, which is never a valid UTF-8 byte. The 4-byte path was only reachable through a UTF-16 surrogate pair, never a scalar astral value. Gate the 3-byte branch with `x <= 0xFFFF` and add a 4-byte branch for non-surrogate scalars. The public `toUtf8(String)` path was never affected, since UTF-16 strings deliver astral characters as surrogate pairs. Adds a regression group asserting `UTF8Encoder().convert([scalar])` matches `dart:convert` for `U+10000`, `U+1F600`, `U+1D11E`, and `U+10FFFF`, and that each round-trips back through the decoder.
The Crockford reverse table was upper-only and marked `I`, `L`, `O` as invalid, so lowercase Crockford strings could not be decoded and the specification's ambiguous-letter substitutions were missing. This contradicted `fromBase32`'s own dartdoc, which promises it handles both upper and lower case. Per Crockford's specification, decoding is now case-insensitive and accepts the ambiguous letters as digits: `I`/`i`/`L`/`l` decode as `1`, and `O`/`o` decode as `0`. `U`/`u` remains outside the alphabet. The encoding alphabet and its output are unchanged. The table is regenerated by `tool/alphabet_maker.dart`, whose `rev` helper now accepts an optional alias map for such substitutions. Adds decode differential tests against `package:base_codecs`' `base32Crockford` (uppercase, lowercase, and the ambiguous letters), plus a check that `U`/`u` is rejected.
`benchmark/bit.dart` exposed the private `_BitEncoder`/`_BitDecoder` types through public fields, tripping `library_private_types_in_public_api` and failing `dart analyze --fatal-infos` (which CI and the documented preflight both run). The Test workflow's `paths:` filter excludes `benchmark/**`, so it went unnoticed. Annotate the fields with the public `BitEncoder`/`BitDecoder` base types. Also alphabetize the `core/bit.dart` export in `lib/src/codecs_base.dart`, and correct the `alphabet_maker` references in `AGENTS.md` and the `new-codec` skill: the tool is `tool/alphabet_maker.dart` (run with `dart run`), not the non-existent `scripts/alphabet_maker.py`.
dipu-bd
commented
Jul 22, 2026
dipu-bd
left a comment
dipu-bd
left a comment
Member
Author
There was a problem hiding this comment.
check if you want to fix these. otherwise present your reason for denial. i will consider to approve or not
- Fold the scalar and surrogate-pair 4-byte cases in `UTF8Encoder.convert` into a single branch: the surrogate pair is combined into the scalar `x`, then both share one emit block. Removes the now-unused `c` local. Output is byte-identical (vm + node green, coverage still 100%). - Remove the verbose comments added to `rev` in `tool/alphabet_maker.dart`.
Codecov Report✅ All modified and coverable lines are covered by tests.
|
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
3 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.
A repo-wide audit surfaced three actionable issues. Each is a separate commit, all verified with the full preflight (format,
dart analyze --fatal-infos,dart teston vm and node, integration smoke test) and 100% line coverage onlib/(911/911).Observable-output changes are filed under a
# _next_CHANGELOG heading so the owner can pick the version number (both are semver-relevant per the operating manual).1. UTF-8 encoder emitted invalid bytes for scalar code points ≥ U+10000
UTF8Encoder.convert's 3-byte branch had no upper bound, so a scalar code point inU+10000..U+10FFFF(a documentedsource: 32input) was force-fit into 3 bytes and produced invalid UTF-8 — e.g.convert([0x1F600])emitted[0xFF, 0x98, 0x80](0xFFis never a valid UTF-8 byte) instead of[0xF0, 0x9F, 0x98, 0x80]. The 4-byte path was only reachable via a UTF-16 surrogate pair, never a scalar astral value.toUtf8(String)was never affected — UTF-16 strings deliver astral characters as surrogate pairs, which already hit the correct 4-byte path. The bug only bit directUTF8Encoder().convert([scalar])callers.convert([scalar])matchesdart:convertforU+10000,U+1F600,U+1D11E,U+10FFFF, and round-trips.2. Crockford Base-32 decoder was not case-insensitive / spec-compliant
The reverse table was upper-only and marked
I/L/Oinvalid, so lowercase Crockford strings were undecodable and the spec's ambiguous-letter substitutions were missing — contradictingfromBase32's own dartdoc. Now, per Crockford's spec, decoding is case-insensitive withI/i/L/l→1andO/o→0(U/ustays outside the alphabet). Encoding output is unchanged.tool/alphabet_maker.dart(itsrevhelper gained an alias map); never hand-typed.package:base_codecs'base32Crockford(uppercase, lowercase, ambiguous letters) plus aU/u-rejection check.3.
benchmark/bit.dartfaileddart analyze --fatal-infosPublic fields exposed the private
_BitEncoder/_BitDecodertypes (library_private_types_in_public_api), failing the analyzer step CI and the documented preflight both run. It slipped through because the Test workflow'spaths:filter excludesbenchmark/**. Fields are now typed as the public base classes. Also alphabetized thecore/bit.dartexport and corrected stalescripts/alphabet_maker.pyreferences (the tool istool/alphabet_maker.dart).Scope note
This PR does not touch the crypt / PHC code. A separate feature-gap roadmap (Base58, Base85/Z85, Bech32, streaming,
constantTimeEquals, …) was produced alongside the audit.