From a526d934cea17ff1b76ddad5c5f2934fb85c11cb Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Wed, 19 Aug 2026 08:09:12 +0200 Subject: fix(render): reject empty tag paths, sharpen the raw-form test F1: test_no_raw_or_partial_form's first assertion only excluded one literal shape (Ok [Var ["{name"]]), so it could not actually catch a future raw/unescaped constructor under a different name. Replace it with an assertion of the real parse result for {{{name}}} (Ok [Var ["{name"]; Text "}"]), documented behaviour rather than a guarantee this test cannot check -- the real guarantee is structural: node has exactly four constructors and none of them is raw. F2: {{.}}, {{#}}, {{^}} and {{/}} used to parse to a Var/Section/ Inverted with an empty path, reachable but never designed. This engine has no "current context" for a bare dot to mean, so a bare-dot or empty-sigil path is now a parse error at lex time, covering all four sigil forms via one path helper. The existing "empty tag {{}}" branch is unchanged and still reachable (a fully empty body is a distinct case from a sigil with an empty path). --- lib/render/template.ml | 18 ++++++++++++++---- test/test_template.ml | 31 ++++++++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/lib/render/template.ml b/lib/render/template.ml index 8de91a6..3e35d53 100644 --- a/lib/render/template.ml +++ b/lib/render/template.ml @@ -38,13 +38,23 @@ let lex src = let b = trim body in if b = "" then Error "empty tag {{}}" else + (* Every sigil form (var, #, ^, /) resolves a dotted path; an + empty path ("{{.}}", "{{#}}", "{{^}}", "{{/}}") names + nothing -- there is no "current context" concept for a bare + dot to mean, so it is a parse error rather than an invented + behaviour Task 3 would have to define. *) + let with_path ctor rest = + match path rest with + | [] -> Error "empty tag path: {{.}} and {{#}} name nothing" + | p -> Ok (ctor p) + in match b.[0] with - | '#' -> Ok (TOpen (path (String.sub b 1 (String.length b - 1)))) - | '^' -> Ok (TInv (path (String.sub b 1 (String.length b - 1)))) - | '/' -> Ok (TClose (path (String.sub b 1 (String.length b - 1)))) + | '#' -> with_path (fun p -> TOpen p) (String.sub b 1 (String.length b - 1)) + | '^' -> with_path (fun p -> TInv p) (String.sub b 1 (String.length b - 1)) + | '/' -> with_path (fun p -> TClose p) (String.sub b 1 (String.length b - 1)) | '!' -> Ok TComment | '>' -> Error "partials are not supported: a template may not include another file" - | _ -> Ok (TVar (path b)) + | _ -> with_path (fun p -> TVar p) b in (match tag with | Error e -> Error e diff --git a/test/test_template.ml b/test/test_template.ml index dee281b..cfb3b04 100644 --- a/test/test_template.ml +++ b/test/test_template.ml @@ -42,12 +42,36 @@ let test_unterminated_tag_is_an_error () = (* The safety promise (spec section 5): a template is data, never a program. There is no raw/triple-brace form to opt out of escaping, and no partial. *) let test_no_raw_or_partial_form () = - Alcotest.(check bool) "triple brace is not an unescaped var" true - (match T.parse "{{{name}}}" with Ok [ T.Var [ "{name" ] ] -> false | Ok _ -> true | Error _ -> true); + (* {{{name}}} is not a triple-brace "unescaped" form -- there is no such + form in this engine, so this assertion states what parsing it ACTUALLY + produces rather than gesturing at a guarantee it cannot check: the + lexer only recognises double braces, so it reads an ordinary var whose + path is the literal string "{name", then a leftover "}" as text. The + real guarantee against a raw form is structural, not this assertion: + [node] has exactly four constructors -- Text, Var, Section, Inverted -- + and none of them is raw/unescaped. If [node] ever grows a fifth, raw + constructor, this test is not what will catch it. *) + Alcotest.(check bool) "triple brace parses as var + stray text" true + (parse_ok "{{{name}}}" = [ T.Var [ "{name" ]; T.Text "}" ]); match T.parse "{{>partial}}" with | Error _ -> () | Ok _ -> Alcotest.fail "partials must not parse: a template may not pull in another file" +(* No "current context" concept exists in this engine (sections push an + object; lookup falls back outward), so a bare-dot or empty-sigil path has + no meaning. Reject at parse time rather than inventing behaviour Task 3's + renderer would otherwise have to define for a case nobody designed. *) +let test_empty_path_is_an_error () = + let expect_error label s = + match T.parse s with + | Error _ -> () + | Ok _ -> Alcotest.failf "%s: an empty tag path must be a parse error" label + in + expect_error "{{.}}" "{{.}}"; + expect_error "{{ . }}" "{{ . }}"; + expect_error "{{#}}{{/}}" "{{#}}{{/}}"; + expect_error "{{^}}{{/}}" "{{^}}{{/}}" + let suite = ( "Template/parse", [ Alcotest.test_case "plain text" `Quick test_plain_text; @@ -58,4 +82,5 @@ let suite = Alcotest.test_case "unclosed section errors" `Quick test_unclosed_section_is_an_error; Alcotest.test_case "mismatched close errors" `Quick test_mismatched_close_is_an_error; Alcotest.test_case "unterminated tag errors" `Quick test_unterminated_tag_is_an_error; - Alcotest.test_case "no raw or partial form" `Quick test_no_raw_or_partial_form ] ) + Alcotest.test_case "no raw or partial form" `Quick test_no_raw_or_partial_form; + Alcotest.test_case "empty path errors" `Quick test_empty_path_is_an_error ] ) -- cgit v1.3