freemarker-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ddek...@apache.org
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, 08 Oct 2017 11:52:18 GMT
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 <ddekany@apache.org>
Authored: Sun Oct 8 13:52:12 2017 +0200
Committer: ddekany <ddekany@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>


Mime
View raw message