Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect infinite loop/crash #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,14 @@
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<source>8</source>
<target>8</target>
</configuration>
</plugin>
</plugins>
</build>

Expand Down
17 changes: 17 additions & 0 deletions src/main/java/de/neuland/jade4j/model/JadeModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@
import java.util.Map;
import java.util.Set;

import de.neuland.jade4j.exceptions.JadeCompilerException;
import de.neuland.jade4j.filter.Filter;
import de.neuland.jade4j.parser.node.MixinNode;
import de.neuland.jade4j.parser.node.Node;
import de.neuland.jade4j.template.JadeTemplate;

public class JadeModel implements Map<String, Object> {

Expand All @@ -22,6 +25,10 @@ public class JadeModel implements Map<String, Object> {
private Map<String, MixinNode> mixins = new HashMap<String, MixinNode>();
private Map<String, Filter> filter = new HashMap<String, Filter>();

// to detect infinite loop

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we also add a comment with the cases we've seen leading to infinite loop?

private Map<String, Integer> visited = new HashMap<String, Integer>();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this class (JadeModel) and therefore this Map will be newly instantiated every time we parse a different document?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could add a unit test to prove this by parsing the same document multiple times.

private Integer MAX_VISITED = 100;

public JadeModel(Map<String, Object> defaults) {
Map<String, Object> rootScope = new HashMap<String, Object>();
scopes.add(rootScope);
Expand Down Expand Up @@ -50,6 +57,16 @@ public MixinNode getMixin(String name) {
return mixins.get(name);
}

public void visit(Node node, JadeTemplate template) {
String visitId = node.getFileName()+":"+node.getLineNumber();
Integer ct = visited.getOrDefault(visitId, 0) +1;
if (ct > MAX_VISITED) {
String error = "Potential infinite loop: "+visitId+" has been visited more then "+MAX_VISITED+" times!";
throw new JadeCompilerException(node, template.getTemplateLoader(), error);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get the Template Name somehow and return it in the Exception message?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

visitId contains file path and line number

}
visited.put(visitId, ct);
}

@Override
public void clear() {
scopes.clear();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ public class BlockCommentNode extends CommentNode {

@Override
public void execute(IndentWriter writer, JadeModel model, JadeTemplate template) throws JadeCompilerException {
model.visit(this, template);
if (!isBuffered()) {
return;
}
Expand Down
1 change: 1 addition & 0 deletions src/main/java/de/neuland/jade4j/parser/node/BlockNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ public class BlockNode extends Node {
private String mode;

public void execute(IndentWriter writer, JadeModel model, JadeTemplate template) throws JadeCompilerException {
model.visit(this, template);
for (Node node : getNodes()) {
node.execute(writer, model, template);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public class ConditionalNode extends Node {

@Override
public void execute(IndentWriter writer, JadeModel model, JadeTemplate template) throws JadeCompilerException {
model.visit(this, template);
for (IfConditionNode conditionNode : this.conditions) {
try {
if (conditionNode.isDefault() || checkCondition(model, conditionNode.getValue()) ^ conditionNode.isInverse()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public IfConditionNode(String condition, int lineNumber) {

@Override
public void execute(IndentWriter writer, JadeModel model, JadeTemplate template) throws JadeCompilerException {
model.visit(this, template);
block.execute(writer, model, template);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public class MixinInjectNode extends AttributedNode {

@Override
public void execute(IndentWriter writer, JadeModel model, JadeTemplate template) throws JadeCompilerException {
model.visit(this, template);
MixinNode mixin = model.getMixin(getName());
if (mixin == null) {
throw new JadeCompilerException(this, template.getTemplateLoader(), "mixin " + getName() + " is not defined");
Expand Down
1 change: 1 addition & 0 deletions src/main/java/de/neuland/jade4j/parser/node/TagNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public boolean hasCodeNode() {

@Override
public void execute(IndentWriter writer, JadeModel model, JadeTemplate template) throws JadeCompilerException {
model.visit(this, template);
writer.newline();
writer.append("<");
writer.append(name);
Expand Down