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 2016/11/28 11:14:30 UTC

svn commit: r1771711 - in /tomcat/trunk: java/org/apache/catalina/webresources/AbstractArchiveResource.java test/org/apache/catalina/webresources/TestJarInputStreamWrapper.java webapps/docs/changelog.xml

Author: markt
Date: Mon Nov 28 11:14:29 2016
New Revision: 1771711

URL: http://svn.apache.org/viewvc?rev=1771711&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=60410
Ensure that multiple calls to JarInputStreamWrapper#close() do not incorrectly trigger the closure of the underlying JAR or WAR file.

Added:
    tomcat/trunk/test/org/apache/catalina/webresources/TestJarInputStreamWrapper.java   (with props)
Modified:
    tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResource.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResource.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResource.java?rev=1771711&r1=1771710&r2=1771711&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResource.java (original)
+++ tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResource.java Mon Nov 28 11:14:29 2016
@@ -21,6 +21,7 @@ import java.io.InputStream;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.security.cert.Certificate;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.jar.JarEntry;
 import java.util.jar.Manifest;
 
@@ -233,7 +234,7 @@ public abstract class AbstractArchiveRes
 
     /**
      * This wrapper assumes that the InputStream was created from a JarFile
-     * obtained from a call to getArchiveResourceSet().getJarFile(). If this is
+     * obtained from a call to getArchiveResourceSet().openJarFile(). If this is
      * not the case then the usage counting in AbstractArchiveResourceSet will
      * break and the JarFile may be unexpectedly closed.
      */
@@ -241,6 +242,7 @@ public abstract class AbstractArchiveRes
 
         private final JarEntry jarEntry;
         private final InputStream is;
+        private final AtomicBoolean closed = new AtomicBoolean(false);
 
 
         public JarInputStreamWrapper(JarEntry jarEntry, InputStream is) {
@@ -281,7 +283,11 @@ public abstract class AbstractArchiveRes
 
         @Override
         public void close() throws IOException {
-            archiveResourceSet.closeJarFile();
+            if (closed.compareAndSet(false, true)) {
+                // Must only call this once else the usage counting will break
+                archiveResourceSet.closeJarFile();
+            }
+            is.close();
         }
 
 

Added: 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=1771711&view=auto
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/webresources/TestJarInputStreamWrapper.java (added)
+++ tomcat/trunk/test/org/apache/catalina/webresources/TestJarInputStreamWrapper.java Mon Nov 28 11:14:29 2016
@@ -0,0 +1,140 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.catalina.webresources;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.reflect.Method;
+import java.util.jar.JarFile;
+import java.util.zip.ZipEntry;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.catalina.WebResource;
+
+public class TestJarInputStreamWrapper {
+
+    @Test
+    public void testReadAfterClose() throws Exception {
+        Method m = InputStream.class.getMethod("read", (Class<?>[]) null);
+        testMethodAfterClose(m, (Object[]) null);
+    }
+
+
+    @Test
+    public void testSkipAfterClose() throws Exception {
+        Method m = InputStream.class.getMethod("skip", long.class);
+        testMethodAfterClose(m, Long.valueOf(1));
+    }
+
+
+    @Test
+    public void testAvailableAfterClose() throws Exception {
+        Method m = InputStream.class.getMethod("available", (Class<?>[]) null);
+        testMethodAfterClose(m, (Object[]) null);
+    }
+
+
+    @Test
+    public void testCloseAfterClose() throws Exception {
+        Method m = InputStream.class.getMethod("close", (Class<?>[]) null);
+        testMethodAfterClose(m, (Object[]) null);
+    }
+
+
+    @Test
+    public void testMarkAfterClose() throws Exception {
+        Method m = InputStream.class.getMethod("mark", int.class);
+        testMethodAfterClose(m, Integer.valueOf(1));
+    }
+
+
+    @Test
+    public void testResetAfterClose() throws Exception {
+        Method m = InputStream.class.getMethod("reset", (Class<?>[]) null);
+        testMethodAfterClose(m, (Object[]) null);
+    }
+
+
+    @Test
+    public void testMarkSupprotedAfterClose() throws Exception {
+        Method m = InputStream.class.getMethod("markSupported", (Class<?>[]) null);
+        testMethodAfterClose(m, (Object[]) null);
+    }
+
+
+    private void testMethodAfterClose(Method m, Object... params) throws IOException {
+        InputStream unwrapped = getUnwrappedClosedInputStream();
+        InputStream wrapped = getWrappedClosedInputStream();
+
+        Object unwrappedReturn = null;
+        Exception unwrappedException = null;
+        Object wrappedReturn = null;
+        Exception wrappedException = null;
+
+        try {
+            unwrappedReturn = m.invoke(unwrapped, params);
+        } catch (Exception e) {
+            unwrappedException = e;
+        }
+
+        try {
+            wrappedReturn = m.invoke(wrapped, params);
+        } catch (Exception e) {
+            wrappedException = e;
+        }
+
+        if (unwrappedReturn == null) {
+            Assert.assertNull(wrappedReturn);
+        } else {
+            Assert.assertNotNull(wrappedReturn);
+            Assert.assertEquals(unwrappedReturn, wrappedReturn);
+        }
+
+        if (unwrappedException == null) {
+            Assert.assertNull(wrappedException);
+        } else {
+            Assert.assertNotNull(wrappedException);
+            Assert.assertEquals(unwrappedException.getClass(), wrappedException.getClass());
+        }
+    }
+
+
+    private InputStream getUnwrappedClosedInputStream() throws IOException {
+        File file = new File("test/webresources/non-static-resources.jar");
+        JarFile jarFile = new JarFile(file);
+        ZipEntry jarEntry = jarFile.getEntry("META-INF/MANIFEST.MF");
+        InputStream unwrapped = jarFile.getInputStream(jarEntry);
+        unwrapped.close();
+        jarFile.close();
+        return unwrapped;
+    }
+
+
+    private InputStream getWrappedClosedInputStream() throws IOException {
+        StandardRoot root = new StandardRoot();
+        root.setCachingAllowed(false);
+        JarResourceSet jarResourceSet =
+                new JarResourceSet(root, "/", "test/webresources/non-static-resources.jar", "/");
+        WebResource webResource = jarResourceSet.getResource("/META-INF/MANIFEST.MF");
+        InputStream wrapped = webResource.getInputStream();
+        wrapped.close();
+        return wrapped;
+    }
+}

Propchange: tomcat/trunk/test/org/apache/catalina/webresources/TestJarInputStreamWrapper.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1771711&r1=1771710&r2=1771711&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Mon Nov 28 11:14:29 2016
@@ -132,6 +132,11 @@
         Refactor the MBean implementations for the internal Tomcat components
         to reduce code duplication. (markt)
       </scode>
+      <fix>
+        <bug>60410</bug>: Ensure that multiple calls to
+        <code>JarInputStreamWrapper#close()</code> do not incorrectly trigger
+        the closure of the underlying JAR or WAR file. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">



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