Repository: incubator-freemarker Updated Branches: refs/heads/3 59d2f30b7 -> 6c8b795f4
Forward ported from 2.3-gae: FREEMARKER-48: Better handling of unchecked exceptions thrown by custom TemplateModel-s. Note that in 3 we don't need the wrapUncheckedExceptions configuration setting. Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/6c8b795f Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/6c8b795f Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/6c8b795f Branch: refs/heads/3 Commit: 6c8b795f4a7f6e0b80183e429a3c6d3e8b8cdf4d Parents: 59d2f30 Author: ddekany <[email protected]> Authored: Thu Sep 28 02:06:39 2017 +0200 Committer: ddekany <[email protected]> Committed: Thu Sep 28 02:36:29 2017 +0200 ---------------------------------------------------------------------- FM3-CHANGE-LOG.txt | 3 +- .../core/UncheckedExceptionHandlingTest.java | 141 +++++++++++++++++++ .../apache/freemarker/core/ASTDirReturn.java | 2 +- .../freemarker/core/ASTDynamicTopLevelCall.java | 27 ++-- .../apache/freemarker/core/ASTExpression.java | 9 +- .../core/BreakOrContinueException.java | 21 ++- .../org/apache/freemarker/core/CallPlace.java | 5 +- .../freemarker/core/FlowControlException.java | 37 +++++ ...nterruptionSupportTemplatePostProcessor.java | 2 +- .../core/model/TemplateDirectiveModel.java | 14 +- 10 files changed, 241 insertions(+), 20 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/FM3-CHANGE-LOG.txt ---------------------------------------------------------------------- diff --git a/FM3-CHANGE-LOG.txt b/FM3-CHANGE-LOG.txt index 0febe03..0480d51 100644 --- a/FM3-CHANGE-LOG.txt +++ b/FM3-CHANGE-LOG.txt @@ -268,7 +268,8 @@ Core / Configuration DefaultTemplateNameFormat24 was renamed to DefaultTemplateNameFormat. - Removed the logTemplateExceptions (log_template_exceptions) setting. FreeMarker now behaves as if it was false. When a FreeMarker method throws an exception, the caller is responsible for either logging it or letting it bubble up. -- Removed the strict_syntax setting, and so also the support for FTL tags without #. This was a FreeMarker 1.x +- Removed the wrapUncheckedExceptions setting. FreeMarker now behaves as if it was true. +- Removed the strictSyntax setting, and so also the support for FTL tags without #. This was a FreeMarker 1.x compatibility option. - Renamed the `cacheStorage` Configuration setting to `templateCacheStorage`. - Renamed the `localizedLookup` Configuration setting to `localizedTemplateLookup`. http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/freemarker-core-test/src/test/java/org/apache/freemarker/core/UncheckedExceptionHandlingTest.java ---------------------------------------------------------------------- diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/UncheckedExceptionHandlingTest.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/UncheckedExceptionHandlingTest.java new file mode 100644 index 0000000..fbe6267 --- /dev/null +++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/UncheckedExceptionHandlingTest.java @@ -0,0 +1,141 @@ +/* + * 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 org.apache.freemarker.core; + +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; + +import java.io.IOException; +import java.io.Writer; + +import org.apache.freemarker.core.model.ArgumentArrayLayout; +import org.apache.freemarker.core.model.TemplateDirectiveModel; +import org.apache.freemarker.core.model.TemplateFunctionModel; +import org.apache.freemarker.core.model.TemplateModel; +import org.apache.freemarker.core.util.CallableUtils; +import org.apache.freemarker.test.TemplateTest; +import org.junit.Test; + +import com.google.common.collect.ImmutableMap; + +public class UncheckedExceptionHandlingTest extends TemplateTest { + + @Override + protected Object createDataModel() { + return ImmutableMap.of( + "f", MyErronousFunction.INSTANCE, + "d", MyErronousDirective.INSTANCE, + "fd", MyFilterDirective.INSTANCE); + } + + @Test + public void test() { + assertThat( + assertErrorContains("${f()}", TemplateException.class, "unchecked exception").getCause(), + instanceOf(MyUncheckedException.class)); + assertThat( + assertErrorContains("<@d />", TemplateException.class, "unchecked exception").getCause(), + instanceOf(MyUncheckedException.class)); + assertThat( + assertErrorContains("${f('NPE')}", TemplateException.class, "unchecked exception").getCause(), + instanceOf(NullPointerException.class)); + assertThat( + assertErrorContains("<@d 'NPE' />", TemplateException.class, "unchecked exception").getCause(), + instanceOf(NullPointerException.class)); + } + + @Test + public void testFlowControlWorks() throws IOException, TemplateException { + assertOutput("<#list 1..2 as i>a<@fd>b<#break>c</@>d</#list>.", "ab."); + assertOutput("<#list 1..2 as i>a<@fd>b<#continue>c</@>d</#list>.", "abab."); + assertOutput("<#function f()><@fd><#return 1></@></#function>${f()}.", "1."); + } + + public static class MyErronousFunction implements TemplateFunctionModel { + private static final MyErronousFunction INSTANCE = new MyErronousFunction(); + + @Override + public TemplateModel execute(TemplateModel[] args, CallPlace callPlace, Environment env) + throws TemplateException { + if ("NPE".equals(CallableUtils.getOptionalStringArgument(args, 0, this))) { + throw new NullPointerException(); + } else { + throw new MyUncheckedException(); + } + } + + @Override + public ArgumentArrayLayout getFunctionArgumentArrayLayout() { + return ArgumentArrayLayout.SINGLE_POSITIONAL_PARAMETER; + } + + } + + public static class MyErronousDirective implements TemplateDirectiveModel { + private static final MyErronousDirective INSTANCE = new MyErronousDirective(); + + @Override + public void execute(TemplateModel[] args, CallPlace callPlace, Writer out, Environment env) + throws TemplateException { + if ("NPE".equals(CallableUtils.getOptionalStringArgument(args, 0, this))) { + throw new NullPointerException(); + } else { + throw new MyUncheckedException(); + } + } + + @Override + public ArgumentArrayLayout getDirectiveArgumentArrayLayout() { + return ArgumentArrayLayout.SINGLE_POSITIONAL_PARAMETER; + } + + @Override + public boolean isNestedContentSupported() { + return false; + } + + } + + public static class MyFilterDirective implements TemplateDirectiveModel { + private static final MyFilterDirective INSTANCE = new MyFilterDirective(); + + @Override + public void execute(TemplateModel[] args, CallPlace callPlace, Writer out, Environment env) + throws TemplateException, IOException { + callPlace.executeNestedContent(null, out, env); + } + + @Override + public ArgumentArrayLayout getDirectiveArgumentArrayLayout() { + return ArgumentArrayLayout.PARAMETERLESS; + } + + @Override + public boolean isNestedContentSupported() { + return true; + } + + } + + public static class MyUncheckedException extends RuntimeException { + // + } + +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDirReturn.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDirReturn.java b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDirReturn.java index 8493310..c5d23b3 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDirReturn.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDirReturn.java @@ -60,7 +60,7 @@ final class ASTDirReturn extends ASTDirective { return "#return"; } - public static class Return extends RuntimeException { + public static class Return extends FlowControlException { static final Return INSTANCE = new Return(); private Return() { } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDynamicTopLevelCall.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDynamicTopLevelCall.java b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDynamicTopLevelCall.java index 7e713bd..dd8394f 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDynamicTopLevelCall.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDynamicTopLevelCall.java @@ -30,6 +30,7 @@ import org.apache.freemarker.core.model.TemplateDirectiveModel; import org.apache.freemarker.core.model.TemplateFunctionModel; import org.apache.freemarker.core.model.TemplateModel; import org.apache.freemarker.core.util.BugException; +import org.apache.freemarker.core.util.CallableUtils; import org.apache.freemarker.core.util.CommonSupplier; import org.apache.freemarker.core.util.StringToIndexMap; import org.apache.freemarker.core.util._StringUtils; @@ -122,15 +123,24 @@ class ASTDynamicTopLevelCall extends ASTDirective implements CallPlace { positionalArgs, namedArgs, argsLayout, callableValue, false, env); - if (directive != null) { - directive.execute(execArgs, this, env.getOut(), env); - } else { - TemplateModel result = function.execute(execArgs, this, env); - if (result == null) { - throw new TemplateException(env, "Function has returned no value (or null)"); + try { + if (directive != null) { + directive.execute(execArgs, this, env.getOut(), env); + } else { + TemplateModel result = function.execute(execArgs, this, env); + if (result == null) { + throw new TemplateException(env, "Function has returned no value (or null)"); + } + // TODO [FM3] Implement it when we have a such language... it should work like `${f()}`. + throw new BugException("Top-level function call not yet implemented"); } - // TODO [FM3] Implement it when we have a such language... it should work like `${f()}`. - throw new BugException("Top-level function call not yet implemented"); + } catch (TemplateException | FlowControlException | IOException e) { + throw e; + } catch (Exception e) { + throw CallableUtils.newGenericExecuteException( + "An unchecked exception was thrown; see the cause exception", + directive != null ? directive : function, directive == null, + e); } return null; @@ -315,6 +325,7 @@ class ASTDynamicTopLevelCall extends ASTDirective implements CallPlace { return null; } + @Override @SuppressWarnings("SynchronizationOnLocalVariableOrMethodParameter") @SuppressFBWarnings(value={ "IS2_INCONSISTENT_SYNC", "DC_DOUBLECHECK" }, justification="Performance tricks") public Object getOrCreateCustomData(Object providerIdentity, CommonSupplier<?> supplier) http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpression.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpression.java b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpression.java index c10beac..d92b477 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpression.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpression.java @@ -67,7 +67,14 @@ abstract class ASTExpression extends ASTNode { } final TemplateModel eval(Environment env) throws TemplateException { - return constantValue != null ? constantValue : _eval(env); + try { + return constantValue != null ? constantValue : _eval(env); + } catch (FlowControlException | TemplateException e) { + throw e; + } catch (Exception e) { + throw new TemplateException( + this, e, env, "Expression has thrown an unchecked exception; see the cause exception."); + } } String evalAndCoerceToPlainText(Environment env) throws TemplateException { http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/freemarker-core/src/main/java/org/apache/freemarker/core/BreakOrContinueException.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/BreakOrContinueException.java b/freemarker-core/src/main/java/org/apache/freemarker/core/BreakOrContinueException.java index c83cb43..5b22161 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/BreakOrContinueException.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/BreakOrContinueException.java @@ -1,3 +1,22 @@ +/* + * 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 org.apache.freemarker.core; /** @@ -5,7 +24,7 @@ package org.apache.freemarker.core; */ // TODO [FM3] This is not a good mechanism (like what if we have <#list ...><@m><#break><@></#list>, and inside `m` // there's <#list ...><#nested></#list>) -class BreakOrContinueException extends RuntimeException { +class BreakOrContinueException extends FlowControlException { static final BreakOrContinueException BREAK_INSTANCE = new BreakOrContinueException(); static final BreakOrContinueException CONTINUE_INSTANCE = new BreakOrContinueException(); http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/freemarker-core/src/main/java/org/apache/freemarker/core/CallPlace.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/CallPlace.java b/freemarker-core/src/main/java/org/apache/freemarker/core/CallPlace.java index 628c9ec..d204084 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/CallPlace.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/CallPlace.java @@ -57,8 +57,9 @@ public interface CallPlace { * Executes the nested content; it there's none, it just does nothing. * * @param nestedContentArgs - * The nested content parameter values to pass to the nested content (as in {@code <@foo bar; i, j>${i}, - * ${j}</@foo>} there are 2 such parameters, whose value you set here), or {@code null} if there's none. + * The nested content parameter values to pass to the nested content (as in <code><@foo bar; i, jgt;${i}, + * ${j}</@foo></code> there are 2 such parameters, whose value you set here), or {@code null} if + * there's none. * This array must be {@link #getNestedContentParameterCount()} long, or else FreeMarker will throw an * {@link TemplateException} with a descriptive error message that tells to user that they need to declare * that many nested content parameters as the length of this array. If you want to allow the caller to not http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/freemarker-core/src/main/java/org/apache/freemarker/core/FlowControlException.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/FlowControlException.java b/freemarker-core/src/main/java/org/apache/freemarker/core/FlowControlException.java new file mode 100644 index 0000000..f7c10bc --- /dev/null +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/FlowControlException.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 org.apache.freemarker.core; + +/** + * Exception that's not really an exception, just used for flow control. + */ +// TODO [FM3] Can we solve controlling program flow without exceptions? +@SuppressWarnings("serial") +class FlowControlException extends RuntimeException { + + FlowControlException() { + super(); + } + + FlowControlException(String message) { + super(message); + } + +} http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/freemarker-core/src/main/java/org/apache/freemarker/core/ThreadInterruptionSupportTemplatePostProcessor.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/ThreadInterruptionSupportTemplatePostProcessor.java b/freemarker-core/src/main/java/org/apache/freemarker/core/ThreadInterruptionSupportTemplatePostProcessor.java index a1b60bf..be7877c 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/ThreadInterruptionSupportTemplatePostProcessor.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/ThreadInterruptionSupportTemplatePostProcessor.java @@ -126,7 +126,7 @@ class ThreadInterruptionSupportTemplatePostProcessor extends TemplatePostProcess * <p>ATTENTION: This is used by https://github.com/kenshoo/freemarker-online. Don't break backward * compatibility without updating that project too! */ - static class TemplateProcessingThreadInterruptedException extends RuntimeException { + static class TemplateProcessingThreadInterruptedException extends FlowControlException { TemplateProcessingThreadInterruptedException() { super("Template processing thread \"interrupted\" flag was set."); http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateDirectiveModel.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateDirectiveModel.java b/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateDirectiveModel.java index 3eb140d..c834f97 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateDirectiveModel.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateDirectiveModel.java @@ -28,7 +28,11 @@ import org.apache.freemarker.core.util.CallableUtils; public interface TemplateDirectiveModel extends TemplateCallableModel { /** - * Invokes the directive. + * Invokes the callable object. + * <p> + * This method shouldn't deliberately throw {@link RuntimeException}, nor {@link IOException} that wasn't caused by + * writing to the output. Such exceptions should be catched inside the method and wrapped inside a + * {@link TemplateException}. * * @param args * The array of argument values. Not {@code null}. If a parameter was omitted on the caller side, the @@ -55,10 +59,10 @@ public interface TemplateDirectiveModel extends TemplateCallableModel { * specifically allow that, typically, implementations that are just adapters towards FreeMarker-unaware * callables (for example, {@link JavaMethodModel} is like that). * - * @throws TemplateException - * If any problem occurs that's not an {@link IOException} during writing the template output. - * @throws IOException - * When writing the template output fails. + * @throws TemplateException If any problem occurs that's not an {@link IOException} during writing the template + * output. + * @throws IOException When writing the template output fails. Other {@link IOException}-s should be catched in this + * method and wrapped into {@link TemplateException}. */ void execute(TemplateModel[] args, CallPlace callPlace, Writer out, Environment env) throws TemplateException, IOException;
