You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@freemarker.apache.org by dd...@apache.org on 2019/08/17 20:16:33 UTC
[freemarker] 04/04: Bug fixed: In <#escape placeholder as
escExpression>,
the placeholder wasn't substituted inside lambda expressions inside
escExpression. Fortunately it's very unlikely that anyone wanted to use
lambdas there (given the few built-ins that accept lambdas).
This is an automated email from the ASF dual-hosted git repository.
ddekany pushed a commit to branch 2.3-gae
in repository https://gitbox.apache.org/repos/asf/freemarker.git
commit c94b7aeed2085d39e2039f9a964b1b06b93750e0
Author: ddekany <dd...@apache.org>
AuthorDate: Sat Aug 17 20:02:11 2019 +0200
Bug fixed: In <#escape placeholder as escExpression>, the placeholder wasn't substituted inside lambda expressions inside escExpression. Fortunately it's very unlikely that anyone wanted to use lambdas there (given the few built-ins that accept lambdas).
---
src/main/java/freemarker/core/EscapeBlock.java | 8 +++--
.../IntermediateStreamOperationLikeBuiltIn.java | 21 +++++++----
.../freemarker/core/LocalLambdaExpression.java | 13 +++++--
.../freemarker/core/UncheckedParseException.java | 37 +++++++++++++++++++
src/main/javacc/FTL.jj | 2 +-
src/manual/en_US/book.xml | 11 ++++++
.../java/freemarker/core/LamdaAndEscapeTest.java | 42 ++++++++++++++++++++++
7 files changed, 123 insertions(+), 11 deletions(-)
diff --git a/src/main/java/freemarker/core/EscapeBlock.java b/src/main/java/freemarker/core/EscapeBlock.java
index f5d5921..0ff9aa0 100644
--- a/src/main/java/freemarker/core/EscapeBlock.java
+++ b/src/main/java/freemarker/core/EscapeBlock.java
@@ -51,8 +51,12 @@ class EscapeBlock extends TemplateElement {
return getChildBuffer();
}
- Expression doEscape(Expression expression) {
- return escapedExpr.deepCloneWithIdentifierReplaced(variable, expression, new ReplacemenetState());
+ Expression doEscape(Expression expression) throws ParseException {
+ try {
+ return escapedExpr.deepCloneWithIdentifierReplaced(variable, expression, new ReplacemenetState());
+ } catch (UncheckedParseException e) {
+ throw e.getParseException();
+ }
}
@Override
diff --git a/src/main/java/freemarker/core/IntermediateStreamOperationLikeBuiltIn.java b/src/main/java/freemarker/core/IntermediateStreamOperationLikeBuiltIn.java
index 0d1378b..d743d8d 100644
--- a/src/main/java/freemarker/core/IntermediateStreamOperationLikeBuiltIn.java
+++ b/src/main/java/freemarker/core/IntermediateStreamOperationLikeBuiltIn.java
@@ -48,9 +48,14 @@ abstract class IntermediateStreamOperationLikeBuiltIn extends BuiltInWithParseTi
if (parameters.size() != 1) {
throw newArgumentCountException("requires exactly 1", openParen, closeParen);
}
- this.elementTransformerExp = parameters.get(0);
- if (elementTransformerExp instanceof LocalLambdaExpression) {
- LocalLambdaExpression localLambdaExp = (LocalLambdaExpression) elementTransformerExp;
+ Expression elementTransformerExp = parameters.get(0);
+ setElementTransformerExp(elementTransformerExp);
+ }
+
+ private void setElementTransformerExp(Expression elementTransformerExp) throws ParseException {
+ this.elementTransformerExp = elementTransformerExp;
+ if (this.elementTransformerExp instanceof LocalLambdaExpression) {
+ LocalLambdaExpression localLambdaExp = (LocalLambdaExpression) this.elementTransformerExp;
checkLocalLambdaParamCount(localLambdaExp, 1);
// We can't do this with other kind of expressions, like a function or method reference, as they
// need to be evaluated on runtime:
@@ -100,9 +105,13 @@ abstract class IntermediateStreamOperationLikeBuiltIn extends BuiltInWithParseTi
protected void cloneArguments(
Expression clone, String replacedIdentifier, Expression replacement, ReplacemenetState replacementState) {
- ((IntermediateStreamOperationLikeBuiltIn) clone).elementTransformerExp
- = elementTransformerExp.deepCloneWithIdentifierReplaced(
- replacedIdentifier, replacement, replacementState);
+ try {
+ ((IntermediateStreamOperationLikeBuiltIn) clone).setElementTransformerExp(
+ elementTransformerExp.deepCloneWithIdentifierReplaced(
+ replacedIdentifier, replacement, replacementState));
+ } catch (ParseException e) {
+ throw new BugException("Deep-clone elementTransformerExp failed", e);
+ }
}
TemplateModel _eval(Environment env) throws TemplateException {
diff --git a/src/main/java/freemarker/core/LocalLambdaExpression.java b/src/main/java/freemarker/core/LocalLambdaExpression.java
index ece8d37..68b3712 100644
--- a/src/main/java/freemarker/core/LocalLambdaExpression.java
+++ b/src/main/java/freemarker/core/LocalLambdaExpression.java
@@ -75,8 +75,17 @@ final class LocalLambdaExpression extends Expression {
@Override
protected Expression deepCloneWithIdentifierReplaced_inner(
String replacedIdentifier, Expression replacement, ReplacemenetState replacementState) {
- // TODO [lambda] replacement in lho is illegal; detect it
- return new LocalLambdaExpression(
+ for (Identifier parameter : lho.getParameters()) {
+ if (parameter.getName().equals(replacedIdentifier)) {
+ // As Expression.deepCloneWithIdentifierReplaced was exposed to users back then, now we can't add
+ // "throws ParseException" to this, therefore, we use UncheckedParseException as a workaround.
+ throw new UncheckedParseException(new ParseException(
+ "Escape placeholder (" + replacedIdentifier + ") can't be used in the " +
+ "parameter list of a lambda expressions.", this));
+ }
+ }
+
+ return new LocalLambdaExpression(
lho,
rho.deepCloneWithIdentifierReplaced(replacedIdentifier, replacement, replacementState));
}
diff --git a/src/main/java/freemarker/core/UncheckedParseException.java b/src/main/java/freemarker/core/UncheckedParseException.java
new file mode 100644
index 0000000..a7b3d86
--- /dev/null
+++ b/src/main/java/freemarker/core/UncheckedParseException.java
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package freemarker.core;
+
+/**
+ * Workaround for throwing {@link ParseException} in methods that wasn't declared to throw it, and we can't change that
+ * for backward compatibility.
+ */
+final class UncheckedParseException extends RuntimeException {
+
+ private final ParseException parseException;
+
+ public UncheckedParseException(ParseException parseException) {
+ this.parseException = parseException;
+ }
+
+ public ParseException getParseException() {
+ return parseException;
+ }
+}
diff --git a/src/main/javacc/FTL.jj b/src/main/javacc/FTL.jj
index 363e8bc..a12f4f1 100644
--- a/src/main/javacc/FTL.jj
+++ b/src/main/javacc/FTL.jj
@@ -477,7 +477,7 @@ public class FMParser {
notNumberLiteral(exp, "boolean (true/false)");
}
- private Expression escapedExpression(Expression exp) {
+ private Expression escapedExpression(Expression exp) throws ParseException {
if (!escapes.isEmpty()) {
return ((EscapeBlock) escapes.getFirst()).doEscape(exp);
} else {
diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml
index afb00b2..d3a4ef1 100644
--- a/src/manual/en_US/book.xml
+++ b/src/manual/en_US/book.xml
@@ -28689,6 +28689,17 @@ TemplateModel x = env.getVariable("x"); // get variable x</programlisting>
<itemizedlist>
<listitem>
+ <para>Bug fixed: In <literal><#escape
+ <replaceable>placeholder</replaceable> as
+ <replaceable>escExpression</replaceable>></literal>, the
+ <literal><replaceable>placeholder</replaceable></literal> wasn't
+ substituted inside lambda expressions inside
+ <literal><replaceable>escExpression</replaceable></literal>.
+ Fortunately it's very unlikely that anyone wanted to use lambdas
+ there (given the few built-ins that accept lambdas).</para>
+ </listitem>
+
+ <listitem>
<para>Bug fixed: AST traversal API now can properly traverse the
inside of lambda expressions (such as the parameter list)</para>
</listitem>
diff --git a/src/test/java/freemarker/core/LamdaAndEscapeTest.java b/src/test/java/freemarker/core/LamdaAndEscapeTest.java
new file mode 100644
index 0000000..2c0ccc7
--- /dev/null
+++ b/src/test/java/freemarker/core/LamdaAndEscapeTest.java
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package freemarker.core;
+
+import org.junit.Test;
+
+import freemarker.test.TemplateTest;
+
+public class LamdaAndEscapeTest extends TemplateTest {
+
+ @Test
+ public void testSubstitutionInLambdaLHO() throws Exception {
+ assertErrorContains("<#escape myPlaceholder as ['a', 'b', 'c']?map(myPlaceholder -> 'x')>${'X'}</#escape>",
+ "myPlaceholder", "lambda");
+ }
+
+ @Test
+ public void testSubstitutionInLambdaRHO() throws Exception {
+ assertOutput("<#escape x as ['a', 'b', 'c']?map(it -> it + x)?join(', ')>" +
+ "${'X'}" +
+ "</#escape>",
+ "aX, bX, cX");
+ }
+
+}