Add support for "sane" strict variable checks

The current "strict variables" checking strategy in Liquid Templates is
somewhat unusable as one cannot even check for the existence of an
object property without raising an error in strict mode.

This is a known limitation, and actively being discussed upstream
(liquid issue 1034).

Several Jekyll template make use of checks like {% if page.mermaid %},
etc., preventing enforcement of strict mode.

This patch adds a "sane" strict variable policy, which allows for such
checks to succeed.

Since this is new functionality specific to liqp, we keep the existing
strict mode configuration unchanged.

Shopify/liquid#1034

src/main/java/liqp/TemplateParser.java

@@ -53,6 +53,30 @@ public enum ErrorMode {
         LAX
     }
 
+    /**
+     * Controls the "strict variables" checking strategy.
+     * 
+     * @see <a href="https://github.com/Shopify/liquid/issues/1034">liquid issue 1034</a>
+     */
+    public enum StrictVariablesMode {
+        /**
+         * No strict variable checking.
+         */
+        OFF,
+
+        /**
+         * Strict variables, but allow checking for the existence of directly-nested variables without
+         * throwing an error (e.g., checking for "object.missing" is acceptable, but
+         * "object.nested.missing" isn't).
+         */
+        SANE,
+
+        /**
+         * Strict variable checking.
+         */
+        STRICT
+    }
+
     public final Flavor flavor;
     public final boolean stripSpacesAroundTags;
     public final boolean stripSingleLine;
