Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 09D11200D17 for ; Sun, 8 Oct 2017 13:52:25 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 0836B1609E6; Sun, 8 Oct 2017 11:52:25 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id F2E191609CB for ; Sun, 8 Oct 2017 13:52:23 +0200 (CEST) Received: (qmail 39215 invoked by uid 500); 8 Oct 2017 11:52:23 -0000 Mailing-List: contact notifications-help@freemarker.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@freemarker.incubator.apache.org Delivered-To: mailing list notifications@freemarker.incubator.apache.org Received: (qmail 39206 invoked by uid 99); 8 Oct 2017 11:52:23 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd1-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 08 Oct 2017 11:52:23 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd1-us-west.apache.org (ASF Mail Server at spamd1-us-west.apache.org) with ESMTP id 62968D464B for ; Sun, 8 Oct 2017 11:52:22 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: -4.222 X-Spam-Level: X-Spam-Status: No, score=-4.222 tagged_above=-999 required=6.31 tests=[KAM_ASCII_DIVIDERS=0.8, RCVD_IN_DNSWL_HI=-5, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, RP_MATCHES_RCVD=-0.001, SPF_PASS=-0.001] autolearn=disabled Received: from mx1-lw-eu.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id 5bEFjd8oSwow for ; Sun, 8 Oct 2017 11:52:20 +0000 (UTC) Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx1-lw-eu.apache.org (ASF Mail Server at mx1-lw-eu.apache.org) with SMTP id 51AC560D88 for ; Sun, 8 Oct 2017 11:52:19 +0000 (UTC) Received: (qmail 38918 invoked by uid 99); 8 Oct 2017 11:52:18 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 08 Oct 2017 11:52:18 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 3EEBFF5718; Sun, 8 Oct 2017 11:52:18 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: ddekany@apache.org To: notifications@freemarker.incubator.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: incubator-freemarker git commit: Added workaround against "ZipException: ZipFile closed" when loading properties. Related to https://github.com/apache/incubator-freemarker/pull/37. Date: Sun, 8 Oct 2017 11:52:18 +0000 (UTC) archived-at: Sun, 08 Oct 2017 11:52:25 -0000 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 Authored: Sun Oct 8 13:52:12 2017 +0200 Committer: ddekany 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 + 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 + freemarker.jar is installed on higher scope + than the web application (like on the Servlet container level), + but the web application contains the TLD. + + + Constants.EMPTY_HASH and GeneralPurposeNothing (the value of missingVar!) now implements @@ -27352,10 +27362,10 @@ TemplateModel x = env.getVariable("x"); // get variable x Added workaround against IllegalStateException: zip - file closed issues (caused by bugs outside of - FreeMarker) when loading resources included in the FreeMarker - jar (see - freemarker.template.utility.ClassUtil.getReasourceAsStream). + file closed and ZipException: ZipFile + closed issues (caused by bugs outside of FreeMarker) + when loading resources included in the FreeMarker jar (see + freemarker.template.utility.ClassUtil.loadProperties).