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 2017/09/26 22:00:57 UTC
incubator-freemarker git commit: FREEMARKER-48: Better handling of
unchecked exceptions thrown by custom TemplateModel-s.
Repository: incubator-freemarker
Updated Branches:
refs/heads/2.3-gae b246de240 -> 53da2fb0b
FREEMARKER-48: Better handling of unchecked exceptions thrown by custom TemplateModel-s.
Added new configuration setting, wrap_unchecked_exceptions (Configurable.setWrapUncheckedExceptions(boolean)). When this is true, unchecked exceptions thrown during evaluating an expression or during executing a custom directive will be wrapped into a TemplateException-s. The advantage of that is that thus the exception will include the location in the template (not just the Java stack trace), and the TemplateExceptionHandler will be invoked for it as well. When this setting is false (which is the default for backward compatibility), the the unchecked exception will bubble up and thrown by Template.process, just as in earlier versions. (Note that plain Java methods called from templates have always wrapped the thrown exception into TemplateException, regardless of this setting.)
When incomplatible_improvements is set to 2.3.27 (or higher), the following unchecked exceptions will be wrapped into TemplateException-s when thrown during evaluating expressions or calling directives: NullPointerException, ClassCastException, IndexOutOfBoundsException, and InvocationTargetException. The goal of this is the same as of setting the wrap_unchecked_exceptions setting to true (see that earlier), but this is more backward compatible, as it avoids wrapping unchecked exceptions that the calling application is likely to catch specifically (like application-specific unchecked exceptions).
Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/53da2fb0
Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/53da2fb0
Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/53da2fb0
Branch: refs/heads/2.3-gae
Commit: 53da2fb0be728416c44f8b158a6b96f9cd61e0f7
Parents: b246de2
Author: ddekany <dd...@apache.org>
Authored: Wed Sep 27 00:00:34 2017 +0200
Committer: ddekany <dd...@apache.org>
Committed: Wed Sep 27 00:00:34 2017 +0200
----------------------------------------------------------------------
.../core/BreakOrContinueException.java | 2 +-
src/main/java/freemarker/core/Configurable.java | 58 +++++++-
src/main/java/freemarker/core/Environment.java | 27 +++-
src/main/java/freemarker/core/EvalUtil.java | 23 +++
src/main/java/freemarker/core/Expression.java | 18 ++-
.../freemarker/core/FlowControlException.java | 9 ++
.../java/freemarker/core/ReturnInstruction.java | 2 +-
.../freemarker/core/TemplateConfiguration.java | 7 +
.../java/freemarker/template/Configuration.java | 64 ++++++++-
.../template/TemplateDirectiveModel.java | 8 +-
.../template/TemplateTransformModel.java | 16 ++-
.../java/freemarker/template/_TemplateAPI.java | 6 +-
src/manual/en_US/book.xml | 73 ++++++++--
.../core/TemplateConfigurationTest.java | 1 +
.../core/UncheckedExceptionHandlingTest.java | 139 +++++++++++++++++++
.../freemarker/template/ConfigurationTest.java | 9 ++
16 files changed, 433 insertions(+), 29 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/53da2fb0/src/main/java/freemarker/core/BreakOrContinueException.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/core/BreakOrContinueException.java b/src/main/java/freemarker/core/BreakOrContinueException.java
index 86bfbd8..222bc28 100644
--- a/src/main/java/freemarker/core/BreakOrContinueException.java
+++ b/src/main/java/freemarker/core/BreakOrContinueException.java
@@ -3,7 +3,7 @@ package freemarker.core;
/**
* Used for implementing #break and #continue.
*/
-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/53da2fb0/src/main/java/freemarker/core/Configurable.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/core/Configurable.java b/src/main/java/freemarker/core/Configurable.java
index 0ae1be4..f1a905b 100644
--- a/src/main/java/freemarker/core/Configurable.java
+++ b/src/main/java/freemarker/core/Configurable.java
@@ -60,6 +60,7 @@ import freemarker.template.SimpleObjectWrapper;
import freemarker.template.Template;
import freemarker.template.TemplateException;
import freemarker.template.TemplateExceptionHandler;
+import freemarker.template.TemplateMethodModel;
import freemarker.template.TemplateModel;
import freemarker.template.Version;
import freemarker.template._TemplateAPI;
@@ -247,6 +248,13 @@ public class Configurable {
/** Alias to the {@code ..._SNAKE_CASE} variation due to backward compatibility constraints. @since 2.3.22 */
public static final String LOG_TEMPLATE_EXCEPTIONS_KEY = LOG_TEMPLATE_EXCEPTIONS_KEY_SNAKE_CASE;
+ /** Legacy, snake case ({@code like_this}) variation of the setting name. @since 2.3.27 */
+ public static final String WRAP_UNCHECKED_EXCEPTIONS_KEY_SNAKE_CASE = "wrap_unchecked_exceptions";
+ /** Modern, camel case ({@code likeThis}) variation of the setting name. @since 2.3.27 */
+ public static final String WRAP_UNCHECKED_EXCEPTIONS_KEY_CAMEL_CASE = "wrapUncheckedExceptions";
+ /** Alias to the {@code ..._SNAKE_CASE} variation due to backward compatibility constraints. @since 2.3.27 */
+ public static final String WRAP_UNCHECKED_EXCEPTIONS_KEY = WRAP_UNCHECKED_EXCEPTIONS_KEY_SNAKE_CASE;
+
/** Legacy, snake case ({@code like_this}) variation of the setting name. @since 2.3.25 */
public static final String LAZY_IMPORTS_KEY_SNAKE_CASE = "lazy_imports";
/** Modern, camel case ({@code likeThis}) variation of the setting name. @since 2.3.25 */
@@ -307,7 +315,8 @@ public class Configurable {
TEMPLATE_EXCEPTION_HANDLER_KEY_SNAKE_CASE,
TIME_FORMAT_KEY_SNAKE_CASE,
TIME_ZONE_KEY_SNAKE_CASE,
- URL_ESCAPING_CHARSET_KEY_SNAKE_CASE
+ URL_ESCAPING_CHARSET_KEY_SNAKE_CASE,
+ WRAP_UNCHECKED_EXCEPTIONS_KEY_SNAKE_CASE
};
private static final String[] SETTING_NAMES_CAMEL_CASE = new String[] {
@@ -338,7 +347,8 @@ public class Configurable {
TEMPLATE_EXCEPTION_HANDLER_KEY_CAMEL_CASE,
TIME_FORMAT_KEY_CAMEL_CASE,
TIME_ZONE_KEY_CAMEL_CASE,
- URL_ESCAPING_CHARSET_KEY_CAMEL_CASE
+ URL_ESCAPING_CHARSET_KEY_CAMEL_CASE,
+ WRAP_UNCHECKED_EXCEPTIONS_KEY_CAMEL_CASE
};
private Configurable parent;
@@ -370,6 +380,7 @@ public class Configurable {
private Boolean showErrorTips;
private Boolean apiBuiltinEnabled;
private Boolean logTemplateExceptions;
+ private Boolean wrapUncheckedExceptions;
private Map<String, ? extends TemplateDateFormatFactory> customDateFormats;
private Map<String, ? extends TemplateNumberFormatFactory> customNumberFormats;
private LinkedHashMap<String, String> autoImports;
@@ -425,6 +436,8 @@ public class Configurable {
templateExceptionHandler = _TemplateAPI.getDefaultTemplateExceptionHandler(incompatibleImprovements);
properties.setProperty(TEMPLATE_EXCEPTION_HANDLER_KEY, templateExceptionHandler.getClass().getName());
+
+ wrapUncheckedExceptions = _TemplateAPI.getDefaultWrapUncheckedExceptions(incompatibleImprovements);
attemptExceptionReporter = _TemplateAPI.getDefaultAttemptExceptionReporter(incompatibleImprovements);
@@ -1690,6 +1703,44 @@ public class Configurable {
public boolean isLogTemplateExceptionsSet() {
return logTemplateExceptions != null;
}
+
+ /**
+ * Specifies if unchecked exceptions thrown during expression evaluation or during executing custom directives (and
+ * transform) will be wrapped into {@link TemplateException}-s, or will bubble up to the caller of
+ * {@link Template#process(Object, Writer, ObjectWrapper)} as is. The default is {@code false} for backward
+ * compatibility (as some applications catch certain unchecked exceptions thrown by the template processing to do
+ * something special), but the recommended value is {@code true}.
+ * When this is {@code true}, the unchecked exceptions will be wrapped into a {@link TemplateException}-s, thus the
+ * exception will include the location in the template (not
+ * just the Java stack trace). Another consequence of the wrapping is that the {@link TemplateExceptionHandler} will
+ * be invoked for the exception (as that only handles {@link TemplateException}-s, it wasn't invoked for unchecked
+ * exceptions). When this setting is {@code false}, unchecked exception will be thrown by
+ * {@link Template#process(Object, Writer, ObjectWrapper)}.
+ * Note that plain Java methods called from templates aren't user defined {@link TemplateMethodModel}-s, and have
+ * always wrapped the thrown exception into {@link TemplateException}, regardless of this setting.
+ *
+ * @since 2.3.27
+ */
+ public void setWrapUncheckedExceptions(boolean wrapUncheckedExceptions) {
+ this.wrapUncheckedExceptions = wrapUncheckedExceptions;
+ }
+
+ /**
+ * The getter pair of {@link #setWrapUncheckedExceptions(boolean)}.
+ *
+ * @since 2.3.27
+ */
+ public boolean getWrapUncheckedExceptions() {
+ return wrapUncheckedExceptions != null ? wrapUncheckedExceptions
+ : (parent != null ? parent.getWrapUncheckedExceptions() : false /* [2.4] true */);
+ }
+
+ /**
+ * @since 2.3.27
+ */
+ public boolean isWrapUncheckedExceptionsSet() {
+ return wrapUncheckedExceptions != null;
+ }
/**
* The getter pair of {@link #setLazyImports(boolean)}.
@@ -2647,6 +2698,9 @@ public class Configurable {
} else if (LOG_TEMPLATE_EXCEPTIONS_KEY_SNAKE_CASE.equals(name)
|| LOG_TEMPLATE_EXCEPTIONS_KEY_CAMEL_CASE.equals(name)) {
setLogTemplateExceptions(StringUtil.getYesNo(value));
+ } else if (WRAP_UNCHECKED_EXCEPTIONS_KEY_SNAKE_CASE.equals(name)
+ || WRAP_UNCHECKED_EXCEPTIONS_KEY_CAMEL_CASE.equals(name)) {
+ setWrapUncheckedExceptions(StringUtil.getYesNo(value));
} else if (LAZY_AUTO_IMPORTS_KEY_SNAKE_CASE.equals(name) || LAZY_AUTO_IMPORTS_KEY_CAMEL_CASE.equals(name)) {
setLazyAutoImports(value.equals(NULL) ? null : Boolean.valueOf(StringUtil.getYesNo(value)));
} else if (LAZY_IMPORTS_KEY_SNAKE_CASE.equals(name) || LAZY_IMPORTS_KEY_CAMEL_CASE.equals(name)) {
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/53da2fb0/src/main/java/freemarker/core/Environment.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/core/Environment.java b/src/main/java/freemarker/core/Environment.java
index 8b03afd..5faacdd 100644
--- a/src/main/java/freemarker/core/Environment.java
+++ b/src/main/java/freemarker/core/Environment.java
@@ -438,6 +438,22 @@ public final class Environment extends Configurable {
}
try {
directiveModel.execute(this, args, outArgs, nested);
+ } catch (FlowControlException e) {
+ throw e;
+ } catch (TemplateException e) {
+ throw e;
+ } catch (IOException e) {
+ // For backward compatibility, we assume that this is because the output Writer has thrown it.
+ throw e;
+ } catch (Exception e) {
+ if (EvalUtil.shouldWrapUncheckedException(e, this)) {
+ throw new _MiscTemplateException(
+ e, this, "Directive has thrown an unchecked exception; see the cause exception.");
+ } else if (e instanceof RuntimeException) {
+ throw (RuntimeException) e;
+ } else {
+ throw new UndeclaredThrowableException(e);
+ }
} finally {
if (outArgs.length > 0) {
localContextStack.pop();
@@ -485,12 +501,17 @@ public final class Environment extends Configurable {
throw e;
} catch (IOException e) {
throw e;
- } catch (RuntimeException e) {
- throw e;
} catch (Error e) {
throw e;
} catch (Throwable e) {
- throw new UndeclaredThrowableException(e);
+ if (EvalUtil.shouldWrapUncheckedException(e, this)) {
+ throw new _MiscTemplateException(
+ e, this, "Transform has thrown an unchecked exception; see the cause exception.");
+ } else if (e instanceof RuntimeException) {
+ throw (RuntimeException) e;
+ } else {
+ throw new UndeclaredThrowableException(e);
+ }
}
} finally {
out = prevOut;
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/53da2fb0/src/main/java/freemarker/core/EvalUtil.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/core/EvalUtil.java b/src/main/java/freemarker/core/EvalUtil.java
index 1dbcfb9..4b03e27 100644
--- a/src/main/java/freemarker/core/EvalUtil.java
+++ b/src/main/java/freemarker/core/EvalUtil.java
@@ -19,6 +19,7 @@
package freemarker.core;
+import java.lang.reflect.InvocationTargetException;
import java.util.Date;
import freemarker.ext.beans.BeanModel;
@@ -32,6 +33,7 @@ import freemarker.template.TemplateModelException;
import freemarker.template.TemplateNumberModel;
import freemarker.template.TemplateScalarModel;
import freemarker.template.TemplateSequenceModel;
+import freemarker.template._TemplateAPI;
/**
* Internally used static utilities for evaluation expressions.
@@ -579,5 +581,26 @@ class EvalUtil {
? env.getArithmeticEngine()
: tObj.getTemplate().getParserConfiguration().getArithmeticEngine();
}
+
+ static boolean shouldWrapUncheckedException(Throwable e, Environment env) {
+ if (FlowControlException.class.isInstance(e)) {
+ return false;
+ }
+ if (env.getWrapUncheckedExceptions()) {
+ return true;
+ } else if (env.getConfiguration().getIncompatibleImprovements().intValue() >= _TemplateAPI.VERSION_INT_2_3_27) {
+ // We have to judge if we dare to wrap this exception, or it's too likely that some applications try to
+ // catch it around the template processing to do something special. For the same reason, we only wrap very
+ // frequent exceptions.
+ // We use "==" instead of "instanceof" deliberately; user defined subclasses must not match.
+ Class<? extends Throwable> c = e.getClass();
+ return c == NullPointerException.class
+ || c == ClassCastException.class
+ || c == IndexOutOfBoundsException.class
+ || c == InvocationTargetException.class;
+ } else {
+ return false;
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/53da2fb0/src/main/java/freemarker/core/Expression.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/core/Expression.java b/src/main/java/freemarker/core/Expression.java
index 2823bb9..45da3c9 100644
--- a/src/main/java/freemarker/core/Expression.java
+++ b/src/main/java/freemarker/core/Expression.java
@@ -32,6 +32,7 @@ import freemarker.template.TemplateModelException;
import freemarker.template.TemplateNumberModel;
import freemarker.template.TemplateScalarModel;
import freemarker.template.TemplateSequenceModel;
+import freemarker.template.utility.UndeclaredThrowableException;
/**
* <b>Internal API - subject to change:</b> Represent expression nodes in the parsed template.
@@ -78,7 +79,22 @@ abstract public class Expression extends TemplateObject {
}
final TemplateModel eval(Environment env) throws TemplateException {
- return constantValue != null ? constantValue : _eval(env);
+ try {
+ return constantValue != null ? constantValue : _eval(env);
+ } catch (FlowControlException e) {
+ throw e;
+ } catch (TemplateException e) {
+ throw e;
+ } catch (Exception e) {
+ if (env != null && EvalUtil.shouldWrapUncheckedException(e, env)) {
+ throw new _MiscTemplateException(
+ this, e, env, "Expression has thrown an unchecked exception; see the cause exception.");
+ } else if (e instanceof RuntimeException) {
+ throw (RuntimeException) e;
+ } else {
+ throw new UndeclaredThrowableException(e);
+ }
+ }
}
String evalAndCoerceToPlainText(Environment env) throws TemplateException {
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/53da2fb0/src/main/java/freemarker/core/FlowControlException.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/core/FlowControlException.java b/src/main/java/freemarker/core/FlowControlException.java
new file mode 100644
index 0000000..7865dc5
--- /dev/null
+++ b/src/main/java/freemarker/core/FlowControlException.java
@@ -0,0 +1,9 @@
+package freemarker.core;
+
+/**
+ * Exception that's not really an exception, just used for flow control.
+ */
+@SuppressWarnings("serial")
+class FlowControlException extends RuntimeException {
+ //
+}
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/53da2fb0/src/main/java/freemarker/core/ReturnInstruction.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/core/ReturnInstruction.java b/src/main/java/freemarker/core/ReturnInstruction.java
index c363046..8b5e923 100644
--- a/src/main/java/freemarker/core/ReturnInstruction.java
+++ b/src/main/java/freemarker/core/ReturnInstruction.java
@@ -62,7 +62,7 @@ public final class ReturnInstruction extends TemplateElement {
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/53da2fb0/src/main/java/freemarker/core/TemplateConfiguration.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/core/TemplateConfiguration.java b/src/main/java/freemarker/core/TemplateConfiguration.java
index 5ec5a02..0835239 100644
--- a/src/main/java/freemarker/core/TemplateConfiguration.java
+++ b/src/main/java/freemarker/core/TemplateConfiguration.java
@@ -198,6 +198,9 @@ public final class TemplateConfiguration extends Configurable implements ParserC
if (tc.isLogTemplateExceptionsSet()) {
setLogTemplateExceptions(tc.getLogTemplateExceptions());
}
+ if (tc.isWrapUncheckedExceptionsSet()) {
+ setWrapUncheckedExceptions(tc.getWrapUncheckedExceptions());
+ }
if (tc.isNamingConventionSet()) {
setNamingConvention(tc.getNamingConvention());
}
@@ -331,6 +334,9 @@ public final class TemplateConfiguration extends Configurable implements ParserC
if (isLogTemplateExceptionsSet() && !template.isLogTemplateExceptionsSet()) {
template.setLogTemplateExceptions(getLogTemplateExceptions());
}
+ if (isWrapUncheckedExceptionsSet() && !template.isWrapUncheckedExceptionsSet()) {
+ template.setWrapUncheckedExceptions(getWrapUncheckedExceptions());
+ }
if (isNewBuiltinClassResolverSet() && !template.isNewBuiltinClassResolverSet()) {
template.setNewBuiltinClassResolver(getNewBuiltinClassResolver());
}
@@ -634,6 +640,7 @@ public final class TemplateConfiguration extends Configurable implements ParserC
|| isLazyAutoImportsSet()
|| isLocaleSet()
|| isLogTemplateExceptionsSet()
+ || isWrapUncheckedExceptionsSet()
|| isNewBuiltinClassResolverSet()
|| isNumberFormatSet()
|| isObjectWrapperSet()
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/53da2fb0/src/main/java/freemarker/template/Configuration.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/template/Configuration.java b/src/main/java/freemarker/template/Configuration.java
index 8b14da6..6e24881 100644
--- a/src/main/java/freemarker/template/Configuration.java
+++ b/src/main/java/freemarker/template/Configuration.java
@@ -22,6 +22,7 @@ package freemarker.template;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
+import java.lang.reflect.InvocationTargetException;
import java.net.URLConnection;
import java.text.DecimalFormat;
import java.text.SimpleDateFormat;
@@ -516,6 +517,7 @@ public class Configuration extends Configurable implements Cloneable, ParserConf
private boolean templateExceptionHandlerExplicitlySet;
private boolean attemptExceptionReporterExplicitlySet;
private boolean logTemplateExceptionsExplicitlySet;
+ private boolean wrapUncheckedExceptionsExplicitlySet;
private boolean localeExplicitlySet;
private boolean defaultEncodingExplicitlySet;
private boolean timeZoneExplicitlySet;
@@ -839,7 +841,15 @@ public class Configuration extends Configurable implements Cloneable, ParserConf
* <li><p>
* {@link BeansWrapper} and {@link DefaultObjectWrapper} now prefers the non-indexed JavaBean property
* read method over the indexed read method when Java 8 exposes both;
- * see {@link BeansWrapper#BeansWrapper(Version)}.
+ * see {@link BeansWrapper#BeansWrapper(Version)}.
+ * <li><p>
+ * The following unchecked exceptions (but not their subclasses) will be wrapped into
+ * {@link TemplateException}-s when thrown during evaluating expressions or calling directives:
+ * {@link NullPointerException}, {@link ClassCastException}, {@link IndexOutOfBoundsException}, and
+ * {@link InvocationTargetException}. The goal of this is the same as of setting
+ * {@link #setWrapUncheckedExceptions(boolean) wrap_unchecked_exceptions} to {@code true} (see more there),
+ * but this is more backward compatible, as it avoids wrapping unchecked exceptions that the calling
+ * application is likely to catch specifically (like application-specific unchecked exceptions).
* </ul>
* </li>
* </ul>
@@ -983,25 +993,34 @@ public class Configuration extends Configurable implements Cloneable, ParserConf
private boolean getDefaultLogTemplateExceptions() {
return getDefaultLogTemplateExceptions(getIncompatibleImprovements());
}
+
+ private boolean getDefaultWrapUncheckedExceptions() {
+ return getDefaultWrapUncheckedExceptions(getIncompatibleImprovements());
+ }
private ObjectWrapper getDefaultObjectWrapper() {
return getDefaultObjectWrapper(getIncompatibleImprovements());
}
// Package visible as Configurable needs this to initialize the field defaults.
- final static TemplateExceptionHandler getDefaultTemplateExceptionHandler(Version incompatibleImprovements) {
+ static TemplateExceptionHandler getDefaultTemplateExceptionHandler(Version incompatibleImprovements) {
return TemplateExceptionHandler.DEBUG_HANDLER;
}
// Package visible as Configurable needs this to initialize the field defaults.
- final static AttemptExceptionReporter getDefaultAttemptExceptionReporter(Version incompatibleImprovements) {
+ static AttemptExceptionReporter getDefaultAttemptExceptionReporter(Version incompatibleImprovements) {
return AttemptExceptionReporter.LOG_ERROR_REPORTER;
}
// Package visible as Configurable needs this to initialize the field defaults.
- final static boolean getDefaultLogTemplateExceptions(Version incompatibleImprovements) {
+ static boolean getDefaultLogTemplateExceptions(Version incompatibleImprovements) {
return true;
}
+
+ // Package visible as Configurable needs this to initialize the field defaults.
+ static boolean getDefaultWrapUncheckedExceptions(Version incompatibleImprovements) {
+ return false;
+ }
@Override
public Object clone() {
@@ -1759,6 +1778,8 @@ public class Configuration extends Configurable implements Cloneable, ParserConf
}
/**
+ * {@inheritDoc}
+ *
* @since 2.3.22
*/
@Override
@@ -1791,6 +1812,36 @@ public class Configuration extends Configurable implements Cloneable, ParserConf
}
/**
+ * {@inheritDoc}
+ *
+ * @since 2.3.27
+ */
+ @Override
+ public void setWrapUncheckedExceptions(boolean value) {
+ super.setWrapUncheckedExceptions(value);
+ wrapUncheckedExceptionsExplicitlySet = true;
+ }
+
+ /**
+ * @since 2.3.27
+ */
+ public void unsetWrapUncheckedExceptions() {
+ if (wrapUncheckedExceptionsExplicitlySet) {
+ setWrapUncheckedExceptions(getDefaultWrapUncheckedExceptions());
+ wrapUncheckedExceptionsExplicitlySet = false;
+ }
+ }
+
+ /**
+ * Tells if {@link #setWrapUncheckedExceptions} (or equivalent) was already called on this instance.
+ *
+ * @since 2.3.27
+ */
+ public boolean isWrapUncheckedExceptionsExplicitlySet() {
+ return wrapUncheckedExceptionsExplicitlySet;
+ }
+
+ /**
* The getter pair of {@link #setStrictSyntaxMode}.
*/
public boolean getStrictSyntaxMode() {
@@ -1853,6 +1904,11 @@ public class Configuration extends Configurable implements Cloneable, ParserConf
unsetLogTemplateExceptions();
}
+ if (!wrapUncheckedExceptionsExplicitlySet) {
+ wrapUncheckedExceptionsExplicitlySet = true;
+ unsetWrapUncheckedExceptions();
+ }
+
if (!objectWrapperExplicitlySet) {
objectWrapperExplicitlySet = true;
unsetObjectWrapper();
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/53da2fb0/src/main/java/freemarker/template/TemplateDirectiveModel.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/template/TemplateDirectiveModel.java b/src/main/java/freemarker/template/TemplateDirectiveModel.java
index 2e1eca0..1de5867 100644
--- a/src/main/java/freemarker/template/TemplateDirectiveModel.java
+++ b/src/main/java/freemarker/template/TemplateDirectiveModel.java
@@ -40,6 +40,11 @@ public interface TemplateDirectiveModel extends TemplateModel {
/**
* Executes this user-defined directive; called by FreeMarker when the user-defined
* directive is called in the template.
+ *
+ * <p>This method should not 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}. (Note that setting {@link Configuration#setWrapUncheckedExceptions(boolean)} to
+ * {@code true} can mitigate the negative effects of implementations that throw {@link RuntimeException}-s.)
*
* @param env the current processing environment. Note that you can access
* the output {@link java.io.Writer Writer} by {@link Environment#getOut()}.
@@ -61,7 +66,8 @@ public interface TemplateDirectiveModel extends TemplateModel {
*
* @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 IOException When writing the template output fails. Other {@link IOException}-s should be catched in this
+ * method and wrapped into {@link TemplateException}.
*/
public void execute(Environment env, Map params, TemplateModel[] loopVars,
TemplateDirectiveBody body) throws TemplateException, IOException;
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/53da2fb0/src/main/java/freemarker/template/TemplateTransformModel.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/template/TemplateTransformModel.java b/src/main/java/freemarker/template/TemplateTransformModel.java
index 9022658..154bd57 100644
--- a/src/main/java/freemarker/template/TemplateTransformModel.java
+++ b/src/main/java/freemarker/template/TemplateTransformModel.java
@@ -37,18 +37,30 @@ public interface TemplateTransformModel extends TemplateModel {
* transformation input to the transform. Each call to this method
* must return a new instance of the writer so that the transformation
* is thread-safe.
+ *
* @param out the character stream to which to write the transformed output
* @param args the arguments (if any) passed to the transformation as a
* map of key/value pairs where the keys are strings and the arguments are
* TemplateModel instances. This is never null. If you need to convert the
* template models to POJOs, you can use the utility methods in the
* {@link DeepUnwrap} class.
+ *
* @return a writer to which the engine will feed the transformation
* input, or null if the transform does not support nested content (body).
* The returned writer can implement the {@link TransformControl}
* interface if it needs advanced control over the evaluation of the
* transformation body.
+ *
+ * <p>This method should not 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 TemplateModelException}. (Note that setting {@link Configuration#setWrapUncheckedExceptions(boolean)} to
+ * {@code true} can mitigate the negative effects of implementations that throw {@link RuntimeException}-s.)
+ *
+ * @throws TemplateModelException 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 TemplateModelException}.
+ *
*/
- Writer getWriter(Writer out, Map args)
- throws TemplateModelException, IOException;
+ Writer getWriter(Writer out, Map args) throws TemplateModelException, IOException;
}
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/53da2fb0/src/main/java/freemarker/template/_TemplateAPI.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/template/_TemplateAPI.java b/src/main/java/freemarker/template/_TemplateAPI.java
index 62960c8..cf5f39c 100644
--- a/src/main/java/freemarker/template/_TemplateAPI.java
+++ b/src/main/java/freemarker/template/_TemplateAPI.java
@@ -81,7 +81,7 @@ public class _TemplateAPI {
Version incompatibleImprovements) {
return Configuration.getDefaultTemplateExceptionHandler(incompatibleImprovements);
}
-
+
public static AttemptExceptionReporter getDefaultAttemptExceptionReporter(
Version incompatibleImprovements) {
return Configuration.getDefaultAttemptExceptionReporter(incompatibleImprovements);
@@ -91,6 +91,10 @@ public class _TemplateAPI {
return Configuration.getDefaultLogTemplateExceptions(incompatibleImprovements);
}
+ public static boolean getDefaultWrapUncheckedExceptions(Version incompatibleImprovements) {
+ return Configuration.getDefaultWrapUncheckedExceptions(incompatibleImprovements);
+ }
+
public static TemplateLoader createDefaultTemplateLoader(Version incompatibleImprovements) {
return Configuration.createDefaultTemplateLoader(incompatibleImprovements);
}
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/53da2fb0/src/manual/en_US/book.xml
----------------------------------------------------------------------
diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml
index edb158b..5135257 100644
--- a/src/manual/en_US/book.xml
+++ b/src/manual/en_US/book.xml
@@ -6300,9 +6300,9 @@ That's all.</programlisting>
beginning of the application (possibly servlet) life-cycle:</para>
<programlisting role="unspecified">// Create your Configuration instance, and specify if up to what FreeMarker
-// version (here 2.3.25) do you want to apply the fixes that are not 100%
+// version (here 2.3.27) do you want to apply the fixes that are not 100%
// backward-compatible. See the Configuration JavaDoc for details.
-Configuration cfg = new Configuration(Configuration.VERSION_2_3_25);
+Configuration cfg = new Configuration(Configuration.VERSION_2_3_27);
// Specify the source where the template files come from. Here I set a
// plain directory for it, but non-file-system sources are possible too:
@@ -6317,7 +6317,10 @@ cfg.setDefaultEncoding("UTF-8");
cfg.setTemplateExceptionHandler(TemplateExceptionHandler.RETHROW_HANDLER);
// Don't log exceptions inside FreeMarker that it will thrown at you anyway:
-cfg.setLogTemplateExceptions(false);</programlisting>
+cfg.setLogTemplateExceptions(false);
+
+// Wrap unchecked exceptions thrown during template processing into TemplateException-s.
+cfg.setWrapUncheckedExceptions(true);</programlisting>
<para>From now you should use this <emphasis>single</emphasis>
configuration instance (i.e., its a singleton). Note however that if a
@@ -6589,11 +6592,12 @@ public class Test {
/* You should do this ONLY ONCE in the whole application life-cycle: */
/* Create and adjust the configuration singleton */
- Configuration cfg = new Configuration(Configuration.VERSION_2_3_25);
+ Configuration cfg = new Configuration(Configuration.VERSION_2_3_27);
cfg.setDirectoryForTemplateLoading(new File("<replaceable>/where/you/store/templates</replaceable>"));
cfg.setDefaultEncoding("UTF-8");
cfg.setTemplateExceptionHandler(TemplateExceptionHandler.RETHROW_HANDLER);
cfg.setLogTemplateExceptions(false);
+ cfg.setWrapUncheckedExceptions(true);
/* ------------------------------------------------------------------------ */
/* You usually do these for MULTIPLE TIMES in the application life-cycle: */
@@ -27053,10 +27057,12 @@ TemplateModel x = env.getVariable("x"); // get variable x</programlisting>
<itemizedlist>
<listitem>
- <para>New directive: <literal>continue</literal>. This can be
- used inside the <literal>list</literal> directive to skip to the
- next iteration (similarly as in Java). <link
- linkend="ref.directive.list.continue">See more...</link></para>
+ <para>New directive: <literal>continue</literal> (<link
+ xlink:href="https://sourceforge.net/p/freemarker/feature-requests/79/">sf.net
+ #79</link>). This can be used inside the <literal>list</literal>
+ directive to skip to the next iteration (similarly as in Java).
+ <link linkend="ref.directive.list.continue">See
+ more...</link></para>
</listitem>
<listitem>
@@ -27073,11 +27079,12 @@ TemplateModel x = env.getVariable("x"); // get variable x</programlisting>
</listitem>
<listitem>
- <para>New built-in, <literal>sequence</literal>. This can be
- used to work around situations where a listable value lacks some
- features that you need in the template (like it can't be listed
- twice, it can't tell its size, etc.), and you can't modify the
- data-model to fix the problem. <link
+ <para>New built-in, <literal>sequence</literal> (<link
+ xlink:href="https://issues.apache.org/jira/browse/FREEMARKER-73">FREEMARKER-73</link>).
+ This can be used to work around situations where a listable
+ value lacks some features that you need in the template (like it
+ can't be listed twice, it can't tell its size, etc.), and you
+ can't modify the data-model to fix the problem. <link
linkend="ref_builtin_sequence">See more...</link></para>
</listitem>
@@ -27117,6 +27124,27 @@ TemplateModel x = env.getVariable("x"); // get variable x</programlisting>
<itemizedlist>
<listitem>
<para>Added new configuration setting,
+ <literal>wrap_unchecked_exceptions</literal>
+ (<literal>Configurable.setWrapUncheckedExceptions(boolean)</literal>).
+ When this is <literal>true</literal>, unchecked exceptions
+ thrown during evaluating an expression or during executing a
+ custom directive will be wrapped into a
+ <literal>TemplateException</literal>-s. The advantage of that is
+ that thus the exception will include the location in the
+ template (not just the Java stack trace), and the
+ <literal>TemplateExceptionHandler</literal> will be invoked for
+ it as well. When this setting is <literal>false</literal> (which
+ is the default for backward compatibility), the the unchecked
+ exception will bubble up and thrown by
+ <literal>Template.process</literal>, just as in earlier
+ versions. (Note that plain Java methods called from templates
+ have always wrapped the thrown exception into
+ <literal>TemplateException</literal>, regardless of this
+ setting.)</para>
+ </listitem>
+
+ <listitem>
+ <para>Added new configuration setting,
<literal>attempt_exception_reporter</literal>
(<literal>Configurable.setAttemptExceptionReporter(AttemptExceptionReporter)</literal>),
to allow the customization of how the exceptions handled (and
@@ -27137,6 +27165,25 @@ TemplateModel x = env.getVariable("x"); // get variable x</programlisting>
</listitem>
<listitem>
+ <para>When <link
+ linkend="pgui_config_incompatible_improvements_how_to_set"><literal>incomplatible_improvements</literal></link>
+ is set to 2.3.27 (or higher), the following unchecked exceptions
+ (but not their subclasses) will be wrapped into
+ <literal>TemplateException</literal>-s when thrown during
+ evaluating expressions or calling directives:
+ <literal>NullPointerException</literal>,
+ <literal>ClassCastException</literal>,
+ <literal>IndexOutOfBoundsException</literal>, and
+ <literal>InvocationTargetException</literal>. The goal of this
+ is the same as of setting the
+ <literal>wrap_unchecked_exceptions</literal> setting to
+ <literal>true</literal> (see that earlier), but this is more
+ backward compatible, as it avoids wrapping unchecked exceptions
+ that some application is likely to catch specifically (like
+ application-specific unchecked exceptions).</para>
+ </listitem>
+
+ <listitem>
<para>Bug fixed: <literal>BeansWrapper</literal> and
<literal>DefaultObjectWrapper</literal>, starting from Java 8,
when the same JavaBeans property has both non-indexed read
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/53da2fb0/src/test/java/freemarker/core/TemplateConfigurationTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/freemarker/core/TemplateConfigurationTest.java b/src/test/java/freemarker/core/TemplateConfigurationTest.java
index bd397d4..665f270 100644
--- a/src/test/java/freemarker/core/TemplateConfigurationTest.java
+++ b/src/test/java/freemarker/core/TemplateConfigurationTest.java
@@ -165,6 +165,7 @@ public class TemplateConfigurationTest {
SETTING_ASSIGNMENTS.put("dateTimeFormat", "yyyy-#DDD-@HH:mm");
SETTING_ASSIGNMENTS.put("locale", NON_DEFAULT_LOCALE);
SETTING_ASSIGNMENTS.put("logTemplateExceptions", false);
+ SETTING_ASSIGNMENTS.put("wrapUncheckedExceptions", true);
SETTING_ASSIGNMENTS.put("newBuiltinClassResolver", TemplateClassResolver.ALLOWS_NOTHING_RESOLVER);
SETTING_ASSIGNMENTS.put("numberFormat", "0.0000");
SETTING_ASSIGNMENTS.put("objectWrapper", new SimpleObjectWrapper(ICI));
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/53da2fb0/src/test/java/freemarker/core/UncheckedExceptionHandlingTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/freemarker/core/UncheckedExceptionHandlingTest.java b/src/test/java/freemarker/core/UncheckedExceptionHandlingTest.java
new file mode 100644
index 0000000..f06d75d
--- /dev/null
+++ b/src/test/java/freemarker/core/UncheckedExceptionHandlingTest.java
@@ -0,0 +1,139 @@
+package freemarker.core;
+
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
+
+import java.io.FilterWriter;
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+
+import org.junit.Test;
+
+import com.google.common.collect.ImmutableMap;
+
+import freemarker.template.Configuration;
+import freemarker.template.TemplateDirectiveBody;
+import freemarker.template.TemplateDirectiveModel;
+import freemarker.template.TemplateException;
+import freemarker.template.TemplateMethodModelEx;
+import freemarker.template.TemplateModel;
+import freemarker.template.TemplateModelException;
+import freemarker.template.TemplateScalarModel;
+import freemarker.template.Version;
+import freemarker.test.TemplateTest;
+
+public class UncheckedExceptionHandlingTest extends TemplateTest {
+
+ @Override
+ protected Object createDataModel() {
+ return ImmutableMap.of(
+ "f", MyErronousFunction.INSTANCE,
+ "d", MyErronousDirective.INSTANCE,
+ "fd", MyFilterDirective.INSTANCE);
+ }
+
+ @Test
+ public void testBackwardCompatible() {
+ getConfiguration().setIncompatibleImprovements(Configuration.VERSION_2_3_26);
+ assertErrorContains("${f()}", MyUncheckedException.class);
+ assertErrorContains("<@d />", MyUncheckedException.class);
+ assertErrorContains("${f('NPE')}", NullPointerException.class);
+ assertErrorContains("<@d type='NPE' />", NullPointerException.class);
+ }
+
+ @Test
+ public void testMostlyBackwardCompatible() {
+ getConfiguration().setIncompatibleImprovements(Configuration.VERSION_2_3_27);
+ assertErrorContains("${f()}", MyUncheckedException.class);
+ assertErrorContains("<@d />", MyUncheckedException.class);
+ assertThat(
+ assertErrorContains("${f('NPE')}", TemplateException.class, "thrown an unchecked").getCause(),
+ instanceOf(NullPointerException.class));
+ assertThat(
+ assertErrorContains("<@d type='NPE' />", TemplateException.class, "thrown an unchecked").getCause(),
+ instanceOf(NullPointerException.class));
+ }
+
+
+ @Test
+ public void testNoBackwardCompatible() {
+ Configuration cfg = getConfiguration();
+ cfg.setWrapUncheckedExceptions(true);
+
+ for (Version ici : new Version[] { Configuration.VERSION_2_3_26, Configuration.VERSION_2_3_27 }) {
+ cfg.setIncompatibleImprovements(ici);
+
+ assertThat(
+ assertErrorContains("${f()}", TemplateException.class, "thrown an unchecked").getCause(),
+ instanceOf(MyUncheckedException.class));
+ assertThat(
+ assertErrorContains("<@d />", TemplateException.class, "thrown an unchecked").getCause(),
+ instanceOf(MyUncheckedException.class));
+ assertThat(
+ assertErrorContains("${f('NPE')}", TemplateException.class, "thrown an unchecked").getCause(),
+ instanceOf(NullPointerException.class));
+ assertThat(
+ assertErrorContains("<@d type='NPE' />", TemplateException.class, "thrown an unchecked").getCause(),
+ instanceOf(NullPointerException.class));
+ }
+ }
+
+ @Test
+ public void testFlowControlWorks() throws IOException, TemplateException {
+ Configuration cfg = getConfiguration();
+ for (boolean wrapUnchecked : new boolean[] { false, true }) {
+ cfg.setWrapUncheckedExceptions(wrapUnchecked);
+
+ 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 TemplateMethodModelEx {
+ private static final MyErronousFunction INSTANCE = new MyErronousFunction();
+
+ public Object exec(List arguments) throws TemplateModelException {
+ if (!arguments.isEmpty() && equalsNPE((TemplateModel) arguments.get(0))) {
+ throw new NullPointerException();
+ } else {
+ throw new MyUncheckedException();
+ }
+ }
+
+ }
+
+ public static class MyErronousDirective implements TemplateDirectiveModel {
+ private static final MyErronousDirective INSTANCE = new MyErronousDirective();
+
+ public void execute(Environment env, Map params, TemplateModel[] loopVars, TemplateDirectiveBody body)
+ throws TemplateException, IOException {
+ if (equalsNPE((TemplateModel) params.get("type"))) {
+ throw new NullPointerException();
+ } else {
+ throw new MyUncheckedException();
+ }
+ }
+
+ }
+
+ public static class MyFilterDirective implements TemplateDirectiveModel {
+ private static final MyFilterDirective INSTANCE = new MyFilterDirective();
+
+ public void execute(Environment env, Map params, TemplateModel[] loopVars, TemplateDirectiveBody body)
+ throws TemplateException, IOException {
+ body.render(new FilterWriter(env.getOut()) { });
+ }
+
+ }
+
+ public static class MyUncheckedException extends RuntimeException {
+ //
+ }
+
+ private static boolean equalsNPE(TemplateModel tm) throws TemplateModelException {
+ return (tm instanceof TemplateScalarModel) && "NPE".equals(((TemplateScalarModel) tm).getAsString());
+ }
+
+}
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/53da2fb0/src/test/java/freemarker/template/ConfigurationTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/freemarker/template/ConfigurationTest.java b/src/test/java/freemarker/template/ConfigurationTest.java
index 69eaf3a..cb46a94 100644
--- a/src/test/java/freemarker/template/ConfigurationTest.java
+++ b/src/test/java/freemarker/template/ConfigurationTest.java
@@ -1213,6 +1213,15 @@ public class ConfigurationTest extends TestCase {
assertEquals(false, cfg.getLogTemplateExceptions());
}
+ public void testSetWrapUncheckedExceptionsViaSetSettingAPI() throws TemplateException {
+ Configuration cfg = new Configuration(Configuration.VERSION_2_3_27);
+ assertEquals(false, cfg.getWrapUncheckedExceptions());
+ cfg.setSetting(Configurable.WRAP_UNCHECKED_EXCEPTIONS_KEY_CAMEL_CASE, "true");
+ assertEquals(true, cfg.getWrapUncheckedExceptions());
+ cfg.setSetting(Configurable.WRAP_UNCHECKED_EXCEPTIONS_KEY_SNAKE_CASE, "false");
+ assertEquals(false, cfg.getWrapUncheckedExceptions());
+ }
+
public void testSetAttemptExceptionReporter() throws TemplateException {
Configuration cfg = new Configuration(Configuration.VERSION_2_3_0);
assertEquals(AttemptExceptionReporter.LOG_ERROR_REPORTER, cfg.getAttemptExceptionReporter());