Minor fixes from code review

rmaucher · rmaucher · commit ebdbc5854e7a · 2026-07-17T09:13:42.000+02:00
diff --git a/java/org/apache/catalina/WebResource.java b/java/org/apache/catalina/WebResource.java
@@ -199,7 +199,7 @@ default URL getCodeBase() {
     /**
      * Returns the certificates that were used to sign this resource to verify it.
      *
-     * @return the certificates that were used to sign this resource to verify it or @null if none.
+     * @return the certificates that were used to sign this resource to verify it or {@code null} if none.
      *
      * @see java.util.jar.JarEntry#getCertificates()
      */
@@ -208,7 +208,7 @@ default URL getCodeBase() {
     /**
      * Returns the manifest associated with this resource.
      *
-     * @return the manifest associated with this resource or @null if none.
+     * @return the manifest associated with this resource or {@code null} if none.
      *
      * @see java.util.jar.JarFile#getManifest()
      */
diff --git a/java/org/apache/catalina/WebResourceSet.java b/java/org/apache/catalina/WebResourceSet.java
@@ -71,7 +71,8 @@ public interface WebResourceSet extends Lifecycle {
      *
      * @param path      The path to be used for the new Resource. It is relative to the root of the web application and
      *                      must start with '/'.
-     * @param is        The InputStream that will provide the content for the new Resource.
+     * @param is        The InputStream that will provide the content for the new Resource. The caller is responsible
+     *                  for closing the stream if required.
      * @param overwrite If <code>true</code> and the resource already exists it will be overwritten. If
      *                      <code>false</code> and the resource already exists the write will fail.
      *
diff --git a/java/org/apache/catalina/tribes/transport/nio/PooledParallelSender.java b/java/org/apache/catalina/tribes/transport/nio/PooledParallelSender.java
@@ -32,6 +32,7 @@
  */
 public class PooledParallelSender extends PooledSender implements PooledParallelSenderMBean {
     private static final Log log = LogFactory.getLog(PooledParallelSender.class);
+    /** StringManager for internationalized log messages. */
     protected static final StringManager sm = StringManager.getManager(PooledParallelSender.class);
 
     /**
diff --git a/java/org/apache/catalina/webresources/AbstractArchiveResource.java b/java/org/apache/catalina/webresources/AbstractArchiveResource.java
@@ -219,7 +219,7 @@ public final byte[] getContent() {
 
         if (len > Integer.MAX_VALUE) {
             // Can't create an array that big
-            throw new ArrayIndexOutOfBoundsException(
+            throw new IllegalStateException(
                     sm.getString("abstractResource.getContentTooLarge", getWebappPath(), Long.valueOf(len)));
         }
 
@@ -244,6 +244,10 @@ public final byte[] getContent() {
                 }
                 pos += n;
             }
+            if (pos < size) {
+                // Stream ended before expected size — return null to avoid partial data
+                return null;
+            }
             // Once the stream has been read, read the certs
             certificates = jisw.getCertificates();
             readCerts = true;
diff --git a/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java b/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java
@@ -356,6 +356,7 @@ public void setReadOnly(boolean readOnly) {
      */
     @Override
     public void setAllowLinking(boolean allowLinking) {
+        // NO-OP
     }
 
     /**
diff --git a/java/org/apache/catalina/webresources/AbstractFileResourceSet.java b/java/org/apache/catalina/webresources/AbstractFileResourceSet.java
@@ -286,7 +286,8 @@ protected void initInternal() throws LifecycleException {
         try {
             this.canonicalBase = fileBase.getCanonicalPath();
         } catch (IOException ioe) {
-            throw new IllegalArgumentException(ioe);
+            throw new LifecycleException(
+                    sm.getString("abstractFileResourceSet.canonicalPathFail", getBase()), ioe);
         }
 
         // Need to handle mapping of the file system root as a special case
diff --git a/java/org/apache/catalina/webresources/AbstractResource.java b/java/org/apache/catalina/webresources/AbstractResource.java
@@ -138,15 +138,19 @@ public final String getStrongETag() {
                         } else {
                             byte[] buf = new byte[4096];
                             try (InputStream is = getInputStream()) {
-                                MessageDigest digest = MessageDigest.getInstance("SHA-256");
-                                while (true) {
-                                    int n = is.read(buf);
-                                    if (n <= 0) {
-                                        break;
+                                if (is == null) {
+                                    strongETag = getETag();
+                                } else {
+                                    MessageDigest digest = MessageDigest.getInstance("SHA-256");
+                                    while (true) {
+                                        int n = is.read(buf);
+                                        if (n <= 0) {
+                                            break;
+                                        }
+                                        digest.update(buf, 0, n);
                                     }
-                                    digest.update(buf, 0, n);
+                                    strongETag = "\"" + HexUtils.toHexString(digest.digest()) + "\"";
                                 }
-                                strongETag = "\"" + HexUtils.toHexString(digest.digest()) + "\"";
                             } catch (Exception e) {
                                 strongETag = getETag();
                             }
diff --git a/java/org/apache/catalina/webresources/AbstractSingleArchiveResourceSet.java b/java/org/apache/catalina/webresources/AbstractSingleArchiveResourceSet.java
@@ -51,11 +51,11 @@ public AbstractSingleArchiveResourceSet() {
      * @param base         The base
      * @param internalPath The internal path
      *
-     * @throws IllegalArgumentException if the {@link WebResourceRoot} is available but this resource set cannot be
+     * @throws IllegalStateException if the {@link WebResourceRoot} is available but this resource set cannot be
      *                                      started
      */
     public AbstractSingleArchiveResourceSet(WebResourceRoot root, String webAppMount, String base, String internalPath)
-            throws IllegalArgumentException {
+            throws IllegalStateException {
         setRoot(root);
         setWebAppMount(webAppMount);
         setBase(base);
@@ -65,7 +65,7 @@ public AbstractSingleArchiveResourceSet(WebResourceRoot root, String webAppMount
             try {
                 start();
             } catch (LifecycleException e) {
-                throw new IllegalStateException(e);
+                throw new IllegalStateException(sm.getString("abstractArchiveResourceSet.startFail"), e);
             }
         }
     }
@@ -87,7 +87,8 @@ protected Map<String,JarEntry> getArchiveEntries(boolean single) {
                 } catch (IOException ioe) {
                     // Should never happen
                     archiveEntries = null;
-                    throw new IllegalStateException(ioe);
+                    throw new IllegalStateException(
+                            sm.getString("abstractArchiveResourceSet.archiveEntriesFail", getBase()), ioe);
                 } finally {
                     if (jarFile != null) {
                         closeJarFile();
@@ -107,7 +108,8 @@ protected JarEntry getArchiveEntry(String pathInArchive) {
             return jarFile.getJarEntry(pathInArchive);
         } catch (IOException ioe) {
             // Should never happen
-            throw new IllegalStateException(ioe);
+            throw new IllegalStateException(
+                    sm.getString("abstractArchiveResourceSet.archiveEntryFail", getBase()), ioe);
         } finally {
             if (jarFile != null) {
                 closeJarFile();
@@ -127,7 +129,8 @@ protected boolean isMultiRelease() {
                         multiRelease = Boolean.valueOf(jarFile.isMultiRelease());
                     } catch (IOException ioe) {
                         // Should never happen
-                        throw new IllegalStateException(ioe);
+                        throw new IllegalStateException(
+                                sm.getString("abstractArchiveResourceSet.multiReleaseFail", getBase()), ioe);
                     } finally {
                         if (jarFile != null) {
                             closeJarFile();
@@ -148,13 +151,15 @@ protected void initInternal() throws LifecycleException {
         try (JarFile jarFile = new JarFile(new File(getBase()), true, ZipFile.OPEN_READ, Runtime.version())) {
             setManifest(jarFile.getManifest());
         } catch (IOException ioe) {
-            throw new IllegalArgumentException(ioe);
+            throw new LifecycleException(
+                    sm.getString("abstractArchiveResourceSet.manifestFail", getBase()), ioe);
         }
 
         try {
             setBaseUrl(UriUtil.buildJarSafeUrl(new File(getBase())));
         } catch (IOException ioe) {
-            throw new IllegalArgumentException(ioe);
+            throw new LifecycleException(
+                    sm.getString("abstractArchiveResourceSet.baseUrlFail", getBase()), ioe);
         }
     }
 }
diff --git a/java/org/apache/catalina/webresources/CachedResource.java b/java/org/apache/catalina/webresources/CachedResource.java
@@ -524,7 +524,7 @@ protected URLConnection openConnection(URL u) throws IOException {
                     constructedURI = new URI(u.toExternalForm());
                 } catch (URISyntaxException e) {
                     // Not ideal but consistent with API
-                    throw new IOException(e);
+                    throw new IOException(sm.getString("cachedResource.invalidURI", u.toExternalForm()), e);
                 }
                 URL constructedURL = constructedURI.toURL();
                 return constructedURL.openConnection();
diff --git a/java/org/apache/catalina/webresources/DirResourceSet.java b/java/org/apache/catalina/webresources/DirResourceSet.java
@@ -89,7 +89,7 @@ public DirResourceSet(WebResourceRoot root, String webAppMount, String base, Str
             try {
                 start();
             } catch (LifecycleException e) {
-                throw new IllegalStateException(e);
+                throw new IllegalStateException(sm.getString("dirResourceSet.startFail"), e);
             }
         }
     }
diff --git a/java/org/apache/catalina/webresources/EmptyResource.java b/java/org/apache/catalina/webresources/EmptyResource.java
@@ -139,7 +139,7 @@ public String getETag() {
 
     @Override
     public void setMimeType(String mimeType) {
-        // NOOP
+        // NO-OP
     }
 
     @Override
diff --git a/java/org/apache/catalina/webresources/EmptyResourceSet.java b/java/org/apache/catalina/webresources/EmptyResourceSet.java
@@ -142,7 +142,7 @@ public URL getBaseUrl() {
      */
     @Override
     public void setReadOnly(boolean readOnly) {
-
+        // NO-OP
     }
 
     /**
@@ -152,6 +152,7 @@ public void setReadOnly(boolean readOnly) {
      */
     @Override
     public void setAllowLinking(boolean allowLinking) {
+        // NO-OP
     }
 
     /**
diff --git a/java/org/apache/catalina/webresources/FileResourceSet.java b/java/org/apache/catalina/webresources/FileResourceSet.java
@@ -59,7 +59,7 @@ public FileResourceSet(WebResourceRoot root, String webAppMount, String base, St
             try {
                 start();
             } catch (LifecycleException e) {
-                throw new IllegalStateException(e);
+                throw new IllegalStateException(sm.getString("fileResourceSet.startFail"), e);
             }
         }
     }
diff --git a/java/org/apache/catalina/webresources/JarWarResourceSet.java b/java/org/apache/catalina/webresources/JarWarResourceSet.java
@@ -69,7 +69,7 @@ public JarWarResourceSet(WebResourceRoot root, String webAppMount, String base,
             try {
                 start();
             } catch (LifecycleException e) {
-                throw new IllegalStateException(e);
+                throw new IllegalStateException(sm.getString("jarWarResourceSet.startFail"), e);
             }
         }
     }
@@ -133,7 +133,8 @@ protected Map<String,JarEntry> getArchiveEntries(boolean single) {
                 } catch (IOException ioe) {
                     // Should never happen
                     archiveEntries = null;
-                    throw new IllegalStateException(ioe);
+                    throw new IllegalStateException(
+                            sm.getString("jarWarResourceSet.archiveEntriesFail", archivePath, getBase()), ioe);
                 } finally {
                     if (warFile != null) {
                         closeJarFile();
@@ -256,13 +257,14 @@ protected void initInternal() throws LifecycleException {
                 setManifest(jarIs.getManifest());
             }
         } catch (IOException ioe) {
-            throw new IllegalArgumentException(ioe);
+            throw new LifecycleException(
+                    sm.getString("jarWarResourceSet.manifestFail", archivePath, getBase()), ioe);
         }
 
         try {
             setBaseUrl(UriUtil.buildJarSafeUrl(new File(getBase())));
         } catch (IOException ioe) {
-            throw new IllegalArgumentException(ioe);
+            throw new LifecycleException(sm.getString("jarWarResourceSet.baseUrlFail", getBase()), ioe);
         }
     }
 
diff --git a/java/org/apache/catalina/webresources/LocalStrings.properties b/java/org/apache/catalina/webresources/LocalStrings.properties
@@ -14,9 +14,16 @@
 # limitations under the License.
 
 abstractArchiveResourceSet.archiveCloseFailed=Error closing archive. Archive may still be open.
+abstractArchiveResourceSet.archiveEntriesFail=Failed to read the archive entries from [{0}]
+abstractArchiveResourceSet.archiveEntryFail=Failed to obtain an entry from the archive [{0}]
+abstractArchiveResourceSet.baseUrlFail=Failed to build the base URL for the archive [{0}]
+abstractArchiveResourceSet.manifestFail=Failed to read the manifest from the archive [{0}]
+abstractArchiveResourceSet.multiReleaseFail=Failed to determine if the archive [{0}] is a multi-release JAR
+abstractArchiveResourceSet.startFail=Failed to start the resource set
 abstractArchiveResourceSet.setReadOnlyFalse=Archive based WebResourceSets such as those based on JARs are hard-coded to be read-only and may not be configured to be read-write
 
 abstractFileResourceSet.canonicalfileCheckFailed=Resource for web application [{0}] at path [{1}] was not loaded as the canonical path [{2}] did not match. Use of symlinks is one possible cause.
+abstractFileResourceSet.canonicalPathFail=Failed to determine the canonical path for [{0}]
 
 abstractResource.getContentFail=Unable to return [{0}] as a byte array
 abstractResource.getContentFirst=getContent must be called before getCertificates
@@ -31,13 +38,15 @@ cache.objectMaxSizeTooBigBytes=The value specified for the maximum object size t
 cache.sizeTracking.add=Increased cache size by [{0}] for item [{1}] at [{2}] making total cache size [{3}]
 cache.sizeTracking.remove=Decreased cache size by [{0}] for item [{1}] at [{2}] making total cache size [{3}]
 
+cachedResource.invalidURI=Unable to construct a URI from the URL [{0}]
 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.isCaseSensitive.fail=Error trying to determine if file system at [{0}] is case sensitive so assuming it is not case sensitive
 dirResourceSet.manifestFail=Failed to read manifest from [{0}]
 dirResourceSet.notDirectory=The directory specified by base and internal path [{0}]{1}[{2}] does not exist.
+dirResourceSet.startFail=Failed to start the resource set
 dirResourceSet.writeNpe=The input stream may not be null
 
 extractingRoot.jarFailed=Failed to extract the JAR file [{0}]
@@ -48,16 +57,23 @@ fileResource.getCreationFail=Unable to determine the creation time for the resou
 fileResource.getUrlFail=Unable to determine a URL for the resource [{0}]
 
 fileResourceSet.notFile=The file specified by base and internal path [{0}]{1}[{2}] does not exist.
+fileResourceSet.startFail=Failed to start the resource set
 
 jarResource.getInputStreamFail=Unable to obtain an InputStream for the resource [{0}] located in the JAR [{1}]
 
 jarResourceRoot.invalidWebAppPath=This resource always refers to a directory so the supplied webAppPath must end with / but the provided webAppPath was [{0}]
 
+jarWarResourceSet.archiveEntriesFail=Failed to read the archive entries from the inner JAR [{0}] located in [{1}]
+jarWarResourceSet.baseUrlFail=Failed to build the base URL for the WAR file [{0}]
 jarWarResourceSet.codingError=Coding error
+jarWarResourceSet.manifestFail=Failed to read the manifest from the inner JAR [{0}] located in [{1}]
+jarWarResourceSet.startFail=Failed to start the resource set
 
 standardRoot.checkStateNotStarted=The resources may not be accessed if they are not currently started
 standardRoot.createInvalidFile=Unable to create WebResourceSet from [{0}]
 standardRoot.createUnknownType=Unable to create WebResourceSet of unknown type [{0}]
+standardRoot.invalidFileUrl=The file URL [{0}] has invalid syntax
+standardRoot.invalidJarUrl=The JAR/WAR URL [{0}] has invalid syntax
 standardRoot.invalidPath=The resource path [{0}] is not valid
 standardRoot.invalidPathNormal=The resource path [{0}] has been normalized to [{1}] which is not valid
 standardRoot.lockedFile=The web application [{0}] failed to close the file [{1}] opened via the following stack trace
diff --git a/java/org/apache/catalina/webresources/StandardRoot.java b/java/org/apache/catalina/webresources/StandardRoot.java
@@ -889,7 +889,8 @@ static class BaseLocation {
                 try {
                     f = new File(new URI(fileUrl));
                 } catch (URISyntaxException e) {
-                    throw new IllegalArgumentException(e);
+                    throw new IllegalArgumentException(
+                            sm.getString("standardRoot.invalidJarUrl", fileUrl), e);
                 }
                 int startOfArchivePath = endOfFileUrl + 2;
                 if (jarUrl.length() > startOfArchivePath) {
@@ -901,7 +902,8 @@ static class BaseLocation {
                 try {
                     f = new File(url.toURI());
                 } catch (URISyntaxException e) {
-                    throw new IllegalArgumentException(e);
+                    throw new IllegalArgumentException(
+                            sm.getString("standardRoot.invalidFileUrl", url.toExternalForm()), e);
                 }
                 archivePath = null;
             } else {

Original file line number	Diff line number	Diff line change
`@@ -199,7 +199,7 @@ default URL getCodeBase() {`
`199`	`199`	`/**`
`200`	`200`	`* Returns the certificates that were used to sign this resource to verify it.`
`201`	`201`	`*`
`202`		`- * @return the certificates that were used to sign this resource to verify it or @null if none.`
	`202`	`+ * @return the certificates that were used to sign this resource to verify it or {@code null} if none.`
`203`	`203`	`*`
`204`	`204`	`* @see java.util.jar.JarEntry#getCertificates()`
`205`	`205`	`*/`
`@@ -208,7 +208,7 @@ default URL getCodeBase() {`
`208`	`208`	`/**`
`209`	`209`	`* Returns the manifest associated with this resource.`
`210`	`210`	`*`
`211`		`- * @return the manifest associated with this resource or @null if none.`
	`211`	`+ * @return the manifest associated with this resource or {@code null} if none.`
`212`	`212`	`*`
`213`	`213`	`* @see java.util.jar.JarFile#getManifest()`
`214`	`214`	`*/`
Original file line number	Diff line number	Diff line change
`@@ -71,7 +71,8 @@ public interface WebResourceSet extends Lifecycle {`
`71`	`71`	`*`
`72`	`72`	`* @param path The path to be used for the new Resource. It is relative to the root of the web application and`
`73`	`73`	`* must start with '/'.`
`74`		`- * @param is The InputStream that will provide the content for the new Resource.`
	`74`	`+ * @param is The InputStream that will provide the content for the new Resource. The caller is responsible`
	`75`	`+ * for closing the stream if required.`
`75`	`76`	`* @param overwrite If <code>true</code> and the resource already exists it will be overwritten. If`
`76`	`77`	`* <code>false</code> and the resource already exists the write will fail.`
`77`	`78`	`*`
Original file line number	Diff line number	Diff line change
`@@ -219,7 +219,7 @@ public final byte[] getContent() {`
`219`	`219`
`220`	`220`	`if (len > Integer.MAX_VALUE) {`
`221`	`221`	`// Can't create an array that big`
`222`		`- throw new ArrayIndexOutOfBoundsException(`
	`222`	`+ throw new IllegalStateException(`
`223`	`223`	`sm.getString("abstractResource.getContentTooLarge", getWebappPath(), Long.valueOf(len)));`
`224`	`224`	`}`
`225`	`225`
`@@ -244,6 +244,10 @@ public final byte[] getContent() {`
`244`	`244`	`}`
`245`	`245`	`pos += n;`
`246`	`246`	`}`
	`247`	`+ if (pos < size) {`
	`248`	`+ // Stream ended before expected size — return null to avoid partial data`
	`249`	`+ return null;`
	`250`	`+ }`
`247`	`251`	`// Once the stream has been read, read the certs`
`248`	`252`	`certificates = jisw.getCertificates();`
`249`	`253`	`readCerts = true;`
Original file line number	Diff line number	Diff line change
`@@ -356,6 +356,7 @@ public void setReadOnly(boolean readOnly) {`
`356`	`356`	`*/`
`357`	`357`	`@Override`
`358`	`358`	`public void setAllowLinking(boolean allowLinking) {`
	`359`	`+ // NO-OP`
`359`	`360`	`}`
`360`	`361`
`361`	`362`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -286,7 +286,8 @@ protected void initInternal() throws LifecycleException {`
`286`	`286`	`try {`
`287`	`287`	`this.canonicalBase = fileBase.getCanonicalPath();`
`288`	`288`	`} catch (IOException ioe) {`
`289`		`- throw new IllegalArgumentException(ioe);`
	`289`	`+ throw new LifecycleException(`
	`290`	`+ sm.getString("abstractFileResourceSet.canonicalPathFail", getBase()), ioe);`
`290`	`291`	`}`
`291`	`292`
`292`	`293`	`// Need to handle mapping of the file system root as a special case`
Original file line number	Diff line number	Diff line change
`@@ -524,7 +524,7 @@ protected URLConnection openConnection(URL u) throws IOException {`
`524`	`524`	`constructedURI = new URI(u.toExternalForm());`
`525`	`525`	`} catch (URISyntaxException e) {`
`526`	`526`	`// Not ideal but consistent with API`
`527`		`- throw new IOException(e);`
	`527`	`+ throw new IOException(sm.getString("cachedResource.invalidURI", u.toExternalForm()), e);`
`528`	`528`	`}`
`529`	`529`	`URL constructedURL = constructedURI.toURL();`
`530`	`530`	`return constructedURL.openConnection();`
Original file line number	Diff line number	Diff line change
`@@ -89,7 +89,7 @@ public DirResourceSet(WebResourceRoot root, String webAppMount, String base, Str`
`89`	`89`	`try {`
`90`	`90`	`start();`
`91`	`91`	`} catch (LifecycleException e) {`
`92`		`- throw new IllegalStateException(e);`
	`92`	`+ throw new IllegalStateException(sm.getString("dirResourceSet.startFail"), e);`
`93`	`93`	`}`
`94`	`94`	`}`
`95`	`95`	`}`