diff --git a/java/org/apache/catalina/core/StandardContext.java b/java/org/apache/catalina/core/StandardContext.java index 8d0471421f21..cc41db1a0dd4 100644 --- a/java/org/apache/catalina/core/StandardContext.java +++ b/java/org/apache/catalina/core/StandardContext.java @@ -4748,6 +4748,7 @@ private void mergeParameters() { for (Map.Entry entry : mergedParams.entrySet()) { sc.setInitParameter(entry.getKey(), entry.getValue()); } + } diff --git a/java/org/apache/catalina/mapper/Mapper.java b/java/org/apache/catalina/mapper/Mapper.java index bfbc6e88b7ce..caac5e5df2b4 100644 --- a/java/org/apache/catalina/mapper/Mapper.java +++ b/java/org/apache/catalina/mapper/Mapper.java @@ -20,8 +20,10 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; @@ -457,10 +459,125 @@ public void addWrappers(String hostName, String contextPath, String version, * @param wrappers Information on wrapper mappings */ private void addWrappers(ContextVersion contextVersion, Collection wrappers) { + // Group the incoming wrappers by category so that each of the copy-on-write wrapper arrays is rebuilt and + // assigned once rather than once per wrapper. Inserting one at a time reallocates the whole array and is + // O(n^2) in array copies for a context with many mappings of the same category. + List newExactWrappers = new ArrayList<>(); + List newWildcardWrappers = new ArrayList<>(); + List newExtensionWrappers = new ArrayList<>(); + MappedWrapper newDefaultWrapper = null; + for (WrapperMappingInfo wrapper : wrappers) { - addWrapper(contextVersion, wrapper.mapping(), wrapper.wrapper(), wrapper.jspWildCard(), - wrapper.resourceOnly()); + String path = wrapper.mapping(); + if (path.endsWith("/*")) { + // Wildcard wrapper + String name = path.substring(0, path.length() - 2); + newWildcardWrappers + .add(new MappedWrapper(name, wrapper.wrapper(), wrapper.jspWildCard(), wrapper.resourceOnly())); + } else if (path.startsWith("*.")) { + // Extension wrapper + String name = path.substring(2); + newExtensionWrappers + .add(new MappedWrapper(name, wrapper.wrapper(), wrapper.jspWildCard(), wrapper.resourceOnly())); + } else if (path.equals("/")) { + // Default wrapper - the last one processed wins, matching sequential insertion + newDefaultWrapper = new MappedWrapper("", wrapper.wrapper(), wrapper.jspWildCard(), + wrapper.resourceOnly()); + } else { + // Exact wrapper + final String name; + if (path.isEmpty()) { + // Special case for the Context Root mapping which is treated as an exact match + name = "/"; + } else { + name = path; + } + newExactWrappers + .add(new MappedWrapper(name, wrapper.wrapper(), wrapper.jspWildCard(), wrapper.resourceOnly())); + } + } + + synchronized (contextVersion) { + if (!newExactWrappers.isEmpty()) { + contextVersion.exactWrappers = mergeWrappers(contextVersion.exactWrappers, newExactWrappers, false, + contextVersion); + } + if (!newExtensionWrappers.isEmpty()) { + contextVersion.extensionWrappers = mergeWrappers(contextVersion.extensionWrappers, + newExtensionWrappers, false, contextVersion); + } + if (!newWildcardWrappers.isEmpty()) { + contextVersion.wildcardWrappers = mergeWrappers(contextVersion.wildcardWrappers, newWildcardWrappers, + true, contextVersion); + } + if (newDefaultWrapper != null) { + contextVersion.defaultWrapper = newDefaultWrapper; + } + } + } + + /** + * Merge a batch of new wrappers into an existing sorted, duplicate-free copy-on-write wrapper array, reproducing + * the behaviour of repeated {@link #insertMap(MapElement[], MapElement[], MapElement)} calls: the result stays + * sorted by name, an entry already present (in the existing array or earlier in the batch) is kept in preference + * to a later duplicate and, for wildcard wrappers, {@link ContextVersion#nesting} is raised to the largest slash + * count amongst the entries actually added. + * + * @param oldWrappers the existing, sorted, duplicate-free array + * @param newWrappers the batch of candidate wrappers, in insertion order + * @param updateNesting whether to update the context nesting (true only for wildcard wrappers) + * @param contextVersion the context whose nesting may be updated + * + * @return the merged array, or {@code oldWrappers} unchanged if every candidate was a duplicate + */ + private static MappedWrapper[] mergeWrappers(MappedWrapper[] oldWrappers, List newWrappers, + boolean updateNesting, ContextVersion contextVersion) { + // Existing names win over new ones; the first occurrence of a name in the batch wins over later duplicates. + Set names = new HashSet<>(); + for (MappedWrapper oldWrapper : oldWrappers) { + names.add(oldWrapper.name); + } + List added = new ArrayList<>(newWrappers.size()); + for (MappedWrapper newWrapper : newWrappers) { + if (names.add(newWrapper.name)) { + added.add(newWrapper); + } + } + if (added.isEmpty()) { + // Every candidate was a duplicate so, as with insertMap() returning false each time, nothing changes. + return oldWrappers; + } + + if (updateNesting) { + for (MappedWrapper addedWrapper : added) { + int slashCount = slashCount(addedWrapper.name); + if (slashCount > contextVersion.nesting) { + contextVersion.nesting = slashCount; + } + } + } + + // Both inputs are sorted by name using String natural ordering (the ordering used by find()) and, having had + // duplicates removed above, share no names. Merge them into a single sorted array. + added.sort((wrapper1, wrapper2) -> wrapper1.name.compareTo(wrapper2.name)); + MappedWrapper[] mergedWrappers = new MappedWrapper[oldWrappers.length + added.size()]; + int oldPos = 0; + int addedPos = 0; + int mergedPos = 0; + while (oldPos < oldWrappers.length && addedPos < added.size()) { + if (oldWrappers[oldPos].name.compareTo(added.get(addedPos).name) < 0) { + mergedWrappers[mergedPos++] = oldWrappers[oldPos++]; + } else { + mergedWrappers[mergedPos++] = added.get(addedPos++); + } + } + while (oldPos < oldWrappers.length) { + mergedWrappers[mergedPos++] = oldWrappers[oldPos++]; + } + while (addedPos < added.size()) { + mergedWrappers[mergedPos++] = added.get(addedPos++); } + return mergedWrappers; } /** diff --git a/java/org/apache/catalina/startup/ContextConfig.java b/java/org/apache/catalina/startup/ContextConfig.java index 6d20b1d672b6..d6e139421fa7 100644 --- a/java/org/apache/catalina/startup/ContextConfig.java +++ b/java/org/apache/catalina/startup/ContextConfig.java @@ -231,6 +231,20 @@ public ContextConfig() { */ protected final Map,Set> typeInitializerMap = new HashMap<>(); + /** + * Index of {@link #typeInitializerMap} entries that represent annotations, keyed by class name. Used to avoid a + * linear scan of {@link #typeInitializerMap} for every annotation of every scanned class. The values are the same + * {@link Set} instances held by {@link #typeInitializerMap}. + */ + private final Map> typeInitializerAnnotationsMap = new HashMap<>(); + + /** + * Index of {@link #typeInitializerMap} entries that represent non-annotations (classes and interfaces), keyed by + * class name. Used to avoid a linear scan of {@link #typeInitializerMap} for every super class and interface of + * every scanned class. The values are the same {@link Set} instances held by {@link #typeInitializerMap}. + */ + private final Map> typeInitializerClassMap = new HashMap<>(); + /** * Flag that indicates if at least one {@link HandlesTypes} entry is present that represents an annotation. */ @@ -241,6 +255,15 @@ public ContextConfig() { */ protected boolean handlesTypesNonAnnotations = false; + /** + * Records, keyed by JAR URL (external form), whether a fragment JAR that was scanned for annotations also contains + * a META-INF/resources/ entry. Populated during the annotation scan so that + * {@link #processResourceJARs(Set)} does not need to re-open and re-scan JARs that have already been fully + * examined. Reset at the start of each {@link #webConfig()}. A concurrent map is used because annotation scanning + * may run in parallel; each JAR writes its own distinct key. + */ + private final Map resourceJarScanCache = new ConcurrentHashMap<>(); + // ------------------------------------------------------------- Properties @@ -1177,6 +1200,8 @@ protected synchronized void configureStop() { // Reset ServletContextInitializer scanning initializerClassMap.clear(); typeInitializerMap.clear(); + typeInitializerAnnotationsMap.clear(); + typeInitializerClassMap.clear(); ok = true; @@ -1314,6 +1339,9 @@ protected void webConfig() { WebXmlParser webXmlParser = new WebXmlParser(context.getXmlNamespaceAware(), context.getXmlValidation(), context.getXmlBlockExternal()); + // Reset any state left over from a previous run on this instance. + resourceJarScanCache.clear(); + Set defaults = new HashSet<>(); defaults.add(getDefaultWebXmlFragment(webXmlParser)); @@ -1427,6 +1455,11 @@ protected void processClasses(WebXml webXml, Set orderedFragments) { // Step 4. Process /WEB-INF/classes for annotations and // @HandlesTypes matches + // Build the name-keyed indexes of typeInitializerMap now that it is + // fully populated and before scanning (which may run in parallel via + // processAnnotations) starts. + buildTypeInitializerIndexes(); + Map javaClassCache; if (context.getParallelAnnotationScanning()) { @@ -1691,30 +1724,8 @@ private WebXml getDefaultWebXmlFragment(WebXmlParser webXmlParser) { InputSource globalWebXml = getGlobalWebXmlSource(); InputSource hostWebXml = getHostWebXmlSource(); - long globalTimeStamp = 0; - long hostTimeStamp = 0; - - if (globalWebXml != null) { - try { - URI uri = new URI(globalWebXml.getSystemId()); - try (CloseableURLConnection uc = new CloseableURLConnection(uri.toURL())) { - globalTimeStamp = uc.getLastModified(); - } - } catch (IOException | URISyntaxException | IllegalArgumentException e) { - globalTimeStamp = -1; - } - } - - if (hostWebXml != null) { - try { - URI uri = new URI(hostWebXml.getSystemId()); - try (CloseableURLConnection uc = new CloseableURLConnection(uri.toURL())) { - hostTimeStamp = uc.getLastModified(); - } - } catch (IOException | URISyntaxException | IllegalArgumentException e) { - hostTimeStamp = -1; - } - } + long globalTimeStamp = getWebXmlTimeStamp(globalWebXml); + long hostTimeStamp = getWebXmlTimeStamp(hostWebXml); if (entry != null && entry.getGlobalTimeStamp() == globalTimeStamp && entry.getHostTimeStamp() == hostTimeStamp) { @@ -1777,6 +1788,35 @@ private WebXml getDefaultWebXmlFragment(WebXmlParser webXmlParser) { } + /** + * Determine the last modified time of a default web.xml {@link InputSource} for the {@link #hostWebXmlCache} + * freshness check. A {@code null} source is reported as {@code 0} (no source). For {@code file:} URIs this uses + * {@link File#lastModified()} rather than opening a {@link java.net.URLConnection} since, for the file protocol, + * reading the last modified time via a connection opens the file as a side effect. Any error is reported as + * {@code -1} so that the result is not cached. + * + * @param source the web.xml source, or {@code null} if none + * + * @return the last modified time in milliseconds, {@code 0} if there is no source, or {@code -1} on error + */ + private long getWebXmlTimeStamp(InputSource source) { + if (source == null) { + return 0; + } + try { + URI uri = new URI(source.getSystemId()); + if ("file".equals(uri.getScheme())) { + return new File(uri).lastModified(); + } + try (CloseableURLConnection uc = new CloseableURLConnection(uri.toURL())) { + return uc.getLastModified(); + } + } catch (IOException | URISyntaxException | IllegalArgumentException e) { + return -1; + } + } + + private void convertJsps(WebXml webXml) { Map jspInitParams; ServletDef jspServlet = webXml.getServlets().get("jsp"); @@ -1889,17 +1929,27 @@ protected void processResourceJARs(Set fragments) { URL url = fragment.getURL(); try { if ("jar".equals(url.getProtocol()) || url.toString().endsWith(".jar")) { - try (Jar jar = JarFactory.newInstance(url)) { - jar.nextEntry(); - String entryName = jar.getEntryName(); - while (entryName != null) { - if (entryName.startsWith("META-INF/resources/")) { - context.getResources().createWebResourceSet( - WebResourceRoot.ResourceSetType.RESOURCE_JAR, "/", url, "/META-INF/resources"); - break; - } + Boolean scannedHasResources = resourceJarScanCache.get(url.toExternalForm()); + if (scannedHasResources != null) { + // This JAR was fully scanned during annotation processing so there is no need to re-open it. + if (scannedHasResources.booleanValue()) { + context.getResources().createWebResourceSet( + WebResourceRoot.ResourceSetType.RESOURCE_JAR, "/", url, "/META-INF/resources"); + } + } else { + try (Jar jar = JarFactory.newInstance(url)) { jar.nextEntry(); - entryName = jar.getEntryName(); + String entryName = jar.getEntryName(); + while (entryName != null) { + if (entryName.startsWith("META-INF/resources/")) { + context.getResources().createWebResourceSet( + WebResourceRoot.ResourceSetType.RESOURCE_JAR, "/", url, + "/META-INF/resources"); + break; + } + jar.nextEntry(); + entryName = jar.getEntryName(); + } } } } else if ("file".equals(url.getProtocol())) { @@ -2157,10 +2207,9 @@ private void scanWebXmlFragment(boolean handlesTypesOnly, WebXml fragment, annotations.setDistributable(true); URL url = fragment.getURL(); processAnnotationsUrl(url, annotations, htOnly, javaClassCache); - Set set = new HashSet<>(); - set.add(annotations); - // Merge annotations into fragment - fragment takes priority - fragment.merge(set); + // Merge annotations into fragment - fragment takes priority. merge() only iterates the set so an immutable + // single-element set avoids allocating a HashMap-backed HashSet per fragment. + fragment.merge(Collections.singleton(annotations)); } /** @@ -2293,6 +2342,10 @@ protected void processAnnotationsJar(URL url, WebXml fragment, boolean handlesTy log.trace(sm.getString("contextConfig.processAnnotationsJar.debug", url)); } + // While iterating every entry, also note whether the JAR contains static resources so that + // processResourceJARs() can avoid re-opening and re-scanning this JAR. + boolean hasResources = false; + jar.nextEntry(); String entryName = jar.getEntryName(); while (entryName != null) { @@ -2303,9 +2356,15 @@ protected void processAnnotationsJar(URL url, WebXml fragment, boolean handlesTy log.error(sm.getString("contextConfig.inputStreamJar", entryName, url), e); } } + if (!hasResources && entryName.startsWith("META-INF/resources/")) { + hasResources = true; + } jar.nextEntry(); entryName = jar.getEntryName(); } + + // Only cache the result once the JAR has been fully and successfully scanned. + resourceJarScanCache.put(url.toExternalForm(), Boolean.valueOf(hasResources)); } catch (IOException ioe) { log.error(sm.getString("contextConfig.jarFile", url), ioe); } @@ -2449,25 +2508,22 @@ protected void checkHandlesTypes(JavaClass javaClass, Map,Set> entry : typeInitializerMap.entrySet()) { - if (entry.getKey().isAnnotation()) { - String entryClassName = entry.getKey().getName(); - for (AnnotationEntry annotationEntry : annotationEntries) { - if (entryClassName.equals(getClassName(annotationEntry.getAnnotationType()))) { - if (clazz == null) { - clazz = Introspection.loadClass(context, className); - if (clazz == null) { - // Can't load the class so no point - // continuing - return; - } - } - for (ServletContainerInitializer sci : entry.getValue()) { - initializerClassMap.get(sci).add(clazz); - } - break; + for (AnnotationEntry annotationEntry : annotationEntries) { + // getAnnotationType() returns the internal descriptor form which the index is keyed on, so no + // per-entry conversion is required. + Set scis = + typeInitializerAnnotationsMap.get(annotationEntry.getAnnotationType()); + if (scis != null) { + if (clazz == null) { + clazz = Introspection.loadClass(context, className); + if (clazz == null) { + // Can't load the class so no point continuing + return; } } + for (ServletContainerInitializer sci : scis) { + initializerClassMap.get(sci).add(clazz); + } } } } @@ -2572,24 +2628,35 @@ private void populateSCIsForCacheEntry(JavaClassCacheEntry cacheEntry, } private Set getSCIsForClass(String className) { - for (Map.Entry,Set> entry : typeInitializerMap.entrySet()) { - Class clazz = entry.getKey(); - if (!clazz.isAnnotation()) { - if (clazz.getName().equals(className)) { - return entry.getValue(); - } - } - } - return EMPTY_SCI_SET; + Set scis = typeInitializerClassMap.get(className); + return scis == null ? EMPTY_SCI_SET : scis; } - private static String getClassName(String internalForm) { - if (!internalForm.startsWith("L")) { - return internalForm; + /** + * (Re)builds the name-keyed indexes of {@link #typeInitializerMap} used by {@link #checkHandlesTypes(JavaClass, + * Map)}. This must be called after {@link #typeInitializerMap} has been fully populated and before scanning starts + * (scanning may run concurrently) so the per-class {@link HandlesTypes} matching can look up interested + * {@link ServletContainerInitializer}s by class name rather than performing a linear scan of the whole map. The + * indexes share the {@link Set} instances held by {@link #typeInitializerMap}. + */ + void buildTypeInitializerIndexes() { + typeInitializerAnnotationsMap.clear(); + typeInitializerClassMap.clear(); + for (Map.Entry,Set> entry : typeInitializerMap.entrySet()) { + Class type = entry.getKey(); + if (type.isAnnotation()) { + Set scis = entry.getValue(); + // Index annotation types by both the internal descriptor form (e.g. "Lpkg/Ann;") and the binary + // class name. Class files reference annotations using the descriptor form, so indexing it lets the + // per-class scan look the type up directly without converting each entry. The binary name is also + // indexed so matching remains identical to the previous descriptor-to-binary conversion for any + // non-descriptor input. + typeInitializerAnnotationsMap.put('L' + type.getName().replace('.', '/') + ';', scis); + typeInitializerAnnotationsMap.put(type.getName(), scis); + } else { + typeInitializerClassMap.put(type.getName(), entry.getValue()); + } } - - // Assume starts with L, ends with ; and uses / rather than . - return internalForm.substring(1, internalForm.length() - 1).replace('/', '.'); } /** diff --git a/java/org/apache/catalina/util/LifecycleBase.java b/java/org/apache/catalina/util/LifecycleBase.java index 5905f7c403d2..b6fb4651e1c5 100644 --- a/java/org/apache/catalina/util/LifecycleBase.java +++ b/java/org/apache/catalina/util/LifecycleBase.java @@ -110,6 +110,11 @@ public void removeLifecycleListener(LifecycleListener listener) { * @param data Data associated with event. */ protected void fireLifecycleEvent(String type, Object data) { + // Many components (e.g. every Wrapper) have no lifecycle listeners but still fire several events per state + // transition, so avoid allocating an event object that no one would receive. + if (lifecycleListeners.isEmpty()) { + return; + } LifecycleEvent event = new LifecycleEvent(this, type, data); for (LifecycleListener listener : lifecycleListeners) { listener.lifecycleEvent(event); diff --git a/java/org/apache/catalina/webresources/AbstractFileResourceSet.java b/java/org/apache/catalina/webresources/AbstractFileResourceSet.java index d1da47776c16..f0ba9c811cb8 100644 --- a/java/org/apache/catalina/webresources/AbstractFileResourceSet.java +++ b/java/org/apache/catalina/webresources/AbstractFileResourceSet.java @@ -90,6 +90,18 @@ public boolean getAllowLinking() { return allowLinking.booleanValue(); } + /** + * A file that has passed this resource set's validation, along with the canonical path that the + * validation resolved. Callers that need the canonical path can reuse it instead of resolving the + * same file again. + * + * @param file the validated file + * @param canonicalPath the canonical path of the file, or {@code null} if it was not resolved + * because linking is allowed + */ + protected record ValidatedFile(File file, String canonicalPath) { + } + /** * Return a File for the specified resource name. * @@ -98,6 +110,19 @@ public boolean getAllowLinking() { * @return the file for the specified resource */ protected final File file(String name, boolean mustExist) { + ValidatedFile validated = validatedFile(name, mustExist); + return validated == null ? null : validated.file(); + } + + /** + * Return a File for the specified resource name, along with the canonical path resolved while + * validating it. + * + * @param name Name of the resource + * @param mustExist Whether the file must exist + * @return the validated file for the specified resource, or {@code null} if there is none + */ + protected final ValidatedFile validatedFile(String name, boolean mustExist) { if (name.equals("/")) { name = ""; @@ -120,7 +145,7 @@ protected final File file(String name, boolean mustExist) { // If allow linking is enabled, files are not limited to being located // under the fileBase so all further checks are disabled. if (getAllowLinking()) { - return file; + return new ValidatedFile(file, null); } // Additional Windows specific checks to handle known problems with @@ -139,6 +164,8 @@ protected final File file(String name, boolean mustExist) { if (canPath == null || !canPath.startsWith(canonicalBase)) { return null; } + // canPath is trimmed to a resource set relative path by the checks below + String fullCanonicalPath = canPath; /* * Ensure that the file is not outside the fileBase. This should not be possible for standard requests (the @@ -186,7 +213,7 @@ protected final File file(String name, boolean mustExist) { return null; } - return file; + return new ValidatedFile(file, fullCanonicalPath); } diff --git a/java/org/apache/catalina/webresources/AbstractSingleArchiveResourceSet.java b/java/org/apache/catalina/webresources/AbstractSingleArchiveResourceSet.java index 2040740b6386..16501401797a 100644 --- a/java/org/apache/catalina/webresources/AbstractSingleArchiveResourceSet.java +++ b/java/org/apache/catalina/webresources/AbstractSingleArchiveResourceSet.java @@ -76,9 +76,10 @@ protected Map getArchiveEntries(boolean single) { synchronized (archiveLock) { if (archiveEntries == null && !single) { JarFile jarFile = null; - archiveEntries = new HashMap<>(); try { jarFile = openJarFile(); + // Pre-size the map from the known entry count to avoid resize/rehash cycles for large JARs. + archiveEntries = new HashMap<>((int) (jarFile.size() / 0.75f) + 1); Enumeration entries = jarFile.entries(); while (entries.hasMoreElements()) { JarEntry entry = entries.nextElement(); diff --git a/java/org/apache/catalina/webresources/DirResourceSet.java b/java/org/apache/catalina/webresources/DirResourceSet.java index 92ebe8249882..a020d5604075 100644 --- a/java/org/apache/catalina/webresources/DirResourceSet.java +++ b/java/org/apache/catalina/webresources/DirResourceSet.java @@ -113,17 +113,19 @@ public WebResource getResource(String path) { readLock.lock(); } try { - File f = file(path.substring(webAppMount.length()), false); - if (f == null) { + ValidatedFile validated = validatedFile(path.substring(webAppMount.length()), false); + if (validated == null) { return new EmptyResource(root, path); } + File f = validated.file(); if (!f.exists()) { return new EmptyResource(root, path, f); } if (f.isDirectory() && path.charAt(path.length() - 1) != '/') { path = path + '/'; } - return new FileResource(root, path, f, readOnly, getManifest(), this, readOnly ? null : path); + return new FileResource(root, path, f, readOnly, getManifest(), this, readOnly ? null : path, + validated.canonicalPath()); } finally { if (readLock != null) { readLock.unlock(); diff --git a/java/org/apache/catalina/webresources/FileResource.java b/java/org/apache/catalina/webresources/FileResource.java index b1ffb69b816d..613997984cd6 100644 --- a/java/org/apache/catalina/webresources/FileResource.java +++ b/java/org/apache/catalina/webresources/FileResource.java @@ -64,6 +64,7 @@ public class FileResource extends AbstractResource { private final boolean needConvert; private final WebResourceLockSet lockSet; private final String lockPath; + private final String canonicalPath; /** * Creates a FileResource without locking support. @@ -78,6 +79,22 @@ public FileResource(WebResourceRoot root, String webAppPath, File resource, bool this(root, webAppPath, resource, readOnly, manifest, null, null); } + /** + * Creates a FileResource with optional locking support. + * + * @param root The web resource root + * @param webAppPath The web application path + * @param resource The underlying file + * @param readOnly Whether the resource is read-only + * @param manifest The JAR manifest, or null if not applicable + * @param lockSet The lock set for concurrent access control, or null if locking is not required + * @param lockPath The path used for locking, or null if locking is not required + */ + public FileResource(WebResourceRoot root, String webAppPath, File resource, boolean readOnly, Manifest manifest, + WebResourceLockSet lockSet, String lockPath) { + this(root, webAppPath, resource, readOnly, manifest, lockSet, lockPath, null); + } + /** * Creates a FileResource with optional locking support. @@ -89,13 +106,16 @@ public FileResource(WebResourceRoot root, String webAppPath, File resource, bool * @param manifest The JAR manifest, or null if not applicable * @param lockSet The lock set for concurrent access control, or null if locking is not required * @param lockPath The path used for locking, or null if locking is not required + * @param canonicalPath The canonical path of the resource if it has already been resolved, else null + * to resolve it on demand */ public FileResource(WebResourceRoot root, String webAppPath, File resource, boolean readOnly, Manifest manifest, - WebResourceLockSet lockSet, String lockPath) { + WebResourceLockSet lockSet, String lockPath, String canonicalPath) { super(root, webAppPath); this.resource = resource; this.lockSet = lockSet; this.lockPath = lockPath; + this.canonicalPath = canonicalPath; if (webAppPath.charAt(webAppPath.length() - 1) == '/') { String realName = resource.getName() + '/'; @@ -195,6 +215,10 @@ private long getContentLengthInternal(boolean convert) { @Override public String getCanonicalPath() { + if (canonicalPath != null) { + // resolved while the resource set validated this file, so no need to resolve it again + return canonicalPath; + } try { return resource.getCanonicalPath(); } catch (IOException ioe) { diff --git a/java/org/apache/catalina/webresources/FileResourceSet.java b/java/org/apache/catalina/webresources/FileResourceSet.java index 1642e607c68f..d6c67c32cc4a 100644 --- a/java/org/apache/catalina/webresources/FileResourceSet.java +++ b/java/org/apache/catalina/webresources/FileResourceSet.java @@ -72,11 +72,12 @@ public WebResource getResource(String path) { String webAppMount = getWebAppMount(); WebResourceRoot root = getRoot(); if (path.equals(webAppMount)) { - File f = file("", true); - if (f == null) { + ValidatedFile validated = validatedFile("", true); + if (validated == null) { return new EmptyResource(root, path); } - return new FileResource(root, path, f, isReadOnly(), null); + return new FileResource(root, path, validated.file(), isReadOnly(), null, null, null, + validated.canonicalPath()); } if (path.charAt(path.length() - 1) != '/') { diff --git a/java/org/apache/catalina/webresources/JarWarResourceSet.java b/java/org/apache/catalina/webresources/JarWarResourceSet.java index e4bea41a30e5..81d9607217b3 100644 --- a/java/org/apache/catalina/webresources/JarWarResourceSet.java +++ b/java/org/apache/catalina/webresources/JarWarResourceSet.java @@ -153,11 +153,13 @@ protected Map getArchiveEntries(boolean single) { } } } - } - WebResourceRoot root = getRoot(); - if (root.getArchiveIndexStrategyEnum().getUsesBloom()) { - jarContents = new JarContents(archiveEntries.values()); - retainBloomFilterForArchives = root.getArchiveIndexStrategyEnum().getRetain(); + // Build the bloom filter once, when the entries are (re)built, rather than on every call. gc() clears + // archiveEntries and jarContents together so the filter is rebuilt whenever the entries are. + WebResourceRoot root = getRoot(); + if (root.getArchiveIndexStrategyEnum().getUsesBloom()) { + jarContents = new JarContents(archiveEntries.values()); + retainBloomFilterForArchives = root.getArchiveIndexStrategyEnum().getRetain(); + } } return archiveEntries; } diff --git a/java/org/apache/catalina/webresources/StandardRoot.java b/java/org/apache/catalina/webresources/StandardRoot.java index cbf5a1628923..fd995bb87955 100644 --- a/java/org/apache/catalina/webresources/StandardRoot.java +++ b/java/org/apache/catalina/webresources/StandardRoot.java @@ -23,7 +23,7 @@ import java.net.URISyntaxException; import java.net.URL; import java.util.ArrayList; -import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; @@ -135,7 +135,7 @@ private String[] list(String path, boolean validate) { for (WebResourceSet webResourceSet : list) { if (!webResourceSet.getClassLoaderOnly()) { String[] entries = webResourceSet.list(path); - result.addAll(Arrays.asList(entries)); + Collections.addAll(result, entries); } } } @@ -379,12 +379,10 @@ protected WebResource[] listResources(String path, boolean validate) { String[] resources = list(path, false); WebResource[] result = new WebResource[resources.length]; + // path is loop invariant so build the prefix (ending in '/') once. + String prefix = path.charAt(path.length() - 1) == '/' ? path : path + '/'; for (int i = 0; i < resources.length; i++) { - if (path.charAt(path.length() - 1) == '/') { - result[i] = getResource(path + resources[i], false, false); - } else { - result[i] = getResource(path + '/' + resources[i], false, false); - } + result[i] = getResource(prefix + resources[i], false, false); } return result; } diff --git a/java/org/apache/coyote/AbstractProtocol.java b/java/org/apache/coyote/AbstractProtocol.java index 776cdc2aa848..7cd970cc6a38 100644 --- a/java/org/apache/coyote/AbstractProtocol.java +++ b/java/org/apache/coyote/AbstractProtocol.java @@ -932,8 +932,11 @@ private ObjectName createObjectName() throws MalformedObjectNameException { */ @Override public void init() throws Exception { + // getName() is not a simple field getter (it builds and quotes the name) and is invariant here, so compute + // it once. + String name = getName(); if (getLog().isInfoEnabled()) { - getLog().info(sm.getString("abstractProtocolHandler.init", getName())); + getLog().info(sm.getString("abstractProtocolHandler.init", name)); logPortOffset(); } @@ -946,13 +949,12 @@ public void init() throws Exception { } if (this.domain != null) { - ObjectName rgOname = new ObjectName(domain + ":type=GlobalRequestProcessor,name=" + getName()); + ObjectName rgOname = new ObjectName(domain + ":type=GlobalRequestProcessor,name=" + name); this.rgOname = rgOname; Registry.getRegistry(null).registerComponent(getHandler().getGlobal(), rgOname, null); } - String endpointName = getName(); - endpoint.setName(endpointName.substring(1, endpointName.length() - 1)); + endpoint.setName(name.substring(1, name.length() - 1)); endpoint.setDomain(domain); endpoint.init(); diff --git a/java/org/apache/tomcat/util/descriptor/web/FragmentJarScannerCallback.java b/java/org/apache/tomcat/util/descriptor/web/FragmentJarScannerCallback.java index 78eb1d9c767f..46744c66537a 100644 --- a/java/org/apache/tomcat/util/descriptor/web/FragmentJarScannerCallback.java +++ b/java/org/apache/tomcat/util/descriptor/web/FragmentJarScannerCallback.java @@ -135,14 +135,15 @@ private void addFragment(WebXml fragment, URL url) { fragment.setName(url.toString()); } fragment.setJarName(extractJarFileName(url)); - if (fragments.containsKey(fragment.getName())) { + WebXml existing = fragments.get(fragment.getName()); + if (existing != null) { // Duplicate. Mark the fragment that has already been found with // this name as having a duplicate so Tomcat can handle it // correctly when the fragments are being ordered. - String duplicateName = fragment.getName(); - fragments.get(duplicateName).addDuplicate(url.toString()); + String urlString = url.toString(); + existing.addDuplicate(urlString); // Rename the current fragment so it doesn't clash - fragment.setName(url.toString()); + fragment.setName(urlString); } fragments.put(fragment.getName(), fragment); } diff --git a/java/org/apache/tomcat/util/digester/Digester.java b/java/org/apache/tomcat/util/digester/Digester.java index bff710ae52f2..80dfec37b584 100644 --- a/java/org/apache/tomcat/util/digester/Digester.java +++ b/java/org/apache/tomcat/util/digester/Digester.java @@ -2126,10 +2126,12 @@ private Attributes updateAttributes(Attributes list) { AttributesImpl newAttrs = new AttributesImpl(list); int nAttributes = newAttrs.getLength(); + // The class loader is invariant for the duration of this call so resolve it once rather than per attribute. + ClassLoader classLoader = getClassLoader(); for (int i = 0; i < nAttributes; ++i) { String value = newAttrs.getValue(i); try { - newAttrs.setValue(i, IntrospectionUtils.replaceProperties(value, null, source, getClassLoader())); + newAttrs.setValue(i, IntrospectionUtils.replaceProperties(value, null, source, classLoader)); } catch (Exception e) { log.warn(sm.getString("digester.failedToUpdateAttributes", newAttrs.getLocalName(i), value), e); } diff --git a/java/org/apache/tomcat/util/modeler/modules/MbeansDescriptorsIntrospectionSource.java b/java/org/apache/tomcat/util/modeler/modules/MbeansDescriptorsIntrospectionSource.java index d61c239b96ef..db12f8f392f6 100644 --- a/java/org/apache/tomcat/util/modeler/modules/MbeansDescriptorsIntrospectionSource.java +++ b/java/org/apache/tomcat/util/modeler/modules/MbeansDescriptorsIntrospectionSource.java @@ -126,10 +126,10 @@ public void execute() throws Exception { specialMethods.put("postDeregister", ""); } - private static final Class[] supportedTypes = new Class[] { Boolean.class, Boolean.TYPE, Byte.class, Byte.TYPE, + private static final Set> supportedTypes = Set.of(Boolean.class, Boolean.TYPE, Byte.class, Byte.TYPE, Character.class, Character.TYPE, Short.class, Short.TYPE, Integer.class, Integer.TYPE, Long.class, Long.TYPE, Float.class, Float.TYPE, Double.class, Double.TYPE, String.class, String[].class, - BigDecimal.class, BigInteger.class, ObjectName.class, Object[].class, java.io.File.class, }; + BigDecimal.class, BigInteger.class, ObjectName.class, Object[].class, java.io.File.class); /** * Check if this class is one of the supported types. If the class is supported, returns true. Otherwise, returns @@ -140,10 +140,9 @@ public void execute() throws Exception { * @return boolean True if class is supported */ private boolean supportedType(Class ret) { - for (Class supportedType : supportedTypes) { - if (ret == supportedType) { - return true; - } + // Class equality is reference identity so a Set membership test matches the previous identity comparison. + if (supportedTypes.contains(ret)) { + return true; } return isBeanCompatible(ret); } @@ -163,7 +162,8 @@ private boolean isBeanCompatible(Class javaType) { // Anything in the java or javax package that // does not have a defined mapping is excluded. - if (javaType.getName().startsWith("java.") || javaType.getName().startsWith("javax.")) { + String name = javaType.getName(); + if (name.startsWith("java.") || name.startsWith("javax.")) { return false; } diff --git a/java/org/apache/tomcat/util/scan/StandardJarScanner.java b/java/org/apache/tomcat/util/scan/StandardJarScanner.java index f985f4a9dd98..cb463873f228 100644 --- a/java/org/apache/tomcat/util/scan/StandardJarScanner.java +++ b/java/org/apache/tomcat/util/scan/StandardJarScanner.java @@ -24,7 +24,6 @@ import java.net.URL; import java.net.URLClassLoader; import java.util.ArrayDeque; -import java.util.Arrays; import java.util.Collections; import java.util.Deque; import java.util.HashSet; @@ -337,7 +336,7 @@ protected void doScanClassPath(JarScanType scanType, ServletContext context, Jar isWebapp = isWebappClassLoader(classLoader); } - classPathUrlsToProcess.addAll(Arrays.asList(((URLClassLoader) classLoader).getURLs())); + Collections.addAll(classPathUrlsToProcess, ((URLClassLoader) classLoader).getURLs()); processURLs(scanType, callback, processedURLs, isWebapp, classPathUrlsToProcess); } @@ -531,15 +530,24 @@ private void processManifest(Jar jar, boolean isWebapp, Deque classPathUrls return; } String[] classPathEntries = classPathAttribute.split(" "); + // The JAR URL and its URI form are invariant across all Class-Path entries so resolve them once. + URL jarURL = jar.getJarFileURL(); + URI jarURI; + try { + jarURI = jarURL.toURI(); + } catch (Exception e) { + if (log.isDebugEnabled()) { + log.debug(sm.getString("jarScan.invalidUri", jarURL), e); + } + return; + } for (String classPathEntry : classPathEntries) { classPathEntry = classPathEntry.trim(); if (classPathEntry.isEmpty()) { continue; } - URL jarURL = jar.getJarFileURL(); URL classPathEntryURL; try { - URI jarURI = jarURL.toURI(); /* * Note: Resolving the relative URLs from the manifest has the potential to introduce security * concerns. However, since only JARs provided by the container and NOT those provided by web diff --git a/test/org/apache/catalina/startup/TestContextConfigAnnotation.java b/test/org/apache/catalina/startup/TestContextConfigAnnotation.java index 69d13c37a7d4..7ba8aa0871d1 100644 --- a/test/org/apache/catalina/startup/TestContextConfigAnnotation.java +++ b/test/org/apache/catalina/startup/TestContextConfigAnnotation.java @@ -296,6 +296,10 @@ public void testCheckHandleTypes() throws Exception { config.typeInitializerMap.put(Object.class, new HashSet<>()); config.typeInitializerMap.get(Object.class).add(sciObject); + // Build the name-keyed indexes derived from typeInitializerMap. In the + // normal flow this happens at the start of processClasses(). + config.buildTypeInitializerIndexes(); + // Scan Servlet, Filter, Servlet, Listener WebXml ignore = new WebXml(); File file = paramClassResource( diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 8402b8ed256f..327159a8d5c3 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -282,10 +282,79 @@ Separate the Context role mapping from the Servlet specification security-role-ref. (remm) + + Improve web application start time by indexing the + @HandlesTypes types by class name so that matching a + scanned class no longer requires a linear scan of all + ServletContainerInitializer handled types. (tandraschko) + + + Avoid an unnecessary file open per deployed web application when + checking whether the cached default web.xml is still + current by reading the last modified time of file: based + global and host web.xml resources directly rather than via + a URLConnection. (tandraschko) + + + Avoid re-opening and re-scanning each web application fragment JAR when + looking for JARs that contain static resources under + META-INF/resources/ by recording that information during + the annotation scan that has already iterated every JAR entry. + (tandraschko) + + + Reduce redundant work during web application start: key the + @HandlesTypes annotation index by the class file descriptor + form to remove a per-annotation string conversion while scanning + classes, hoist loop invariant lookups out of the manifest + Class-Path, attribute property replacement and class path + scanning loops, and avoid an unnecessary per fragment collection + allocation. (tandraschko) + + + Build the bloom filter used to index the entries of a JAR nested inside + a packed WAR only once when the entries are read rather than rebuilding + it on every resource lookup. (tandraschko) + + + Further reduce allocations while reading web application resources: + pre-size the archive entry map from the known JAR entry count and hoist + a loop invariant out of resource listing. (tandraschko) + + + When registering a web application with the Mapper, add its + servlet mappings to each of the copy-on-write wrapper arrays in a single + merge per mapping category rather than reallocating the array once per + mapping, which was O(n^2) for contexts with many mappings. + (tandraschko) + + + Do not allocate a LifecycleEvent when a component has no + lifecycle listeners. Components such as each Wrapper fire + several state transition events during start but commonly have no + listeners. (tandraschko) + + + Use a Set rather than a linear array scan when checking + whether a type is a supported attribute type during JMX MBean + introspection. (tandraschko) + + + Avoid resolving the canonical path of a file based web resource twice. + The path is already resolved when the resource set validates that the + file is located under its base, so pass it to the created + FileResource rather than having + FileResource.getCanonicalPath() resolve the same file + again. (tandraschko) + + + Compute the protocol handler name once during init() + instead of rebuilding and re-quoting it several times. (tandraschko) + Change the default value of the cookiesWithoutEquals attribute of the Rfc6265CookieProcessor from