diff --git a/.claude/skills/new-codec/SKILL.md b/.claude/skills/new-codec/SKILL.md index 348dc03..aad5e3c 100644 --- a/.claude/skills/new-codec/SKILL.md +++ b/.claude/skills/new-codec/SKILL.md @@ -26,10 +26,11 @@ against itself is how this repo got the wordSafe bug. ## 2. Generate the alphabet tables — never type them -Edit `scripts/alphabet_maker.py`: add the alphabet as a bytes literal, print +Edit `tool/alphabet_maker.dart`: add the alphabet as a string literal, print with `fwd()` (forward table) and `rev()` (reverse table; pass multiple -alphabets to one `rev()` call to make a case-insensitive decoder). Run -`python3 scripts/alphabet_maker.py` and paste the output. Leave the new section +alphabets to one `rev()` call to make a case-insensitive decoder, and an +optional alias map for substitutions like Crockford's `I/L -> 1`). Run +`dart run tool/alphabet_maker.dart` and paste the output. Leave the new section in the script (commented like the others) so the table is regenerable. Keep the trailing `//` after the first row of each table so `dart format` preserves the grid layout. diff --git a/AGENTS.md b/AGENTS.md index 9ef4467..45b1762 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -24,7 +24,7 @@ cd test_integration && dart pub get && dart run main.dart # public-API smoke t bash scripts/globals.sh # one-time: activate coverage/cobertura/junitreport/pana bash scripts/coverage.sh # lcov + cobertura into coverage/ (gitignored) dart run benchmark/.dart # perf comparisons vs other packages -python3 scripts/alphabet_maker.py # generates alphabet tables (edit script, run, paste) +dart run tool/alphabet_maker.dart # generates alphabet tables (edit script, run, paste) ``` There is no build step. `master` is the only branch; commits land on it directly. @@ -109,7 +109,7 @@ converters accept `List` (not `Iterable`, per the 2.6.0 breaking change) and return `Uint8List` where the output is bytes. **Alphabet tables** are generated, never typed. Edit the relevant section of -`scripts/alphabet_maker.py` (`fwd()` for forward, `rev()` for reverse — `rev` +`tool/alphabet_maker.dart` (`fwd()` for forward, `rev()` for reverse — `rev` accepts multiple alphabets to build case-insensitive tables), run it, paste the output. Keep the trailing `//` after the first row so `dart format` preserves the table layout. @@ -183,7 +183,7 @@ codebase wrong. Read this list before writing code, not after. 3. **The hand-typed alphabet.** 32–128 entry integer tables cannot be reviewed by eye; the `wordSafe` bug was a paste error in exactly such a table. **Rule: never hand-write or hand-edit an alphabet table. Generate it with - `scripts/alphabet_maker.py` and add the alphabet-string assertion test.** + `tool/alphabet_maker.dart` and add the alphabet-string assertion test.** 4. **The missing mask.** Hand-rolled bit surgery fails on inputs where flag bits and payload bits disagree (`fromUtf8(toUtf8('Ā'))` returned `'ƀ'` because a continuation byte was OR-ed in without `& 0x3F`; U+00A9 masked the @@ -253,7 +253,7 @@ Adjectives don't count; these checklists do. "Done" means every box checks. **New codec or new alphabet:** -- [ ] Tables generated by `scripts/alphabet_maker.py` (script section committed/updated) +- [ ] Tables generated by `tool/alphabet_maker.dart` (script section committed/updated) - [ ] Codec class: private ctor, `static const` instances, dartdoc with alphabet block + spec link on every instance - [ ] Top-level `to`/`from` in `lib/src/.dart` with `codec:` override diff --git a/CHANGELOG.md b/CHANGELOG.md index 8de41a3..b06e7b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,24 @@ +# _next_ + +- Fix the UTF-8 encoder producing invalid output for scalar code points in + `U+10000..U+10FFFF`. The `UTF8Encoder.convert` 3-byte branch had no upper + bound, so an astral scalar (e.g. `U+1F600`, a documented `source: 32` input) + was force-fit into 3 bytes and emitted invalid UTF-8 (including the byte + `0xFF`). It now emits the correct 4-byte sequence, verified against + `dart:convert`. The public `toUtf8(String)` path was never affected, since + UTF-16 strings deliver astral characters as surrogate pairs. **Observable + output change** for direct `UTF8Encoder().convert([scalar])` callers. +- Make the Crockford Base-32 decoder spec-compliant: it is now case-insensitive + and substitutes the ambiguous letters `I`/`i`/`L`/`l` -> `1` and `O`/`o` -> + `0` (the letter `U`/`u` is still not part of the alphabet). Previously + lowercase Crockford strings and the ambiguous letters were rejected. Verified + against `package:base_codecs`. Encoding output is unchanged. **Observable + change**: inputs that previously threw now decode. +- Fix `benchmark/bit.dart` exposing the private `_BitEncoder`/`_BitDecoder` + types through public fields, which failed `dart analyze --fatal-infos` + (benchmark tooling only; no library code or output is affected). +- Alphabetize the `core/bit.dart` export in `lib/src/codecs_base.dart`. + # 3.5.2 - Renames internal abstract class `CipherlibConverter` -> `BitConverter`. diff --git a/benchmark/bit.dart b/benchmark/bit.dart index ff6e4f3..b133d26 100644 --- a/benchmark/bit.dart +++ b/benchmark/bit.dart @@ -40,7 +40,7 @@ Uint8List _sample(int size) => Uint8List.fromList(List.filled(size, 0x5a)); class BitEncode extends SyncBenchmark { final int target; final Uint8List input; - late final _BitEncoder enc = _BitEncoder(8, target); + late final BitEncoder enc = _BitEncoder(8, target); BitEncode(int size, this.target) : input = _sample(size), @@ -55,7 +55,7 @@ class BitEncode extends SyncBenchmark { class BitDecode extends SyncBenchmark { final int source; final int _size; - late final _BitDecoder dec = _BitDecoder(source, 8); + late final BitDecoder dec = _BitDecoder(source, 8); Uint8List encoded = Uint8List(0); BitDecode(int size, this.source) diff --git a/lib/src/codecs/base32.dart b/lib/src/codecs/base32.dart index 428923d..c8dcd91 100644 --- a/lib/src/codecs/base32.dart +++ b/lib/src/codecs/base32.dart @@ -78,12 +78,17 @@ const _base32EncodingCrockford = [ ]; // Crockford's Base32 Reversed +// +// Case-insensitive, and per the Crockford spec the ambiguous letters decode to +// digits: I/i/L/l -> 1 and O/o -> 0. The letter U/u is not decoded. const _base32DecodingCrockford = [ __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, // __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, __, 00, 01, 02, 03, 04, 05, 06, 07, 08, - 09, __, __, __, __, __, __, __, 10, 11, 12, 13, 14, 15, 16, 17, __, 18, 19, - __, 20, 21, __, 22, 23, 24, 25, 26, __, 27, 28, 29, 30, 31, __, __, __, __, + 09, __, __, __, __, __, __, __, 10, 11, 12, 13, 14, 15, 16, 17, 01, 18, 19, + 01, 20, 21, 00, 22, 23, 24, 25, 26, __, 27, 28, 29, 30, 31, __, __, __, __, + __, __, 10, 11, 12, 13, 14, 15, 16, 17, 01, 18, 19, 01, 20, 21, 00, 22, 23, + 24, 25, 26, __, 27, 28, 29, 30, 31, __, __, __, __, __, __, __, __, __, __, ]; // GeoHash's Base32 @@ -509,6 +514,10 @@ class Base32Codec extends IterableCodec { /// This alphabet uses additional characters for a mod-37 checksum, and avoid /// the character U to reduce the likelihood of accidental obscenity. /// + /// Decoding is case-insensitive and, following the specification, accepts the + /// ambiguous letters as digits: `I`, `i`, `L`, `l` decode as `1`, and `O`, + /// `o` decode as `0`. The letter `U`/`u` is not part of the alphabet. + /// /// It is not padded. static const Base32Codec crockford = Base32Codec._( encoder: Base32Encoder( diff --git a/lib/src/codecs/utf8.dart b/lib/src/codecs/utf8.dart index 1672ef6..9ce74d7 100644 --- a/lib/src/codecs/utf8.dart +++ b/lib/src/codecs/utf8.dart @@ -39,7 +39,7 @@ class UTF8Encoder extends BitEncoder { @override Uint8List convert(List input) { int len = input.length; - int l = 0, p = 0, x, y, c; + int l = 0, p = 0, x, y; var out = Uint8List(len << 2); while (p < len) { x = input[p]; @@ -62,31 +62,34 @@ class UTF8Encoder extends BitEncoder { out[l++] = 0x80 | (x & 0x3F); p++; } - // Case: 3-byte - else if (x < 0xD800 || x > 0xDFFF) { + // Case: 3-byte (rest of the Basic Multilingual Plane, sans surrogates) + else if (x <= 0xFFFF && (x < 0xD800 || x > 0xDFFF)) { out[l++] = 0xE0 | (x >>> 12); out[l++] = 0x80 | ((x >>> 6) & 0x3F); out[l++] = 0x80 | (x & 0x3F); p++; } - // Case: 4-byte from a UTF-16 surrogate pair - else if (x <= 0xDBFF) { - p++; - if (p >= len) { - throw FormatException('Unpaired high surrogate $x at ${p - 1}'); - } - y = input[p]; - if (y < 0xDC00 || y > 0xDFFF) { - throw FormatException('Invalid surrogate pair ($x, $y) at $p'); + // Case: 4-byte, either a scalar in U+10000..U+10FFFF or a UTF-16 + // high surrogate combined with the following low surrogate into one. + else { + if (x <= 0xDBFF) { + p++; + if (p >= len) { + throw FormatException('Unpaired high surrogate $x at ${p - 1}'); + } + y = input[p]; + if (y < 0xDC00 || y > 0xDFFF) { + throw FormatException('Invalid surrogate pair ($x, $y) at $p'); + } + x = 0x10000 + (((x - 0xD800) << 10) | (y - 0xDC00)); + } else if (x <= 0xDFFF) { + throw FormatException('Unpaired low surrogate $x at $p'); } - c = 0x10000 + (((x - 0xD800) << 10) | (y - 0xDC00)); - out[l++] = 0xF0 | (c >>> 18); - out[l++] = 0x80 | ((c >>> 12) & 0x3F); - out[l++] = 0x80 | ((c >>> 6) & 0x3F); - out[l++] = 0x80 | (c & 0x3F); + out[l++] = 0xF0 | (x >>> 18); + out[l++] = 0x80 | ((x >>> 12) & 0x3F); + out[l++] = 0x80 | ((x >>> 6) & 0x3F); + out[l++] = 0x80 | (x & 0x3F); p++; - } else { - throw FormatException('Unpaired low surrogate $x at $p'); } } diff --git a/lib/src/codecs_base.dart b/lib/src/codecs_base.dart index d0e7b20..8feeedd 100644 --- a/lib/src/codecs_base.dart +++ b/lib/src/codecs_base.dart @@ -16,8 +16,8 @@ export 'codecs/base8.dart'; export 'codecs/bigint.dart'; export 'codecs/utf8.dart'; export 'core/alphabet.dart'; +export 'core/bit.dart'; export 'core/byte.dart'; export 'core/codec.dart'; -export 'core/bit.dart'; export 'crypt.dart'; export 'utf8.dart'; diff --git a/test/base32_test.dart b/test/base32_test.dart index 9f02af5..247ef36 100644 --- a/test/base32_test.dart +++ b/test/base32_test.dart @@ -498,6 +498,56 @@ void main() { }); }); + // Crockford decoding is case-insensitive and, per the specification, + // substitutes the ambiguous letters (I/i/L/l -> 1 and O/o -> 0); U/u is not + // part of the alphabet. The external oracle is `package:base_codecs`' + // `base32Crockford`, which applies the same rules. + // https://www.crockford.com/base32.html + group('crockford decode (spec-compliant, vs base_codecs)', () { + test('encoding matches base_codecs', () { + for (int i = 0; i < 100; ++i) { + var b = randomBytes(i); + var ours = toBase32(b, codec: Base32Codec.crockford); + expect(ours, base_codecs.base32CrockfordEncode(b), reason: '$i'); + } + }); + test('decoding uppercase matches base_codecs', () { + for (int i = 0; i < 100; ++i) { + var b = randomBytes(i); + var s = toBase32(b, codec: Base32Codec.crockford); + expect(fromBase32(s, codec: Base32Codec.crockford), + base_codecs.base32CrockfordDecode(s), + reason: '$i'); + } + }); + test('decoding lowercase matches base_codecs', () { + for (int i = 0; i < 100; ++i) { + var b = randomBytes(i); + var s = toBase32(b, codec: Base32Codec.crockford).toLowerCase(); + expect(fromBase32(s, codec: Base32Codec.crockford), + base_codecs.base32CrockfordDecode(s), + reason: '$i'); + } + }); + test('ambiguous letters I/i/L/l -> 1 and O/o -> 0', () { + // "foo" encodes to "CSQPY"; lowercase must decode identically. + expect(fromBase32('csqpy', codec: Base32Codec.crockford), + equals('foo'.codeUnits)); + // Every spelling of the ambiguous symbols matches the reference. + for (final s in ['10', '1O', 'IO', 'io', 'i0', 'L0', 'lo']) { + expect(fromBase32(s, codec: Base32Codec.crockford), + base_codecs.base32CrockfordDecode(s), + reason: s); + } + }); + test('U and u are rejected', () { + expect(() => fromBase32('U0', codec: Base32Codec.crockford), + throwsFormatException); + expect(() => fromBase32('u0', codec: Base32Codec.crockford), + throwsFormatException); + }); + }); + group('compare against package: base32', () { test('encoding (uppercase)', () { for (int i = 0; i < 100; ++i) { diff --git a/test/utf8_test.dart b/test/utf8_test.dart index 941018f..1e3e694 100644 --- a/test/utf8_test.dart +++ b/test/utf8_test.dart @@ -189,6 +189,32 @@ void main() { test('with empty input for decoder returns empty list', () { expect(encoder.convert([]), isEmpty); }); + + // Regression: the 3-byte branch used to lack an upper bound, so a scalar + // code point >= U+10000 fed to `convert` (a documented `source: 32` + // input) was force-fit into 3 bytes and produced invalid UTF-8 (e.g. + // U+1F600 emitted the byte 0xFF). The 4-byte path was only reachable via + // a UTF-16 surrogate pair, never a scalar astral value. The external + // oracle here is `dart:convert`'s `utf8.encode`. + group('scalar code point >= U+10000 uses the 4-byte form', () { + const scalars = [ + 0x10000, // first 4-byte scalar + 0x1F600, // 😀, whose broken output contained the invalid byte 0xFF + 0x1D11E, // 𝄞 musical symbol G clef + 0x10FFFF, // last valid code point + ]; + for (final cp in scalars) { + test('U+${cp.toRadixString(16)}', () { + final expected = utf8.encode(String.fromCharCode(cp)); + expect(encoder.convert([cp]), equals(expected)); + // And it must round-trip back to the same scalar. + expect( + UTF8Codec.standard.decoder.convert(encoder.convert([cp])), + equals([cp]), + ); + }); + } + }); }); group('Decoder', () { diff --git a/tool/alphabet_maker.dart b/tool/alphabet_maker.dart index 85316f9..db390a5 100644 --- a/tool/alphabet_maker.dart +++ b/tool/alphabet_maker.dart @@ -1,5 +1,5 @@ // Generates the forward (encode) and reverse (decode) alphabet lookup tables -// used by the codecs. Dart port of tool/alphabet_maker.py. +// used by the codecs. // // Run with: dart run tool/alphabet_maker.dart @@ -17,18 +17,22 @@ void show(List list, int n) { stdout.write("\n"); } -void rev(List merge) { +void rev(List merge, [Map alias = const {}]) { var v = []; + void put(int c, int value) { + while (c >= v.length) { + v.addAll(List.filled(19, -1)); + } + v[c] = value; + } + for (var s in merge) { var chars = s.codeUnits; for (var i = 0; i < chars.length; i++) { - var c = chars[i]; - while (c >= v.length) { - v.addAll(List.filled(19, -1)); - } - v[c] = i; + put(chars[i], i); } } + alias.forEach((ch, value) => put(ch.codeUnitAt(0), value)); show([ for (var x in v) x < 0 ? "__" : x.toString().padLeft(2, "0"), ], 19); @@ -55,7 +59,15 @@ void main() { // fwd("0123456789ABCDEFGHJKMNPQRSTVWXYZ"); // stdout.write("];\n"); // stdout.write("const _base32DecodingCrockford = ["); - // rev(["0123456789ABCDEFGHJKMNPQRSTVWXYZ"]); + // rev( + // [ + // "0123456789ABCDEFGHJKMNPQRSTVWXYZ", + // "0123456789abcdefghjkmnpqrstvwxyz", + // ], + // // Crockford decoders accept lowercase and substitute the ambiguous + // // letters: I/i/L/l -> 1 and O/o -> 0. The letter U is not decoded. + // {"I": 1, "i": 1, "L": 1, "l": 1, "O": 0, "o": 0}, + // ); // stdout.write("];\n"); // stdout.write("const _base32EncodingGeoHash = [");