Skip to content

Commit

Permalink
Differentiate between null and missing values
Browse files Browse the repository at this point in the history
Currently, we would get a VariableNotExistException when in strict mode,
even if a page property, for example, is knowingly set to null.

This is specifically important since {% if val %} returns true even for
empty strings, making it almost impossible to avoid an exception and
still support properties like page.last_modified_at.

Introduce a "found" state when traversing structures, and only consider
a non-existant variable if found=false

This change breaks backwards compatibility with classes implementing
LookupNode.Indexable. This is considered low-risk at the moment.
  • Loading branch information
kohlschuetter committed Dec 27, 2023
1 parent 4dd315e commit 0d572d1
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 15 deletions.
15 changes: 10 additions & 5 deletions src/main/java/liqp/TemplateContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;

import liqp.RenderTransformer.ObjectAppender;
import liqp.exceptions.ExceededMaxIterationsException;

public class TemplateContext {
private static final AtomicBoolean FOUND_DUMMY = new AtomicBoolean(false);

public static final String REGISTRY_CYCLE = "cycle";
public static final String REGISTRY_IFCHANGED = "ifchanged";
Expand Down Expand Up @@ -99,20 +101,23 @@ public boolean containsKey(String key) {
}

public Object get(String key) {
return get(key, FOUND_DUMMY);
}

public Object get(String key, AtomicBoolean found) {
// First try to retrieve the key from the local context
Object value = this.variables.get(key);

if (value != null) {
return value;
if (this.variables.containsKey(key)) {
found.set(true);
return this.variables.get(key);
}

if (parent != null) {
// Not available locally, try the parent context
return parent.get(key);
return parent.get(key, found);
}

// Not available
found.set(false);
return null;
}

Expand Down
27 changes: 17 additions & 10 deletions src/main/java/liqp/nodes/LookupNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;

import liqp.TemplateContext;
import liqp.TemplateParser;
Expand All @@ -13,7 +14,6 @@
import liqp.parser.LiquidSupport;

public class LookupNode implements LNode {

private final String id;
private final List<Indexable> indexes;

Expand All @@ -38,8 +38,11 @@ public Object render(TemplateContext context) {
} else {
realId = id;
}

AtomicBoolean found = new AtomicBoolean(false);
if (context.containsKey(realId)) {
value = context.get(realId);
found.set(true);
}
if (value == null) {
Map<String, Object> environmentMap = context.getEnvironmentMap();
Expand All @@ -49,10 +52,10 @@ public Object render(TemplateContext context) {
}

for(Indexable index : indexes) {
value = index.get(value, context);
value = index.get(value, context, found);
}

if(value == null && context.getParser().strictVariables) {
if(value == null && !found.get() && context.getParser().strictVariables) {
RuntimeException e = new VariableNotExistException(getVariableName());
context.addError(e);
if (context.getErrorMode() == TemplateParser.ErrorMode.STRICT) {
Expand All @@ -72,7 +75,7 @@ private String getVariableName() {
}

public interface Indexable {
Object get(Object value, TemplateContext context);
Object get(Object value, TemplateContext context, AtomicBoolean found);
}

public static class Hash implements Indexable {
Expand All @@ -84,8 +87,8 @@ public Hash(String hash) {
}

@Override
public Object get(Object value, TemplateContext context) {

public Object get(Object value, TemplateContext context, AtomicBoolean found) {
found.set(true);
if(value == null) {
return null;
}
Expand Down Expand Up @@ -141,10 +144,14 @@ else if(value.getClass().isArray()) {
} else {
map = (Map<?,?>) value;
}
if (!map.containsKey(hash)) {
found.set(false);
return null;
}
return map.get(hash);
}
else if(value instanceof TemplateContext) {
return ((TemplateContext)value).get(hash);
return ((TemplateContext)value).get(hash, found);
} else {
return null;
}
Expand All @@ -168,8 +175,8 @@ public Index(LNode expression, String text) {
}

@Override
public Object get(Object value, TemplateContext context) {

public Object get(Object value, TemplateContext context, AtomicBoolean found) {
found.set(true);
if(value == null) {
return null;
}
Expand Down Expand Up @@ -238,7 +245,7 @@ else if (value instanceof List) {
}

String hash = String.valueOf(key);
return new Hash(hash).get(value, context);
return new Hash(hash).get(value, context, found);
}
}

Expand Down

0 comments on commit 0d572d1

Please sign in to comment.