xls-codec: work toward a genuine 100% mutation score

packages/xls-codec/src/biff/builder.test.ts

@@ -9,12 +9,12 @@ function view(bytes: Uint8Array<ArrayBuffer>): DataView {
 describe("RecordBuilder", () => {
   it("writes a u8 as a single byte", () => {
     const bytes = new RecordBuilder().u8(0xab).build();
-    expect(Array.from(bytes)).toEqual([0xab]);
+    expect(Array.from(bytes)).toStrictEqual([0xab]);
   });
 
   it("writes a u16 little-endian", () => {
     const bytes = new RecordBuilder().u16(0x1234).build();
-    expect(Array.from(bytes)).toEqual([0x34, 0x12]);
+    expect(Array.from(bytes)).toStrictEqual([0x34, 0x12]);
   });
 
   it("truncates a u16 to its own 16 bits", () => {
@@ -39,7 +39,7 @@ describe("RecordBuilder", () => {
       .bytes(new Uint8Array([0xaa, 0xbb]))
       .u8(0x02)
       .build();
-    expect(Array.from(bytes)).toEqual([0x01, 0xaa, 0xbb, 0x02]);
+    expect(Array.from(bytes)).toStrictEqual([0x01, 0xaa, 0xbb, 0x02]);
   });
 
   it("chains fields in call order into one contiguous buffer", () => {

packages/xls-codec/src/biff/cursor.test.ts

@@ -1,7 +1,6 @@
 import { describe, expect, it } from "vitest";
 
 import { BlockCursor } from "./cursor";
-import { BiffFormatError } from "./records";
 
 function bytes(...values: readonly number[]): Uint8Array<ArrayBuffer> {
   return new Uint8Array(values);
@@ -70,20 +69,35 @@ describe("BlockCursor", () => {
   it("reads a run of raw bytes", () => {
     const cursor = new BlockCursor([bytes(0x01, 0x02, 0x03, 0x04)]);
 
-    expect(cursor.take(3)).toEqual(bytes(0x01, 0x02, 0x03));
+    expect(cursor.take(3)).toStrictEqual(bytes(0x01, 0x02, 0x03));
   });
 
   it("reads a run of raw bytes spanning a block boundary", () => {
     const cursor = new BlockCursor([bytes(0x01, 0x02), bytes(0x03, 0x04)]);
 
-    expect(cursor.take(3)).toEqual(bytes(0x01, 0x02, 0x03));
+    expect(cursor.take(3)).toStrictEqual(bytes(0x01, 0x02, 0x03));
   });
 
   it("rejects a length-prefixed take() before allocating, rather than after reading runs out", () => {
-    // A length field taken straight from untrusted BIFF8 input (e.g. CFEx's own cbDxf, [MS-XLS] 2.4.64) can name up to 4 GiB from a record only a few real bytes long. take() must reject a count larger than the data actually remaining before it allocates, not merely fail partway through copying bytes -- an allocate-then-fail sequence still pays the allocation cost the check exists to avoid.
+    // A length field taken straight from untrusted BIFF8 input (e.g. CFEx's own cbDxf, [MS-XLS] 2.4.64) can name up to 4 GiB from a record only a few real bytes long. take() must reject a count larger than the data actually remaining before it allocates, not merely fail partway through copying bytes -- an allocate-then-fail sequence still pays the allocation cost the check exists to avoid. Asserting the up-front check's OWN wording, not just that some BiffFormatError was thrown, is what actually proves this: the later per-byte read inside the copy loop throws a BiffFormatError too, with different wording, so a generic class-only assertion cannot tell the two apart.
     const cursor = new BlockCursor([bytes(0x01, 0x02, 0x03)]);
 
-    expect(() => cursor.take(0xffffffff)).toThrow(BiffFormatError);
+    expect(() => cursor.take(0xffffffff)).toThrow(
+      /requests more data than remains/,
+    );
+  });
+
+  it("computes remaining bytes correctly when the cursor sits exactly on an exhausted block, not just at construction", () => {
+    // A block exhausted by a prior read (offset === that block's own length) is a different unsettled moment than a freshly constructed cursor -- remainingTotal() must still settle from here before totalling, or it would count the already-exhausted block's own length a second time on top of the real remaining block's.
+    const cursor = new BlockCursor([
+      bytes(0x01, 0x02),
+      bytes(0x03, 0x04, 0x05),
+    ]);
+    cursor.u8();
+    cursor.u8(); // exactly exhausts the first block, without yet triggering another settle()
+
+    expect(() => cursor.take(4)).toThrow(/requests more data than remains/);
+    expect(cursor.take(3)).toStrictEqual(bytes(0x03, 0x04, 0x05));
   });
 
   it("skips forward without returning the bytes", () => {
@@ -100,18 +114,41 @@ describe("BlockCursor", () => {
     expect(cursor.u8()).toBe(0x04);
   });
 
-  it("rejects a read running past the end of the last block", () => {
+  it("rejects a u8 read running past the end of the last block, naming which field was being read", () => {
+    const cursor = new BlockCursor([bytes()]);
+
+    expect(() => cursor.u8()).toThrow(/^u8 runs past the end/);
+  });
+
+  it("rejects a u16 read running past the end of the last block on its very first byte, not just its second", () => {
+    // An empty cursor, not a one-byte one: u16() calls nextByte("u16") twice, once for its low byte and once for its high, and a fixture with exactly one byte available only ever exercises the SECOND call's own failure -- the first would have succeeded. Only a cursor with no bytes at all forces the first call itself to fail.
+    const cursor = new BlockCursor([bytes()]);
+
+    expect(() => cursor.u16()).toThrow(/^u16 runs past the end/);
+  });
+
+  it("rejects a u16 read that runs out after its low byte but before its high one", () => {
     const cursor = new BlockCursor([bytes(0x01)]);
 
-    expect(() => cursor.u16()).toThrow(BiffFormatError);
+    expect(() => cursor.u16()).toThrow(/^u16 runs past the end/);
   });
 
-  it("rejects a skip running past the end of the last block", () => {
+  it("rejects a skip running past the end of the last block, naming the byte count it was skipping", () => {
     const cursor = new BlockCursor([bytes(0x01, 0x02)]);
 
     expect(() => {
       cursor.skip(3);
-    }).toThrow(BiffFormatError);
+    }).toThrow(/^3-byte skip runs past the end/);
+  });
+
+  it("reports the current block index, correctly settled even once every block is fully consumed", () => {
+    const cursor = new BlockCursor([bytes(0x01)]);
+
+    cursor.u8();
+
+    // A cursor with no unread bytes anywhere still rests at a specific, well-defined block index -- one past the single block just consumed, not two past it or further, however many times settle() re-runs afterwards.
+    expect(cursor.blockPosition()).toBe(1);
+    expect(cursor.blockPosition()).toBe(1);
   });
 
   it("treats a zero-length block as empty rather than as the end of the data", () => {

packages/xls-codec/src/biff/cursor.ts

@@ -1,5 +1,8 @@
 import { BiffFormatError } from "./records";
 
+/** u8()'s own context label -- reused, not restated, by take()'s per-byte copy loop below: once take()'s own upfront remainingTotal() check has passed, that loop's nextByte() call can never actually run out (there are provably at least `count` bytes left across the blocks it is about to walk), so its label has nothing of its own to name and borrows the one real caller's already-exercised text instead of building an independent, permanently unobservable template literal every copy. */
+const U8_CONTEXT = "u8";
+
 // A field-reading cursor over one record's data, or over a base record's data followed by its Continue records' ([MS-XLS] 2.4.58) -- one sequence of blocks read as if it were contiguous, while still knowing where each block ends.
 //
 // Both halves of that matter. Reads span a block boundary transparently, because a record's fields do not stop where a Continue happens to split them. But the boundary stays observable, because for a string ([MS-XLS] 2.5.293) the first byte after a boundary is a re-stated fHighByte flag rather than character data -- so the string reader needs to ask "did I just cross into a new block?" mid-field. A plain concatenation of the blocks would answer that question with silence and splice the flag byte into the text; see biff/strings.ts for the reader that consumes the boundary correctly.
@@ -11,7 +14,7 @@ export class BlockCursor {
 
   constructor(blocks: readonly Uint8Array<ArrayBuffer>[]) {
     this.blocks = blocks;
-    this.settle();
+    // No settle() call here: every public method below (hasMore, remainingInBlock, blockPosition, nextByte) already calls it as its own first step, so a freshly constructed cursor needs no separate normalisation pass before its first use -- one that ran here would only ever redo what the first real call does anyway.
   }
 
   /** Advances past any exhausted or empty blocks, so the cursor always rests either on a readable byte or past the end of the last block. A Continue carrying no data is legal and must not read as the end of the record. */
@@ -27,13 +30,8 @@ export class BlockCursor {
 
   private nextByte(context: string): number {
     this.settle();
-    const block = this.blocks[this.blockIndex];
-    if (block === undefined) {
-      throw new BiffFormatError(
-        `${context} runs past the end of the record data`,
-      );
-    }
-    const byte = block[this.offset];
+    // One absence check covers both ways this can run out: no block left at all, or (impossible in practice, since settle() above already guarantees offset < block.length whenever a block IS left, but not a distinction noUncheckedIndexedAccess's own typing can see) an in-range block with nothing at this offset. Folding them into the optional-chained lookup's own single undefined case is what keeps this to one throw and one reachable message, rather than a second copy the first branch already made unreachable.
+    const byte = this.blocks[this.blockIndex]?.[this.offset];
     if (byte === undefined) {
       throw new BiffFormatError(
         `${context} runs past the end of the record data`,
@@ -63,7 +61,7 @@ export class BlockCursor {
   }
 
   u8(): number {
-    return this.nextByte("u8");
+    return this.nextByte(U8_CONTEXT);
   }
 
   u16(): number {
@@ -97,17 +95,13 @@ export class BlockCursor {
     );
   }
 
-  /** Total unread bytes across the current block and every block after it -- what a length-prefixed field's own prefix must be checked against before that many bytes are allocated, so a record cannot claim a length far larger than the data actually behind it. */
+  /** Total unread bytes across the current block and every block after it -- what a length-prefixed field's own prefix must be checked against before that many bytes are allocated, so a record cannot claim a length far larger than the data actually behind it. remainingInBlock() is read FIRST and the later blocks sliced off `this.blockIndex` only afterwards, so a cursor sitting exactly on an exhausted block (offset === that block's own length, reached mid-read rather than only at construction) settles before either figure is taken -- computing the slice first would capture the stale, pre-settle index instead. */
   private remainingTotal(): number {
-    this.settle();
-    let total = 0;
-    for (let index = this.blockIndex; index < this.blocks.length; index += 1) {
-      const block = this.blocks[index];
-      total +=
-        (index === this.blockIndex ? this.remainingInBlock() : block?.length) ??
-        0;
-    }
-    return total;
+    const inCurrentBlock = this.remainingInBlock();
+    const rest = this.blocks
+      .slice(this.blockIndex + 1)
+      .reduce((sum, block) => sum + block.length, 0);
+    return inCurrentBlock + rest;
   }
 
   /** The next `count` bytes, copied out. Spans block boundaries. Rejects a `count` larger than the data actually remaining before allocating, so a length-prefixed field taken from untrusted input (e.g. CFEx's own `cbDxf`) cannot force a multi-gigabyte allocation from a tiny record. */
@@ -119,7 +113,7 @@ export class BlockCursor {
     }
     const out = new Uint8Array(count);
     for (let index = 0; index < count; index += 1) {
-      out[index] = this.nextByte(`${count}-byte run`);
+      out[index] = this.nextByte(U8_CONTEXT);
     }
     return out;
   }

packages/xls-codec/src/biff/font.test.ts

@@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest";
 import {
   NORMAL_FONT_FIELDS,
   cellFontDiffersFromNormal,
+  contentFontOf,
   readFontRecord,
   writeFontRecord,
   xfFontFieldsOf,
@@ -33,11 +34,11 @@ describe("writeFontRecord", () => {
       underline: true,
       colorIcv: 10,
     };
-    expect(readBack(writeFontRecord(fields))).toEqual(fields);
+    expect(readBack(writeFontRecord(fields))).toStrictEqual(fields);
   });
 
   it("writes the Normal font's own fields verbatim", () => {
-    expect(readBack(writeFontRecord(NORMAL_FONT_FIELDS))).toEqual(
+    expect(readBack(writeFontRecord(NORMAL_FONT_FIELDS))).toStrictEqual(
       NORMAL_FONT_FIELDS,
     );
   });
@@ -65,18 +66,90 @@ describe("writeFontRecord", () => {
       }),
     ).toThrow(BiffWriteError);
   });
+
+  it("accepts a height and a name length sitting exactly on dyHeight's and fontName's own boundaries, not just short of them", () => {
+    // 20/8191/1/31 are the field's own documented MUSTs (>= 20/<= 8191/>= 1/<= 31), not the one-past values the sibling test throws on, so the four checks above must each be a strict boundary rather than an off-by-one -- 21/8190/2/30 could not distinguish `>=`/`<=` from `>`/`<` the way exactly-on-the-edge values do.
+    expect(() =>
+      writeFontRecord({ ...NORMAL_FONT_FIELDS, heightTwips: 20 }),
+    ).not.toThrow();
+    expect(() =>
+      writeFontRecord({ ...NORMAL_FONT_FIELDS, heightTwips: 8191 }),
+    ).not.toThrow();
+    expect(() =>
+      writeFontRecord({ ...NORMAL_FONT_FIELDS, name: "A" }),
+    ).not.toThrow();
+    expect(() =>
+      writeFontRecord({ ...NORMAL_FONT_FIELDS, name: "A".repeat(31) }),
+    ).not.toThrow();
+  });
+
+  it("accepts a height of exactly 0 even though it sits outside dyHeight's own 20-8191 range", () => {
+    // heightTwips 0 is the one value this check lets through despite failing the range test outright -- proving the exception is real, and not merely the range check never firing, needs a height that WOULD throw under the range alone (0 is well below 20) to still pass.
+    expect(() =>
+      writeFontRecord({ ...NORMAL_FONT_FIELDS, heightTwips: 0 }),
+    ).not.toThrow();
+  });
+
+  it("names the field, the offending value, and the allowed range in each refusal's own message", () => {
+    expect(() =>
+      writeFontRecord({ ...NORMAL_FONT_FIELDS, heightTwips: 19 }),
+    ).toThrow(/font height 19 twips is outside the 20-8191 range/);
+    expect(() => writeFontRecord({ ...NORMAL_FONT_FIELDS, name: "" })).toThrow(
+      /font name "" is 0 UTF-16 code units, outside the 1-31/,
+    );
+  });
+
+  it("writes exactly cch characters of the font name, not one more", () => {
+    // fontNameBytes' own for loop must stop at name.length, not run one iteration past it: an off-by-one there would append a spurious extra UTF-16 unit (charCodeAt past the end reads as NaN, which the record builder's own u16 coerces to 0) two bytes long, growing the record beyond what a correctly-written one needs -- invisible to a round trip through this package's own reader, which stops reading the name at cch regardless (and invisible too to a bare LENGTH DIFFERENCE between two names of different lengths, since a constant one-unit overshoot shifts both by the identical two bytes). Only the record's own absolute total length, for one fixed name, pins the real byte count down.
+    const record = writeFontRecord({ ...NORMAL_FONT_FIELDS, name: "AB" });
+
+    // 4 (record header: type + size) + 14 (Font's own fixed fields) + 2 (fontNameBytes' own cch + flags) + 2*2 (one uncompressed UTF-16 unit per character).
+    expect(record.length).toBe(4 + 14 + 2 + 2 * 2);
+  });
+});
+
+describe("contentFontOf", () => {
+  it("states no colour when the cell's own icv is the same index the baseline font already carries, even where a real colour would resolve", () => {
+    // The comparison is on the raw icv, not the colour it resolves to: two fonts sharing the SAME index state no colour of their own, regardless of whether resolveColor would happily produce one for it.
+    const font: XfFontFields = {
+      ...NORMAL_FONT_FIELDS,
+      colorIcv: NORMAL_FONT_FIELDS.colorIcv,
+    };
+    const resolveColor = () => ({ r: 1, g: 0, b: 0 });
+
+    expect(
+      contentFontOf(font, NORMAL_FONT_FIELDS, resolveColor)?.color,
+    ).toBeUndefined();
+  });
+
+  it("states the resolved colour when the cell's own icv genuinely differs from the baseline's", () => {
+    const font: XfFontFields = {
+      ...NORMAL_FONT_FIELDS,
+      colorIcv: NORMAL_FONT_FIELDS.colorIcv + 1,
+    };
+    const resolveColor = (icv: number) =>
+      icv === font.colorIcv ? { r: 0, g: 1, b: 0 } : undefined;
+
+    expect(
+      contentFontOf(font, NORMAL_FONT_FIELDS, resolveColor)?.color,
+    ).toStrictEqual({
+      r: 0,
+      g: 1,
+      b: 0,
+    });
+  });
 });
 
 describe("xfFontFieldsOf", () => {
   /** A palette resolution standing in for the real plan: enough to prove which properties survive, since the plan's own exact icv assignment is write.ts's concern. */
   const icvOf = (color: { readonly r: number }) => (color.r === 1 ? 10 : 12);
 
   it("normalises an absent, empty, or all-default font to the Normal font's own fields", () => {
-    expect(xfFontFieldsOf(undefined, icvOf)).toEqual(NORMAL_FONT_FIELDS);
-    expect(xfFontFieldsOf({}, icvOf)).toEqual(NORMAL_FONT_FIELDS);
+    expect(xfFontFieldsOf(undefined, icvOf)).toStrictEqual(NORMAL_FONT_FIELDS);
+    expect(xfFontFieldsOf({}, icvOf)).toStrictEqual(NORMAL_FONT_FIELDS);
     expect(
       xfFontFieldsOf({ bold: false, fontFamily: "Arial", sizePt: 10 }, icvOf),
-    ).toEqual(NORMAL_FONT_FIELDS);
+    ).toStrictEqual(NORMAL_FONT_FIELDS);
   });
 
   it("resolves each stated property and defaults each unstated one", () => {
@@ -85,7 +158,7 @@ describe("xfFontFieldsOf", () => {
         { bold: true, fontFamily: "Courier New", sizePt: 12 },
         icvOf,
       ),
-    ).toEqual({
+    ).toStrictEqual({
       ...NORMAL_FONT_FIELDS,
       bold: true,
       name: "Courier New",