Skip to content

Commit

Permalink
Ignore unrecognized #directives rather than throwing an exception.
Browse files Browse the repository at this point in the history
This is consistent with Velocity, and convenient when templates include URLs with `#anchors`.

RELNOTES=An unrecognized `#directive` is now ignored rather than causing an exception.
PiperOrigin-RevId: 498005681
  • Loading branch information
eamonnmcmanus authored and EscapeVelocity Team committed Dec 27, 2022
1 parent 4158ad5 commit a0ae2ba
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 2 deletions.
26 changes: 24 additions & 2 deletions src/main/java/com/google/escapevelocity/Parser.java
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,15 @@ private Node parseMacroDefinition() throws IOException {
return Node.emptyNode(resourceName, lineNumber());
}

/**
* {@code #directives} that Velocity supports but we currently don't, and that don't have to be
* followed by {@code (}. If we see one of these, we should complain, rather than just ignoring it
* the way we would for {@code #random} or whatever. If it <i>does</i> have to be followed by
* {@code (} then we will treat it as an undefined macro, which is fine.
*/
private static final ImmutableSet<String> UNSUPPORTED_VELOCITY_DIRECTIVES =
ImmutableSet.of("break", "stop");

/**
* Parses an identifier after {@code #} that is not one of the standard directives. The assumption
* is that it is a call of a macro that is defined in the template. Macro definitions are
Expand All @@ -558,9 +567,22 @@ private Node parseMacroDefinition() throws IOException {
* }</pre>
*/
private Node parsePossibleMacroCall(String directive) throws IOException {
skipSpace();
StringBuilder sb = new StringBuilder("#").append(directive);
while (Character.isWhitespace(c)) {
sb.appendCodePoint(c);
next();
}
if (c != '(') {
throw parseException("Unrecognized directive #" + directive);
if (UNSUPPORTED_VELOCITY_DIRECTIVES.contains(directive)) {
throw parseException("EscapeVelocity does not currently support #" + directive);
}
// Velocity allows #foo, where #foo is not a directive and is not followed by `(` (so it can't
// be a macro call). Then it is just plain text. BUT, sometimes but not always, Velocity will
// reject #endfoo, a string beginning with #end. So we do always reject that.
if (directive.startsWith("end")) {
throw parseException("Unrecognized directive #" + directive);
}
return parsePlainText(sb);
}
next();
ImmutableList.Builder<ExpressionNode> parameterNodes = ImmutableList.builder();
Expand Down
60 changes: 60 additions & 0 deletions src/test/java/com/google/escapevelocity/TemplateTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.fail;

import com.google.common.base.Joiner;
Expand Down Expand Up @@ -199,6 +200,65 @@ public void ignoreHashIfNotDirectiveOrComment() {
compare("${foo}#${bar}", ImmutableMap.of("foo", "xxx", "bar", "yyy"));
}

@Test
public void ignoreUnrecognizedDirective() {
compare("<a href=\"http://google.com/foo#bar\">");
compare("#bar");
// Using a name like #endfoo sometimes triggers an exception with Velocity. Here we check
// whether any other standard directive string has this behaviour, and apparently none do.
compare("#breakx");
compare("#definex");
compare("#elseifx");
compare("#elsex");
compare("#evaluatex");
compare("#foreachx");
compare("#ifx");
compare("#includex");
compare("#macrox");
compare("#parsex");
compare("#setx");
compare("#stopx");
expectException("#endx", "Unrecognized directive #endx");
}

// Since we are lax with #foo in general, make sure we don't just ignore Velocity directives that
// we don't support. There are two varieties: ones that must be followed by `(`, which we will
// treat as undefined macros; and ones that are not necessarily followed by `(`, which we handle
// explicitly.

@Test
public void unsupportedDirectives_paren() throws Exception {
String[] unsupportedDirectives = {
"#break($foreach)", "#define($foo)", "#evaluate('x')", "#include('x.vm')",
};
for (String unsupportedDirective : unsupportedDirectives) {
Template template = Template.parseFrom(new StringReader(unsupportedDirective));
EvaluationException e =
assertThrows(
unsupportedDirective,
EvaluationException.class,
() -> template.evaluate(ImmutableMap.of()));
assertThat(e)
.hasMessageThat()
.contains("is neither a standard directive nor a macro");
}
}

@Test
public void unsupportedDirectives_noParen() {
String[] unsupportedDirectives = {"#break", "#stop"};
for (String unsupportedDirective : unsupportedDirectives) {
ParseException e =
assertThrows(
unsupportedDirective,
ParseException.class,
() -> Template.parseFrom(new StringReader(unsupportedDirective)));
assertThat(e)
.hasMessageThat()
.contains("EscapeVelocity does not currently support " + unsupportedDirective);
}
}

@Test
public void blockQuote() {
compare("#[[]]#");
Expand Down

0 comments on commit a0ae2ba

Please sign in to comment.