From 1988d350242b47aa52aa07904c495e7e2c0eba82 Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Thu, 20 Aug 2026 22:11:17 +0200 Subject: fix: audit findings — parser strictness, name ambiguity, and errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by auditing the shipped program rather than the diff. The parser accepted OCaml integer-literal syntax, so "Luke 1_1:5" read as chapter ELEVEN and "+5" as 5 -- a typo silently becoming a different chapter, reachable through any user overlay. Numbers are now plain digits and positive, and a descending range is rejected: 1:20-10 is always a transcription error. No shipped citation changed. FOUR PAIRS OF DIFFERENT BOOKS SHARED A FULL TITLE. 1 and 2 Corinthians both rendered "Epistola ad Corinthios", as did Thessalonians, Timothy and Peter -- 108 citations in 2027 alone that a reader cannot resolve to a book. This is the Kings defect fixed earlier and not generalised. The titles now carry their volume numeral, marked CONSTRUCTED, and a test asserts no two books share a name -- while allowing the case where two ids ARE the same book under different numbering, which a tradition relates. Spec section 8.5 is now delivered rather than merely recorded. Shipped styles did not re-parse their own output: 32 of 52 Latin abbreviations and 49 of 52 full titles failed, so a citation copied from colitur's own output into an overlay was passed through untouched and printed in the wrong language, silently. Every shipped name is registered as a spelling and split_book learned multi-word titles by longest-token match. Now 0 of 52 fail beyond the same-book aliases. Overlay errors were written for a compiler author: they named an OCaml source file the reader does not have and buried the useful token. The existing five-path rewriter is replaced by a generic one, applied to every load path rather than one, so "rank: is not one of the allowed values (at Class9)" replaces the raw Of_sexp_error dump. Also: the new-overlay scaffold documented citations and layer without showing them, and its comment implied the wrong nesting -- the single easiest thing to get wrong; error messages echoed whole file lines, copying an unrelated file's contents into stderr when a flag pointed at one; and config --show validated partway down its table, exiting 2 after writing five rows to stdout. --- lib/citation/parse.ml | 54 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-) (limited to 'lib/citation/parse.ml') diff --git a/lib/citation/parse.ml b/lib/citation/parse.ml index 45fbb72..2f2316e 100644 --- a/lib/citation/parse.ml +++ b/lib/citation/parse.ml @@ -9,7 +9,41 @@ let split_on c s = String.split_on_char c s |> List.map String.trim (* The book is the longest leading run of non-digit words, allowing one leading ordinal ("1 Cor", "3 Kings"). Everything after it is the reference tail. *) +(* Longest REGISTERED token that prefixes [s] and is followed by a space and + a digit. This is what lets a MULTI-WORD title parse: the heuristic below + stops at the first space, so "Evangelium secundum Lucam 5:12-14" would + otherwise split as the book "Evangelium" and fail. + + Longest-match matters and is not decoration: "Liber Regum III" and + "Liber Regum IV" share a prefix with each other, and a shortest-match + would read both as some other book entirely. *) +let longest_token_prefix s = + let n = String.length s in + let best = ref None in + List.iter + (fun (tok, _) -> + let tl = String.length tok in + if + tl < n + && String.sub s 0 tl = tok + && s.[tl] = ' ' + (* a digit must follow, or "Job" would swallow the start of a + different book whose name merely begins the same way *) + && (let j = ref (tl + 1) in + while !j < n && s.[!j] = ' ' do incr j done; + !j < n && s.[!j] >= '0' && s.[!j] <= '9') + then + match !best with + | Some (b, _) when String.length b >= tl -> () + | _ -> best := Some (tok, String.trim (String.sub s tl (n - tl))) + ) + Book.tokens; + !best + let split_book s = + match longest_token_prefix s with + | Some (book, tail) when tail <> "" -> Some (book, tail) + | _ -> let n = String.length s in let i = ref 0 in (* optional leading ordinal digit *) @@ -25,7 +59,21 @@ let split_book s = let tail = String.trim (String.sub s !i (n - !i)) in if book = "" || tail = "" then None else Some (book, tail) -let int_opt s = int_of_string_opt (String.trim s) +(* A citation number is PLAIN DIGITS and positive -- nothing else. + [int_of_string_opt] also accepts OCaml's own integer-literal syntax, so + "1_1" would read as 11 and "+5" as 5: a transcription typo silently + becoming a DIFFERENT chapter, which nothing downstream could detect. A + user overlay supplies arbitrary strings, so this is reachable, not + theoretical. Chapter and verse numbering both start at 1, so zero is + rejected too. *) +let int_opt s = + let s = String.trim s in + let ok = + s <> "" + && String.for_all (function '0' .. '9' -> true | _ -> false) s + in + if not ok then None + else match int_of_string_opt s with Some n when n > 0 -> Some n | _ -> None (* "20-32" -> {first=20; last=Some 32}; "21" -> {first=21; last=None} *) let parse_range s = @@ -33,7 +81,9 @@ let parse_range s = | [ a ] -> ( match int_opt a with Some f -> Some { first = f; last = None } | None -> None) | [ a; b ] -> ( match (int_opt a, int_opt b) with - | Some f, Some l -> Some { first = f; last = Some l } + (* A descending range ("1:20-10") is always a transcription error; + accepting it would render back out as a citation nobody can follow. *) + | Some f, Some l when l >= f -> Some { first = f; last = Some l } | _ -> None) | _ -> None -- cgit v1.3