[tomcat] branch master updated: Fix JSP compilation showing old content reported on users@ list

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[tomcat] branch master updated: Fix JSP compilation showing old content reported on users@ list

markt
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/master by this push:
     new 03e7bc8  Fix JSP compilation showing old content reported on users@ list
03e7bc8 is described below

commit 03e7bc8487cb706adf1f56586948a7762dd42d14
Author: Mark Thomas <[hidden email]>
AuthorDate: Thu Nov 7 11:37:18 2019 +0000

    Fix JSP compilation showing old content reported on users@ list
---
 .../catalina/webresources/CachedResource.java      | 126 ++++++++++++++++++++-
 .../catalina/webresources/LocalStrings.properties  |   2 +
 webapps/docs/changelog.xml                         |   7 ++
 3 files changed, 134 insertions(+), 1 deletion(-)

diff --git a/java/org/apache/catalina/webresources/CachedResource.java b/java/org/apache/catalina/webresources/CachedResource.java
index 88d2453..72dba84 100644
--- a/java/org/apache/catalina/webresources/CachedResource.java
+++ b/java/org/apache/catalina/webresources/CachedResource.java
@@ -17,13 +17,21 @@
 package org.apache.catalina.webresources;
 
 import java.io.ByteArrayInputStream;
+import java.io.IOException;
 import java.io.InputStream;
+import java.net.MalformedURLException;
 import java.net.URL;
+import java.net.URLConnection;
+import java.net.URLStreamHandler;
+import java.security.Permission;
 import java.security.cert.Certificate;
 import java.util.jar.Manifest;
 
 import org.apache.catalina.WebResource;
 import org.apache.catalina.WebResourceRoot;
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.res.StringManager;
 
 /**
  * This class is designed to wrap a 'raw' WebResource and providing caching for
@@ -32,6 +40,9 @@ import org.apache.catalina.WebResourceRoot;
  */
 public class CachedResource implements WebResource {
 
+    private static final Log log = LogFactory.getLog(CachedResource.class);
+    private static final StringManager sm = StringManager.getManager(CachedResource.class);
+
     // Estimate (on high side to be safe) of average size excluding content
     // based on profiler data.
     private static final long CACHE_ENTRY_SIZE = 500;
@@ -314,7 +325,45 @@ public class CachedResource implements WebResource {
 
     @Override
     public URL getURL() {
-        return webResource.getURL();
+        /*
+         * We don't want applications using this URL to access the resource
+         * directly as that could lead to inconsistent results when the resource
+         * is updated on the file system but the cache entry has not yet
+         * expired. We saw this, for example, in JSP compilation.
+         * - last modified time was obtained via
+         *   ServletContext.getResource("path").openConnection().getLastModified()
+         * - JSP content was obtained via
+         *   ServletContext.getResourceAsStream("path")
+         * The result was that the JSP modification was detected but the JSP
+         * content was read from the cache so the non-updated JSP page was
+         * used to generate the .java and .class file
+         *
+         * One option to resolve this issue is to use a custom URL scheme for
+         * resource URLs. This would allow us, via registration of a
+         * URLStreamHandlerFactory, to control how the resources are accessed
+         * and ensure that all access go via the cache We took this approach for
+         * war: URLs so we can use jar:war:file: URLs to reference resources in
+         * unpacked WAR files. However, because URL.setURLStreamHandlerFactory()
+         * may only be caused once, this can cause problems when using other
+         * libraries that also want to use a custom URL scheme.
+         *
+         * The approach below allows us to insert a custom URLStreamHandler
+         * without registering a custom protocol. The only limitation (compared
+         * to registering a custom protocol) is that if the application
+         * constructs the same URL from a String, they will access the resource
+         * directly and not via the cache.
+         */
+        URL resourceURL = webResource.getURL();
+        if (resourceURL == null) {
+            return null;
+        }
+        try {
+            return new URL(null, resourceURL.toExternalForm(),
+                    new CachedResourceURLStreamHandler(resourceURL, root, webAppPath, usesClassLoaderResources));
+        } catch (MalformedURLException e) {
+            log.error(sm.getString("cachedResource.invalidURL", resourceURL.toExternalForm()), e);
+            return null;
+        }
     }
 
     @Override
@@ -355,4 +404,79 @@ public class CachedResource implements WebResource {
         }
         return result;
     }
+
+
+    private static class CachedResourceURLStreamHandler extends URLStreamHandler {
+
+        private final URL resourceURL;
+        private final StandardRoot root;
+        private final String webAppPath;
+        private final boolean usesClassLoaderResources;
+
+        public CachedResourceURLStreamHandler(URL resourceURL, StandardRoot root, String webAppPath,
+                boolean usesClassLoaderResources) {
+            this.resourceURL = resourceURL;
+            this.root = root;
+            this.webAppPath = webAppPath;
+            this.usesClassLoaderResources = usesClassLoaderResources;
+        }
+
+        @Override
+        protected URLConnection openConnection(URL u) throws IOException {
+            return new CachedResourceURLConnection(resourceURL, root, webAppPath, usesClassLoaderResources);
+        }
+    }
+
+
+    private static class CachedResourceURLConnection extends URLConnection {
+
+        private final StandardRoot root;
+        private final String webAppPath;
+        private final boolean usesClassLoaderResources;
+        private final URLConnection resourceURLConnection;
+        private boolean connected;
+
+        protected CachedResourceURLConnection(URL resourceURL, StandardRoot root, String webAppPath,
+                boolean usesClassLoaderResources) throws IOException {
+            super(resourceURL);
+            this.root = root;
+            this.webAppPath = webAppPath;
+            this.usesClassLoaderResources = usesClassLoaderResources;
+            this.resourceURLConnection = url.openConnection();
+        }
+
+        @Override
+        public void connect() throws IOException {
+            if (!connected) {
+                resourceURLConnection.connect();
+                connected = true;
+            }
+        }
+
+        @Override
+        public InputStream getInputStream() throws IOException {
+            connect();
+            InputStream is = getResource().getInputStream();
+            return is;
+        }
+
+        @Override
+        public Permission getPermission() throws IOException {
+            return resourceURLConnection.getPermission();
+        }
+
+        @Override
+        public long getLastModified() {
+            return getResource().getLastModified();
+        }
+
+        @Override
+        public long getContentLengthLong() {
+            return getResource().getContentLength();
+        }
+
+        private WebResource getResource() {
+            return root.getResourceInternal(webAppPath, usesClassLoaderResources);
+        }
+    }
 }
