Repository: mitsuhiko/minijinja
Pull request #479 (2024-04-08 10:31:33)

Fix dotted error
The parser allowed some syntax constructs that would create bad code.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a399418..933e4b2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -47,6 +47,7 @@ particular when implementing dynamic objects.  In short:
 - `DynObject` now replaces `Arc<Object>`.
 - The debug printing of some objects was simplified.
 - Added the `iterable` test.  #475
+- The parser no longer panics when using dotted assignments in unexpected places. #479
 
 ## 1.0.17
 
diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock
new file mode 100644
index 0000000..d83883f
--- /dev/null
+++ b/fuzz/Cargo.lock
@@ -0,0 +1,168 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 3
+
+[[package]]
+name = "arbitrary"
+version = "1.3.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7d5a26814d8dcb93b0e5a0ff3c6d80a8843bafb21b39e8e18a6f05471870e110"
+dependencies = [
+ "derive_arbitrary",
+]
+
+[[package]]
+name = "cc"
+version = "1.0.91"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "1fd97381a8cc6493395a5afc4c691c1084b3768db713b73aa215217aa245d153"
+dependencies = [
+ "jobserver",
+ "libc",
+]
+
+[[package]]
+name = "derive_arbitrary"
+version = "1.3.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "67e77553c4162a157adbf834ebae5b415acbecbeafc7a74b0e886657506a7611"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
+[[package]]
+name = "itoa"
+version = "1.0.11"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "49f1f14873335454500d59611f1cf4a4b0f786f9ac11f4312a78e4cf2566695b"
+
+[[package]]
+name = "jobserver"
+version = "0.1.28"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ab46a6e9526ddef3ae7f787c06f0f2600639ba80ea3eade3d8e670a2230f51d6"
+dependencies = [
+ "libc",
+]
+
+[[package]]
+name = "libc"
+version = "0.2.153"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9c198f91728a82281a64e1f4f9eeb25d82cb32a5de251c6bd1b5154d63a8e7bd"
+
+[[package]]
+name = "libfuzzer-sys"
+version = "0.4.7"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a96cfd5557eb82f2b83fed4955246c988d331975a002961b07c81584d107e7f7"
+dependencies = [
+ "arbitrary",
+ "cc",
+ "once_cell",
+]
+
+[[package]]
+name = "minijinja"
+version = "1.0.16"
+dependencies = [
+ "percent-encoding",
+ "serde",
+ "serde_json",
+]
+
+[[package]]
+name = "minijinja-fuzz"
+version = "0.0.0"
+dependencies = [
+ "arbitrary",
+ "libfuzzer-sys",
+ "minijinja",
+ "serde",
+]
+
+[[package]]
+name = "once_cell"
+version = "1.19.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3fdb12b2476b595f9358c5161aa467c2438859caa136dec86c26fdd2efe17b92"
+
+[[package]]
+name = "percent-encoding"
+version = "2.3.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e3148f5046208a5d56bcfc03053e3ca6334e51da8dfb19b6cdc8b306fae3283e"
+
+[[package]]
+name = "proc-macro2"
+version = "1.0.79"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e835ff2298f5721608eb1a980ecaee1aef2c132bf95ecc026a11b7bf3c01c02e"
+dependencies = [
+ "unicode-ident",
+]
+
+[[package]]
+name = "quote"
+version = "1.0.35"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "291ec9ab5efd934aaf503a6466c5d5251535d108ee747472c3977cc5acc868ef"
+dependencies = [
+ "proc-macro2",
+]
+
+[[package]]
+name = "ryu"
+version = "1.0.17"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e86697c916019a8588c99b5fac3cead74ec0b4b819707a682fd4d23fa0ce1ba1"
+
+[[package]]
+name = "serde"
+version = "1.0.197"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3fb1c873e1b9b056a4dc4c0c198b24c3ffa059243875552b2bd0933b1aee4ce2"
+dependencies = [
+ "serde_derive",
+]
+
+[[package]]
+name = "serde_derive"
+version = "1.0.197"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7eb0b34b42edc17f6b7cac84a52a1c5f0e1bb2227e997ca9011ea3dd34e8610b"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
+[[package]]
+name = "serde_json"
+version = "1.0.115"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "12dc5c46daa8e9fdf4f5e71b6cf9a53f2487da0e86e55808e2d35539666497dd"
+dependencies = [
+ "itoa",
+ "ryu",
+ "serde",
+]
+
+[[package]]
+name = "syn"
+version = "2.0.58"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "44cfb93f38070beee36b3fef7d4f5a16f27751d94b187b666a5cc5e9b0d30687"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "unicode-ident",
+]
+
+[[package]]
+name = "unicode-ident"
+version = "1.0.12"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b"
diff --git a/minijinja/src/compiler/parser.rs b/minijinja/src/compiler/parser.rs
index 44f3731..74d5316 100644
--- a/minijinja/src/compiler/parser.rs
+++ b/minijinja/src/compiler/parser.rs
@@ -686,21 +686,23 @@ impl<'a> Parser<'a> {
         })
     }
 