@@ -68,8 +92,13 @@ public enum ErrorMode {
 
     /**
      * The same as <code>template.render!({}, strict_variables: true)</code> in ruby
+     * 
+     * @see #strictVariablesMode
      */
+    @Deprecated(forRemoval = true)
     public final boolean strictVariables;
+    public final StrictVariablesMode strictVariablesMode;
+
     /**
      * This field doesn't have equivalent in ruby.
      */
@@ -138,7 +167,7 @@ public static class Builder {
         private Boolean liquidStyleWhere;
 
 
-        private boolean strictVariables = false;
+        private StrictVariablesMode strictVariablesMode = StrictVariablesMode.OFF;
         private boolean showExceptionsFromInclude;
         private EvaluateMode evaluateMode = EvaluateMode.LAZY;
         private Locale locale = DEFAULT_LOCALE;
@@ -165,7 +194,7 @@ public Builder(TemplateParser parser) {
             this.insertions = new ArrayList<>(parser.insertions.values());
             this.filters = new ArrayList<>(parser.filters.values());
 
-            this.strictVariables = parser.strictVariables;
+            this.strictVariablesMode = parser.strictVariablesMode;
             this.evaluateMode = parser.evaluateMode;
             this.locale = parser.locale;
             this.renderTransformer = parser.renderTransformer;
@@ -248,9 +277,15 @@ public Builder withLiquidStyleInclude(boolean liquidStyleInclude) {
             return this;
         }
 
-        @SuppressWarnings("hiding")
         public Builder withStrictVariables(boolean strictVariables) {
-            this.strictVariables = strictVariables;
+            this.strictVariablesMode = strictVariables ? StrictVariablesMode.STRICT
+                : StrictVariablesMode.OFF;
+            return this;
+        }
+
+        @SuppressWarnings("hiding")
+        public Builder withStrictVariables(StrictVariablesMode strictVariablesMode) {
+            this.strictVariablesMode = Objects.requireNonNull(strictVariablesMode);
             return this;
         }
 
@@ -404,12 +439,12 @@ public TemplateParser build() {
                 nameResolver = new LocalFSNameResolver(snippetsFolderName);
             }
 
-            return new TemplateParser(strictVariables, showExceptionsFromInclude, evaluateMode, renderTransformer, locale, defaultTimeZone, environmentMapConfigurator, errorMode, fl, stripSpacesAroundTags, stripSingleLine, mapper,
+            return new TemplateParser(strictVariablesMode, showExceptionsFromInclude, evaluateMode, renderTransformer, locale, defaultTimeZone, environmentMapConfigurator, errorMode, fl, stripSpacesAroundTags, stripSingleLine, mapper,
                     allInsertions, finalFilters, evaluateInOutputTag, strictTypedExpressions, liquidStyleInclude, liquidStyleWhere, nameResolver, limitMaxIterations, limitMaxSizeRenderedString, limitMaxRenderTimeMillis, limitMaxTemplateSizeBytes);
         }
     }
 
-    TemplateParser(boolean strictVariables, boolean showExceptionsFromInclude, EvaluateMode evaluateMode,
+    TemplateParser(StrictVariablesMode strictVariablesMode, boolean showExceptionsFromInclude, EvaluateMode evaluateMode,
                    RenderTransformer renderTransformer, Locale locale, ZoneId defaultTimeZone,
                    Consumer<Map<String, Object>> environmentMapConfigurator, ErrorMode errorMode, Flavor flavor, boolean stripSpacesAroundTags, boolean stripSingleLine,
                    ObjectMapper mapper, Insertions insertions, Filters filters, boolean evaluateInOutputTag,
@@ -427,7 +462,8 @@ public TemplateParser build() {
         this.liquidStyleInclude = liquidStyleInclude;
         this.liquidStyleWhere = liquidStyleWhere;
 
-        this.strictVariables = strictVariables;
+        this.strictVariablesMode = strictVariablesMode;
+        this.strictVariables = strictVariablesMode != StrictVariablesMode.OFF;
         this.showExceptionsFromInclude = showExceptionsFromInclude;
         this.evaluateMode = evaluateMode;
         this.renderTransformer = renderTransformer == null ? RenderTransformerDefaultImpl.INSTANCE

src/main/java/liqp/nodes/LookupNode.java

@@ -51,15 +51,46 @@ public Object render(TemplateContext context) {
             }
         }
 
-        for(Indexable index : indexes) {
-            value = index.get(value, context, found);
+        boolean foundAllButLastOne = false;
+        for (Iterator<Indexable> it = indexes.iterator(); it.hasNext();) {
+            Indexable index = it.next();
+            if (it.hasNext()) {
+                value = index.get(value, context, found);
+                if (value == null) {
+                    found.set(false);
+                    break;
+                }
+            } else {
+                // last item
+                value = index.get(value, context, found);
+                if (!found.get()) {
+                    foundAllButLastOne = true;
+                }
+            }
         }
 
-        if(value == null && !found.get() && context.getParser().strictVariables) {
-            RuntimeException e = new VariableNotExistException(getVariableName());
-            context.addError(e);
-            if (context.getErrorMode() == TemplateParser.ErrorMode.STRICT) {
-                throw e;
+        if (value == null && !found.get()) {
+            final boolean error;
+            switch (context.getParser().strictVariablesMode) {
+                case OFF:
+                    error = false;
+                    break;
+                case STRICT:
+                    error = true;
+                    break;
+                case SANE:
+                    error = !foundAllButLastOne;
+                    break;
+                default:
+                    throw new UnsupportedOperationException();
+            }
+
+            if (error) {
+                RuntimeException e = new VariableNotExistException(getVariableName());
+                context.addError(e);
+                if (context.getErrorMode() == TemplateParser.ErrorMode.STRICT) {
+                    throw e;
+                }
             }
         }
 

src/test/java/liqp/blocks/IfTest.java

@@ -2,13 +2,18 @@
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertThrows;
+
+import java.util.Collections;
 
 import org.antlr.v4.runtime.RecognitionException;
 import org.junit.Test;
 
 import liqp.Template;
 import liqp.TemplateParser;
+import liqp.TemplateParser.StrictVariablesMode;
 import liqp.exceptions.LiquidException;
+import liqp.exceptions.VariableNotExistException;
 
 public class IfTest {
 
@@ -410,4 +415,35 @@ public void and_or_evaluation_orderTest() throws RecognitionException {
         assertThat(TemplateParser.DEFAULT.parse("{% if true or false and false %}TRUE{% else %}FALSE{% endif %}").render(), is("TRUE"));
         assertThat(TemplateParser.DEFAULT.parse("{% if true and false and false or true %}TRUE{% else %}FALSE{% endif %}").render(), is("FALSE"));
     }
+
+    @Test
+    public void strictVariablesTest() throws RecognitionException {
+        assertThat(new TemplateParser.Builder().withStrictVariables(StrictVariablesMode.STRICT).build()
+            .parse("{% if obj.var %}true{% else %}false{% endif %}").render(Collections.singletonMap(
+                "obj", Collections.singletonMap("var", "val"))), is("true"));
+
+        assertThrows(VariableNotExistException.class, () -> {
+            new TemplateParser.Builder().withStrictVariables(StrictVariablesMode.STRICT).build().parse(
+                "{% if obj.missing %}true{% else %}false{% endif %}").render(Collections.singletonMap(
+                    "obj", Collections.singletonMap("var", "val")));
+        });
+
+        assertThat(new TemplateParser.Builder().withStrictVariables(StrictVariablesMode.SANE).build()
+            .parse("{% if obj.missing %}true{% else %}false{% endif %}").render(Collections.singletonMap(
+                "obj", Collections.singletonMap("var", "val"))), is("false"));
+
+        assertThrows(VariableNotExistException.class, () -> {
+            new TemplateParser.Builder().withStrictVariables(StrictVariablesMode.SANE).build().parse(
+                "{% if obj.nested.missing %}true{% else %}false{% endif %}").render(Collections
+                    .singletonMap("obj", Collections.singletonMap("var", "val")));
+        });
+
+        assertThat(new TemplateParser.Builder().withStrictVariables(StrictVariablesMode.OFF).build()
+            .parse("{% if obj.missing %}true{% else %}false{% endif %}").render(Collections.singletonMap(
+                "obj", Collections.singletonMap("var", "val"))), is("false"));
+
+        assertThat(new TemplateParser.Builder().withStrictVariables(StrictVariablesMode.OFF).build()
+            .parse("{% if obj.nested.missing %}true{% else %}false{% endif %}").render(Collections
+                .singletonMap("obj", Collections.singletonMap("var", "val"))), is("false"));
+    }
 }