From ab174d101526029d336b4f9cdd436c228dc96692 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Mon, 29 Apr 2019 12:05:34 +0200 Subject: [PATCH] Issue #3597 Fix session persistence classloading for deep structures (#3602) * Issue #3597 Fix session persistence classloading for deep structures with system classes. --- .../architecture/jetty-classloading.adoc | 4 ++ .../jetty/server/session/SessionData.java | 57 ++++++++++++++----- .../jetty/util/ClassVisibilityChecker.java | 51 +++++++++++++++++ .../jetty/webapp/WebAppClassLoader.java | 37 +++++------- .../session/AbstractSessionDataStoreTest.java | 16 ++++++ 5 files changed, 129 insertions(+), 36 deletions(-) create mode 100644 jetty-util/src/main/java/org/eclipse/jetty/util/ClassVisibilityChecker.java diff --git a/jetty-documentation/src/main/asciidoc/reference/architecture/jetty-classloading.adoc b/jetty-documentation/src/main/asciidoc/reference/architecture/jetty-classloading.adoc index e5f07735802..316f73487a7 100644 --- a/jetty-documentation/src/main/asciidoc/reference/architecture/jetty-classloading.adoc +++ b/jetty-documentation/src/main/asciidoc/reference/architecture/jetty-classloading.adoc @@ -162,6 +162,10 @@ You can do so directly to the API via a context XML file such as the following: If none of the alternatives already described meet your needs, you can always provide a custom classloader for your webapp. We recommend, but do not require, that your custom loader subclasses link:{JDURL}/org/eclipse/jetty/webapp/WebAppClassLoader.html[WebAppClassLoader]. + +If you do not subclass WebAppClassLoader, we recommend that you implement the link:{JDURL}/org/eclipse/jetty/util/ClassVisibilityChecker.html[ClassVisibilityChecker] interface. +Without this interface, session persistence will be slower. + You configure the classloader for the webapp like so: [source, java, subs="{sub-order}"] diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java index 6d1e7b8018f..bb8d15d8d6b 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java @@ -28,6 +28,7 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import org.eclipse.jetty.util.ClassLoadingObjectInputStream; +import org.eclipse.jetty.util.ClassVisibilityChecker; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -78,22 +79,47 @@ public class SessionData implements Serializable out.writeObject(entries); for (Entry entry: data._attributes.entrySet()) { - out.writeUTF(entry.getKey()); - ClassLoader loader = entry.getValue().getClass().getClassLoader(); - boolean isServerLoader = false; - - if (loader == Thread.currentThread().getContextClassLoader()) //is it the webapp classloader? - isServerLoader = false; - else if (loader == Thread.currentThread().getContextClassLoader().getParent() || loader == SessionData.class.getClassLoader() || loader == null) // is it the container loader? - isServerLoader = true; - else - throw new IOException ("Unknown loader"); // we don't know what loader to use + out.writeUTF(entry.getKey()); - out.writeBoolean(isServerLoader); + Class clazz = entry.getValue().getClass(); + ClassLoader loader = clazz.getClassLoader(); + ClassLoader contextLoader = Thread.currentThread().getContextClassLoader(); + boolean isContextLoader; + + if (loader == contextLoader) //is it the context classloader? + isContextLoader = true; + else if (contextLoader == null) //not context classloader + isContextLoader = false; + else if (contextLoader instanceof ClassVisibilityChecker) + { + //Clazz not loaded by context classloader, but ask if loadable by context classloader, + //because preferable to use context classloader if possible (eg for deep structures). + ClassVisibilityChecker checker = (ClassVisibilityChecker)(contextLoader); + isContextLoader = (checker.isSystemClass(clazz) && !(checker.isServerClass(clazz))); + } + else + { + //Class wasn't loaded by context classloader, but try loading from context loader, + //because preferable to use context classloader if possible (eg for deep structures). + try + { + Class result = contextLoader.loadClass(clazz.getName()); + isContextLoader = (result == clazz); //only if TTCL loaded this instance of the class + } + catch (Throwable e) + { + isContextLoader = false; //TCCL can't see the class + } + } + + if (LOG.isDebugEnabled()) + LOG.debug("Attribute {} class={} isServerLoader={}", entry.getKey(),clazz.getName(),(!isContextLoader)); + out.writeBoolean(!isContextLoader); out.writeObject(entry.getValue()); } } - + + /** * De-serialize the attribute map of a session. * @@ -118,12 +144,15 @@ public class SessionData implements Serializable data._attributes = new ConcurrentHashMap<>(); int entries = ((Integer)o).intValue(); + ClassLoader contextLoader = Thread.currentThread().getContextClassLoader(); + ClassLoader serverLoader = SessionData.class.getClassLoader(); for (int i=0; i < entries; i++) { String name = in.readUTF(); //attribute name boolean isServerClassLoader = in.readBoolean(); //use server or webapp classloader to load - - Object value = ((ClassLoadingObjectInputStream)in).readObject(isServerClassLoader?SessionData.class.getClassLoader():Thread.currentThread().getContextClassLoader()); + if (LOG.isDebugEnabled()) + LOG.debug("Deserialize {} isServerLoader={} serverLoader={} tccl={}", name, isServerClassLoader,serverLoader, contextLoader); + Object value = ((ClassLoadingObjectInputStream)in).readObject(isServerClassLoader?serverLoader:contextLoader); data._attributes.put(name, value); } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ClassVisibilityChecker.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ClassVisibilityChecker.java new file mode 100644 index 00000000000..25da1cea73e --- /dev/null +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ClassVisibilityChecker.java @@ -0,0 +1,51 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + + +package org.eclipse.jetty.util; + +/** + * ClassVisibilityChecker + * + * Interface to be implemented by classes capable of checking class visibility + * for a context. + * + */ +public interface ClassVisibilityChecker +{ + /* ------------------------------------------------------------ */ + /** Is the class a System Class. + * A System class is a class that is visible to a webapplication, + * but that cannot be overridden by the contents of WEB-INF/lib or + * WEB-INF/classes + * @param clazz The fully qualified name of the class. + * @return True if the class is a system class. + */ + boolean isSystemClass(Class clazz); + + /* ------------------------------------------------------------ */ + /** Is the class a Server Class. + * A Server class is a class that is part of the implementation of + * the server and is NIT visible to a webapplication. The web + * application may provide it's own implementation of the class, + * to be loaded from WEB-INF/lib or WEB-INF/classes + * @param clazz The fully qualified name of the class. + * @return True if the class is a server class. + */ + boolean isServerClass(Class clazz); +} diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java index f7d165c4367..967b9e5feaa 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java @@ -39,6 +39,7 @@ import java.util.Set; import java.util.StringTokenizer; import java.util.concurrent.CopyOnWriteArrayList; +import org.eclipse.jetty.util.ClassVisibilityChecker; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.log.Log; @@ -65,7 +66,7 @@ import org.eclipse.jetty.util.resource.ResourceCollection; * context classloader will be used. If that is null then the * classloader that loaded this class is used as the parent. */ -public class WebAppClassLoader extends URLClassLoader +public class WebAppClassLoader extends URLClassLoader implements ClassVisibilityChecker { static { @@ -85,7 +86,7 @@ public class WebAppClassLoader extends URLClassLoader /* ------------------------------------------------------------ */ /** The Context in which the classloader operates. */ - public interface Context + public interface Context extends ClassVisibilityChecker { /* ------------------------------------------------------------ */ /** Convert a URL or path to a Resource. @@ -103,26 +104,6 @@ public class WebAppClassLoader extends URLClassLoader */ PermissionCollection getPermissions(); - /* ------------------------------------------------------------ */ - /** Is the class a System Class. - * A System class is a class that is visible to a webapplication, - * but that cannot be overridden by the contents of WEB-INF/lib or - * WEB-INF/classes - * @param clazz The fully qualified name of the class. - * @return True if the class is a system class. - */ - boolean isSystemClass(Class clazz); - - /* ------------------------------------------------------------ */ - /** Is the class a Server Class. - * A Server class is a class that is part of the implementation of - * the server and is NIT visible to a webapplication. The web - * application may provide it's own implementation of the class, - * to be loaded from WEB-INF/lib or WEB-INF/classes - * @param clazz The fully qualified name of the class. - * @return True if the class is a server class. - */ - boolean isServerClass(Class clazz); /* ------------------------------------------------------------ */ /** @@ -744,5 +725,17 @@ public class WebAppClassLoader extends URLClassLoader { return "WebAppClassLoader=" + _name+"@"+Long.toHexString(hashCode()); } + + @Override + public boolean isSystemClass(Class clazz) + { + return _context.isSystemClass(clazz); + } + + @Override + public boolean isServerClass(Class clazz) + { + return _context.isServerClass(clazz); + } } diff --git a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStoreTest.java b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStoreTest.java index ce6d4c0f1c7..166f8952d04 100644 --- a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStoreTest.java +++ b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStoreTest.java @@ -31,10 +31,12 @@ import static org.junit.jupiter.api.Assertions.fail; import java.io.File; import java.io.FileOutputStream; import java.io.InputStream; +import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.lang.reflect.Proxy; import java.net.URL; import java.net.URLClassLoader; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; import java.util.Set; @@ -238,6 +240,11 @@ public abstract class AbstractSessionDataStoreTest File factoryClass = new File (proxyabledir, "ProxyableFactory.class"); IO.copy(is, new FileOutputStream(factoryClass)); is.close(); + + is = Thread.currentThread().getContextClassLoader().getResourceAsStream("Foo.clazz"); + File fooClass = new File (proxyabledir, "Foo.class"); + IO.copy(is, new FileOutputStream(fooClass)); + is.close(); URL[] proxyabledirUrls = new URL[]{proxyabledir.toURI().toURL()}; _contextClassLoader = new URLClassLoader(proxyabledirUrls, Thread.currentThread().getContextClassLoader()); @@ -272,6 +279,15 @@ public abstract class AbstractSessionDataStoreTest //Make an attribute that uses the proxy only known to the webapp classloader data.setAttribute("a", proxy); + + //Now make an attribute that uses a system class to store a webapp classes + //see issue #3597 + Class fooclazz = Class.forName("Foo", true, _contextClassLoader); + Constructor constructor = fooclazz.getConstructor(null); + Object foo = constructor.newInstance(null); + ArrayList list = new ArrayList(); + list.add(foo); + data.setAttribute("foo", list); } finally {