freemarker-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ddek...@apache.org
Subject [2/2] incubator-freemarker git commit: - Fixed connection "releasing" issue in URLTemplateLoader
Date Sat, 11 Feb 2017 17:56:29 GMT
- Fixed connection "releasing" issue in URLTemplateLoader

- Added more protected methods to URLTemplateLoader, so that (hopefully) it can be used for
HTTP and such more complex cases.


Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/de975ebf
Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/de975ebf
Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/de975ebf

Branch: refs/heads/3
Commit: de975ebf446e7848561ecabb93e4b6d9fdada4a7
Parents: 3e11ead
Author: ddekany <ddekany@apache.org>
Authored: Sat Feb 11 18:56:19 2017 +0100
Committer: ddekany <ddekany@apache.org>
Committed: Sat Feb 11 18:56:19 2017 +0100

----------------------------------------------------------------------
 .../freemarker/cache/ClassTemplateLoader.java   |  33 +++--
 .../freemarker/cache/URLTemplateLoader.java     | 135 +++++++++++++------
 2 files changed, 112 insertions(+), 56 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/de975ebf/src/main/java/freemarker/cache/ClassTemplateLoader.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/cache/ClassTemplateLoader.java b/src/main/java/freemarker/cache/ClassTemplateLoader.java
index e9825fe..c4e7987 100644
--- a/src/main/java/freemarker/cache/ClassTemplateLoader.java
+++ b/src/main/java/freemarker/cache/ClassTemplateLoader.java
@@ -19,7 +19,9 @@
 
 package freemarker.cache;
 
+import java.io.IOException;
 import java.net.URL;
+import java.net.URLConnection;
 
 import freemarker.template.utility.NullArgumentException;
 import freemarker.template.utility.StringUtil;
@@ -98,19 +100,6 @@ public class ClassTemplateLoader extends URLTemplateLoader {
         this.basePackagePath = canonBasePackagePath;
     }
 
