Skip to content

Commit 134674f

Browse files
Prithvi-Webclaude
andcommitted
fix(store): a name that begins with a byte-order mark keeps it - the store reads every stored byte back, so "\uFEFFfoo" beside "foo", and "\uFEFF" alone, are each spelled as the file they name, and no query hit, scan JSON or delete names the file beside it (CI4 review F2.1, F2.2)
The packed store keeps a name as its UTF-8 bytes, and a name may begin with U+FEFF: fs.readdir keeps it, and the encoder stores its three bytes (EF BB BF). The store read names back with a TextDecoder made without ignoreBOM, which reads a leading mark as nothing. So the 900-byte "\uFEFFfoo" was named "foo" and spelled "/data/foo" - the path of the 5-byte file beside it - and a file named "\uFEFF" alone was spelled as its folder, "/data/". path(), childPath(), the query's hits and the scan JSON all build on name(), and the path lookup, which compares bytes, then found the other file. DELETE /api/files, the cart, an Autopilot query policy and an offload act on those paths. The decoder was the store's from its start (3fd871f); CI4 review F1.1 (44dfcb7) had made childPath agree with it rather than fix it. The decoder now keeps the mark (ignoreBOM: true, as gitSafe's does), so a name reads back as it was stored. childPath's branch for a parent whose name begins with the mark is gone: with the mark kept, a name is empty or ends with the separator exactly when its bytes do, so the plain byte rule is exact, and CI4's byte-for-byte differential, which keeps its mark-led names, still holds it to what reading the parent path answered. Tests, RED before the fix for the reviewers' reason: - tests/packedStore.test.ts: siblings "foo" and "\uFEFFfoo", a file "\uFEFF", "\uFEFFsub/a" beside "sub/a", and a folder "\uFEFF" holding a file, under "/data" and "C:\data": name(), path(), childPath(), findByPath(path()) and the streamed scan JSON agree with the object store, which keeps names as given. RED: the name "\uFEFFfoo" read as "foo". - tests/queryWalkWork.test.ts: type:file answers every file under its own name and path, held to the listing the store was built from. RED: "/data/foo" for both the 5-byte and the 900-byte file, "/data/" for "\uFEFF". Mutants, each RED for the right reason, restored and checked by SHA-256: the old decoder (both tests, and the differential); childPath reading a parent named "\uFEFF" alone as empty ("/data/sub/\uFEFFb"); path() dropping the mark; the path lookup dropping it (finds the plain "foo"); the scan JSON dropping it; the child list leaving a child out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 2bac59f commit 134674f

3 files changed

Lines changed: 101 additions & 19 deletions

File tree

‎src/services/scanStore.ts‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -795,7 +795,14 @@ const CLOUD_PROVIDERS: readonly CloudProviderName[] = ['icloud', 'onedrive', 'dr
795795
const CLOUD_ID: Record<string, number> = { icloud: 1, onedrive: 2, dropbox: 3 };
796796

797797
const utf8Encoder = new TextEncoder();
798-
const utf8Decoder = new TextDecoder();
798+
/**
799+
* Reads a stored name back byte for byte. `ignoreBOM`: a name may begin with
800+
* U+FEFF — fs.readdir keeps it, and the encoder stores its three bytes — and
801+
* a decoder made without it reads them as nothing, so '\uFEFFfoo' was spelled
802+
* as the 'foo' beside it and '\uFEFF' alone as its folder (CI4 review F2.1,
803+
* F2.2): a path that names another file, which a delete would act on.
804+
*/
805+
const utf8Decoder = new TextDecoder('utf-8', { ignoreBOM: true });
799806

800807
const INITIAL_CAP = 1024;
801808
const INITIAL_POOL = 16 * 1024;
@@ -1509,20 +1516,12 @@ export class PackedScanStore implements ScanStore {
15091516
} else {
15101517
const from = this.nameOff[parent];
15111518
const to = this.nameOff[parent + 1];
1512-
const bytes = this.nameBytes;
1513-
if (to - from >= 3 && bytes[from] === 0xef && bytes[from + 1] === 0xbb && bytes[from + 2] === 0xbf) {
1514-
// A leading byte-order mark, which name()'s TextDecoder reads as
1515-
// nothing (CI4 review F1.1): '\uFEFF' alone is the empty name though
1516-
// its three bytes are stored, and '\uFEFF/' is the separator alone.
1517-
// Decided from the parent's name as name() reads it — its name, never
1518-
// its path.
1519-
const parentName = this.name(parent);
1520-
endsWithSep = parentName.length === 0 || parentName.endsWith(this.sep);
1521-
} else {
1522-
// The parent's stored name is empty or ends in the separator's byte
1523-
// (UTF-8: an ASCII byte is never part of a longer character).
1524-
endsWithSep = to === from || bytes[to - 1] === this.sepByte;
1525-
}
1519+
// The parent's stored name is empty or ends in the separator's byte
1520+
// (UTF-8: an ASCII byte is never part of a longer character). name()
1521+
// reads every stored byte back, a leading byte-order mark's too (CI4
1522+
// review F2.1, F2.2), so the name is empty, or ends with the separator,
1523+
// exactly when its bytes do.
1524+
endsWithSep = to === from || this.nameBytes[to - 1] === this.sepByte;
15261525
}
15271526
return endsWithSep ? parentPath + name : parentPath + this.sep + name;
15281527
}

‎tests/packedStore.test.ts‎

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,10 @@ test('CI4: childPath answers byte for byte what reading its parent path answered
164164
for (const [rootPath, sep] of roots) {
165165
// '' after 'a': an empty name's stored bytes sit after a name that does
166166
// not end in the separator, so only its emptiness can say it does.
167-
// A byte-order mark (CI4 review F1.1): name() reads a leading one as
168-
// nothing, so '\uFEFF' alone is the empty name though three bytes are
169-
// stored, and '\uFEFF' + sep is the separator alone; one anywhere else,
170-
// or a second, is part of the name.
167+
// A byte-order mark (CI4 review F1.1, F2.1/F2.2): name() reads one back
168+
// as it was stored, a leading one too, so '\uFEFF' alone is a name of one
169+
// character that does not end in the separator, and '\uFEFF' + sep one
170+
// that does.
171171
const names = ['a', '', `a${sep}`, 'é', `é${sep}`, 'b\\', 'c/', `${sep}${sep}`, ':', 'b:', 'z::', 'Ʀ', '\uFEFF', `\uFEFF${sep}`, 'a\uFEFF', '\uFEFF\uFEFF'];
172172
const packed = new PackedScanStore(rootPath, sep, folderInput('r'));
173173
// Every name under the root, under every name, under every name: a parent
@@ -203,6 +203,66 @@ test('CI4: childPath answers byte for byte what reading its parent path answered
203203
}
204204
});
205205

206+
test('CI4 review F2.1/F2.2: a name that begins with a byte-order mark keeps it — "\\uFEFFfoo" beside "foo", and "\\uFEFF" alone, are each spelled as the file they name, by name(), path(), childPath(), the path lookup and the scan JSON, as the object store spells them', async () => {
207+
// A walker lists a folder with fs.readdir, whose names keep a leading
208+
// U+FEFF, and the store keeps its three bytes (EF BB BF). A TextDecoder
209+
// made without ignoreBOM reads them as nothing, so the 900-byte
210+
// '\uFEFFfoo' was spelled '/data/foo' — the 5-byte 'foo' beside it — and a
211+
// file named '\uFEFF' alone was spelled as its folder. A path is what a
212+
// delete acts on: one node, its own spelling.
213+
const BOM = '\uFEFF';
214+
interface Listed { name: string; size?: number; children?: Listed[] }
215+
// Siblings that a decoder dropping the mark spells alike: 'foo' and
216+
// '\uFEFFfoo'; '\uFEFF' and its folder; '\uFEFFsub/a' and 'sub/a'; and a
217+
// folder named '\uFEFF' alone with a file in it.
218+
const listing: Listed[] = [
219+
{ name: 'foo', size: 5 },
220+
{ name: `${BOM}foo`, size: 900 },
221+
{ name: BOM, size: 7 },
222+
{ name: `${BOM}sub`, children: [{ name: 'a', size: 11 }, { name: BOM, size: 13 }] },
223+
{ name: 'sub', children: [{ name: 'a', size: 17 }, { name: BOM, children: [{ name: 'b', size: 19 }] }] },
224+
];
225+
const input = (l: Listed): NodeInput => ({ name: l.name, isDir: l.children !== undefined, size: l.size ?? 0, modifiedAt: 1, isHidden: false });
226+
for (const [rootPath, sep] of [['/data', '/'], ['C:\\data', '\\']] as const) {
227+
const stores = [new ObjectScanStore(rootPath, sep, folderInput('data')), new PackedScanStore(rootPath, sep, folderInput('data'))];
228+
for (const store of stores) {
229+
const build = (parent: number, list: Listed[]): void => {
230+
for (const l of list) {
231+
const id = store.addNode(parent, input(l));
232+
if (l.children) build(id, l.children);
233+
}
234+
};
235+
build(store.rootId, listing);
236+
store.finalize();
237+
store.sumSizes();
238+
}
239+
for (const store of stores) {
240+
// Walked from the root by the store's own child lists, in the order they
241+
// were listed: a packed store numbers its nodes anew when it finalizes.
242+
const label = `${rootPath} on the ${store.constructor.name}`;
243+
const stack: Array<{ id: number; path: string; list: Listed[] }> = [{ id: store.rootId, path: rootPath, list: listing }];
244+
while (stack.length > 0) {
245+
const { id, path: parentPath, list } = stack.pop() as { id: number; path: string; list: Listed[] };
246+
const kids = store.childIds(id);
247+
assert.equal(kids.length, list.length, `${label}: ${JSON.stringify(parentPath)} holds what was listed in it`);
248+
kids.forEach((kid, i) => {
249+
const { name, children } = list[i];
250+
const path = `${parentPath}${sep}${name}`;
251+
assert.equal(store.name(kid), name, `${label}: the name ${JSON.stringify(name)} as it was listed`);
252+
assert.equal(store.path(kid), path, `${label}: path() of ${JSON.stringify(path)}`);
253+
assert.equal(store.childPath(kid, store.path(id)), path, `${label}: childPath() of ${JSON.stringify(path)}`);
254+
// A lookup answers one node for a path: each node's finding itself is
255+
// every path being its own.
256+
assert.equal(store.findByPath(store.path(kid)), kid, `${label}: path() of ${JSON.stringify(path)} finds that node, not another`);
257+
if (children) stack.push({ id: kid, path, list: children });
258+
});
259+
}
260+
}
261+
const [obj, packed] = stores;
262+
assert.equal((await streamed(packed, 13)).json, wholeTreeJson(obj), `${rootPath}: the packed store's scan JSON is the object store's`);
263+
}
264+
});
265+
206266
test('eviction semantics: dropping the store frees it for GC (no registry leaks)', () => {
207267
// The store keeps no module-level registries — everything is instance state,
208268
// so dropping the last reference is sufficient for GC. This guards against

‎tests/queryWalkWork.test.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,3 +442,26 @@ test('CI4 review F1.2: a sort tie that reads a kept folder\'s path while its fil
442442
assert.equal(probes, 1, 'the deepest folder\'s path is built once, for its tie');
443443
assert.ok(held < HELD_PER_FOLDER * DEPTH, heldMessage('type:dir', held, DEPTH, keptPath));
444444
});
445+
446+
test('CI4 review F2.1/F2.2: a hit whose name begins with a byte-order mark keeps it — "\\uFEFFfoo" beside "foo", and "\\uFEFF" alone, are answered as the files they are, each under its own path', async () => {
447+
// A hit's path is what DELETE /api/files, the cart and an Autopilot query
448+
// policy act on. Read without its mark, the 900-byte '\uFEFFfoo' was
449+
// answered as '/data/foo', the 5-byte file beside it, and the 7-byte
450+
// '\uFEFF' as '/data/', its folder. The oracle is the listing the store was
451+
// built from, as a walker's fs.readdir gives the names.
452+
const BOM = '\uFEFF';
453+
const file = (dir: string, name: string, size: number): FileNode => ({ name, path: `${dir}/${name}`, type: 'file', extension: '', size, modifiedAt: T - YEAR, isHidden: false });
454+
const sub = `/data/${BOM}sub`;
455+
const listed = [file('/data', 'foo', 5), file('/data', `${BOM}foo`, 900), file('/data', BOM, 7), file(sub, BOM, 13), file(sub, 'foo', 11)];
456+
const tree: FileNode = {
457+
name: 'data', path: '/data', type: 'dir', size: 936, modifiedAt: T - YEAR, isHidden: false,
458+
children: [...listed.slice(0, 3), { name: `${BOM}sub`, path: sub, type: 'dir', size: 24, modifiedAt: T - YEAR, isHidden: false, children: listed.slice(3) }],
459+
};
460+
const out = await run(scanOf(tree), 'type:file');
461+
const bySize = (a: { size: number }, b: { size: number }): number => a.size - b.size;
462+
assert.deepEqual(
463+
out.hits.map((h) => ({ path: h.path, name: h.name, size: h.size })).sort(bySize),
464+
listed.map((n) => ({ path: n.path, name: n.name, size: n.size })).sort(bySize),
465+
'every file answered under its own name and path',
466+
);
467+
});

0 commit comments

Comments
 (0)