You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2017/10/13 10:10:33 UTC

svn commit: r1812103 - in /tomcat/trunk: java/org/apache/catalina/webresources/ java/org/apache/tomcat/util/compat/ java/org/apache/tomcat/util/scan/ test/org/apache/catalina/webresources/

Author: markt
Date: Fri Oct 13 10:10:33 2017
New Revision: 1812103

URL: http://svn.apache.org/viewvc?rev=1812103&view=rev
Log:
Partial fix for https://bz.apache.org/bugzilla/show_bug.cgi?id=61601
Handle multi-release JARs for unpacked web applications

Modified:
    tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java
    tomcat/trunk/java/org/apache/catalina/webresources/AbstractSingleArchiveResourceSet.java
    tomcat/trunk/java/org/apache/catalina/webresources/JarWarResourceSet.java
    tomcat/trunk/java/org/apache/tomcat/util/compat/Jre9Compat.java
    tomcat/trunk/java/org/apache/tomcat/util/compat/JreCompat.java
    tomcat/trunk/java/org/apache/tomcat/util/scan/JarFileUrlJar.java
    tomcat/trunk/test/org/apache/catalina/webresources/TestJarInputStreamWrapper.java

Modified: tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java?rev=1812103&r1=1812102&r2=1812103&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java (original)
+++ tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java Fri Oct 13 10:10:33 2017
@@ -30,6 +30,7 @@ import java.util.jar.Manifest;
 import org.apache.catalina.WebResource;
 import org.apache.catalina.WebResourceRoot;
 import org.apache.catalina.util.ResourceSet;
