This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 60008aaaf7 Code review fixes
60008aaaf7 is described below
commit 60008aaaf73a5e864f382f4667d576ea2d5b2f2a
Author: remm <[email protected]>
AuthorDate: Wed Jul 15 17:06:29 2026 +0200
Code review fixes
Some refinements for DOMWriter, co authored by OpenCode.
Similar in XMLWriter.
---
.../apache/catalina/servlets/DefaultServlet.java | 8 +++--
.../catalina/servlets/LocalStrings.properties | 1 +
java/org/apache/catalina/util/ContextName.java | 2 +-
.../catalina/util/CustomObjectInputStream.java | 4 +--
java/org/apache/catalina/util/DOMWriter.java | 37 +++++++++++++++++++---
.../org/apache/catalina/util/ErrorPageSupport.java | 12 +++----
.../org/apache/catalina/util/ExactRateLimiter.java | 2 +-
java/org/apache/catalina/util/IOTools.java | 2 +-
java/org/apache/catalina/util/Introspection.java | 5 +--
java/org/apache/catalina/util/NetMask.java | 23 +++++++++++---
java/org/apache/catalina/util/NetMaskSet.java | 2 +-
java/org/apache/catalina/util/RateLimiterBase.java | 2 +-
java/org/apache/catalina/util/ResourceSet.java | 26 ++++++++++++---
java/org/apache/catalina/util/ServerInfo.java | 2 +-
java/org/apache/catalina/util/SessionConfig.java | 2 +-
.../catalina/util/SessionIdGeneratorBase.java | 2 +-
java/org/apache/catalina/util/Strftime.java | 2 +-
.../apache/catalina/util/TimeBucketCounter.java | 2 +-
java/org/apache/catalina/util/XMLWriter.java | 14 ++++++--
19 files changed, 114 insertions(+), 36 deletions(-)
diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java
b/java/org/apache/catalina/servlets/DefaultServlet.java
index 0d8a1b1f8e..0642052c06 100644
--- a/java/org/apache/catalina/servlets/DefaultServlet.java
+++ b/java/org/apache/catalina/servlets/DefaultServlet.java
@@ -2128,8 +2128,12 @@ public class DefaultServlet extends HttpServlet {
log(sm.getString("defaultServlet.globalXSLTTooBig",
f.getAbsolutePath()));
} else {
try (FileInputStream fis = new FileInputStream(f)) {
- byte[] b = new byte[(int) f.length()];
- IOTools.readFully(fis, b);
+ int xsltLength = (int) globalXsltFileSize;
+ byte[] b = new byte[xsltLength];
+ if (xsltLength != IOTools.readFully(fis, b)) {
+ log(sm.getString("defaultServlet.truncatedXSLT",
f.getAbsolutePath()));
+ return null;
+ }
return new StreamSource(new ByteArrayInputStream(b));
}
}
diff --git a/java/org/apache/catalina/servlets/LocalStrings.properties
b/java/org/apache/catalina/servlets/LocalStrings.properties
index 2628a1e8e2..896d0c0aa1 100644
--- a/java/org/apache/catalina/servlets/LocalStrings.properties
+++ b/java/org/apache/catalina/servlets/LocalStrings.properties
@@ -53,6 +53,7 @@ defaultServlet.resource.lastModified=Last Modified
defaultServlet.resource.name=Name
defaultServlet.resource.size=Size
defaultServlet.skipfail=Read failed because only [{0}] bytes were available
but needed to skip [{1}] bytes to reach the start of the requested range
+defaultServlet.truncatedXSLT=The global XSLT file [{0}] was truncated
defaultServlet.unknownBomConfig=Unrecognised value of [{0}] provided for
useBomIfPresent initialization parameter
defaultServlet.wrongByteCountForRange=An invalid amount [{0}] of bytes were
received for range with length [{1}]
defaultServlet.xslError=XSL transformer error
diff --git a/java/org/apache/catalina/util/ContextName.java
b/java/org/apache/catalina/util/ContextName.java
index 18c224c431..34d2e38f5b 100644
--- a/java/org/apache/catalina/util/ContextName.java
+++ b/java/org/apache/catalina/util/ContextName.java
@@ -100,7 +100,7 @@ public final class ContextName {
/**
* Construct an instance from a path and version.
*
- * @param path Context path to use
+ * @param path Context path to use, which must not contain a version
marker
* @param version Context version to use
*/
public ContextName(String path, String version) {
diff --git a/java/org/apache/catalina/util/CustomObjectInputStream.java
b/java/org/apache/catalina/util/CustomObjectInputStream.java
index 80f869b997..79b3de1ed6 100644
--- a/java/org/apache/catalina/util/CustomObjectInputStream.java
+++ b/java/org/apache/catalina/util/CustomObjectInputStream.java
@@ -73,8 +73,8 @@ public final class CustomObjectInputStream extends
ObjectInputStream {
* @param log The logger to use to report any issues.
It may only be null if the filterMode does
* not require logging
* @param allowedClassNamePattern The regular expression to use to filter
deserialized classes. The fully qualified
- * class name must match this pattern
for deserialization to be allowed if
- * filtering is enabled.
+ * class name must match this pattern
for deserialization to be allowed.
+ * If null, filtering will be disabled.
* @param warnOnFailure Should any failures be logged?
*
* @exception IOException if an input/output error occurs
diff --git a/java/org/apache/catalina/util/DOMWriter.java
b/java/org/apache/catalina/util/DOMWriter.java
index 6218dfb4e2..8f47594394 100644
--- a/java/org/apache/catalina/util/DOMWriter.java
+++ b/java/org/apache/catalina/util/DOMWriter.java
@@ -112,7 +112,6 @@ public class DOMWriter {
case Node.PROCESSING_INSTRUCTION_NODE:
out.print("<?");
out.print(node.getLocalName());
-
String piData = node.getNodeValue();
if (piData != null && !piData.isEmpty()) {
out.print(' ');
@@ -122,15 +121,45 @@ public class DOMWriter {
int start = 0;
int end = piData.indexOf("?>");
while (end >= 0) {
- out.print(piData.substring(start, end + 1));
- out.print(' ');
- start = end + 1;
+ out.print(piData.substring(start, end));
+ out.print("? >");
+ start = end + 2;
end = piData.indexOf("?>", start);
}
out.print(piData.substring(start));
}
out.print("?>");
break;
+
+ // print comment
+ case Node.COMMENT_NODE:
+ out.print("<!--");
+ String commentValue = node.getNodeValue();
+ if (commentValue != null) {
+ // The only illegal sequence in comment data is --> which
+ // would terminate the comment early. Break it with a
space.
+ int start = 0;
+ int end = commentValue.indexOf("-->");
+ while (end >= 0) {
+ out.print(commentValue.substring(start, end));
+ out.print("- >");
+ start = end + 3;
+ end = commentValue.indexOf("-->", start);
+ }
+ out.print(commentValue.substring(start));
+ }
+ out.print("-->");
+ break;
+
+ // print document fragment (just its children)
+ case Node.DOCUMENT_FRAGMENT_NODE:
+ printChildren(node);
+ break;
+
+ default:
+ // Unhandled node types (ENTITY_NODE, NOTATION_NODE,
+ // ATTRIBUTE_NODE) are silently skipped.
+ break;
}
if (type == Node.ELEMENT_NODE) {
diff --git a/java/org/apache/catalina/util/ErrorPageSupport.java
b/java/org/apache/catalina/util/ErrorPageSupport.java
index 7e85599f55..af87b33a02 100644
--- a/java/org/apache/catalina/util/ErrorPageSupport.java
+++ b/java/org/apache/catalina/util/ErrorPageSupport.java
@@ -97,18 +97,18 @@ public class ErrorPageSupport {
/**
- * Find the ErrorPage, if any, for the given exception type, searching up
the
+ * Find the ErrorPage, if any, for the given exception, searching up the
* exception's class hierarchy.
*
- * @param exceptionType The exception instance
+ * @param exception The exception instance
*
- * @return The ErrorPage for the exception type, or {@code null} if none
is configured
+ * @return The ErrorPage for the exception, or {@code null} if none is
configured
*/
- public ErrorPage find(Throwable exceptionType) {
- if (exceptionType == null) {
+ public ErrorPage find(Throwable exception) {
+ if (exception == null) {
return null;
}
- Class<?> clazz = exceptionType.getClass();
+ Class<?> clazz = exception.getClass();
String name = clazz.getName();
while (!Object.class.equals(clazz)) {
ErrorPage errorPage = exceptionPages.get(name);
diff --git a/java/org/apache/catalina/util/ExactRateLimiter.java
b/java/org/apache/catalina/util/ExactRateLimiter.java
index 246b5e8a77..8fb5cd9eb4 100644
--- a/java/org/apache/catalina/util/ExactRateLimiter.java
+++ b/java/org/apache/catalina/util/ExactRateLimiter.java
@@ -19,7 +19,7 @@ package org.apache.catalina.util;
import java.util.concurrent.ScheduledExecutorService;
/**
- * A RateLimiter that compromises efficiency for accuracy in order to provide
exact rate limiting.
+ * A RateLimiter that trades efficiency for accuracy in order to provide exact
rate limiting.
*/
public class ExactRateLimiter extends RateLimiterBase {
diff --git a/java/org/apache/catalina/util/IOTools.java
b/java/org/apache/catalina/util/IOTools.java
index c6cd67ccf8..e16cf76054 100644
--- a/java/org/apache/catalina/util/IOTools.java
+++ b/java/org/apache/catalina/util/IOTools.java
@@ -94,7 +94,7 @@ public class IOTools {
* @param is The source to read from
* @param buf The buffer to write to
*
- * @return The number of bytes read
+ * @return The number of bytes actually read
*
* @throws IOException If an I/O error occurs during the read
*/
diff --git a/java/org/apache/catalina/util/Introspection.java
b/java/org/apache/catalina/util/Introspection.java
index 8dc41fbcd3..5a27ca39b8 100644
--- a/java/org/apache/catalina/util/Introspection.java
+++ b/java/org/apache/catalina/util/Introspection.java
@@ -41,8 +41,9 @@ public class Introspection {
/**
- * Extract the Java Bean property name from the setter name. Note: This
method assumes that the method name has
- * already been checked for correctness.
+ * Extract the Java Bean property name from the setter name. Note: This
+ * method assumes that the method name starts with "set" and has a length
+ * greater than 3.
*
* @param setter The setter method
*
diff --git a/java/org/apache/catalina/util/NetMask.java
b/java/org/apache/catalina/util/NetMask.java
index 51c53769c8..8b5f7e02ce 100644
--- a/java/org/apache/catalina/util/NetMask.java
+++ b/java/org/apache/catalina/util/NetMask.java
@@ -19,6 +19,7 @@ package org.apache.catalina.util;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.Arrays;
+import java.util.Objects;
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;
@@ -296,13 +297,27 @@ public final class NetMask {
return false;
}
NetMask other = (NetMask) o;
- return nrBytes == other.nrBytes && lastByteShift ==
other.lastByteShift &&
- Arrays.equals(netaddr, other.netaddr);
+ if (foundPort != other.foundPort) {
+ return false;
+ }
+ if (nrBytes != other.nrBytes || lastByteShift != other.lastByteShift) {
+ return false;
+ }
+ if (!Arrays.equals(netaddr, other.netaddr)) {
+ return false;
+ }
+ if (foundPort) {
+ return Objects.equals(portPattern, other.portPattern);
+ }
+ return true;
}
@Override
public int hashCode() {
- return 31 * Arrays.hashCode(netaddr) + lastByteShift;
+ int result = 31 * (31 * Arrays.hashCode(netaddr) + nrBytes) +
lastByteShift;
+ if (foundPort) {
+ result = 31 * result + portPattern.hashCode();
+ }
+ return result;
}
-
}
diff --git a/java/org/apache/catalina/util/NetMaskSet.java
b/java/org/apache/catalina/util/NetMaskSet.java
index 91d7067b05..6a1e6cd042 100644
--- a/java/org/apache/catalina/util/NetMaskSet.java
+++ b/java/org/apache/catalina/util/NetMaskSet.java
@@ -104,7 +104,7 @@ public class NetMaskSet {
}
/**
- * removes all entries from the set
+ * Removes all entries from the set.
*/
public void clear() {
netmasks.clear();
diff --git a/java/org/apache/catalina/util/RateLimiterBase.java
b/java/org/apache/catalina/util/RateLimiterBase.java
index cfd8b4fc6b..5ff2f7cb27 100644
--- a/java/org/apache/catalina/util/RateLimiterBase.java
+++ b/java/org/apache/catalina/util/RateLimiterBase.java
@@ -24,7 +24,7 @@ import java.util.concurrent.atomic.AtomicInteger;
import jakarta.servlet.FilterConfig;
/**
- * Base implementation of {@link RateLimiter}, provides runtime data
maintenance mechanism monitor.
+ * Base implementation of {@link RateLimiter}, provides a runtime data
maintenance mechanism and monitor.
*/
public abstract class RateLimiterBase implements RateLimiter {
diff --git a/java/org/apache/catalina/util/ResourceSet.java
b/java/org/apache/catalina/util/ResourceSet.java
index f7a82f7451..8fcc5cdc81 100644
--- a/java/org/apache/catalina/util/ResourceSet.java
+++ b/java/org/apache/catalina/util/ResourceSet.java
@@ -19,6 +19,7 @@ package org.apache.catalina.util;
import java.io.Serial;
import java.util.Collection;
+import java.util.Comparator;
import java.util.HashSet;
import java.util.Iterator;
@@ -89,15 +90,15 @@ public final class ResourceSet<T> extends HashSet<T> {
/**
- * The current lock state of this parameter map.
+ * The current lock state of this set.
*/
private boolean locked = false;
/**
- * Return the locked state of this parameter map.
+ * Return the locked state of this set.
*
- * @return the locked state of this parameter map
+ * @return the locked state of this set
*/
public boolean isLocked() {
return this.locked;
@@ -105,7 +106,7 @@ public final class ResourceSet<T> extends HashSet<T> {
/**
- * Set the locked state of this parameter map.
+ * Set the locked state of this set.
*
* @param locked The new locked state
*/
@@ -221,6 +222,23 @@ public final class ResourceSet<T> extends HashSet<T> {
}
+ @Override
+ public java.util.Spliterator<T> spliterator() {
+ if (locked) {
+ java.util.Spliterator<T> base = super.spliterator();
+ return new java.util.Spliterator<T>() {
+ @Override public boolean
tryAdvance(java.util.function.Consumer<? super T> action) { return
base.tryAdvance(action); }
+ @Override public java.util.Spliterator<T> trySplit() { return
null; }
+ @Override public long estimateSize() { return
base.estimateSize(); }
+ @Override public int characteristics() { return
base.characteristics() & ~java.util.Spliterator.SORTED; }
+ @Override public void
forEachRemaining(java.util.function.Consumer<? super T> action) {
base.forEachRemaining(action); }
+ @Override public Comparator<? super T> getComparator() {
return base.getComparator(); }
+ };
+ }
+ return super.spliterator();
+ }
+
+
/**
* {@inheritDoc}
*
diff --git a/java/org/apache/catalina/util/ServerInfo.java
b/java/org/apache/catalina/util/ServerInfo.java
index 5c8090d550..2a1e32f2fe 100644
--- a/java/org/apache/catalina/util/ServerInfo.java
+++ b/java/org/apache/catalina/util/ServerInfo.java
@@ -54,7 +54,7 @@ public class ServerInfo {
private static final String serverBuilt;
/**
- * The server built String, in ISO-8604 date format.
+ * The server built String, in ISO-8601 date format.
*/
private static final String serverBuiltIso;
diff --git a/java/org/apache/catalina/util/SessionConfig.java
b/java/org/apache/catalina/util/SessionConfig.java
index a2106fb817..9c55cf5cbc 100644
--- a/java/org/apache/catalina/util/SessionConfig.java
+++ b/java/org/apache/catalina/util/SessionConfig.java
@@ -77,7 +77,7 @@ public class SessionConfig {
*
* @param context The context
*
- * @return the parameter name for the session
+ * @return the cookie path for the session
*/
public static String getSessionCookiePath(Context context) {
diff --git a/java/org/apache/catalina/util/SessionIdGeneratorBase.java
b/java/org/apache/catalina/util/SessionIdGeneratorBase.java
index 45bf5bb34b..5785f4ea8b 100644
--- a/java/org/apache/catalina/util/SessionIdGeneratorBase.java
+++ b/java/org/apache/catalina/util/SessionIdGeneratorBase.java
@@ -277,7 +277,7 @@ public abstract class SessionIdGeneratorBase extends
LifecycleBase implements Se
try {
result =
SecureRandom.getInstance(DEFAULT_SECURE_RANDOM_ALGORITHM);
} catch (NoSuchAlgorithmException e) {
-
log.error(sm.getString("sessionIdGeneratorBase.randomAlgorithm",
secureRandomAlgorithm), e);
+
log.error(sm.getString("sessionIdGeneratorBase.randomAlgorithm",
DEFAULT_SECURE_RANDOM_ALGORITHM), e);
}
}
diff --git a/java/org/apache/catalina/util/Strftime.java
b/java/org/apache/catalina/util/Strftime.java
index c1dd436670..c9b56fab45 100644
--- a/java/org/apache/catalina/util/Strftime.java
+++ b/java/org/apache/catalina/util/Strftime.java
@@ -29,7 +29,7 @@ import java.util.TimeZone;
* were literals.</li>
* <li>Certain complicated commands, like those dealing with the week of the
year probably don't have exactly the same
* behavior as strftime.</li>
- * <li>These limitations are due to use SimpleDateTime. If the conversion was
done manually, all these limitations could
+ * <li>These limitations are due to use SimpleDateFormat. If the conversion
was done manually, all these limitations could
* be eliminated.</li>
* <li>The interface looks like a subset of DateFormat. Maybe someday someone
will make this class extend
* DateFormat.</li>
diff --git a/java/org/apache/catalina/util/TimeBucketCounter.java
b/java/org/apache/catalina/util/TimeBucketCounter.java
index 91e96aa150..4f54ade92f 100644
--- a/java/org/apache/catalina/util/TimeBucketCounter.java
+++ b/java/org/apache/catalina/util/TimeBucketCounter.java
@@ -126,7 +126,7 @@ public class TimeBucketCounter extends
TimeBucketCounterBase {
/**
* Returns the ratio to the next power of 2 so that we can adjust the
value.
*
- * @param value of target duration in seconds
+ * @param value of target duration in milliseconds
*
* @return the ratio to the next power of 2 so that we can adjust the value
*/
diff --git a/java/org/apache/catalina/util/XMLWriter.java
b/java/org/apache/catalina/util/XMLWriter.java
index 146136cccc..a2eb721e8f 100644
--- a/java/org/apache/catalina/util/XMLWriter.java
+++ b/java/org/apache/catalina/util/XMLWriter.java
@@ -111,7 +111,7 @@ public class XMLWriter {
*/
public void writeProperty(String namespace, String name, String value) {
writeElement(namespace, name, OPENING);
- buffer.append(value);
+ buffer.append(Escape.xml(value));
writeElement(namespace, name, CLOSING);
}
@@ -243,7 +243,17 @@ public class XMLWriter {
* @param data Data to append
*/
public void writeData(String data) {
- buffer.append("<![CDATA[").append(data).append("]]>");
+ buffer.append("<![CDATA[");
+ int start = 0;
+ int idx;
+ while ((idx = data.indexOf("]]>", start)) >= 0) {
+ buffer.append(data, start, idx);
+ // We terminate, then append the ]]>, and restart the sequence
+ buffer.append("]]>]]><![CDATA[");
+ start = idx + 3;
+ }
+ buffer.append(data.substring(start));
+ buffer.append("]]>");
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
