Prevent macros from panicking when called with wrong state (#330)

CHANGELOG.md

@@ -4,6 +4,7 @@ All notable changes to MiniJinja are documented here.
 
 ## 1.0.7
 
+- Macros called with the wrong state will no longer panic.  #330
 - Debug output of `Value::UNDEFINED` and `Value::from(())` is now
   `undefined` and `none` rather than `Undefined` and `None`.  This was
   an accidental inconsistency.

minijinja/src/vm/macro_object.rs

@@ -19,6 +19,7 @@ pub(crate) struct MacroData {
     // reference to the macro instruction (and the jump offset) in the
     // state under `state.macros`.
     pub macro_ref_id: usize,
+    pub state_id: isize,
     pub closure: Value,
     pub caller_reference: bool,
 }
@@ -47,6 +48,14 @@ impl Object for Macro {
     }
 
     fn call(&self, state: &State, args: &[Value]) -> Result<Value, Error> {
+        // we can only call macros that point to loaded template state.
+        if state.id != self.data.state_id {
+            return Err(Error::new(
+                ErrorKind::InvalidOperation,
+                "cannot call this macro. template state went away.",
+            ));
+        }
+
         let (args, kwargs) = match args.last() {
             Some(Value(ValueRepr::Map(kwargs, MapType::Kwargs))) => {
                 (&args[..args.len() - 1], Some(kwargs))

minijinja/src/vm/mod.rs

@@ -127,6 +127,8 @@ impl<'env> Vm<'env> {
                 blocks: BTreeMap::default(),
                 loaded_templates: Default::default(),
                 #[cfg(feature = "macros")]
+                id: state.id,
+                #[cfg(feature = "macros")]
                 macros: state.macros.clone(),
                 #[cfg(feature = "fuel")]
                 fuel_tracker: state.fuel_tracker.clone(),
@@ -946,6 +948,7 @@ impl<'env> Vm<'env> {
                 name: Arc::from(name.to_string()),
                 arg_spec,
                 macro_ref_id,
+                state_id: state.id,
                 closure,
                 caller_reference: (flags & MACRO_CALLER) != 0,
             }),

minijinja/src/vm/state.rs

@@ -13,6 +13,14 @@ use crate::vm::context::Context;
 #[cfg(feature = "fuel")]
 use crate::vm::fuel::FuelTracker;
 
+/// When macros are used, the state carries an `id` counter.  Whenever a state is
+/// created, the counter is incremented.  This exists because macros can keep a reference
+/// to instructions from another state by index.  Without this counter it would
+/// be possible for a macro to be called with a different state (different id)
+/// which mean we likely panic.
+#[cfg(feature = "macros")]
+static STATE_ID: std::sync::atomic::AtomicIsize = std::sync::atomic::AtomicIsize::new(0);
+
 /// Provides access to the current execution state of the engine.
 ///
 /// A read only reference is passed to filter functions and similar objects to
@@ -40,6 +48,8 @@ pub struct State<'template, 'env> {
     #[allow(unused)]
     pub(crate) loaded_templates: BTreeSet<&'env str>,
     #[cfg(feature = "macros")]
+    pub(crate) id: isize,
+    #[cfg(feature = "macros")]
     pub(crate) macros: std::sync::Arc<Vec<(&'template Instructions<'env>, usize)>>,
     #[cfg(feature = "fuel")]
     pub(crate) fuel_tracker: Option<std::sync::Arc<FuelTracker>>,
@@ -67,6 +77,8 @@ impl<'template, 'env> State<'template, 'env> {
         blocks: BTreeMap<&'env str, BlockStack<'template, 'env>>,
     ) -> State<'template, 'env> {
         State {
+            #[cfg(feature = "macros")]
+            id: STATE_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed),
             env,
             ctx,
             current_block: None,

minijinja/tests/test_macros.rs

@@ -1,7 +1,7 @@
 use similar_asserts::assert_eq;
 
 use minijinja::value::{Kwargs, StructObject, Value};
-use minijinja::{args, context, render, Environment};
+use minijinja::{args, context, render, Environment, ErrorKind};
 
 #[test]
 fn test_context() {
@@ -92,3 +92,25 @@ fn test_args() {
     assert_eq!(kwargs.get::<i32>("bar").unwrap(), 23);
     assert_eq!(type_name_of_val(args), "[minijinja::value::Value]");
 }
+
+#[test]
+fn test_macro_passing() {
+    let env = Environment::new();
+    let tmpl = env
+        .template_from_str("{% macro m(a) %}{{ a }}{% endmacro %}")
+        .unwrap();
+    let (_, state) = tmpl.render_and_return_state(()).unwrap();
+    let m = state.lookup("m").unwrap();
+    assert_eq!(m.get_attr("name").unwrap().as_str(), Some("m"));
+    let rv = m.call(&state, args!(42)).unwrap();
+    assert_eq!(rv.as_str(), Some("42"));
+
+    // if we call the macro on an empty state it errors
+    let empty_state = env.empty_state();
+    let err = m.call(&empty_state, args!(42)).unwrap_err();
+    assert_eq!(err.kind(), ErrorKind::InvalidOperation);
+    assert_eq!(
+        err.detail(),
+        Some("cannot call this macro. template state went away.")
+    );
+}