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/kernel/overlay.ml | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) (limited to 'lib/kernel/overlay.ml') diff --git a/lib/kernel/overlay.ml b/lib/kernel/overlay.ml index 58d8847..516379d 100644 --- a/lib/kernel/overlay.ml +++ b/lib/kernel/overlay.ml @@ -132,23 +132,6 @@ let fill_defaults ~id sexp = actually reach a user into the vocabulary of the file they are looking at. Anything unrecognised passes through verbatim rather than being reworded into something possibly wrong. *) -let humanise_error msg = - let replace ~sub ~by s = - let n = String.length sub and len = String.length s in - let rec go i acc = - if i > len - n then acc ^ String.sub s i (len - i) - else if String.equal (String.sub s i n) sub then go (i + n) (acc ^ by) - else go (i + 1) (acc ^ String.make 1 s.[i]) - in - if n = 0 then s else go 0 "" - in - msg - |> replace ~sub:"lib/kernel/celebration.ml.t_of_sexp" ~by:"celebration" - |> replace ~sub:"lib/kernel/colour.ml.t_of_sexp" ~by:"colour" - |> replace ~sub:"lib/kernel/subject.ml.t_of_sexp" ~by:"subject" - |> replace ~sub:"lib/kernel/date_spec.ml.t_of_sexp" ~by:"date" - |> replace ~sub:"lib/kernel/overlay.ml.directive_of_sexp" ~by:"directive" - let load rank_of_sexp path = match Sexplib.Sexp.load_sexp path with | exception Sys_error msg -> Error msg @@ -157,7 +140,8 @@ let load rank_of_sexp path = [Sexplib.Sexp.Parse_error], so a catch-all here -- placed last among the exception branches -- is what actually keeps every parse failure inside [Error] instead of escaping. *) - | exception exn -> Error (Printf.sprintf "%s: %s" path (Printexc.to_string exn)) + | exception exn -> + Error (Printf.sprintf "%s: %s" path (Sexp_error.humanise (Printexc.to_string exn))) | sexp -> ( (* The overlay's own [id] is needed to default [layer], so read it off the raw sexp first. If it is missing or malformed the derived parser @@ -174,4 +158,4 @@ let load rank_of_sexp path = match t_of_sexp rank_of_sexp (fill_defaults ~id sexp) with | t -> Ok t | exception exn -> - Error (Printf.sprintf "%s: %s" path (humanise_error (Printexc.to_string exn)))) + Error (Printf.sprintf "%s: %s" path (Sexp_error.humanise (Printexc.to_string exn)))) -- cgit v1.3