From 9473b01a79484c1eb5d3ffd4875a1f49cfb3bf1c Mon Sep 17 00:00:00 2001 From: huntedbytheirs Date: Sat, 8 Aug 2026 18:54:05 -0400 Subject: [PATCH] fix(info): handle missing deps field without unsigned indexOf wrap --- .omo/evidence/task-27-tofu-core.log | 24 +++++++++++++++++ .omo/notepads/tofu-core/learnings.md | 19 +++++++++++++ .omo/notepads/tofu-core/problems.md | 13 +++++++++ src/tofu/commands/info.d | 40 ++++++++++++++++++++++++---- 4 files changed, 91 insertions(+), 5 deletions(-) diff --git a/.omo/evidence/task-27-tofu-core.log b/.omo/evidence/task-27-tofu-core.log index de4256d..06e87d6 100644 --- a/.omo/evidence/task-27-tofu-core.log +++ b/.omo/evidence/task-27-tofu-core.log @@ -42,3 +42,27 @@ step 13: cleanup... PASS === smoketest complete === PASS: 16 checks passed All checks passed. + +=== FIX (task 27): info.d unsigned indexOf wrap === +Bug: src/tofu/commands/info.d:93 — `size_t pos = content.indexOf("deps")`. +indexOf returns ptrdiff_t (-1 for not found); unsigned size_t wraps -1 to +SIZE_MAX, making `if (pos < 0)` a no-op → ArrayIndexError on recipes +without a deps field (uncaught D Error, crashed -Si). + +Fix applied: + 1. scanDepsArray: size_t pos → ptrdiff_t pos (+ explanatory comment). + 2. Added regression unittest (Test 4): cached recipe WITHOUT deps field + (name/ver/summary/build_system only) → infoCommand returns 0. + Recipe written via existing writeRecipe helper, mirrors Test 3 pattern. + +Audit of same pattern elsewhere (checked, NOT bugs): + - info.d:36 auto kp = ...indexOf(key) → auto infers ptrdiff_t ✓ + - recipeparse.d:52,112 auto idx = indexOf(...) → ptrdiff_t ✓ + - fetch.d:82 auto idx = indexOf(...) → ptrdiff_t ✓ + - search.d / remove.d / install.d → inline >= 0 / == -1 ✓ + No other unsigned indexOf assignment found. + +Verification: + dub build → Linking tofu (pass) + dub test → 23 modules passed unittests + smoketest → PASS: 16 checks passed, All checks passed. diff --git a/.omo/notepads/tofu-core/learnings.md b/.omo/notepads/tofu-core/learnings.md index 372b29b..268be22 100644 --- a/.omo/notepads/tofu-core/learnings.md +++ b/.omo/notepads/tofu-core/learnings.md @@ -1399,3 +1399,22 @@ The sandbox test (test 2) creates a sentinel file, serves an index containing `o - `-S nonexistent --noconfirm` → exit 2 (package not found in index) - No-args → exit 1, `--help` → exit 0 + +## Task 27 — `std.string.indexOf` unsigned-wrap trap (info.d fix) + +- **`std.string.indexOf` returns `ptrdiff_t`** — `-1` means "not found". Assigning it to + an unsigned type (`size_t`/`uint`) silently wraps `-1` to `SIZE_MAX`, and a following + `if (pos < 0)` guard becomes a compile-time-legal but always-false no-op. Downstream + `content[pos .. $]` then throws `ArrayIndexError` — a D `Error` (NOT an `Exception`), + so `catch (Exception)` blocks do not trap it and the process aborts. +- **`auto` is the safe default**: `auto idx = content.indexOf(key)` infers `ptrdiff_t`, + so `if (idx < 0)` works. Only explicit unsigned typing (`size_t pos = ...`) is broken. + Audit rule: when declaring a named variable for an `indexOf`/`indexOfSlice` result, + always use `ptrdiff_t` (or `auto`). +- **D `Error` vs `Exception`**: out-of-bounds slice indexing throws `ArrayIndexError` + which derives from `Error`, bypassing `catch (Exception)`. A whole-program crash + (uncaught Error) is a strong smell of this class of bug — the run-time failure mode + is a naked `core.exception.ArrayIndexError` on stderr. +- **Escape in unit tests**: all 8 pre-existing info.d tests used recipes WITH a `deps` + field, so the absent-deps path was never exercised. Regression tests must cover the + *negative* field, not just happy paths. diff --git a/.omo/notepads/tofu-core/problems.md b/.omo/notepads/tofu-core/problems.md index d0f41e8..b771301 100644 --- a/.omo/notepads/tofu-core/problems.md +++ b/.omo/notepads/tofu-core/problems.md @@ -23,3 +23,16 @@ _Auto-scaffolded by /start-work. Append new entries below - never overwrite._ **Workaround in smoketest:** Recipe includes `deps = {}` to avoid triggering this bug. +--- + +## RESOLVED (task 27) + +**Fixed:** `scanDepsArray` in `src/tofu/commands/info.d` — `size_t pos` → `ptrdiff_t pos` +so the `pos < 0` guard fires on `indexOf` returning -1. Added a regression unittest +(recipe without deps → infoCommand returns 0). Audit of the same pattern across +`info.d` (line 36), `recipeparse.d` (52, 112), `fetch.d` (82), `search.d`, `remove.d`, +`install.d` found no other unsigned indexOf assignment (`auto` infers `ptrdiff_t` +everywhere else). + +Verified: `dub build` ✓, `dub test` — 23 modules passed ✓, smoketest — 16 checks PASS ✓. + diff --git a/src/tofu/commands/info.d b/src/tofu/commands/info.d index ba52a24..64c5039 100644 --- a/src/tofu/commands/info.d +++ b/src/tofu/commands/info.d @@ -90,7 +90,11 @@ private string scanStringField(string content, string key) { /// and return the joined dependency list as `"v1, v2, ..."`. /// Returns `""` when `deps` is absent or the table is empty. private string scanDepsArray(string content) { - size_t pos = content.indexOf("deps"); + // indexOf returns ptrdiff_t (-1 for not found); store in a SIGNED type + // so the `pos < 0` guard below actually fires. An unsigned size_t would + // wrap -1 to SIZE_MAX and the guard becomes a no-op, crashing later on + // content[pos .. $] with ArrayIndexError when "deps" is absent. + ptrdiff_t pos = content.indexOf("deps"); if (pos < 0) return ""; // Word-boundary check @@ -352,7 +356,33 @@ LUA"); assert(ec == 0); } -// ── Test (4): installed + up to date → status line ──────────── +// ── Test (4): recipe with NO deps field → no crash (regression) ─ +// Regression for the unsigned indexOf wrap: scanDepsArray previously +// stored indexOf's -1 into size_t (→ SIZE_MAX), making the `pos < 0` +// guard a no-op and crashing with ArrayIndexError on content[pos .. $]. +@safe unittest { + auto cfg = testCfg("nodeps"); + scope (exit) removeDir(cfg.cacheDir); + + writeRecipe(cfg, "ripgrep", q"LUA +return { + name = "ripgrep", + ver = "14.1.0", + summary = "Fast grep", + build_system = "cargo", +} +LUA"); + + auto flags = dummyArgs("ripgrep"); + auto fetcher = delegate PackageIndex[](Config _) @safe { + return makeFakeIndex(); + }; + + int ec = infoCommand("ripgrep", flags, cfg, fetcher); + assert(ec == 0, "expected exit 0 for recipe without deps, got " ~ to!string(ec)); +} + +// ── Test (5): installed + up to date → status line ──────────── @safe unittest { auto cfg = testCfg("uptodate"); scope (exit) removeDir(cfg.cacheDir); @@ -369,7 +399,7 @@ LUA"); assert(ec == 0); } -// ── Test (5): installed + outdated → status line with zuur ver ─ +// ── Test (6): installed + outdated → status line with zuur ver ─ @safe unittest { auto cfg = testCfg("outdated"); scope (exit) removeDir(cfg.cacheDir); @@ -386,7 +416,7 @@ LUA"); assert(ec == 0); } -// ── Test (6): not installed → no status line ───────────────── +// ── Test (7): not installed → no status line ───────────────── @safe unittest { auto cfg = testCfg("notinst"); scope (exit) removeDir(cfg.cacheDir); @@ -400,7 +430,7 @@ LUA"); assert(ec == 0); } -// ── Test (7): pool=binary only → no recipe section ──────────── +// ── Test (8): pool=binary only → no recipe section ──────────── @safe unittest { auto cfg = testCfg("binaryonly"); scope (exit) removeDir(cfg.cacheDir);