diff --git a/java/org/apache/catalina/webresources/LocalStrings.properties b/java/org/apache/catalina/webresources/LocalStrings.properties
index a69c178..fb9badc 100644
--- a/java/org/apache/catalina/webresources/LocalStrings.properties
+++ b/java/org/apache/catalina/webresources/LocalStrings.properties
@@ -25,6 +25,8 @@ cache.backgroundEvictFail=The background cache eviction process was unable to fr
 cache.objectMaxSizeTooBig=The value of [{0}]kB for objectMaxSize is larger than the limit of maxSize/20 so has been reduced to [{1}]kB
 cache.objectMaxSizeTooBigBytes=The value specified for the maximum object size to cache [{0}]kB is greater than Integer.MAX_VALUE bytes which is the maximum size that can be cached. The limit will be set to Integer.MAX_VALUE bytes.
 
+cachedResource.invalidURL=Unable to create an instance of CachedResourceURLStreamHandler because the URL [{0}] is malformed
+
 classpathUrlStreamHandler.notFound=Unable to load the resource [{0}] using the thread context class loader or the current class''s class loader
 
 dirResourceSet.manifestFail=Failed to read manifest from [{0}]
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 926f6ce..fcc8dd5 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -63,6 +63,13 @@
         <bug>63836</bug> Ensure that references to the Host object are cleared
         once the Host instance is destroyed. (markt)
       </fix>
+      <fix>
+        Ensure that, when static resource caching is enabled for a web
+        application, all access to static files (including JSP files) goes via
+        the cache so that a consistent view of the static files is seen. Prior
+        to this change it was possible to see an updated last modified time but
+        the content would be that prior to the modification. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [tomcat] branch master updated: Fix JSP compilation showing old content reported on users@ list

Rémy Maucherat
On Thu, Nov 7, 2019 at 12:38 PM <[hidden email]> wrote:
+         * One option to resolve this issue is to use a custom URL scheme for
+         * resource URLs. This would allow us, via registration of a
+         * URLStreamHandlerFactory, to control how the resources are accessed
+         * and ensure that all access go via the cache We took this approach for
+         * war: URLs so we can use jar:war:file: URLs to reference resources in
+         * unpacked WAR files. However, because URL.setURLStreamHandlerFactory()
+         * may only be caused once, this can cause problems when using other
+         * libraries that also want to use a custom URL scheme.


That's how it was done in Tomcat 7, and going down that route would mean bringing back everything including the classloader bindings. So let's try to avoid it.

Rémy

Reply | Threaded
Open this post in threaded view
|

Re: [tomcat] branch master updated: Fix JSP compilation showing old content reported on users@ list

markt
On 07/11/2019 11:50, Rémy Maucherat wrote:

> On Thu, Nov 7, 2019 at 12:38 PM <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     +         * One option to resolve this issue is to use a custom URL
>     scheme for
>     +         * resource URLs. This would allow us, via registration of a
>     +         * URLStreamHandlerFactory, to control how the resources
>     are accessed
>     +         * and ensure that all access go via the cache We took this
>     approach for
>     +         * war: URLs so we can use jar:war:file: URLs to reference
>     resources in
>     +         * unpacked WAR files. However, because
>     URL.setURLStreamHandlerFactory()
>     +         * may only be caused once, this can cause problems when
>     using other
>     +         * libraries that also want to use a custom URL scheme.
>
>
> That's how it was done in Tomcat 7, and going down that route would mean
> bringing back everything including the classloader bindings. So let's
> try to avoid it.

+1

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]