+import org.apache.tomcat.util.compat.JreCompat;
 
 public abstract class AbstractArchiveResourceSet extends AbstractResourceSet {
 
@@ -247,23 +248,28 @@ public abstract class AbstractArchiveRes
                 return new JarResourceRoot(root, new File(getBase()),
                         baseUrlString, path);
             } else {
-                Map<String,JarEntry> jarEntries = getArchiveEntries(true);
                 JarEntry jarEntry = null;
-                if (!(pathInJar.charAt(pathInJar.length() - 1) == '/')) {
-                    if (jarEntries == null) {
-                        jarEntry = getArchiveEntry(pathInJar + '/');
-                    } else {
-                        jarEntry = jarEntries.get(pathInJar + '/');
-                    }
-                    if (jarEntry != null) {
-                        path = path + '/';
+                if (isMultiRelease()) {
+                    // Calls JarFile.getJarEntry() which is multi-release aware
+                    jarEntry = getArchiveEntry(pathInJar);
+                } else {
+                    Map<String,JarEntry> jarEntries = getArchiveEntries(true);
+                    if (!(pathInJar.charAt(pathInJar.length() - 1) == '/')) {
+                        if (jarEntries == null) {
+                            jarEntry = getArchiveEntry(pathInJar + '/');
+                        } else {
+                            jarEntry = jarEntries.get(pathInJar + '/');
+                        }
+                        if (jarEntry != null) {
+                            path = path + '/';
+                        }
                     }
-                }
-                if (jarEntry == null) {
-                    if (jarEntries == null) {
-                        jarEntry = getArchiveEntry(pathInJar);
-                    } else {
-                        jarEntry = jarEntries.get(pathInJar);
+                    if (jarEntry == null) {
+                        if (jarEntries == null) {
+                            jarEntry = getArchiveEntry(pathInJar);
+                        } else {
+                            jarEntry = jarEntries.get(pathInJar);
+                        }
                     }
                 }
                 if (jarEntry == null) {
@@ -277,6 +283,8 @@ public abstract class AbstractArchiveRes
         }
     }
 
+    protected abstract boolean isMultiRelease();
+
     protected abstract WebResource createArchiveResource(JarEntry jarEntry,
             String webAppPath, Manifest manifest);
 
@@ -299,7 +307,7 @@ public abstract class AbstractArchiveRes
     protected JarFile openJarFile() throws IOException {
         synchronized (archiveLock) {
             if (archive == null) {
-                archive = new JarFile(getBase());
+                archive = JreCompat.getInstance().jarFileNewInstance(getBase());
             }
             archiveUseCount++;
             return archive;

Modified: tomcat/trunk/java/org/apache/catalina/webresources/AbstractSingleArchiveResourceSet.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/webresources/AbstractSingleArchiveResourceSet.java?rev=1812103&r1=1812102&r2=1812103&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/webresources/AbstractSingleArchiveResourceSet.java (original)
+++ tomcat/trunk/java/org/apache/catalina/webresources/AbstractSingleArchiveResourceSet.java Fri Oct 13 10:10:33 2017
@@ -28,6 +28,7 @@ import java.util.jar.JarFile;
 import org.apache.catalina.LifecycleException;
 import org.apache.catalina.WebResourceRoot;
 import org.apache.tomcat.util.buf.UriUtil;
+import org.apache.tomcat.util.compat.JreCompat;
 
 /**
  * Base class for a {@link org.apache.catalina.WebResourceSet} based on a
@@ -35,6 +36,8 @@ import org.apache.tomcat.util.buf.UriUti
  */
 public abstract class AbstractSingleArchiveResourceSet extends AbstractArchiveResourceSet {
 
+    private Boolean multiRelease;
+
     /**
      * A no argument constructor is required for this to work with the digester.
      */
@@ -104,11 +107,37 @@ public abstract class AbstractSingleArch
     }
 
 
+    @Override
+    protected boolean isMultiRelease() {
+        if (multiRelease == null) {
+            synchronized (archiveLock) {
+                if (multiRelease == null) {
+                    JarFile jarFile = null;
+                    try {
+                        jarFile = openJarFile();
+                        multiRelease = Boolean.valueOf(
+                                JreCompat.getInstance().jarFileIsMultiRelease(jarFile));
+                    } catch (IOException ioe) {
+                        // Should never happen
+                        throw new IllegalStateException(ioe);
+                    } finally {
+                        if (jarFile != null) {
+                            closeJarFile();
+                        }
+                    }
+                }
+            }
+        }
+
+        return multiRelease.booleanValue();
+    }
+
+
     //-------------------------------------------------------- Lifecycle methods
     @Override
     protected void initInternal() throws LifecycleException {
 
-        try (JarFile jarFile = new JarFile(getBase())) {
+        try (JarFile jarFile = JreCompat.getInstance().jarFileNewInstance(getBase())) {
             setManifest(jarFile.getManifest());
         } catch (IOException ioe) {
             throw new IllegalArgumentException(ioe);

Modified: tomcat/trunk/java/org/apache/catalina/webresources/JarWarResourceSet.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/webresources/JarWarResourceSet.java?rev=1812103&r1=1812102&r2=1812103&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/webresources/JarWarResourceSet.java (original)
+++ tomcat/trunk/java/org/apache/catalina/webresources/JarWarResourceSet.java Fri Oct 13 10:10:33 2017
@@ -162,6 +162,13 @@ public class JarWarResourceSet extends A
     }
 
 
+    @Override
+    protected boolean isMultiRelease() {
+        // TODO: multi-release support for packed WAR files
+        return false;
+    }
+
+
     //-------------------------------------------------------- Lifecycle methods
     @Override
     protected void initInternal() throws LifecycleException {

Modified: tomcat/trunk/java/org/apache/tomcat/util/compat/Jre9Compat.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/compat/Jre9Compat.java?rev=1812103&r1=1812102&r2=1812103&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/compat/Jre9Compat.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/compat/Jre9Compat.java Fri Oct 13 10:10:33 2017
@@ -16,7 +16,9 @@
  */
 package org.apache.tomcat.util.compat;
 
+import java.io.File;
 import java.io.IOException;
+import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.net.MalformedURLException;
@@ -25,6 +27,8 @@ import java.net.URL;
 import java.net.URLConnection;
 import java.util.Deque;
 import java.util.Set;
+import java.util.jar.JarFile;
+import java.util.zip.ZipFile;
 
 import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLParameters;
@@ -49,14 +53,12 @@ class Jre9Compat extends JreCompat {
     private static final Method locationMethod;
     private static final Method isPresentMethod;
     private static final Method getMethod;
+    private static final Constructor<JarFile> jarFileConstructor;
+    private static final Method isMultiReleaseMethod;
 
-    static {
-        Class<?> moduleLayerClazz = null;
-        Class<?> configurationClazz = null;
-        Class<?> resolvedModuleClazz = null;
-        Class<?> moduleReferenceClazz = null;
-        Class<?> optionalClazz = null;
+    private static final Object RUNTIME_VERSION;
 
+    static {
         Class<?> c1 = null;
         Method m2 = null;
         Method m3 = null;
@@ -68,13 +70,18 @@ class Jre9Compat extends JreCompat {
         Method m9 = null;
         Method m10 = null;
         Method m11 = null;
+        Constructor<JarFile> c12 = null;
+        Method m13 = null;
+        Object o14 = null;
 
         try {
-            moduleLayerClazz = Class.forName("java.lang.ModuleLayer");
-            configurationClazz = Class.forName("java.lang.module.Configuration");
-            resolvedModuleClazz = Class.forName("java.lang.module.ResolvedModule");
-            moduleReferenceClazz = Class.forName("java.lang.module.ModuleReference");
-            optionalClazz = Class.forName("java.util.Optional");
+            Class<?> moduleLayerClazz = Class.forName("java.lang.ModuleLayer");
+            Class<?> configurationClazz = Class.forName("java.lang.module.Configuration");
+            Class<?> resolvedModuleClazz = Class.forName("java.lang.module.ResolvedModule");
+            Class<?> moduleReferenceClazz = Class.forName("java.lang.module.ModuleReference");
+            Class<?> optionalClazz = Class.forName("java.util.Optional");
+            Class<?> versionClazz = Class.forName("java.lang.Runtime$Version");
+            Method versionMethod = JarFile.class.getMethod("runtimeVersion");
 
             c1 = Class.forName("java.lang.reflect.InaccessibleObjectException");
             m2 = SSLParameters.class.getMethod("setApplicationProtocols", String[].class);
@@ -87,11 +94,15 @@ class Jre9Compat extends JreCompat {
             m9 = moduleReferenceClazz.getMethod("location");
             m10 = optionalClazz.getMethod("isPresent");
             m11 = optionalClazz.getMethod("get");
-        } catch (SecurityException | NoSuchMethodException e) {
-            // Should never happen
+            c12 = JarFile.class.getConstructor(File.class, boolean.class, int.class, versionClazz);
+            m13 = JarFile.class.getMethod("isMultiRelease");
+            o14 = versionMethod.invoke(null);
         } catch (ClassNotFoundException e) {
             // Must be Java 8
+        } catch (ReflectiveOperationException | IllegalArgumentException e) {
+            // Should never happen
         }
+
         inaccessibleObjectExceptionClazz = c1;
         setApplicationProtocolsMethod = m2;
         getApplicationProtocolMethod = m3;
@@ -103,6 +114,10 @@ class Jre9Compat extends JreCompat {
         locationMethod = m9;
         isPresentMethod = m10;
         getMethod = m11;
+        jarFileConstructor = c12;
+        isMultiReleaseMethod = m13;
+
+        RUNTIME_VERSION = o14;
     }
 
 
@@ -175,4 +190,25 @@ class Jre9Compat extends JreCompat {
             throw new UnsupportedOperationException(e);
         }
     }
+
+
+    @Override
+    public JarFile jarFileNewInstance(File f) throws IOException {
+        try {
+            return jarFileConstructor.newInstance(
+                    f, Boolean.TRUE, Integer.valueOf(ZipFile.OPEN_READ), RUNTIME_VERSION);
+        } catch (ReflectiveOperationException | IllegalArgumentException e) {
+            throw new IOException(e);
+        }
+    }
+
+
+    @Override
+    public boolean jarFileIsMultiRelease(JarFile jarFile) {
+        try {
+            return ((Boolean) isMultiReleaseMethod.invoke(jarFile)).booleanValue();
+        } catch (ReflectiveOperationException | IllegalArgumentException e) {
+            return false;
+        }
+    }
 }

Modified: tomcat/trunk/java/org/apache/tomcat/util/compat/JreCompat.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/compat/JreCompat.java?rev=1812103&r1=1812102&r2=1812103&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/compat/JreCompat.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/compat/JreCompat.java Fri Oct 13 10:10:33 2017
@@ -16,10 +16,12 @@
  */
 package org.apache.tomcat.util.compat;
 
+import java.io.File;
 import java.io.IOException;
 import java.net.URL;
 import java.net.URLConnection;
 import java.util.Deque;
+import java.util.jar.JarFile;
 
 import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLParameters;
@@ -119,7 +121,7 @@ public class JreCompat {
 
 
     /**
-     * Obtains the URls for all the JARs on the module path when the JVM starts
+     * Obtains the URLs for all the JARs on the module path when the JVM starts
      * and adds them to the provided Deque.
      *
      * @param classPathUrlsToProcess    The Deque to which the modules should be
@@ -129,4 +131,44 @@ public class JreCompat {
         // NO-OP for Java 8. There is no module path.
     }
 
+
+    /**
+     * Creates a new JarFile instance. When running on Java 9 and later, the
+     * JarFile will be multi-release JAR aware. While this isn't strictly
+     * required to be in this package, it is provided as a convenience method.
+     *
+     * @param s The JAR file to open
+     *
+     * @throws IOException  If an I/O error occurs creating the JarFile instance
+     */
+    public final JarFile jarFileNewInstance(String s) throws IOException {
+        return jarFileNewInstance(new File(s));
+    }
+
+
+    /**
+     * Creates a new JarFile instance. When running on Java 9 and later, the
+     * JarFile will be multi-release JAR aware.
+     *
+     * @param f The JAR file to open
+     *
+     * @throws IOException  If an I/O error occurs creating the JarFile instance
+     */
+    public JarFile jarFileNewInstance(File f) throws IOException {
+        return new JarFile(f);
+    }
+
+
+    /**
+     * Is this JarFile a multi-release JAR file.
+     *
+     * @param jarFile   The JarFile to test
+     *
+     * @return {@code true} If it is a multi-release JAR file and is configured
+     *         to behave as such.
+     */
+    public boolean jarFileIsMultiRelease(JarFile jarFile) {
+        // Java 8 doesn't support multi-release so default to false
+        return false;
+    }
 }

Modified: tomcat/trunk/java/org/apache/tomcat/util/scan/JarFileUrlJar.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/scan/JarFileUrlJar.java?rev=1812103&r1=1812102&r2=1812103&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/scan/JarFileUrlJar.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/scan/JarFileUrlJar.java Fri Oct 13 10:10:33 2017
@@ -23,23 +23,28 @@ import java.net.JarURLConnection;
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.util.Enumeration;
+import java.util.HashSet;
+import java.util.Set;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 import java.util.jar.Manifest;
 import java.util.zip.ZipEntry;
 
 import org.apache.tomcat.Jar;
+import org.apache.tomcat.util.compat.JreCompat;
 
 /**
  * Implementation of {@link Jar} that is optimised for file based JAR URLs that
  * refer directly to a JAR file (e.g URLs of the form jar:file: ... .jar!/ or
- * file:... .jar) .
+ * file:... .jar).
  */
 public class JarFileUrlJar implements Jar {
 
     private final JarFile jarFile;
     private final URL jarFileURL;
+    private final boolean multiRelease;
     private Enumeration<JarEntry> entries;
+    private Set<String> entryNamesSeen;
     private JarEntry entry = null;
 
     public JarFileUrlJar(URL url, boolean startsWithJar) throws IOException {
@@ -57,9 +62,10 @@ public class JarFileUrlJar implements Ja
             } catch (URISyntaxException e) {
                 throw new IOException(e);
             }
-            jarFile = new JarFile(f);
+            jarFile = JreCompat.getInstance().jarFileNewInstance(f);
             jarFileURL = url;
         }
+        multiRelease = JreCompat.getInstance().jarFileIsMultiRelease(jarFile);
     }
 
 
@@ -71,6 +77,7 @@ public class JarFileUrlJar implements Ja
 
     @Override
     public InputStream getInputStream(String name) throws IOException {
+        // JarFile#getEntry() is multi-release aware
         ZipEntry entry = jarFile.getEntry(name);
         if (entry == null) {
             return null;
@@ -81,6 +88,7 @@ public class JarFileUrlJar implements Ja
 
     @Override
     public long getLastModified(String name) throws IOException {
+        // JarFile#getEntry() is multi-release aware
         ZipEntry entry = jarFile.getEntry(name);
         if (entry == null) {
             return -1;
@@ -112,13 +120,58 @@ public class JarFileUrlJar implements Ja
 
     @Override
     public void nextEntry() {
+        // JarFile#entries() is NOT multi-release aware
         if (entries == null) {
             entries = jarFile.entries();
+            if (multiRelease) {
+                entryNamesSeen = new HashSet<>();
+            }
         }
-        if (entries.hasMoreElements()) {
-            entry = entries.nextElement();
+
+        if (multiRelease) {
+            // Need to ensure that:
+            // - the one, correct entry is returned where multiple versions
+            //   are available
+            // - that the order of entries in the JAR doesn't prevent the
+            //   correct entries being returned
+            // - the case where an entry appears in the versions location
+            //   but not in the the base location is handled correctly
+
+            // Enumerate the entries until one is reached that represents an
+            // entry that has not been seen before.
+            String name = null;
+            while (true) {
+                if (entries.hasMoreElements()) {
+                    entry = entries.nextElement();
+                    name = entry.getName();
+                    // Get 'base' name
+                    if (name.startsWith("META-INF/versions/")) {
+                        int i = name.indexOf('/', 18);
+                        if (i == -1) {
+                            continue;
+                        }
+                        name = name.substring(i + 1);
+                    }
+                    if (name.length() == 0 || entryNamesSeen.contains(name)) {
+                        continue;
+                    }
+
+                    entryNamesSeen.add(name);
+
+                    // JarFile.getJarEntry is version aware so use it
+                    entry = jarFile.getJarEntry(entry.getName());
+                    break;
+                } else {
+                    entry = null;
+                    break;
+                }
+            }
         } else {
-            entry = null;
+            if (entries.hasMoreElements()) {
+                entry = entries.nextElement();
+            } else {
+                entry = null;
+            }
         }
     }
 
@@ -149,6 +202,7 @@ public class JarFileUrlJar implements Ja
     @Override
     public void reset() throws IOException {
         entries = null;
+        entryNamesSeen = null;
         entry = null;
     }
 }

Modified: tomcat/trunk/test/org/apache/catalina/webresources/TestJarInputStreamWrapper.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/webresources/TestJarInputStreamWrapper.java?rev=1812103&r1=1812102&r2=1812103&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/webresources/TestJarInputStreamWrapper.java (original)
+++ tomcat/trunk/test/org/apache/catalina/webresources/TestJarInputStreamWrapper.java Fri Oct 13 10:10:33 2017
@@ -27,6 +27,7 @@ import org.junit.Assert;
 import org.junit.Test;
 
 import org.apache.catalina.WebResource;
+import org.apache.tomcat.util.compat.JreCompat;
 
 public class TestJarInputStreamWrapper {
 
@@ -118,7 +119,7 @@ public class TestJarInputStreamWrapper {
 
     private InputStream getUnwrappedClosedInputStream() throws IOException {
         File file = new File("test/webresources/non-static-resources.jar");
-        JarFile jarFile = new JarFile(file);
+        JarFile jarFile = JreCompat.getInstance().jarFileNewInstance(file);
         ZipEntry jarEntry = jarFile.getEntry("META-INF/MANIFEST.MF");
         InputStream unwrapped = jarFile.getInputStream(jarEntry);
         unwrapped.close();



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org