-    fn parse_assign_name(&mut self) -> Result<ast::Expr<'a>, Error> {
+    fn parse_assign_name(&mut self, dotted: bool) -> Result<ast::Expr<'a>, Error> {
         let (id, span) = expect_token!(self, Token::Ident(name) => name, "identifier");
         if RESERVED_NAMES.contains(&id) {
             syntax_error!("cannot assign to reserved variable name {}", id);
         }
         let mut rv = ast::Expr::Var(ast::Spanned::new(ast::Var { id }, span));
-        while skip_token!(self, Token::Dot) {
-            let (attr, span) = expect_token!(self, Token::Ident(name) => name, "identifier");
-            rv = ast::Expr::GetAttr(ast::Spanned::new(
-                ast::GetAttr {
-                    expr: rv,
-                    name: attr,
-                },
-                span,
-            ));
+        if dotted {
+            while skip_token!(self, Token::Dot) {
+                let (attr, span) = expect_token!(self, Token::Ident(name) => name, "identifier");
+                rv = ast::Expr::GetAttr(ast::Spanned::new(
+                    ast::GetAttr {
+                        expr: rv,
+                        name: attr,
+                    },
+                    span,
+                ));
+            }
         }
         Ok(rv)
     }
@@ -725,7 +727,7 @@ impl<'a> Parser<'a> {
                 expect_token!(self, Token::ParenClose, "`)`");
                 rv
             } else {
-                ok!(self.parse_assign_name())
+                ok!(self.parse_assign_name(false))
             });
             if matches_token!(self, Token::Comma) {
                 is_tuple = true;
@@ -811,7 +813,7 @@ impl<'a> Parser<'a> {
                 expect_token!(self, Token::ParenClose, "`)`");
                 assign
             } else {
-                ok!(self.parse_assign_name())
+                ok!(self.parse_assign_name(false))
             };
             expect_token!(self, Token::Assign, "assignment operator");
             let expr = ok!(self.parse_expr());
@@ -830,7 +832,7 @@ impl<'a> Parser<'a> {
             expect_token!(self, Token::ParenClose, "`)`");
             (assign, true)
         } else {
-            (ok!(self.parse_assign_name()), false)
+            (ok!(self.parse_assign_name(true)), false)
         };
 
         if !in_paren && matches_token!(self, Token::BlockEnd | Token::Pipe) {
@@ -977,9 +979,9 @@ impl<'a> Parser<'a> {
             if matches_token!(self, Token::BlockEnd) {
                 break;
             }
-            let name = ok!(self.parse_assign_name());
+            let name = ok!(self.parse_assign_name(false));
             let alias = if skip_token!(self, Token::Ident("as")) {
-                Some(ok!(self.parse_assign_name()))
+                Some(ok!(self.parse_assign_name(false)))
             } else {
                 None
             };
@@ -1004,7 +1006,7 @@ impl<'a> Parser<'a> {
                     break;
                 }
             }
-            args.push(ok!(self.parse_assign_name()));
+            args.push(ok!(self.parse_assign_name(false)));
             if skip_token!(self, Token::Assign) {
                 defaults.push(ok!(self.parse_expr()));
             } else if !defaults.is_empty() {


Test patch:
diff --git a/minijinja/tests/inputs/err_bad_dotted_assign_forloop.txt b/minijinja/tests/inputs/err_bad_dotted_assign_forloop.txt
new file mode 100644
index 0000000..1d17175
--- /dev/null
+++ b/minijinja/tests/inputs/err_bad_dotted_assign_forloop.txt
@@ -0,0 +1,5 @@
+{}
+---
+{% for foo.baz in [] %}
+  ...
+{% endfor %}
\ No newline at end of file
diff --git a/minijinja/tests/inputs/err_bad_dotted_assign_macro.txt b/minijinja/tests/inputs/err_bad_dotted_assign_macro.txt
new file mode 100644
index 0000000..d3d532d
--- /dev/null
+++ b/minijinja/tests/inputs/err_bad_dotted_assign_macro.txt
@@ -0,0 +1,5 @@
+{}
+---
+{% macro foo(bar.baz) %}
+  ...
+{% endmacro %}
\ No newline at end of file
diff --git a/minijinja/tests/inputs/err_bad_dotted_assign_with.txt b/minijinja/tests/inputs/err_bad_dotted_assign_with.txt
new file mode 100644
index 0000000..ac88d79
--- /dev/null
+++ b/minijinja/tests/inputs/err_bad_dotted_assign_with.txt
@@ -0,0 +1,5 @@
+{}
+---
+{% with foo.bar = 42 %}
+  ...
+{% endwith %}
\ No newline at end of file
diff --git a/minijinja/tests/snapshots/test_templates__vm@err_bad_dotted_assign_forloop.txt.snap b/minijinja/tests/snapshots/test_templates__vm@err_bad_dotted_assign_forloop.txt.snap
new file mode 100644
index 0000000..c1794a7
--- /dev/null
+++ b/minijinja/tests/snapshots/test_templates__vm@err_bad_dotted_assign_forloop.txt.snap
@@ -0,0 +1,24 @@
+---
+source: minijinja/tests/test_templates.rs
+description: "{% for foo.baz in [] %}\n  ...\n{% endfor %}"
+info: {}
+input_file: minijinja/tests/inputs/err_bad_dotted_assign_forloop.txt
+---
+!!!SYNTAX ERROR!!!
+
+Error {
+    kind: SyntaxError,
+    detail: "unexpected `.`, expected in",
+    name: "err_bad_dotted_assign_forloop.txt",
+    line: 1,
+}
+
+syntax error: unexpected `.`, expected in (in err_bad_dotted_assign_forloop.txt:1)
+---------------------- err_bad_dotted_assign_forloop.txt ----------------------
+   1 > {% for foo.baz in [] %}
+     i           ^ syntax error
+   2 |   ...
+   3 | {% endfor %}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+No referenced variables
+-------------------------------------------------------------------------------
diff --git a/minijinja/tests/snapshots/test_templates__vm@err_bad_dotted_assign_macro.txt.snap b/minijinja/tests/snapshots/test_templates__vm@err_bad_dotted_assign_macro.txt.snap
new file mode 100644
index 0000000..4c87320
--- /dev/null
+++ b/minijinja/tests/snapshots/test_templates__vm@err_bad_dotted_assign_macro.txt.snap
@@ -0,0 +1,24 @@
+---
+source: minijinja/tests/test_templates.rs
+description: "{% macro foo(bar.baz) %}\n  ...\n{% endmacro %}"
+info: {}
+input_file: minijinja/tests/inputs/err_bad_dotted_assign_macro.txt
+---
+!!!SYNTAX ERROR!!!
+
+Error {
+    kind: SyntaxError,
+    detail: "unexpected `.`, expected `,`",
+    name: "err_bad_dotted_assign_macro.txt",
+    line: 1,
+}
+
+syntax error: unexpected `.`, expected `,` (in err_bad_dotted_assign_macro.txt:1)
+----------------------- err_bad_dotted_assign_macro.txt -----------------------
+   1 > {% macro foo(bar.baz) %}
+     i                 ^ syntax error
+   2 |   ...
+   3 | {% endmacro %}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+No referenced variables
+-------------------------------------------------------------------------------
diff --git a/minijinja/tests/snapshots/test_templates__vm@err_bad_dotted_assign_with.txt.snap b/minijinja/tests/snapshots/test_templates__vm@err_bad_dotted_assign_with.txt.snap
new file mode 100644
index 0000000..4d9b956
--- /dev/null
+++ b/minijinja/tests/snapshots/test_templates__vm@err_bad_dotted_assign_with.txt.snap
@@ -0,0 +1,24 @@
+---
+source: minijinja/tests/test_templates.rs
+description: "{% with foo.bar = 42 %}\n  ...\n{% endwith %}"
+info: {}
+input_file: minijinja/tests/inputs/err_bad_dotted_assign_with.txt
+---
+!!!SYNTAX ERROR!!!
+
+Error {
+    kind: SyntaxError,
+    detail: "unexpected `.`, expected assignment operator",
+    name: "err_bad_dotted_assign_with.txt",
+    line: 1,
+}
+
+syntax error: unexpected `.`, expected assignment operator (in err_bad_dotted_assign_with.txt:1)
+----------------------- err_bad_dotted_assign_with.txt ------------------------
+   1 > {% with foo.bar = 42 %}
+     i            ^ syntax error
+   2 |   ...
+   3 | {% endwith %}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+No referenced variables
+-------------------------------------------------------------------------------