-    @Override
-    protected URL getURL(String name) {
-        String fullPath = basePackagePath + name;
-
-        // Block java.net.URLClassLoader exploits:
-        if (basePackagePath.equals("/") && !isSchemeless(fullPath)) {
-            return null;
-        }
-
-        return resourceLoaderClass != null ? resourceLoaderClass.getResource(fullPath) :
classLoader
-                .getResource(fullPath);
-    }
-
     private static boolean isSchemeless(String fullPath) {
         int i = 0;
         int ln = fullPath.length();
@@ -172,4 +161,22 @@ public class ClassTemplateLoader extends URLTemplateLoader {
         return basePackagePath;
     }
 
+    @Override
+    protected URL getURL(String name) {
+        String fullPath = basePackagePath + name;
+    
+        // Block java.net.URLClassLoader exploits:
+        if (basePackagePath.equals("/") && !isSchemeless(fullPath)) {
+            return null;
+        }
+    
+        return resourceLoaderClass != null ? resourceLoaderClass.getResource(fullPath) :
classLoader
+                .getResource(fullPath);
+    }
+
+    @Override
+    protected TemplateLoadingResult extractNegativeResult(URLConnection conn) throws IOException
{
+        return null;
+    }
+
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/de975ebf/src/main/java/freemarker/cache/URLTemplateLoader.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/cache/URLTemplateLoader.java b/src/main/java/freemarker/cache/URLTemplateLoader.java
index bd5219c..56d5bdd 100644
--- a/src/main/java/freemarker/cache/URLTemplateLoader.java
+++ b/src/main/java/freemarker/cache/URLTemplateLoader.java
@@ -17,11 +17,11 @@
  * under the License.
  */
 
-
 package freemarker.cache;
 
 import java.io.File;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.Serializable;
 import java.net.JarURLConnection;
 import java.net.URL;
@@ -34,11 +34,11 @@ import org.slf4j.LoggerFactory;
 import freemarker.template.Configuration;
 
 /**
- * This is an abstract template loader that can load templates whose
- * location can be described by an URL. Subclasses only need to override
- * the {@link #getURL(String)} method.
+ * This is an abstract template loader that can load templates whose location can be described
by an URL. Subclasses
+ * only need to override the {@link #getURL(String)}, {@link #extractNegativeResult(URLConnection)},
and perhaps the
+ * {@link #prepareConnection(URLConnection)} method.
  */
-// TODO JUnit test
+// TODO JUnit test (including implementing a HTTP-based template loader to test the new protected
methods)
 public abstract class URLTemplateLoader implements TemplateLoader {
     
     private static final Logger LOG = LoggerFactory.getLogger("freemarker.cache");
@@ -46,33 +46,6 @@ public abstract class URLTemplateLoader implements TemplateLoader {
     private Boolean urlConnectionUsesCaches = false;
     
     /**
-     * Given a template name (plus potential locale decorations) retrieves
-     * an URL that points the template source.
-     * @param name the name of the sought template, including the locale
-     * decorations.
-     * @return an URL that points to the template source, or null if it can
-     * determine that the template source does not exist.
-     */
-    protected abstract URL getURL(String name);
-    
-    /**
-     * Can be used by subclasses to canonicalize URL path prefixes.
-     * @param prefix the path prefix to canonicalize
-     * @return the canonicalized prefix. All backslashes are replaced with
-     * forward slashes, and a trailing slash is appended if the original
-     * prefix wasn't empty and didn't already end with a slash.
-     */
-    protected static String canonicalizePrefix(String prefix) {
-        // make it foolproof
-        prefix = prefix.replace('\\', '/');
-        // ensure there's a trailing slash
-        if (prefix.length() > 0 && !prefix.endsWith("/")) {
-            prefix += "/";
-        }
-        return prefix;
-    }
-
-    /**
      * Getter pair of {@link #setURLConnectionUsesCaches(Boolean)}.
      * 
      * @since 2.3.21
@@ -83,7 +56,7 @@ public abstract class URLTemplateLoader implements TemplateLoader {
 
     /**
      * Sets if {@link URLConnection#setUseCaches(boolean)} will be called, and with what
value. By default this is
-     * {@code false}, becase FreeMarker has its own template cache with its own update delay
setting
+     * {@code false}, because FreeMarker has its own template cache with its own update delay
setting
      * ({@link Configuration#setTemplateUpdateDelay(int)}). If this is set to {@code null},
      * {@link URLConnection#setUseCaches(boolean)} won't be called.
      */
@@ -110,18 +83,43 @@ public abstract class URLTemplateLoader implements TemplateLoader {
             conn.setUseCaches(urlConnectionUsesCaches);
         }
         
-        // To prevent clustering issues, getLastModified(fallbackToJarLMD=false)
-        long lmd = getLastModified(conn, false);
-        Long version = lmd != -1 ? lmd : null;
+        prepareConnection(conn);
+        conn.connect();
         
-        URLTemplateLoadingSource source = new URLTemplateLoadingSource(url);
-        
-        if (ifSourceDiffersFrom != null && ifSourceDiffersFrom.equals(source)
-                && Objects.equals(ifVersionDiffersFrom, version)) {
-            return TemplateLoadingResult.NOT_MODIFIED;
+        InputStream inputStream = null;
+        Long version;
+        URLTemplateLoadingSource source;
+        try {
+            TemplateLoadingResult negativeResult = extractNegativeResult(conn);
+            if (negativeResult != null) {
+                return negativeResult;
+            }
+            
+            // To prevent clustering issues, getLastModified(fallbackToJarLMD=false)
+            long lmd = getLastModified(conn, false);
+            version = lmd != -1 ? lmd : null;
+            
+            source = new URLTemplateLoadingSource(url);
+            
+            if (ifSourceDiffersFrom != null && ifSourceDiffersFrom.equals(source)
+                    && Objects.equals(ifVersionDiffersFrom, version)) {
+                return TemplateLoadingResult.NOT_MODIFIED;
+            }
+            
+            inputStream = conn.getInputStream();
+        } catch (Throwable e) {
+            try {
+                if (inputStream == null) {
+                    // There's no URLConnection.close(), so we do this hack. In case of HttpURLConnection
we could call
+                    // disconnect(), but that's perhaps too aggressive.
+                    conn.getInputStream().close();
+                }
+            } catch (IOException e2) {
+                LOG.debug("Failed to close connection inputStream", e2);
+            }
+            throw e;
         }
-        
-        return new TemplateLoadingResult(source, version, conn.getInputStream(), null);
+        return new TemplateLoadingResult(source, version, inputStream, null);
     }
 
     @Override
@@ -172,5 +170,56 @@ public abstract class URLTemplateLoader implements TemplateLoader {
           return conn.getLastModified();
         }
     }
+
+    /**
+     * Given a template name (plus potential locale decorations) retrieves an URL that points
the template source.
+     * 
+     * @param name
+     *            the name of the sought template (including the locale decorations, or other
decorations the
+     *            {@link TemplateLookupStrategy} uses).
+     *            
+     * @return An URL that points to the template source, or null if it can be determined
that the template source does
+     *         not exist. For many implementations the existence of the template can't be
decided at this point, and you
+     *         rely on {@link #extractNegativeResult(URLConnection)} instead.
+     */
+    protected abstract URL getURL(String name);
+
+    /**
+     * Called before the resource if read, checks if we can immediately return a {@link TemplateLoadingResult#NOT_FOUND}
+     * or {@link TemplateLoadingResult#NOT_MODIFIED}, or throw an {@link IOException}. For
example, for a HTTP-based
+     * storage, the HTTP response status 404 could result in {@link TemplateLoadingResult#NOT_FOUND},
304 in
+     * {@link TemplateLoadingResult#NOT_MODIFIED}, and some others, like 500 in throwing
an {@link IOException}.
+     * 
+     * <p>Some
+     * implementations rely on {@link #getURL(String)} returning {@code null} when a template
is missing, in which case
+     * this method is certainly not applicable.
+     */
+    protected abstract TemplateLoadingResult extractNegativeResult(URLConnection conn) throws
IOException;
+
+    /**
+     * Called before anything that causes the connection to actually build up. This is where
+     * {@link URLConnection#setIfModifiedSince(long)} and such can be called if someone overrides
this.
+     * The default implementation in {@link URLTemplateLoader} does nothing. 
+     */
+    protected void prepareConnection(URLConnection conn) {
+        // Does nothing
+    }
+
+    /**
+     * Can be used by subclasses to canonicalize URL path prefixes.
+     * @param prefix the path prefix to canonicalize
+     * @return the canonicalized prefix. All backslashes are replaced with
+     * forward slashes, and a trailing slash is appended if the original
+     * prefix wasn't empty and didn't already end with a slash.
+     */
+    protected static String canonicalizePrefix(String prefix) {
+        // make it foolproof
+        prefix = prefix.replace('\\', '/');
+        // ensure there's a trailing slash
+        if (prefix.length() > 0 && !prefix.endsWith("/")) {
+            prefix += "/";
+        }
+        return prefix;
+    }
     
 }


Mime
View raw message