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 e816f81e55d9cd00b7c17525fd665eff1f630bb2 Author: ddekany <[email protected]> AuthorDate: Sun Feb 16 12:26:47 2020 +0100 Setting Configuration.incompatibleImprovements to the object returned by Configuration.getVersion() will now be logged as an error, but for backward compatibility it will still work. The typical bad pattern is this: new Configuration(Configuration.getVersion()). Doing that defeats the purpose of incompatibleImprovements, and makes upgrading FreeMarker a potentially breaking change. Furthermore, doing this probably won't be allowed starting from 2.4.0, and will throw exception. --- .../ext/beans/BeansWrapperConfiguration.java | 7 ++++++ .../java/freemarker/template/Configuration.java | 17 ++++++++++--- .../DefaultObjectWrapperConfiguration.java | 3 +++ .../java/freemarker/template/_TemplateAPI.java | 22 ++++++++++++++++- src/manual/en_US/book.xml | 28 ++++++++++++++++++++++ src/test/java/freemarker/manual/ExamplesTest.java | 3 ++- 6 files changed, 75 insertions(+), 5 deletions(-) diff --git a/src/main/java/freemarker/ext/beans/BeansWrapperConfiguration.java b/src/main/java/freemarker/ext/beans/BeansWrapperConfiguration.java index 323275e..7be7ee4 100644 --- a/src/main/java/freemarker/ext/beans/BeansWrapperConfiguration.java +++ b/src/main/java/freemarker/ext/beans/BeansWrapperConfiguration.java @@ -76,6 +76,13 @@ public abstract class BeansWrapperConfiguration implements Cloneable { */ protected BeansWrapperConfiguration(Version incompatibleImprovements, boolean isIncompImprsAlreadyNormalized) { _TemplateAPI.checkVersionNotNullAndSupported(incompatibleImprovements); + + // We can't do this in the BeansWrapper constructor, as by that time the version is normalized. + if (!isIncompImprsAlreadyNormalized) { + _TemplateAPI.checkCurrentVersionNotRecycled( + incompatibleImprovements, + "freemarker.beans", "BeansWrapper"); + } incompatibleImprovements = isIncompImprsAlreadyNormalized ? incompatibleImprovements diff --git a/src/main/java/freemarker/template/Configuration.java b/src/main/java/freemarker/template/Configuration.java index db736df..93139ae 100644 --- a/src/main/java/freemarker/template/Configuration.java +++ b/src/main/java/freemarker/template/Configuration.java @@ -609,9 +609,10 @@ public class Configuration extends Configurable implements Cloneable, ParserConf * * <p>Bugfixes and improvements that are fully backward compatible, also those that are important security fixes, * are enabled regardless of the incompatible improvements setting. - * + * * <p>Do NOT ever use {@link #getVersion()} to set the "incompatible improvements". Always use a fixed value, like - * {@link #VERSION_2_3_28}. Otherwise your application can break as you upgrade FreeMarker. + * {@link #VERSION_2_3_30}. Otherwise your application can break as you upgrade FreeMarker. (As of 2.3.30, doing + * this will be logged as an error. As of 2.4.0, it will be probably disallowed, by throwing exception.) * * <p>An important consequence of setting this setting is that now your application will check if the stated minimum * FreeMarker version requirement is met. Like if you set this setting to 2.3.22, but accidentally the application @@ -944,6 +945,7 @@ public class Configuration extends Configurable implements Cloneable, ParserConf checkFreeMarkerVersionClash(); NullArgumentException.check("incompatibleImprovements", incompatibleImprovements); + checkCurrentVersionNotRecycled(incompatibleImprovements); this.incompatibleImprovements = incompatibleImprovements; createTemplateCache(); @@ -1928,7 +1930,8 @@ public class Configuration extends Configurable implements Cloneable, ParserConf * Use {@link #Configuration(Version)} instead if possible; see the meaning of the parameter there. * * <p>Do NOT ever use {@link #getVersion()} to set the "incompatible improvements". Always use a fixed value, like - * {@link #VERSION_2_3_28}. Otherwise your application can break as you upgrade FreeMarker. + * {@link #VERSION_2_3_30}. Otherwise your application can break as you upgrade FreeMarker. (As of 2.3.30, doing + * this will be logged as an error. As of 2.4.0, it will be probably disallowed, by throwing exception.) * * <p>If the default value of a setting depends on the {@code incompatibleImprovements} and the value of that setting * was never set in this {@link Configuration} object through the public API, its value will be set to the default @@ -1947,6 +1950,8 @@ public class Configuration extends Configurable implements Cloneable, ParserConf _TemplateAPI.checkVersionNotNullAndSupported(incompatibleImprovements); if (!this.incompatibleImprovements.equals(incompatibleImprovements)) { + checkCurrentVersionNotRecycled(incompatibleImprovements); + this.incompatibleImprovements = incompatibleImprovements; if (!templateLoaderExplicitlySet) { @@ -1998,6 +2003,12 @@ public class Configuration extends Configurable implements Cloneable, ParserConf } } + private static void checkCurrentVersionNotRecycled(Version incompatibleImprovements) { + _TemplateAPI.checkCurrentVersionNotRecycled( + incompatibleImprovements, + "freemarker.configuration", "Configuration"); + } + /** * @see #setIncompatibleImprovements(Version) * @return Never {@code null}. diff --git a/src/main/java/freemarker/template/DefaultObjectWrapperConfiguration.java b/src/main/java/freemarker/template/DefaultObjectWrapperConfiguration.java index 81b4836..ff474fa 100644 --- a/src/main/java/freemarker/template/DefaultObjectWrapperConfiguration.java +++ b/src/main/java/freemarker/template/DefaultObjectWrapperConfiguration.java @@ -38,6 +38,9 @@ public abstract class DefaultObjectWrapperConfiguration extends BeansWrapperConf protected DefaultObjectWrapperConfiguration(Version incompatibleImprovements) { super(DefaultObjectWrapper.normalizeIncompatibleImprovementsVersion(incompatibleImprovements), true); + _TemplateAPI.checkCurrentVersionNotRecycled( + incompatibleImprovements, + "freemarker.configuration", "DefaultObjectWrapper"); useAdaptersForContainers = getIncompatibleImprovements().intValue() >= _TemplateAPI.VERSION_INT_2_3_22; forceLegacyNonListCollections = true; // [2.4]: = IcI < _TemplateAPI.VERSION_INT_2_4_0; } diff --git a/src/main/java/freemarker/template/_TemplateAPI.java b/src/main/java/freemarker/template/_TemplateAPI.java index 8d1683f..18c16e4 100644 --- a/src/main/java/freemarker/template/_TemplateAPI.java +++ b/src/main/java/freemarker/template/_TemplateAPI.java @@ -30,6 +30,7 @@ import freemarker.cache.TemplateNameFormat; import freemarker.core.Expression; import freemarker.core.OutputFormat; import freemarker.core.TemplateObject; +import freemarker.log.Logger; import freemarker.template.utility.NullArgumentException; /** @@ -94,7 +95,26 @@ public class _TemplateAPI { throw new IllegalArgumentException("\"incompatibleImprovements\" must be at least 2.3.0."); } } - + + /** + * Checks if the object return by {@link Configuration#getVersion()} was used for setting + * "incompatibleImprovements", which shouldn't be done. + * + * @since 2.3.30 + */ + public static void checkCurrentVersionNotRecycled( + Version incompatibleImprovements, + String logCategory, String configuredClassShortName) { + if (incompatibleImprovements == Configuration.getVersion()) { + Logger.getLogger(logCategory) + .error(configuredClassShortName + ".incompatibleImprovements was set to the object returned by " + + "Configuration.getVersion(). That defeats the purpose of incompatibleImprovements, " + + "and makes upgrading FreeMarker a potentially breaking change. Also, this " + + "probably won't be allowed starting from 2.4.0. Instead, set incompatibleImprovements to " + + "the highest concrete version that's known to be compatible with your application."); + } + } + public static int getTemplateLanguageVersionAsInt(TemplateObject to) { return getTemplateLanguageVersionAsInt(to.getTemplate()); } diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml index 5d4d545..8bb74f2 100644 --- a/src/manual/en_US/book.xml +++ b/src/manual/en_US/book.xml @@ -11389,6 +11389,13 @@ TemplateHashModel roundingModeEnums = <td>Logs messages of the FreeMarker JSP support.</td> </tr> + + <tr> + <td><literal>freemarker.configuration</literal></td> + + <td>Logs messages related to configuring FreeMarker, and not + fitting any other categories.</td> + </tr> </tbody> </informaltable> @@ -29288,6 +29295,27 @@ TemplateModel x = env.getVariable("x"); // get variable x</programlisting> </listitem> <listitem> + <para>Setting <literal>incompatibleImprovements</literal> to the + instance returned by + <literal>Configuration.getVersion()</literal> will now be logged + as an error, but for backward compatibility it will still work. + This applies to said setting of + <literal>Configuration</literal>, + <literal>DefaultObjectWrapper</literal>, and + <literal>BeansWrapper</literal>. The typical bad pattern is + this: <literal>new + Configuration(Configuration.getVersion())</literal>. Doing that + defeats the purpose of incompatibleImprovements, and makes + upgrading FreeMarker a potentially breaking change. Furthermore, + doing this probably won't be allowed starting from 2.4.0, and + will throw exception. So if above mistake is present in your + application, it should be fixed by setting + <literal>incompatibleImprovements</literal> to the highest + concrete version that's known to be compatible with the + application.</para> + </listitem> + + <listitem> <para>Added <literal>Environment.getDataModelOrSharedVariable(String)</literal>.</para> </listitem> diff --git a/src/test/java/freemarker/manual/ExamplesTest.java b/src/test/java/freemarker/manual/ExamplesTest.java index 7478d74..5d3a932 100644 --- a/src/test/java/freemarker/manual/ExamplesTest.java +++ b/src/test/java/freemarker/manual/ExamplesTest.java @@ -29,6 +29,7 @@ import freemarker.cache.MultiTemplateLoader; import freemarker.cache.StringTemplateLoader; import freemarker.cache.TemplateLoader; import freemarker.template.Configuration; +import freemarker.template.Version; import freemarker.test.TemplateTest; @Ignore @@ -44,7 +45,7 @@ public abstract class ExamplesTest extends TemplateTest { @Override protected final Configuration createConfiguration() { - Configuration cfg = new Configuration(Configuration.getVersion()); + Configuration cfg = new Configuration(new Version(Configuration.getVersion().toString())); setupTemplateLoaders(cfg); return cfg; }
