Repository: incubator-freemarker Updated Branches: refs/heads/2.3-gae bd7498e8d -> 15b97afa5
Added workaround against "ZipException: ZipFile closed" when loading properties. Related to https://github.com/apache/incubator-freemarker/pull/37. Also, fixed bug where we didn't try to find TLD resources with the thread context class loader. Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/15b97afa Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/15b97afa Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/15b97afa Branch: refs/heads/2.3-gae Commit: 15b97afa5026f6a53101f582840c11cc4d07e365 Parents: bd7498e Author: ddekany <ddek...@apache.org> Authored: Sun Oct 8 13:52:12 2017 +0200 Committer: ddekany <ddek...@apache.org> Committed: Sun Oct 8 13:52:12 2017 +0200 ---------------------------------------------------------------------- .../freemarker/ext/beans/UnsafeMethods.java | 15 +--- .../java/freemarker/ext/jsp/TaglibFactory.java | 9 +- .../java/freemarker/template/Configuration.java | 16 +--- .../freemarker/template/utility/ClassUtil.java | 94 ++++++++++++++++---- src/manual/en_US/book.xml | 18 +++- 5 files changed, 104 insertions(+), 48 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/15b97afa/src/main/java/freemarker/ext/beans/UnsafeMethods.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/ext/beans/UnsafeMethods.java b/src/main/java/freemarker/ext/beans/UnsafeMethods.java index f590085..c7b5f46 100644 --- a/src/main/java/freemarker/ext/beans/UnsafeMethods.java +++ b/src/main/java/freemarker/ext/beans/UnsafeMethods.java @@ -19,7 +19,6 @@ package freemarker.ext.beans; -import java.io.InputStream; import java.lang.reflect.Method; import java.util.HashMap; import java.util.HashSet; @@ -43,21 +42,13 @@ class UnsafeMethods { } private static final Set createUnsafeMethodsSet() { - Properties props = new Properties(); - String methodSpec = null; try { - InputStream in = ClassUtil.getReasourceAsStream(BeansWrapper.class, UNSAFE_METHODS_PROPERTIES); - try { - props.load(in); - } finally { - in.close(); - } + Properties props = ClassUtil.loadProperties(BeansWrapper.class, UNSAFE_METHODS_PROPERTIES); Set set = new HashSet(props.size() * 4 / 3, 1f); Map primClasses = createPrimitiveClassesMap(); for (Iterator iterator = props.keySet().iterator(); iterator.hasNext(); ) { - methodSpec = (String) iterator.next(); try { - set.add(parseMethodSpec(methodSpec, primClasses)); + set.add(parseMethodSpec((String) iterator.next(), primClasses)); } catch (ClassNotFoundException e) { if (ClassIntrospector.DEVELOPMENT_MODE) { throw e; @@ -70,7 +61,7 @@ class UnsafeMethods { } return set; } catch (Exception e) { - throw new RuntimeException("Could not load unsafe method " + methodSpec + " " + e.getClass().getName() + " " + e.getMessage()); + throw new RuntimeException("Could not load unsafe method set", e); } } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/15b97afa/src/main/java/freemarker/ext/jsp/TaglibFactory.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/ext/jsp/TaglibFactory.java b/src/main/java/freemarker/ext/jsp/TaglibFactory.java index 0dc07b1..de4bcf3 100644 --- a/src/main/java/freemarker/ext/jsp/TaglibFactory.java +++ b/src/main/java/freemarker/ext/jsp/TaglibFactory.java @@ -1257,16 +1257,19 @@ public class TaglibFactory implements TemplateHashModel { public InputStream getInputStream() throws IOException { ClassLoader tccl = tryGetThreadContextClassLoader(); if (tccl != null) { - return ClassUtil.getReasourceAsStream(getClass(), resourcePath); + InputStream r = ClassUtil.getReasourceAsStream(tccl, resourcePath, true); + if (r != null) { + return r; + } } - return ClassUtil.getReasourceAsStream(getClass(), resourcePath); + return ClassUtil.getReasourceAsStream(getClass(), resourcePath, false); } public String getXmlSystemId() throws IOException { ClassLoader tccl = tryGetThreadContextClassLoader(); if (tccl != null) { - final URL url = getClass().getResource(resourcePath); + final URL url = tccl.getResource(resourcePath); if (url != null) { return url.toExternalForm(); } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/15b97afa/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 53358b1..d4d6863 100644 --- a/src/main/java/freemarker/template/Configuration.java +++ b/src/main/java/freemarker/template/Configuration.java @@ -21,7 +21,6 @@ package freemarker.template; import java.io.File; import java.io.IOException; -import java.io.InputStream; import java.io.Writer; import java.lang.reflect.InvocationTargetException; import java.net.URLConnection; @@ -435,19 +434,12 @@ public class Configuration extends Configurable implements Cloneable, ParserConf private static final Version VERSION; static { try { - Properties vp = new Properties(); - InputStream ins = ClassUtil.getReasourceAsStream(Configuration.class, VERSION_PROPERTIES_PATH); - try { - vp.load(ins); - } finally { - ins.close(); - } - - String versionString = getRequiredVersionProperty(vp, "version"); + Properties props = ClassUtil.loadProperties(Configuration.class, VERSION_PROPERTIES_PATH); + String versionString = getRequiredVersionProperty(props, "version"); Date buildDate; { - String buildDateStr = getRequiredVersionProperty(vp, "buildTimestamp"); + String buildDateStr = getRequiredVersionProperty(props, "buildTimestamp"); if (buildDateStr.endsWith("Z")) { buildDateStr = buildDateStr.substring(0, buildDateStr.length() - 1) + "+0000"; } @@ -458,7 +450,7 @@ public class Configuration extends Configurable implements Cloneable, ParserConf } } - final Boolean gaeCompliant = Boolean.valueOf(getRequiredVersionProperty(vp, "isGAECompliant")); + final Boolean gaeCompliant = Boolean.valueOf(getRequiredVersionProperty(props, "isGAECompliant")); VERSION = new Version(versionString, gaeCompliant, buildDate); } catch (IOException e) { http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/15b97afa/src/main/java/freemarker/template/utility/ClassUtil.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/template/utility/ClassUtil.java b/src/main/java/freemarker/template/utility/ClassUtil.java index 7d97839..d5601f2 100644 --- a/src/main/java/freemarker/template/utility/ClassUtil.java +++ b/src/main/java/freemarker/template/utility/ClassUtil.java @@ -23,7 +23,9 @@ import java.io.IOException; import java.io.InputStream; import java.net.URL; import java.util.HashSet; +import java.util.Properties; import java.util.Set; +import java.util.zip.ZipException; import freemarker.core.Environment; import freemarker.core.Macro; @@ -384,34 +386,40 @@ public class ClassUtil { /** * Very similar to {@link Class#getResourceAsStream(String)}, but throws {@link IOException} instead of returning - * {@code null}, and attempts to work around "IllegalStateException: zip file closed" issues (caused by bugs - * outside of FreeMarker). - * - * @return Never {@code null} - * @throws IOException If the resource wasn't found, or other {@link IOException} occurs. + * {@code null} if {@code optional} is {@code false}, and attempts to work around "IllegalStateException: zip file + * closed" issues (caused by bugs outside of FreeMarker). + * + * @return If {@code optional} is {@code false}, it's never {@code null}, otherwise {@code null} indicates that the + * resource doesn't exist. + * @throws IOException + * If the resource wasn't found, or other {@link IOException} occurs. + * + * @since 2.3.27 */ - public static InputStream getReasourceAsStream(Class<?> baseClass, String resource) throws IOException { + public static InputStream getReasourceAsStream(Class<?> baseClass, String resource, boolean optional) + throws IOException { InputStream ins; try { ins = baseClass.getResourceAsStream(resource); } catch (IllegalStateException e) { - // Workaround for "IllegalStateException: zip file closed". This happens due to bugs outside of FreeMarker - // (sometimes caused by the caching of jar URL connection streams), but we try to work it around anyway. + // Workaround for "IllegalStateException: zip file closed". This happens due to bugs outside of FreeMarker, + // but we try to work it around anyway. URL url = baseClass.getResource(resource); ins = url != null ? url.openStream() : null; } - if (ins == null) { - throw new IOException("Class-loader resource not found (shown quoted): \"" - + StringUtil.jQuote(resource) + "\". The base class was " + baseClass.getName() + "."); + if (!optional) { + checkInputStreamNotNull(ins, baseClass, resource); } return ins; } /** - * Same as {@link #getReasourceAsStream(Class, String)}, but uses a {@link ClassLoader} directly instead of a - * {@link Class}. + * Same as {@link #getReasourceAsStream(Class, String, boolean)}, but uses a {@link ClassLoader} directly + * instead of a {@link Class}. + * + * @since 2.3.27 */ - public static InputStream getReasourceAsStream(ClassLoader classLoader, String resource) + public static InputStream getReasourceAsStream(ClassLoader classLoader, String resource, boolean optional) throws IOException { InputStream ins; try { @@ -420,11 +428,63 @@ public class ClassUtil { URL url = classLoader.getResource(resource); ins = url != null ? url.openStream() : null; } - if (ins == null) { - throw new IOException("Class-loader resource not found (shown quoted): \"" - + StringUtil.jQuote(resource) + "\". The base ClassLoader was: " + classLoader); + if (ins == null && !optional) { + throw new IOException("Class-loader resource not found (shown quoted): " + + StringUtil.jQuote(resource) + ". The base ClassLoader was: " + classLoader); } return ins; } + + /** + * Loads a class loader resource into a {@link Properties}; tries to work around "zip file closed" errors. + * + * @since 2.3.27 + */ + public static Properties loadProperties(Class<?> baseClass, String resource) throws IOException { + Properties props = new Properties(); + + InputStream ins = null; + try { + try { + ins = baseClass.getResourceAsStream(resource); + } catch (IllegalStateException e) { + throw new MaybeZipFileClosedException(); + } + checkInputStreamNotNull(ins, baseClass, resource); + try { + props.load(ins); + } catch (ZipException e) { + throw new MaybeZipFileClosedException(); + } + } catch (MaybeZipFileClosedException e) { + // Workaround for "zip file closed" exception. This happens due to bugs outside of FreeMarker, but we try + // to work it around anyway. + URL url = baseClass.getResource(resource); + ins = url != null ? url.openStream() : null; + checkInputStreamNotNull(ins, baseClass, resource); + props.load(ins); + } finally { + if (ins != null) { + try { + ins.close(); + } catch (ZipException e) { + // Do nothing to suppress "ZipFile closed" exceptions. + } + } + } + return props; + } + + private static void checkInputStreamNotNull(InputStream ins, Class<?> baseClass, String resource) + throws IOException { + if (ins == null) { + throw new IOException("Class-loader resource not found (shown quoted): " + + StringUtil.jQuote(resource) + ". The base class was " + baseClass.getName() + "."); + } + } + + private static class MaybeZipFileClosedException extends Exception { + // + } } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/15b97afa/src/manual/en_US/book.xml ---------------------------------------------------------------------- diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml index 2068fe3..1e0e4d9 100644 --- a/src/manual/en_US/book.xml +++ b/src/manual/en_US/book.xml @@ -27293,6 +27293,16 @@ TemplateModel x = env.getVariable("x"); // get variable x</programlisting> </listitem> <listitem> + <para>Bug fixed: JSP support haven't tried using the thread + context class-loader to load the TLD, instead, it has only used + the defining class loader of the FreeMarker classes. This can + cause problem in the rare case where + <literal>freemarker.jar</literal> is installed on higher scope + than the web application (like on the Servlet container level), + but the web application contains the TLD.</para> + </listitem> + + <listitem> <para><literal>Constants.EMPTY_HASH</literal> and <literal>GeneralPurposeNothing</literal> (the value of <literal>missingVar!</literal>) now implements @@ -27352,10 +27362,10 @@ TemplateModel x = env.getVariable("x"); // get variable x</programlisting> <listitem> <para>Added workaround against <quote>IllegalStateException: zip - file closed</quote> issues (caused by bugs outside of - FreeMarker) when loading resources included in the FreeMarker - jar (see - <literal>freemarker.template.utility.ClassUtil.getReasourceAsStream</literal>).</para> + file closed</quote> and <quote>ZipException: ZipFile + closed</quote> issues (caused by bugs outside of FreeMarker) + when loading resources included in the FreeMarker jar (see + <literal>freemarker.template.utility.ClassUtil.loadProperties</literal>).</para> </listitem> </itemizedlist> </section>
