You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2026/10/20 21:33:36 UTC
(tomcat) branch 10.1.x updated (30968b0d96 -> bc8b3f47f4)
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a change to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
from 30968b0d96 Refactor AJP tests to use a common base class
new 013d1eeb4f Fix infinite loop in JsonFormatter for cyclic cause chains
new 6704676af2 Clear handler prefix ThreadLocal when handler creation fails
new a2b46ac588 Open the current log file when no date is set
new c605292fab Avoid NPE in juli formatters for records with no message
new 6baf03eb93 Fix recovery after write failures in JULI FileHandler
new c2da82e743 Ignore unusable JULI Log service providers instead of failing
new 00390ed5d4 Release the log handlers of the given class loader in JULI
new 1e34b34a7a Synchronize access to the bindings of a naming context
new 17cbcb737a Bind the initial context of a container in one step
new aab838c6d1 Fix the class loader used to check the type of a JNDI lookup reference
new 310eaaf375 Ignore the resource description in the Bean resource factory
new 512c27ee6d Report a bean property value which cannot be converted as a NamingException
new 9956636f60 Create the package directory when generating the reflection less code
new fb2c3aa85b Create the storage directory in the back-port tools and do not swallow export failures
new 3020c7b266 Do not swallow exceptions when loading translation property files
new c112834caa Restore the original archive when normalising the timestamps fails
new 5f89ca73f5 Close the per-entry stream when normalising archive timestamps
new 76843eb193 Log the Txt2Html converted file count once per task with the correct total
new 5edca2207a Do not swallow write failures when converting text files to HTML
new a884d3d52b Report a resource link which omits the global attribute as an IllegalArgumentException
new d50dd8b698 Return null from ResourceLinkFactory when the global naming context is unavailable
new bc8b3f47f4 Compatibility
The 22 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../catalina/core/NamingContextListener.java | 12 +-
java/org/apache/juli/ClassLoaderLogManager.java | 5 +-
java/org/apache/juli/FileHandler.java | 70 +++++-
java/org/apache/juli/JsonFormatter.java | 17 +-
java/org/apache/juli/LogUtil.java | 5 +-
java/org/apache/juli/logging/LogFactory.java | 60 +++--
java/org/apache/naming/ContextBindings.java | 27 +++
java/org/apache/naming/NamingContext.java | 71 ++++--
java/org/apache/naming/SelectorContext.java | 10 +-
java/org/apache/naming/factory/BeanFactory.java | 73 +++---
.../apache/naming/factory/LocalStrings.properties | 4 +
java/org/apache/naming/factory/LookupFactory.java | 46 ++--
.../apache/naming/factory/ResourceLinkFactory.java | 19 +-
.../apache/tomcat/buildutil/RepeatableArchive.java | 23 +-
java/org/apache/tomcat/buildutil/Txt2Html.java | 19 +-
.../tomcat/buildutil/translate/BackportBase.java | 5 +-
.../apache/tomcat/buildutil/translate/Utils.java | 8 +-
.../ObjectReflectionPropertyInspector.java | 8 +-
.../org/apache/juli/TestClassLoaderLogManager.java | 42 +++-
test/org/apache/juli/TestFileHandler.java | 139 ++++++++++++
test/org/apache/juli/TestJdkLoggerFormatter.java | 45 ++++
test/org/apache/juli/TestJsonFormatter.java | 41 ++++
test/org/apache/juli/TestLogUtil.java | 6 +
.../TestOneLineFormatter.java} | 31 +--
test/org/apache/juli/logging/TestLogFactory.java | 165 ++++++++++++++
.../juli/logging/TesterLogNoStringConstructor.java | 117 ++++++++++
.../juli/logging/TesterLogStringConstructor.java | 125 +++++++++++
test/org/apache/naming/TestNamingContext.java | 27 +++
.../naming/TestNamingContextConcurrency.java | 137 ++++++++++++
.../naming/TestSelectorContextConcurrency.java | 143 ++++++++++++
.../org/apache/naming/factory/TestBeanFactory.java | 78 +++++++
.../apache/naming/factory/TestLookupFactory.java | 248 +++++++++++++++++++++
.../naming/factory/TestResourceLinkFactory.java | 54 +++++
test/org/apache/naming/factory/TesterBean.java | 20 ++
.../buildutil/translate/TestFixedStrings.java | 7 +-
35 files changed, 1771 insertions(+), 136 deletions(-)
create mode 100644 test/org/apache/juli/TestJdkLoggerFormatter.java
copy test/org/apache/{tomcat/util/http/TestMethod.java => juli/TestOneLineFormatter.java} (51%)
create mode 100644 test/org/apache/juli/logging/TestLogFactory.java
create mode 100644 test/org/apache/juli/logging/TesterLogNoStringConstructor.java
create mode 100644 test/org/apache/juli/logging/TesterLogStringConstructor.java
create mode 100644 test/org/apache/naming/TestNamingContextConcurrency.java
create mode 100644 test/org/apache/naming/TestSelectorContextConcurrency.java
create mode 100644 test/org/apache/naming/factory/TestLookupFactory.java
create mode 100644 test/org/apache/naming/factory/TestResourceLinkFactory.java
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 15/22: Do not swallow exceptions when loading translation property files
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 3020c7b2668753babd0d37367a99621f5c84bd72
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 13:13:41 2026 +0200
Do not swallow exceptions when loading translation property files
Utils.load() caught IOException while reading a LocalStrings*.properties
file, only printed a stack trace, and returned an empty or partially
filled Properties object. All callers treated the result as an empty
file, so a file that could not be read silently lost all its keys in
Export and back-port output, and Import silently produced no output for
the affected language while leaving any stale file from a previous run
in place. Propagate the exception instead. All callers already declared
throws IOException except the TestFixedStrings helper and test methods
which are updated accordingly.
---
java/org/apache/tomcat/buildutil/translate/Utils.java | 4 +---
test/org/apache/tomcat/buildutil/translate/TestFixedStrings.java | 7 ++++---
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/java/org/apache/tomcat/buildutil/translate/Utils.java b/java/org/apache/tomcat/buildutil/translate/Utils.java
index 9d20b5d6ed..34bce56664 100644
--- a/java/org/apache/tomcat/buildutil/translate/Utils.java
+++ b/java/org/apache/tomcat/buildutil/translate/Utils.java
@@ -61,14 +61,12 @@ public class Utils {
}
- static Properties load(File f) {
+ static Properties load(File f) throws IOException {
Properties props = new Properties();
try (FileInputStream fis = new FileInputStream(f);
Reader r = new InputStreamReader(fis, StandardCharsets.UTF_8)) {
props.load(r);
- } catch (IOException ioe) {
- ioe.printStackTrace();
}
return props;
}
diff --git a/test/org/apache/tomcat/buildutil/translate/TestFixedStrings.java b/test/org/apache/tomcat/buildutil/translate/TestFixedStrings.java
index a401611814..42942d86f7 100644
--- a/test/org/apache/tomcat/buildutil/translate/TestFixedStrings.java
+++ b/test/org/apache/tomcat/buildutil/translate/TestFixedStrings.java
@@ -17,6 +17,7 @@
package org.apache.tomcat.buildutil.translate;
import java.io.File;
+import java.io.IOException;
import java.util.Properties;
import org.junit.Assert;
@@ -25,13 +26,13 @@ import org.junit.Test;
public class TestFixedStrings {
@Test
- public void testManagerApps() {
+ public void testManagerApps() throws IOException {
doTestStartsWith("org.apache.catalina.manager", "OK -");
doTestStartsWith("org.apache.catalina.manager.host", "OK -");
}
- private void doTestStartsWith(String packageName, String text) {
+ private void doTestStartsWith(String packageName, String text) throws IOException {
File dir = new File("java/" + packageName.replace('.', '/'));
// English
@@ -52,7 +53,7 @@ public class TestFixedStrings {
}
- private void doTestStartsFile(File fileTranslated, Properties propsEn, String text) {
+ private void doTestStartsFile(File fileTranslated, Properties propsEn, String text) throws IOException {
Properties propsTranslated = Utils.load(fileTranslated);
for (Object key : propsEn.keySet()) {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 19/22: Do not swallow write failures when converting text files to HTML
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 5edca2207aa62b50dc96cf7b10eefd45541ae2d2
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 13:34:57 2026 +0200
Do not swallow write failures when converting text files to HTML
The converted pages were written through a PrintWriter, which swallows
IOExceptions raised by the underlying stream. A mid-stream write failure
such as a full disk was therefore silently ignored and the build
completed successfully with a truncated or empty .html file. Write
directly through an OutputStreamWriter instead, so write and close
failures propagate and are reported as a BuildException by the existing
error handling in execute(). The produced output is unchanged.
---
java/org/apache/tomcat/buildutil/Txt2Html.java | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/java/org/apache/tomcat/buildutil/Txt2Html.java b/java/org/apache/tomcat/buildutil/Txt2Html.java
index 84fb7b9d5f..a61ab337df 100644
--- a/java/org/apache/tomcat/buildutil/Txt2Html.java
+++ b/java/org/apache/tomcat/buildutil/Txt2Html.java
@@ -24,6 +24,8 @@ import java.io.IOException;
import java.io.InputStreamReader;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
+import java.io.Writer;
+import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
@@ -131,10 +133,10 @@ public class Txt2Html extends Task {
// Open files:
try (BufferedReader in =
new BufferedReader(new InputStreamReader(new FileInputStream(from), SOURCE_ENCODING))) {
- try (PrintWriter out = new PrintWriter(new OutputStreamWriter(new FileOutputStream(to), "UTF-8"))) {
+ try (Writer out = new OutputStreamWriter(new FileOutputStream(to), StandardCharsets.UTF_8)) {
// Output header:
- out.print("<!DOCTYPE html><html><head><meta charset=\"UTF-8\" />" +
+ out.write("<!DOCTYPE html><html><head><meta charset=\"UTF-8\" />" +
"<title>Source Code</title></head><body><pre>");
// Convert, line-by-line:
@@ -155,11 +157,11 @@ public class Txt2Html extends Task {
result.append(c);
}
}
- out.print(result.toString() + LINE_SEPARATOR);
+ out.write(result.toString() + LINE_SEPARATOR);
}
// Output footer:
- out.print("</pre></body></html>");
+ out.write("</pre></body></html>");
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 18/22: Log the Txt2Html converted file count once per task with the correct total
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 76843eb19316f98a3f03b1c64d95e22853ed35af
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 13:28:54 2026 +0200
Log the Txt2Html converted file count once per task with the correct total
The converted file counter was declared before the fileset loop but the
'Converted N files to <todir>' message was logged at the end of each
fileset iteration. Every build.xml usage configures several filesets
with a single todir, so the message was printed once per fileset with
an ever-growing cumulative count that overstated the number of files
converted by the later filesets. Move the summary log after the fileset
loop so it is emitted once per task with the total. Also close the
unbalanced quote in the per-file verbose log message.
---
java/org/apache/tomcat/buildutil/Txt2Html.java | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/java/org/apache/tomcat/buildutil/Txt2Html.java b/java/org/apache/tomcat/buildutil/Txt2Html.java
index b54bd26458..84fb7b9d5f 100644
--- a/java/org/apache/tomcat/buildutil/Txt2Html.java
+++ b/java/org/apache/tomcat/buildutil/Txt2Html.java
@@ -101,7 +101,7 @@ public class Txt2Html extends Task {
File from = new File(basedir, file);
File to = new File(todir, file + ".html");
if (!to.exists() || (from.lastModified() > to.lastModified())) {
- log("Converting file '" + from.getAbsolutePath() + "' to '" + to.getAbsolutePath(),
+ log("Converting file '" + from.getAbsolutePath() + "' to '" + to.getAbsolutePath() + "'",
Project.MSG_VERBOSE);
try {
convert(from, to);
@@ -113,9 +113,9 @@ public class Txt2Html extends Task {
count++;
}
}
- if (count > 0) {
- log("Converted " + count + " file" + (count > 1 ? "s" : "") + " to " + todir.getAbsolutePath());
- }
+ }
+ if (count > 0) {
+ log("Converted " + count + " file" + (count > 1 ? "s" : "") + " to " + todir.getAbsolutePath());
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 12/22: Report a bean property value which cannot be converted as a NamingException
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 512c27ee6dafd9de63c39a68f7748ae6b465fc95
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 11:46:14 2026 +0200
Report a bean property value which cannot be converted as a NamingException
BeanFactory converts the value of each configured address of a resource
reference to the type of the bean property which it names, using the
conversion methods of the numeric types and, for a character property,
the first character of the value. A value which does not match the type,
such as a word for a property which is an integer, made one of those
methods throw a NumberFormatException and an empty value for a character
property made the reading of the first character throw a
StringIndexOutOfBoundsException. As neither is one of the exceptions
which the factory catches, it escaped the factory despite the
NamingException which its contract promises for an error while the
instance is created. The failure which reached the operator was then the
unchecked exception alone, which names neither the property nor its
value and so gave no clue which attribute of the resource was
misconfigured.
Such a value now leads to the NamingException which the contract
promises, naming the value, the property and the type, with the
exception of the conversion as its cause. A value which can be converted
is converted as before.
Add tests which check both a value which is not a number for a property
which is an integer and an empty value for a character property.
---
java/org/apache/naming/factory/BeanFactory.java | 70 +++++++++++++---------
.../apache/naming/factory/LocalStrings.properties | 1 +
.../org/apache/naming/factory/TestBeanFactory.java | 51 ++++++++++++++++
test/org/apache/naming/factory/TesterBean.java | 20 +++++++
4 files changed, 114 insertions(+), 28 deletions(-)
diff --git a/java/org/apache/naming/factory/BeanFactory.java b/java/org/apache/naming/factory/BeanFactory.java
index 0d5a785cec..c9d9badb7f 100644
--- a/java/org/apache/naming/factory/BeanFactory.java
+++ b/java/org/apache/naming/factory/BeanFactory.java
@@ -139,38 +139,52 @@ public class BeanFactory implements ObjectFactory {
Class<?> propType = pda[i].getPropertyType();
Method setProp = pda[i].getWriteMethod();
- if (propType.equals(String.class)) {
- valueArray[0] = value;
- } else if (propType.equals(Character.class) || propType.equals(char.class)) {
- valueArray[0] = Character.valueOf(value.charAt(0));
- } else if (propType.equals(Byte.class) || propType.equals(byte.class)) {
- valueArray[0] = Byte.valueOf(value);
- } else if (propType.equals(Short.class) || propType.equals(short.class)) {
- valueArray[0] = Short.valueOf(value);
- } else if (propType.equals(Integer.class) || propType.equals(int.class)) {
- valueArray[0] = Integer.valueOf(value);
- } else if (propType.equals(Long.class) || propType.equals(long.class)) {
- valueArray[0] = Long.valueOf(value);
- } else if (propType.equals(Float.class) || propType.equals(float.class)) {
- valueArray[0] = Float.valueOf(value);
- } else if (propType.equals(Double.class) || propType.equals(double.class)) {
- valueArray[0] = Double.valueOf(value);
- } else if (propType.equals(Boolean.class) || propType.equals(boolean.class)) {
- valueArray[0] = Boolean.valueOf(value);
- } else if (setProp != null) {
- // This is a Tomcat specific extension and is not part of the
- // Java Bean specification.
- String setterName = setProp.getName();
- try {
- setProp = bean.getClass().getMethod(setterName, String.class);
+ // The conversion methods of the numeric types and
+ // the reading of the first character report an
+ // invalid value of a configured property with an
+ // unchecked exception, which is replaced here by a
+ // NamingException which names the property, its
+ // value and its type.
+ try {
+ if (propType.equals(String.class)) {
valueArray[0] = value;
- } catch (NoSuchMethodException nsme) {
+ } else if (propType.equals(Character.class) || propType.equals(char.class)) {
+ valueArray[0] = Character.valueOf(value.charAt(0));
+ } else if (propType.equals(Byte.class) || propType.equals(byte.class)) {
+ valueArray[0] = Byte.valueOf(value);
+ } else if (propType.equals(Short.class) || propType.equals(short.class)) {
+ valueArray[0] = Short.valueOf(value);
+ } else if (propType.equals(Integer.class) || propType.equals(int.class)) {
+ valueArray[0] = Integer.valueOf(value);
+ } else if (propType.equals(Long.class) || propType.equals(long.class)) {
+ valueArray[0] = Long.valueOf(value);
+ } else if (propType.equals(Float.class) || propType.equals(float.class)) {
+ valueArray[0] = Float.valueOf(value);
+ } else if (propType.equals(Double.class) || propType.equals(double.class)) {
+ valueArray[0] = Double.valueOf(value);
+ } else if (propType.equals(Boolean.class) || propType.equals(boolean.class)) {
+ valueArray[0] = Boolean.valueOf(value);
+ } else if (setProp != null) {
+ // This is a Tomcat specific extension and is not part of the
+ // Java Bean specification.
+ String setterName = setProp.getName();
+ try {
+ setProp = bean.getClass().getMethod(setterName, String.class);
+ valueArray[0] = value;
+ } catch (NoSuchMethodException nsme) {
+ throw new NamingException(
+ sm.getString("beanFactory.noStringConversion", propName,
+ propType.getName()));
+ }
+ } else {
throw new NamingException(sm.getString("beanFactory.noStringConversion", propName,
propType.getName()));
}
- } else {
- throw new NamingException(
- sm.getString("beanFactory.noStringConversion", propName, propType.getName()));
+ } catch (NumberFormatException | StringIndexOutOfBoundsException ex) {
+ NamingException ne = new NamingException(sm.getString("beanFactory.invalidValue",
+ value, propName, propType.getName()));
+ ne.initCause(ex);
+ throw ne;
}
if (setProp != null) {
diff --git a/java/org/apache/naming/factory/LocalStrings.properties b/java/org/apache/naming/factory/LocalStrings.properties
index 4a54e94849..04843fa857 100644
--- a/java/org/apache/naming/factory/LocalStrings.properties
+++ b/java/org/apache/naming/factory/LocalStrings.properties
@@ -17,6 +17,7 @@
# To edit translations see: https://tomcat.apache.org/getinvolved.html#Translations
beanFactory.classNotFound=Class not found: [{0}]
+beanFactory.invalidValue=The value [{0}] of the property [{1}] cannot be converted to the type [{2}]
beanFactory.noForceString=The forceString option has been removed as a security hardening measure. Instead, if the setter method doesn't use String, a primitive or a primitive wrapper, the factory will look for a method with the same name as the setter that accepts a String and use that if found.
beanFactory.noSetMethod=No set method found for property [{0}]
beanFactory.noStringConversion=String conversion for property [{0}] of type [{1}] not available
diff --git a/test/org/apache/naming/factory/TestBeanFactory.java b/test/org/apache/naming/factory/TestBeanFactory.java
index 79912c7d0c..ceb2211a7b 100644
--- a/test/org/apache/naming/factory/TestBeanFactory.java
+++ b/test/org/apache/naming/factory/TestBeanFactory.java
@@ -16,6 +16,7 @@
*/
package org.apache.naming.factory;
+import javax.naming.NamingException;
import javax.naming.StringRefAddr;
import org.junit.Assert;
@@ -91,4 +92,54 @@ public class TestBeanFactory {
Assert.assertNotNull(result.getServer());
Assert.assertEquals(IP_ADDRESS, result.getServer().getHostAddress());
}
+
+
+ /*
+ * A value which does not match the type of the property is a configuration
+ * error which has to be reported as a NamingException with the property as
+ * context, not as the unchecked exception of the conversion.
+ */
+ @Test
+ public void testValueNotMatchingThePropertyTypeIsReported() throws Exception {
+
+ ResourceRef resourceRef = new ResourceRef(TesterBean.class.getName(), null, null, null, false);
+ resourceRef.add(new StringRefAddr("limit", "not-a-number"));
+
+ BeanFactory factory = new BeanFactory();
+
+ try {
+ Object obj = factory.getObjectInstance(resourceRef, null, null, null);
+ Assert.fail("The invalid value was accepted: " + obj);
+ } catch (NamingException e) {
+ Assert.assertTrue("The property is not named: " + e.getMessage(), e.getMessage().contains("limit"));
+ Assert.assertTrue("The value is not named: " + e.getMessage(), e.getMessage().contains("not-a-number"));
+ Assert.assertTrue("The reason for the failure is not reported",
+ e.getCause() instanceof NumberFormatException);
+ }
+ }
+
+
+ /*
+ * The value of a character property is read from the first character of
+ * the configured value, so an empty value has to be reported in the same
+ * way as a value which cannot be converted.
+ */
+ @Test
+ public void testEmptyValueForCharacterPropertyIsReported() throws Exception {
+
+ ResourceRef resourceRef = new ResourceRef(TesterBean.class.getName(), null, null, null, false);
+ resourceRef.add(new StringRefAddr("letter", ""));
+
+ BeanFactory factory = new BeanFactory();
+
+ try {
+ Object obj = factory.getObjectInstance(resourceRef, null, null, null);
+ Assert.fail("The invalid value was accepted: " + obj);
+ } catch (NamingException e) {
+ Assert.assertTrue("The property is not named: " + e.getMessage(), e.getMessage().contains("letter"));
+ Assert.assertTrue("The type is not named: " + e.getMessage(), e.getMessage().contains("char"));
+ Assert.assertTrue("The reason for the failure is not reported",
+ e.getCause() instanceof StringIndexOutOfBoundsException);
+ }
+ }
}
diff --git a/test/org/apache/naming/factory/TesterBean.java b/test/org/apache/naming/factory/TesterBean.java
index d2beef275c..654e9fef28 100644
--- a/test/org/apache/naming/factory/TesterBean.java
+++ b/test/org/apache/naming/factory/TesterBean.java
@@ -23,6 +23,10 @@ public class TesterBean {
private InetAddress server;
+ private int limit;
+
+ private char letter;
+
public InetAddress getServer() {
return server;
}
@@ -38,4 +42,20 @@ public class TesterBean {
throw new IllegalArgumentException(e);
}
}
+
+ public int getLimit() {
+ return limit;
+ }
+
+ public void setLimit(int limit) {
+ this.limit = limit;
+ }
+
+ public char getLetter() {
+ return letter;
+ }
+
+ public void setLetter(char letter) {
+ this.letter = letter;
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 08/22: Synchronize access to the bindings of a naming context
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 1e34b34a7aa6295422dd3e8c185f784f81e56fdd
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 10:44:26 2026 +0200
Synchronize access to the bindings of a naming context
The bindings of a NamingContext are held in a HashMap that is also shared
with the views of the context which a lookup of the empty name returns.
Application threads read that map while they look resources up, while other
threads bind and unbind resources, for example when NamingContextListener
reacts to the JMX notifications that are sent while a web application is
started or stopped. As the map was used without any synchronization, list()
and listBindings() regularly failed with a ConcurrentModificationException,
and the map itself could be damaged by a write which resized it while
another thread was reading it, which loses bindings and can make a binding
that is there invisible to a reader.
Synchronize every access to the map itself on the map object. The map is
final and it is the same object in the views of the context, so this also
covers the paths which go through a view. Lookups of a sub-context and the
creation of the object which a resource reference points at are left
outside the lock, so no user code runs while the lock is held. The
enumeration which list() and listBindings() return now walks a copy of the
entries which was taken under the lock, rather than the live view of the
values of the map, which is what threw the exception when the map was
written to while the enumeration was still in progress. In unbind(), an
entry is now read and removed under the same lock; for a name which denotes
a binding in a sub-context, the call into that sub-context is made once the
lock is released. The requirement that the map is only used under its own
lock is now documented on the field, since the field is protected and the
constructor which takes the map is public.
---
java/org/apache/naming/NamingContext.java | 71 ++++++++---
.../naming/TestNamingContextConcurrency.java | 137 +++++++++++++++++++++
2 files changed, 193 insertions(+), 15 deletions(-)
diff --git a/java/org/apache/naming/NamingContext.java b/java/org/apache/naming/NamingContext.java
index 2b9c104dde..5fa94b81c5 100644
--- a/java/org/apache/naming/NamingContext.java
+++ b/java/org/apache/naming/NamingContext.java
@@ -16,9 +16,11 @@
*/
package org.apache.naming;
+import java.util.ArrayList;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Hashtable;
+import java.util.List;
import javax.naming.Binding;
import javax.naming.CompositeName;
@@ -113,7 +115,10 @@ public class NamingContext implements Context {
/**
- * Bindings in this Context.
+ * Bindings in this Context. The map is shared with the views of this context that a lookup of the empty name
+ * returns and it is read by the threads performing lookups while resources are bound and unbound on admin
+ * threads, so every access to the map itself must be synchronized on it. The {@link NamingEntry} values carry
+ * their own lock for the creation of singleton objects.
*/
protected final HashMap<String,NamingEntry> bindings;
@@ -200,20 +205,26 @@ public class NamingContext implements Context {
throw new NamingException(sm.getString("namingContext.invalidName"));
}
- NamingEntry entry = bindings.get(name.get(0));
+ NamingEntry entry;
+ boolean leaf;
+ synchronized (bindings) {
+ entry = bindings.get(name.get(0));
+ leaf = name.size() == 1;
+ if (leaf) {
+ bindings.remove(name.get(0));
+ }
+ }
if (entry == null) {
throw new NameNotFoundException(sm.getString("namingContext.nameNotBound", name, name.get(0)));
}
- if (name.size() > 1) {
+ if (!leaf) {
if (entry.type == NamingEntry.CONTEXT) {
((Context) entry.value).unbind(name.getSuffix(1));
} else {
throw new NamingException(sm.getString("namingContext.contextExpected", name.get(0)));
}
- } else {
- bindings.remove(name.get(0));
}
}
@@ -246,10 +257,17 @@ public class NamingContext implements Context {
name = name.getSuffix(1);
}
if (name.isEmpty()) {
- return new NamingContextEnumeration(bindings.values().iterator());
+ List<NamingEntry> entries;
+ synchronized (bindings) {
+ entries = new ArrayList<>(bindings.values());
+ }
+ return new NamingContextEnumeration(entries.iterator());
}
- NamingEntry entry = bindings.get(name.get(0));
+ NamingEntry entry;
+ synchronized (bindings) {
+ entry = bindings.get(name.get(0));
+ }
if (entry == null) {
throw new NameNotFoundException(sm.getString("namingContext.nameNotBound", name, name.get(0)));
@@ -275,10 +293,17 @@ public class NamingContext implements Context {
name = name.getSuffix(1);
}
if (name.isEmpty()) {
- return new NamingContextBindingsEnumeration(bindings.values().iterator(), this);
+ List<NamingEntry> entries;
+ synchronized (bindings) {
+ entries = new ArrayList<>(bindings.values());
+ }
+ return new NamingContextBindingsEnumeration(entries.iterator(), this);
}
- NamingEntry entry = bindings.get(name.get(0));
+ NamingEntry entry;
+ synchronized (bindings) {
+ entry = bindings.get(name.get(0));
+ }
if (entry == null) {
throw new NameNotFoundException(sm.getString("namingContext.nameNotBound", name, name.get(0)));
@@ -311,7 +336,10 @@ public class NamingContext implements Context {
throw new NamingException(sm.getString("namingContext.invalidName"));
}
- NamingEntry entry = bindings.get(name.get(0));
+ NamingEntry entry;
+ synchronized (bindings) {
+ entry = bindings.get(name.get(0));
+ }
if (entry == null) {
throw new NameNotFoundException(sm.getString("namingContext.nameNotBound", name, name.get(0)));
@@ -326,7 +354,9 @@ public class NamingContext implements Context {
} else {
if (entry.type == NamingEntry.CONTEXT) {
((Context) entry.value).close();
- bindings.remove(name.get(0));
+ synchronized (bindings) {
+ bindings.remove(name.get(0));
+ }
} else {
throw new NotContextException(sm.getString("namingContext.contextExpected", name.get(0)));
}
@@ -384,7 +414,10 @@ public class NamingContext implements Context {
return nameParser;
}
- NamingEntry entry = bindings.get(name.get(0));
+ NamingEntry entry;
+ synchronized (bindings) {
+ entry = bindings.get(name.get(0));
+ }
if (entry == null) {
throw new NameNotFoundException(sm.getString("namingContext.nameNotBound", name, name.get(0)));
}
@@ -493,7 +526,10 @@ public class NamingContext implements Context {
return new NamingContext(env, this.name, bindings);
}
- NamingEntry entry = bindings.get(name.get(0));
+ NamingEntry entry;
+ synchronized (bindings) {
+ entry = bindings.get(name.get(0));
+ }
if (entry == null) {
throw new NameNotFoundException(sm.getString("namingContext.nameNotBound", name, name.get(0)));
@@ -604,7 +640,10 @@ public class NamingContext implements Context {
throw new NamingException(sm.getString("namingContext.invalidName"));
}
- NamingEntry entry = bindings.get(name.get(0));
+ NamingEntry entry;
+ synchronized (bindings) {
+ entry = bindings.get(name.get(0));
+ }
if (name.size() > 1) {
if (entry == null) {
@@ -638,7 +677,9 @@ public class NamingContext implements Context {
} else {
entry = new NamingEntry(name.get(0), toBind, NamingEntry.ENTRY);
}
- bindings.put(name.get(0), entry);
+ synchronized (bindings) {
+ bindings.put(name.get(0), entry);
+ }
}
}
diff --git a/test/org/apache/naming/TestNamingContextConcurrency.java b/test/org/apache/naming/TestNamingContextConcurrency.java
new file mode 100644
index 0000000000..00243fc0cf
--- /dev/null
+++ b/test/org/apache/naming/TestNamingContextConcurrency.java
@@ -0,0 +1,137 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.naming;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
+
+import javax.naming.Binding;
+import javax.naming.NameClassPair;
+import javax.naming.NamingEnumeration;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+/*
+ * The bindings of a NamingContext are read by the threads performing lookups while admin
+ * threads bind and unbind resources, as NamingContextListener does on a JMX notification.
+ * Before the map accesses were synchronized, this regularly led to ConcurrentModification-
+ * Exception in list() and listBindings() and could lose updates to the underlying HashMap.
+ */
+public class TestNamingContextConcurrency {
+
+ @Test
+ public void testConcurrentBindUnbindAndListing() throws Exception {
+ // A context that is not registered with the ContextAccessController is writable
+ NamingContext context = new NamingContext(null, "test");
+ // The context returned for the empty name shares the bindings map and its lock
+ NamingContext view = (NamingContext) context.lookup("");
+
+ int stableEntries = 200;
+ for (int i = 0; i < stableEntries; i++) {
+ context.bind("stable" + i, "value" + i);
+ }
+
+ AtomicBoolean stop = new AtomicBoolean(false);
+ AtomicReference<Throwable> failure = new AtomicReference<>();
+ AtomicInteger writes = new AtomicInteger();
+
+ Thread writer = new Thread(() -> {
+ int i = 0;
+ try {
+ while (!stop.get()) {
+ String name = "dynamic" + (i % 50);
+ context.bind(name, "value" + i);
+ context.unbind(name);
+ writes.incrementAndGet();
+ i++;
+ }
+ } catch (Throwable t) {
+ failure.compareAndSet(null, t);
+ }
+ });
+
+ List<Thread> readers = new ArrayList<>();
+ for (int r = 0; r < 4; r++) {
+ readers.add(new Thread(() -> {
+ try {
+ while (!stop.get()) {
+ NamingEnumeration<NameClassPair> names = context.list("");
+ int count = 0;
+ while (names.hasMore()) {
+ names.next();
+ count++;
+ }
+ // The stable entries are never removed, so they must all be there
+ Assert.assertTrue("Missing stable entries in list()", count >= stableEntries);
+ NamingEnumeration<Binding> bindings = context.listBindings("");
+ while (bindings.hasMore()) {
+ bindings.next();
+ }
+ view.list("");
+ view.lookup("stable0");
+ context.lookup("stable0");
+ try {
+ Thread.sleep(20);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ }
+ }
+ } catch (Throwable t) {
+ failure.compareAndSet(null, t);
+ }
+ }));
+ }
+
+ writer.start();
+ for (Thread reader : readers) {
+ reader.start();
+ }
+ Thread.sleep(2000);
+ stop.set(true);
+ writer.join(30000);
+ for (Thread reader : readers) {
+ reader.join(30000);
+ }
+
+ Assert.assertFalse("The writer did not terminate.", writer.isAlive());
+ for (Thread reader : readers) {
+ Assert.assertFalse("A reader did not terminate.", reader.isAlive());
+ }
+ Throwable seen = failure.get();
+ Assert.assertNull("Concurrent access failed: " + seen, seen);
+ Assert.assertTrue("The writer made no progress", writes.get() > 0);
+
+ // The map must still be intact: all stable entries present, no dynamic leftovers
+ NamingEnumeration<Binding> remaining = context.listBindings("");
+ int stableCount = 0;
+ int dynamicCount = 0;
+ while (remaining.hasMore()) {
+ Binding binding = remaining.next();
+ if (binding.getName().startsWith("stable")) {
+ stableCount++;
+ } else if (binding.getName().startsWith("dynamic")) {
+ dynamicCount++;
+ }
+ }
+ Assert.assertEquals(stableEntries, stableCount);
+ Assert.assertEquals("Leftover dynamic entries", 0, dynamicCount);
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 09/22: Bind the initial context of a container in one step
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 17cbcb737a301bb086d3bfbc0465d35938ff955c
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 11:07:47 2026 +0200
Bind the initial context of a container in one step
The context which the initial context of a container uses is created when it
is first needed and is bound to a name built from the name of the object the
current thread is bound to. That name was looked up and the new context was
bound in two steps, so two threads of a container which needed the context
at the same moment could both find that no context was bound yet, both
create one and both bind it, the second binding replacing the first. The
thread whose context was replaced kept using the context it had created, so
the names it bound there were not visible to the other threads of that
container, which showed up as a name which was bound successfully and was
then not bound. The names concerned are the ones which an application binds
through an initial context with a name that does not start with the java:
scheme. The bindings the container makes itself are made under the name of
the container and are not affected.
Read the name and bind the context in a single step instead, with a helper
in ContextBindings which binds the context only if no context is bound to
that name yet and returns the context which is in use. A thread which finds
that another thread was faster then uses the context which was bound rather
than the one it created itself. The helper takes the security token as a
parameter, like the other methods which bind a context, and it returns the
context which is already bound when the caller may not bind anything under
that name.
---
java/org/apache/naming/ContextBindings.java | 27 ++++
java/org/apache/naming/SelectorContext.java | 10 +-
.../naming/TestSelectorContextConcurrency.java | 143 +++++++++++++++++++++
3 files changed, 177 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/naming/ContextBindings.java b/java/org/apache/naming/ContextBindings.java
index 066b9c4ec5..231e30e4ad 100644
--- a/java/org/apache/naming/ContextBindings.java
+++ b/java/org/apache/naming/ContextBindings.java
@@ -105,6 +105,33 @@ public class ContextBindings {
}
+ /**
+ * Binds an object and a naming context, unless a naming context is already bound to that object, and returns the
+ * naming context which is in use for that object. The read and the binding are done in a single step, so that a
+ * caller which finds that another thread bound a context in the meantime uses the context which that thread bound
+ * rather than the one it created itself. The returned context is never <code>null</code>: it is the context which
+ * is bound to the object, or the given one when nothing is bound, which happens when the caller may not bind
+ * anything under that name.
+ *
+ * @param obj Object to bind with naming context
+ * @param context Associated naming context instance
+ * @param token Security token
+ *
+ * @return the naming context which is in use for the object
+ */
+ static Context bindContextIfAbsent(Object obj, Context context, Object token) {
+ if (!ContextAccessController.checkSecurityToken(obj, token)) {
+ // Nothing may be bound under that name by this caller, so the
+ // context which is in use is the one which is already bound, if
+ // there is one, and the given one otherwise.
+ Context bound = getContext(obj);
+ return bound == null ? context : bound;
+ }
+ Context bound = objectBindings.putIfAbsent(obj, context);
+ return bound == null ? context : bound;
+ }
+
+
/**
* Unbinds an object and a naming context.
*
diff --git a/java/org/apache/naming/SelectorContext.java b/java/org/apache/naming/SelectorContext.java
index 8f36da94d7..dc09a3d6e0 100644
--- a/java/org/apache/naming/SelectorContext.java
+++ b/java/org/apache/naming/SelectorContext.java
@@ -354,9 +354,13 @@ public class SelectorContext implements Context {
Context initialContext = ContextBindings.getContext(ICName);
if (initialContext == null) {
// Allocating a new context and binding it to the appropriate
- // name
- initialContext = new NamingContext(env, ICName);
- ContextBindings.bindContext(ICName, initialContext);
+ // name. Other threads may do the same for the same name at the
+ // same time, so the context which is returned is the one which
+ // ended up being bound rather than the one which was created
+ // here. Otherwise the bindings made on a context which was not
+ // bound would not be visible to the other threads using that
+ // name.
+ initialContext = ContextBindings.bindContextIfAbsent(ICName, new NamingContext(env, ICName), null);
}
return initialContext;
} else {
diff --git a/test/org/apache/naming/TestSelectorContextConcurrency.java b/test/org/apache/naming/TestSelectorContextConcurrency.java
new file mode 100644
index 0000000000..e1f4c86c9e
--- /dev/null
+++ b/test/org/apache/naming/TestSelectorContextConcurrency.java
@@ -0,0 +1,143 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.naming;
+
+import java.util.ArrayList;
+import java.util.Enumeration;
+import java.util.Hashtable;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicReference;
+
+import javax.naming.Context;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+/*
+ * The context which the initial context of a container uses is created when it
+ * is first needed and is bound to a name built from the name of the object the
+ * current thread is bound to. Every thread of the container which needs that
+ * context at the same moment may find that no context is bound yet and create
+ * one of its own, and only one of those contexts can be bound. Before the
+ * context was bound with a single atomic step, the bindings made on the
+ * contexts which were not bound were lost, which showed up as names which were
+ * bound successfully and were then not bound.
+ */
+public class TestSelectorContextConcurrency {
+
+ private static final int THREAD_COUNT = 4;
+
+ private static final String CONTAINER_NAME = "container=/concurrency";
+
+
+ @Test
+ public void testConcurrentInitialContextCreation() throws Exception {
+ ContextBindings.bindContext(CONTAINER_NAME, new NamingContext(null, CONTAINER_NAME));
+
+ try {
+ CountDownLatch ready = new CountDownLatch(THREAD_COUNT);
+ CountDownLatch start = new CountDownLatch(1);
+ AtomicReference<Throwable> bindingFailure = new AtomicReference<>();
+ List<Thread> binders = new ArrayList<>();
+
+ for (int i = 0; i < THREAD_COUNT; i++) {
+ final String name = "binding" + i;
+ Thread binder = new Thread(() -> {
+ try {
+ ContextBindings.bindThread(CONTAINER_NAME, null);
+ ready.countDown();
+ start.await();
+ new SelectorContext(new SlowEnvironment(), true).bind(name, name);
+ } catch (Throwable t) {
+ bindingFailure.compareAndSet(null, t);
+ ready.countDown();
+ } finally {
+ ContextBindings.unbindThread(CONTAINER_NAME, null);
+ }
+ });
+ binders.add(binder);
+ binder.start();
+ }
+
+ ready.await();
+ start.countDown();
+ for (Thread binder : binders) {
+ binder.join(30000);
+ }
+
+ Assert.assertNull("A thread failed to bind: " + bindingFailure.get(), bindingFailure.get());
+ for (Thread binder : binders) {
+ Assert.assertFalse("A thread did not terminate.", binder.isAlive());
+ }
+
+ // Every thread bound a name on the initial context of the container,
+ // so all of them must be visible from another thread which is bound
+ // to the same container. The bindings which were made on a context
+ // which was not bound are not visible. A thread is needed because a
+ // naming context is bound to the thread which uses it.
+ AtomicReference<Throwable> lookupFailure = new AtomicReference<>();
+ Thread reader = new Thread(() -> {
+ try {
+ ContextBindings.bindThread(CONTAINER_NAME, null);
+ Context initialContext = new SelectorContext(new Hashtable<>(), true);
+ for (int i = 0; i < THREAD_COUNT; i++) {
+ String name = "binding" + i;
+ Assert.assertEquals("The initial context does not have the binding of thread " + i, name,
+ initialContext.lookup(name));
+ }
+ } catch (Throwable t) {
+ lookupFailure.compareAndSet(null, t);
+ } finally {
+ ContextBindings.unbindThread(CONTAINER_NAME, null);
+ }
+ });
+ reader.start();
+ reader.join(30000);
+
+ Assert.assertFalse("The reader did not terminate.", reader.isAlive());
+ Assert.assertNull("The initial context does not have all the bindings: " + lookupFailure.get(),
+ lookupFailure.get());
+ } finally {
+ ContextBindings.unbindContext(SelectorContext.IC_PREFIX + CONTAINER_NAME, null);
+ ContextBindings.unbindContext(CONTAINER_NAME, null);
+ }
+ }
+
+
+ /**
+ * An environment which is slow to read, so that the threads are still
+ * copying it into the context they created when the first of them binds
+ * that context. Without the delay, the moment in which two threads can both
+ * find that no context is bound is too short for the test to reach it
+ * reliably.
+ */
+ private static class SlowEnvironment extends Hashtable<String,Object> {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ public Enumeration<String> keys() {
+ try {
+ Thread.sleep(200);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ }
+ return super.keys();
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 17/22: Close the per-entry stream when normalising archive timestamps
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 5f89ca73f5b46bb1c85c77854d99034c3073d8e4
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 13:27:44 2026 +0200
Close the per-entry stream when normalising archive timestamps
ZipFile.getInputStream() returns a new stream for every entry and each
stream holds an Inflater until it is closed or the ZipFile is closed.
The entry copy loop never closed the per-entry stream, so these native
resources accumulated for the lifetime of the archive rewrite. Copy each
entry inside a try-with-resources block so the stream is released as
soon as the entry has been written.
---
java/org/apache/tomcat/buildutil/RepeatableArchive.java | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/java/org/apache/tomcat/buildutil/RepeatableArchive.java b/java/org/apache/tomcat/buildutil/RepeatableArchive.java
index 761976d165..81c36b2059 100644
--- a/java/org/apache/tomcat/buildutil/RepeatableArchive.java
+++ b/java/org/apache/tomcat/buildutil/RepeatableArchive.java
@@ -127,11 +127,11 @@ public class RepeatableArchive extends Task {
zipOut.putNextEntry(entry);
- InputStream is = oldZipFile.getInputStream(oldEntry);
-
- int numRead;
- while ((numRead = is.read(buf)) >= 0) {
- zipOut.write(buf, 0, numRead);
+ try (InputStream is = oldZipFile.getInputStream(oldEntry)) {
+ int numRead;
+ while ((numRead = is.read(buf)) >= 0) {
+ zipOut.write(buf, 0, numRead);
+ }
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 05/22: Fix recovery after write failures in JULI FileHandler
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 6baf03eb9307f13e6ed62846e75338869023614a
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 09:54:42 2026 +0200
Fix recovery after write failures in JULI FileHandler
A failure while writing a record was reported through the error
manager but the writer was never reset. Since PrintWriter swallows
the exceptions of the underlying stream instead of propagating them,
every later flush reported nothing either, and the handler stayed
attached to a writer that could no longer complete writes, silently
losing all further records for the lifetime of the handler. The
error flag is now checked explicitly after the write through flush
of the unbuffered mode and a failed writer is discarded, so the
next record reopens the file and logging recovers on its own.
closeWriter() had a related problem. Writing the tail, flushing or
closing could fail, in which case the writer field was left set.
Every subsequent close() then repeated the failure report for the
same writer, which was never released. The writer is now closed and
discarded unconditionally, with a failure of the tail and a failure
of the close each reported at most once.
Added regression tests that fail a write through /dev/full and a
tail that throws, asserting the number of reports and that a failed
writer is not carried over.
---
java/org/apache/juli/FileHandler.java | 38 +++++++++---
test/org/apache/juli/TestFileHandler.java | 96 +++++++++++++++++++++++++++++++
2 files changed, 127 insertions(+), 7 deletions(-)
diff --git a/java/org/apache/juli/FileHandler.java b/java/org/apache/juli/FileHandler.java
index 56de314fe3..48089272b7 100644
--- a/java/org/apache/juli/FileHandler.java
+++ b/java/org/apache/juli/FileHandler.java
@@ -275,6 +275,13 @@ public class FileHandler extends Handler {
writer.write(result);
if (bufferSize.intValue() < 0) {
writer.flush();
+ // PrintWriter swallows the exceptions from the underlying stream instead of
+ // propagating them, so the error flag needs to be checked explicitly after the
+ // flush. Throwing below discards the writer and lets a later record open a new
+ // one, as the next record will find the date unset and re-open the file.
+ if (writer.checkError()) {
+ throw new IOException("Unable to write to the log file");
+ }
}
} else {
reportError("FileHandler is closed or not yet initialized, unable to log [" + result + "]", null,
@@ -282,6 +289,14 @@ public class FileHandler extends Handler {
}
} catch (Exception e) {
reportError(null, e, ErrorManager.WRITE_FAILURE);
+ // Discard the writer, so the next record attempts to reopen the file rather than
+ // remaining attached to a writer that can no longer complete writes
+ writerLock.readLock().unlock();
+ try {
+ closeWriter();
+ } finally {
+ writerLock.readLock().lock();
+ }
}
} finally {
writerLock.readLock().unlock();
@@ -310,13 +325,22 @@ public class FileHandler extends Handler {
if (writer == null) {
return;
}
- writer.write(getFormatter().getTail(this));
- writer.flush();
- writer.close();
- writer = null;
- date = null;
- } catch (Exception e) {
- reportError(null, e, ErrorManager.CLOSE_FAILURE);
+ try {
+ writer.write(getFormatter().getTail(this));
+ writer.flush();
+ } catch (Exception e) {
+ reportError(null, e, ErrorManager.CLOSE_FAILURE);
+ } finally {
+ // The writer is closed and discarded even when the tail could not be written, so a
+ // failure is not repeated by every later close() for a writer that is never released
+ try {
+ writer.close();
+ } catch (Exception e) {
+ reportError(null, e, ErrorManager.CLOSE_FAILURE);
+ }
+ writer = null;
+ date = null;
+ }
} finally {
writerLock.writeLock().unlock();
}
diff --git a/test/org/apache/juli/TestFileHandler.java b/test/org/apache/juli/TestFileHandler.java
index 7b7d9725c5..30c83e735d 100644
--- a/test/org/apache/juli/TestFileHandler.java
+++ b/test/org/apache/juli/TestFileHandler.java
@@ -23,11 +23,15 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.time.LocalDate;
import java.time.LocalDateTime;
+import java.util.logging.ErrorManager;
+import java.util.logging.Formatter;
+import java.util.logging.Handler;
import java.util.logging.Level;
import java.util.logging.LogRecord;
import org.junit.After;
import org.junit.Assert;
+import org.junit.Assume;
import org.junit.Before;
import org.junit.Test;
@@ -166,6 +170,98 @@ public class TestFileHandler {
return count;
}
+ /*
+ * A write failure must be reported, and the failed writer must be discarded so
+ * that the next write retries opening the file rather than silently writing into
+ * the void for the lifetime of the handler.
+ */
+ @Test
+ public void testWriterDiscardedOnWriteFailure() throws Exception {
+ Assume.assumeTrue("Requires /dev/full", new File("/dev/full").canWrite());
+
+ // Buffer size below zero means: write through, no buffering
+ FileHandler handler = new FileHandler("/dev/", "full", "", Integer.valueOf(-1),
+ Boolean.FALSE, Integer.valueOf(-1));
+ try {
+ CountingErrorManager errorManager = new CountingErrorManager();
+ handler.setErrorManager(errorManager);
+
+ // Neither of these can succeed. PrintWriter swallows the resulting
+ // IOExceptions, so a check is needed after the flush.
+ handler.publish(createRecord("first"));
+ handler.publish(createRecord("second"));
+
+ // Opening /dev/full itself must succeed. Otherwise the records would
+ // be reported as failures without the write path ever being exercised.
+ Assert.assertEquals(0, errorManager.openFailures);
+
+ // Before the fix the failures were swallowed and never reported (zero
+ // here). After the fix each failed write is reported once.
+ Assert.assertEquals(2, errorManager.writeFailures);
+
+ // The failed writer must not be re-used: closing it must succeed (it is
+ // a writer to /dev/full), so no further failures are reported.
+ handler.close();
+ Assert.assertEquals(0, errorManager.closeFailures);
+ } finally {
+ handler.close();
+ }
+ }
+
+ /*
+ * If the tail cannot be written or the writer cannot be closed, close() must
+ * still discard the writer. Otherwise every subsequent close() repeats the
+ * failure report for a writer that is never released.
+ */
+ @Test
+ public void testWriterDiscardedOnTailFailure() throws Exception {
+ FileHandler handler = new FileHandler(logsDir.getAbsolutePath(), "tail", "",
+ Integer.valueOf(-1), Boolean.FALSE, Integer.valueOf(-1));
+ handler.setFormatter(new Formatter() {
+ @Override
+ public String format(LogRecord record) {
+ return record.getMessage() + System.lineSeparator();
+ }
+
+ @Override
+ public String getTail(Handler h) {
+ throw new IllegalStateException("Test tail failure");
+ }
+ });
+ CountingErrorManager errorManager = new CountingErrorManager();
+ handler.setErrorManager(errorManager);
+
+ handler.publish(createRecord("message"));
+ handler.close();
+ // Before the fix the writer was left open, so this reported a second time.
+ handler.close();
+
+ Assert.assertEquals(1, errorManager.closeFailures);
+ }
+
+ private static LogRecord createRecord(String message) {
+ LogRecord record = new LogRecord(Level.INFO, message);
+ record.setLoggerName("test");
+ return record;
+ }
+
+ private static final class CountingErrorManager extends ErrorManager {
+ private int closeFailures;
+ private int writeFailures;
+ private int openFailures;
+
+ @Override
+ public void error(String msg, Exception ex, int code) {
+ if (code == ErrorManager.CLOSE_FAILURE) {
+ closeFailures++;
+ } else if (code == ErrorManager.WRITE_FAILURE) {
+ writeFailures++;
+ } else if (code == ErrorManager.OPEN_FAILURE) {
+ openFailures++;
+ }
+ }
+ }
+
private void generateLogFiles(File dir, String prefix, String suffix, int amount)
throws IOException {
for (int i = 0; i < amount; i++) {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 02/22: Clear handler prefix ThreadLocal when handler creation fails
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 6704676af27ca39ef84506e0b4ee11971f5fb622
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 08:59:15 2026 +0200
Clear handler prefix ThreadLocal when handler creation fails
In ClassLoaderLogManager.readConfiguration(InputStream, ClassLoader)
the prefix of the handler being configured is held in a ThreadLocal
while the handler is instantiated, so the handler reads its own
prefixed properties during construction. The reset of that ThreadLocal
only ran on the success path, so a handler that could not be created,
for example due to a typo in the handler class name in a web
application's logging.properties, left its prefix on the configuring
thread. Subsequent property lookups on that thread were then resolved
against the prefixed properties of the failed handler before the
unprefixed ones, which could corrupt the configuration of loggers and
handlers created afterwards.
The reset now runs in a finally block so the prefix is cleared on every
path, including when an Error escapes the handler's static
initialization. The side effect is that the prefix is cleared after the
handler has been added to the handlers map and to the root logger.
Neither operation resolves properties, so there is no change in
behaviour on the success path. Added a regression test that resolves an
unprefixed property after configuring a single broken handler.
---
java/org/apache/juli/ClassLoaderLogManager.java | 5 ++-
.../org/apache/juli/TestClassLoaderLogManager.java | 42 ++++++++++++++++++++--
2 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/juli/ClassLoaderLogManager.java b/java/org/apache/juli/ClassLoaderLogManager.java
index d943d6512e..4e834c3a3d 100644
--- a/java/org/apache/juli/ClassLoaderLogManager.java
+++ b/java/org/apache/juli/ClassLoaderLogManager.java
@@ -566,7 +566,6 @@ public class ClassLoaderLogManager extends LogManager {
// The specification strongly implies all configuration should be done
// during the creation of the handler object.
// This includes setting level, filter, formatter and encoding.
- this.prefix.set(null);
info.handlers.put(handlerName, handler);
if (rootHandlers == null) {
localRootLogger.addHandler(handler);
@@ -575,6 +574,10 @@ public class ClassLoaderLogManager extends LogManager {
// Report error
System.err.println("Handler error");
e.printStackTrace();
+ } finally {
+ // Clear the prefix even when the handler could not be created, so a
+ // stale prefix does not affect property resolution on this thread
+ this.prefix.set(null);
}
}
diff --git a/test/org/apache/juli/TestClassLoaderLogManager.java b/test/org/apache/juli/TestClassLoaderLogManager.java
index b2482af790..dc05850c9f 100644
--- a/test/org/apache/juli/TestClassLoaderLogManager.java
+++ b/test/org/apache/juli/TestClassLoaderLogManager.java
@@ -20,6 +20,9 @@ import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.Random;
import java.util.logging.Level;
@@ -108,6 +111,30 @@ public class TestClassLoaderLogManager {
}
}
+ /*
+ * The prefix of a handler must not remain set on the configuring thread after the instantiation of the handler
+ * fails, as subsequent property lookups on that thread would otherwise first be resolved against the prefixed
+ * properties of the failed handler.
+ */
+ @Test
+ public void testStaleHandlerPrefixAfterHandlerFailure() throws IOException {
+ final String config = "handlers = 10Foo.MissingHandler" + System.lineSeparator()
+ + "10Foo.marker = STALE_PREFIX_VALUE" + System.lineSeparator()
+ + "marker = CLEAN_VALUE" + System.lineSeparator();
+ final ClassLoader cl = new TestClassLoader(config.getBytes(StandardCharsets.UTF_8));
+ final Thread currentThread = Thread.currentThread();
+ final ClassLoader oldCL = currentThread.getContextClassLoader();
+ try {
+ currentThread.setContextClassLoader(cl);
+ final ClassLoaderLogManager logManager = new ClassLoaderLogManager();
+ logManager.readConfiguration();
+ // The failed handler must not leave its prefix behind for this thread
+ Assert.assertEquals("CLEAN_VALUE", logManager.getProperty("marker"));
+ } finally {
+ currentThread.setContextClassLoader(oldCL);
+ }
+ }
+
private static class LoggerCreateThread extends Thread {
private final LogManager logManager;
@@ -161,7 +188,18 @@ public class TestClassLoaderLogManager {
}
}
- private static class TestClassLoader extends ClassLoader implements WebappProperties {
+ private static class TestClassLoader extends URLClassLoader implements WebappProperties {
+
+ private final byte[] loggingProperties;
+
+ TestClassLoader() {
+ this(EMPTY_BYTES);
+ }
+
+ TestClassLoader(byte[] loggingProperties) {
+ super(new URL[0]);
+ this.loggingProperties = loggingProperties;
+ }
@Override
public String getWebappName() {
@@ -186,7 +224,7 @@ public class TestClassLoaderLogManager {
@Override
public InputStream getResourceAsStream(final String resource) {
if ("logging.properties".equals(resource)) {
- return new ByteArrayInputStream(EMPTY_BYTES);
+ return new ByteArrayInputStream(loggingProperties);
}
return null;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 11/22: Ignore the resource description in the Bean resource factory
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 310eaaf37514dbdabf4dff451a8627b6e5e33561
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 11:41:13 2026 +0200
Ignore the resource description in the Bean resource factory
BeanFactory maps each address of the resource reference to a property of
the bean, skipping only the attributes which describe the definition of
the resource rather than the resource itself. The description is such an
attribute: ResourceRef stores it as an address whenever a resource is
defined with a description and NamingContextListener always passes it on.
Almost no bean has a property named description, so the effect was that a
bean resource defined with a description failed every lookup with the
message that no set method was found for the property [description]. For
the rare bean which does have such a property, the description of the
resource definition was silently set on the bean, which was equally wrong
since the description describes the resource, not the object which
implements it.
The description is now skipped in the same way as the scope and the
authentication type. This matches the default factory of the data source,
which only ever reads the property names it knows of and therefore never
passed the description to the object it created.
Add a regression test.
---
java/org/apache/naming/factory/BeanFactory.java | 3 ++-
.../org/apache/naming/factory/TestBeanFactory.java | 27 ++++++++++++++++++++++
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/naming/factory/BeanFactory.java b/java/org/apache/naming/factory/BeanFactory.java
index fde7c5e4a5..0d5a785cec 100644
--- a/java/org/apache/naming/factory/BeanFactory.java
+++ b/java/org/apache/naming/factory/BeanFactory.java
@@ -122,7 +122,8 @@ public class BeanFactory implements ObjectFactory {
String propName = ra.getType();
if (propName.equals(Constants.FACTORY) || propName.equals("scope") || propName.equals("auth") ||
- propName.equals("forceString") || propName.equals("singleton")) {
+ propName.equals("description") || propName.equals("forceString") ||
+ propName.equals("singleton")) {
continue;
}
diff --git a/test/org/apache/naming/factory/TestBeanFactory.java b/test/org/apache/naming/factory/TestBeanFactory.java
index ec85ff6136..79912c7d0c 100644
--- a/test/org/apache/naming/factory/TestBeanFactory.java
+++ b/test/org/apache/naming/factory/TestBeanFactory.java
@@ -64,4 +64,31 @@ public class TestBeanFactory {
Assert.assertNotNull(result.getServer());
Assert.assertEquals(IP_ADDRESS, result.getServer().getHostAddress());
}
+
+
+ /*
+ * The description of a resource is a property of the resource definition
+ * and not a property of the bean, so the factory has to ignore it.
+ */
+ @Test
+ public void testDescriptionIsNotPassedToTheBean() throws Exception {
+
+ // Create the resource definition with a description
+ ResourceRef resourceRef = new ResourceRef(TesterBean.class.getName(), "The tester bean", null, null, false);
+ resourceRef.add(new StringRefAddr("server", IP_ADDRESS));
+
+ // Create the factory
+ BeanFactory factory = new BeanFactory();
+
+ // Use the factory to create the resource from the definition
+ Object obj = factory.getObjectInstance(resourceRef, null, null, null);
+
+ // Check the correct type was created
+ Assert.assertNotNull(obj);
+ Assert.assertEquals(obj.getClass(), TesterBean.class);
+ // Check the description was ignored and only the server field was set
+ TesterBean result = (TesterBean) obj;
+ Assert.assertNotNull(result.getServer());
+ Assert.assertEquals(IP_ADDRESS, result.getServer().getHostAddress());
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 22/22: Compatibility
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit bc8b3f47f4f79cbfc37bb4c1b0d8473e90d3f8de
Author: remm <re...@apache.org>
AuthorDate: Tue Oct 20 23:33:11 2026 +0200
Compatibility
---
java/org/apache/tomcat/buildutil/Txt2Html.java | 1 -
.../apache/naming/factory/TestLookupFactory.java | 32 +++++++++++++++++-----
2 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/java/org/apache/tomcat/buildutil/Txt2Html.java b/java/org/apache/tomcat/buildutil/Txt2Html.java
index a61ab337df..a08dd9c6f8 100644
--- a/java/org/apache/tomcat/buildutil/Txt2Html.java
+++ b/java/org/apache/tomcat/buildutil/Txt2Html.java
@@ -23,7 +23,6 @@ import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.OutputStreamWriter;
-import java.io.PrintWriter;
import java.io.Writer;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
diff --git a/test/org/apache/naming/factory/TestLookupFactory.java b/test/org/apache/naming/factory/TestLookupFactory.java
index 4af5e7ebeb..b73bbe7682 100644
--- a/test/org/apache/naming/factory/TestLookupFactory.java
+++ b/test/org/apache/naming/factory/TestLookupFactory.java
@@ -63,7 +63,7 @@ public class TestLookupFactory extends TomcatBaseTest {
container.bind("comp/env/ref", new LookupRef(MARKER_CLASS, "java:comp/env/target"));
Object found = new javax.naming.InitialContext().lookup("java:comp/env/ref");
- Assert.assertSame(webapp.marker(), found);
+ Assert.assertSame(webapp.getMarker(), found);
}
@@ -79,10 +79,10 @@ public class TestLookupFactory extends TomcatBaseTest {
ejb.setType("Session");
ejb.setLookupName("java:comp/env/target");
ejb.setRemote(MARKER_CLASS);
- webapp.listener().addEjb(ejb);
+ webapp.getListener().addEjb(ejb);
Object found = new javax.naming.InitialContext().lookup("java:comp/env/ejb");
- Assert.assertSame(webapp.marker(), found);
+ Assert.assertSame(webapp.getMarker(), found);
}
@@ -118,7 +118,7 @@ public class TestLookupFactory extends TomcatBaseTest {
ejb.setLookupName("java:comp/env/target");
// The object bound as the target is not an instance of this interface
ejb.setRemote(String.class.getName());
- webapp.listener().addEjb(ejb);
+ webapp.getListener().addEjb(ejb);
try {
Object found = new javax.naming.InitialContext().lookup("java:comp/env/ejb");
@@ -186,8 +186,8 @@ public class TestLookupFactory extends TomcatBaseTest {
// The context of the web application is read only once it is started
NamingContextListener listener = null;
for (LifecycleListener lifecycleListener : ctx.findLifecycleListeners()) {
- if (lifecycleListener instanceof NamingContextListener namingContextListener) {
- listener = namingContextListener;
+ if (lifecycleListener instanceof NamingContextListener) {
+ listener = (NamingContextListener) lifecycleListener;
}
}
Assert.assertNotNull(listener);
@@ -225,6 +225,24 @@ public class TestLookupFactory extends TomcatBaseTest {
* The listener which binds the naming resources of the web application and
* the instance of the class which only that web application can see.
*/
- private record TesterWebapp(NamingContextListener listener, Object marker) {
+ private class TesterWebapp {
+ private final NamingContextListener listener;
+ private final Object marker;
+ TesterWebapp(NamingContextListener listener, Object marker) {
+ this.listener = listener;
+ this.marker = marker;
+ }
+ /**
+ * @return the listener
+ */
+ public NamingContextListener getListener() {
+ return this.listener;
+ }
+ /**
+ * @return the marker
+ */
+ public Object getMarker() {
+ return this.marker;
+ }
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 16/22: Restore the original archive when normalising the timestamps fails
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit c112834caab7f6d653f678f6df83ca9bdfb13f4d
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 13:20:34 2026 +0200
Restore the original archive when normalising the timestamps fails
The archive was atomically renamed to <name>.old before being rewritten
and the .old file was only deleted on success. If an IOException
occurred while rewriting (corrupt entry, disk full, ...) the task
aborted leaving a truncated archive in place of the original, with the
original surviving only as a stray .old file that required manual
recovery. When the operation fails after the rename, move the .old file
back over the partial output before rethrowing, so the input archive is
restored byte for byte. Report a failing restore as a suppressed
exception of the original error.
---
java/org/apache/tomcat/buildutil/RepeatableArchive.java | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/java/org/apache/tomcat/buildutil/RepeatableArchive.java b/java/org/apache/tomcat/buildutil/RepeatableArchive.java
index a67939a285..761976d165 100644
--- a/java/org/apache/tomcat/buildutil/RepeatableArchive.java
+++ b/java/org/apache/tomcat/buildutil/RepeatableArchive.java
@@ -109,9 +109,11 @@ public class RepeatableArchive extends Task {
for (String file : files) {
File archive = new File(basedir, file);
File oldArchive = new File(basedir, file + ".old");
+ boolean renamed = false;
try {
Files.move(archive.toPath(), oldArchive.toPath(), StandardCopyOption.ATOMIC_MOVE);
+ renamed = true;
try (ZipFile oldZipFile = new ZipFile(oldArchive);
ZipOutputStream zipOut = new ZipOutputStream(new FileOutputStream(archive))) {
@@ -139,6 +141,17 @@ public class RepeatableArchive extends Task {
}
Files.delete(oldArchive.toPath());
} catch (IOException ioe) {
+ // The task did not run to completion for this archive. If the original was
+ // renamed, restore it rather than leaving the archive in its current, possibly
+ // truncated, state together with a stray ".old".
+ try {
+ if (renamed) {
+ Files.move(oldArchive.toPath(), archive.toPath(), StandardCopyOption.REPLACE_EXISTING,
+ StandardCopyOption.ATOMIC_MOVE);
+ }
+ } catch (IOException restoreException) {
+ ioe.addSuppressed(restoreException);
+ }
throw new BuildException(ioe);
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 20/22: Report a resource link which omits the global attribute as an IllegalArgumentException
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit a884d3d52b34817351cb4f5e904019bc12a76ccd
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 13:48:54 2026 +0200
Report a resource link which omits the global attribute as an IllegalArgumentException
The global attribute of a resource link is documented as required but
nothing checked it at runtime. A web application configured with a
resource link which omitted the global attribute made the registration
of the link pass a null value to ConcurrentHashMap.put which rejected it
with a NullPointerException. The exception escaped the creation of the
naming context which only handles a NamingException, and the failure
which reached the operator was an unchecked exception naming neither
the link nor the missing attribute, while the deployment of the whole
web application failed.
The registration of the access to a global resource now checks both
names up front and reports the misconfiguration as an
IllegalArgumentException naming the link, consistent with the other
validation failures of the factory. The deployment fails as before but
with a message which identifies the faulty resource link. Add a test
which checks the rejection of a registration missing either name.
---
.../apache/naming/factory/LocalStrings.properties | 1 +
.../apache/naming/factory/ResourceLinkFactory.java | 6 +++++
test/org/apache/naming/TestNamingContext.java | 27 ++++++++++++++++++++++
3 files changed, 34 insertions(+)
diff --git a/java/org/apache/naming/factory/LocalStrings.properties b/java/org/apache/naming/factory/LocalStrings.properties
index 04843fa857..a9c5074c0e 100644
--- a/java/org/apache/naming/factory/LocalStrings.properties
+++ b/java/org/apache/naming/factory/LocalStrings.properties
@@ -38,6 +38,7 @@ lookupFactory.typeMismatch=The JNDI reference [{0}] was expected to be of type [
resourceFactory.factoryCreationError=Could not create resource factory instance
resourceLinkFactory.invalidGlobalContext=Caller provided invalid global context
+resourceLinkFactory.nullName=The local resource link [{0}] that refers to global resource [{1}] does not specify the required name or global attribute
resourceLinkFactory.nullType=The local resource link [{0}] that refers to global resource [{1}] does not specify the required attribute type
resourceLinkFactory.unknownType=The local resource link [{0}] that refers to global resource [{1}] specified the unknown type [{2}]
resourceLinkFactory.wrongType=The local resource link [{0}] that refers to global resource [{1}] was expected to return an instance of [{2}] but returned an instance of [{3}]
diff --git a/java/org/apache/naming/factory/ResourceLinkFactory.java b/java/org/apache/naming/factory/ResourceLinkFactory.java
index eb993c08d6..40f8ad4e57 100644
--- a/java/org/apache/naming/factory/ResourceLinkFactory.java
+++ b/java/org/apache/naming/factory/ResourceLinkFactory.java
@@ -77,9 +77,15 @@ public class ResourceLinkFactory implements ObjectFactory {
* @param globalContext The global naming context
* @param localName The local name of the resource
* @param globalName The global name of the resource
+ *
+ * @throws IllegalArgumentException if the local or global name is not specified
*/
public static void registerGlobalResourceAccess(Context globalContext, String localName, String globalName) {
validateGlobalContext(globalContext);
+ if (localName == null || globalName == null) {
+ throw new IllegalArgumentException(
+ sm.getString("resourceLinkFactory.nullName", localName, globalName));
+ }
ClassLoader cl = Thread.currentThread().getContextClassLoader();
globalResourceRegistrations.computeIfAbsent(cl, k -> new ConcurrentHashMap<>()).put(localName, globalName);
}
diff --git a/test/org/apache/naming/TestNamingContext.java b/test/org/apache/naming/TestNamingContext.java
index 576f59fbd9..c3aae8dc30 100644
--- a/test/org/apache/naming/TestNamingContext.java
+++ b/test/org/apache/naming/TestNamingContext.java
@@ -16,6 +16,8 @@
*/
package org.apache.naming;
+import java.util.Hashtable;
+
import javax.naming.Context;
import javax.naming.NamingException;
@@ -95,6 +97,31 @@ public class TestNamingContext extends TomcatBaseTest {
}
+ @Test
+ public void testResourceLinkWithoutGlobalName() {
+ // Use a private context to keep the global context validation out of
+ // the way, whatever test order is used within the JVM
+ Context context = new NamingContext(new Hashtable<String,Object>(), "test");
+ ResourceLinkFactory.setGlobalContext(context);
+
+ // A resource link without a global name is a configuration error that
+ // must be reported as such, not as a NullPointerException
+ try {
+ ResourceLinkFactory.registerGlobalResourceAccess(context, LOCAL_NAME, null);
+ Assert.fail("Expected an IllegalArgumentException for the missing global name");
+ } catch (IllegalArgumentException iae) {
+ // Expected
+ }
+
+ try {
+ ResourceLinkFactory.registerGlobalResourceAccess(context, null, GLOBAL_NAME);
+ Assert.fail("Expected an IllegalArgumentException for the missing local name");
+ } catch (IllegalArgumentException iae) {
+ // Expected
+ }
+ }
+
+
@Test
public void testModuleEquivalentToComp() throws Exception {
Tomcat tomcat = getTomcatInstance();
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 14/22: Create the storage directory in the back-port tools and do not swallow export failures
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit fb2c3aa85b49c574edb3b140f957b49e726ec121
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 13:03:01 2026 +0200
Create the storage directory in the back-port tools and do not swallow
export failures
The back-port tools computed <target>/.settings/translations but never
created it. Since .settings is git-ignored, a freshly checked out
target tree - the normal back-port scenario - lacks the directory, the
FileOutputStream in Utils.export() failed with FileNotFoundException
and, as the exception was only printed to stderr, both BackportEnglish
and BackportTranslations completed with exit code 0 having produced no
output at all. Create the storage directory in the BackportBase
constructor and make Utils.export() propagate IOException instead of
swallowing it, so failures abort the run with a clear error. Export
already created the directory itself. Update the now stale constructor
Javadoc accordingly.
---
java/org/apache/tomcat/buildutil/translate/BackportBase.java | 5 ++++-
java/org/apache/tomcat/buildutil/translate/Utils.java | 4 +---
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/java/org/apache/tomcat/buildutil/translate/BackportBase.java b/java/org/apache/tomcat/buildutil/translate/BackportBase.java
index ce12815c74..2e0d83acf5 100644
--- a/java/org/apache/tomcat/buildutil/translate/BackportBase.java
+++ b/java/org/apache/tomcat/buildutil/translate/BackportBase.java
@@ -62,7 +62,7 @@ public abstract class BackportBase {
*
* @param args array with a single element: the target directory path
*
- * @throws IOException if an I/O error occurs while reading translation files
+ * @throws IOException if an I/O error occurs
*/
protected BackportBase(String... args) throws IOException {
if (args.length != 1) {
@@ -89,6 +89,9 @@ public abstract class BackportBase {
targetEnglish = targetTranslations.get("");
storageDir = new File(targetRoot, Constants.STORAGE_DIR);
+ if (!storageDir.exists() && !storageDir.mkdirs()) {
+ throw new IOException("Unable to create directory [" + storageDir.getAbsolutePath() + "]");
+ }
}
/**
diff --git a/java/org/apache/tomcat/buildutil/translate/Utils.java b/java/org/apache/tomcat/buildutil/translate/Utils.java
index 8352b132b7..9d20b5d6ed 100644
--- a/java/org/apache/tomcat/buildutil/translate/Utils.java
+++ b/java/org/apache/tomcat/buildutil/translate/Utils.java
@@ -208,7 +208,7 @@ public class Utils {
}
- static void export(String language, Properties translation, File storageDir) {
+ static void export(String language, Properties translation, File storageDir) throws IOException {
File out = new File(storageDir, Constants.L10N_PREFIX + language + Constants.L10N_SUFFIX);
try (FileOutputStream fos = new FileOutputStream(out);
Writer w = new OutputStreamWriter(fos, StandardCharsets.UTF_8)) {
@@ -217,8 +217,6 @@ public class Utils {
for (String key : keys) {
w.write(key + "=" + formatValueExport(translation.getProperty(key)) + "\n");
}
- } catch (IOException ioe) {
- ioe.printStackTrace();
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 06/22: Ignore unusable JULI Log service providers instead of failing
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit c2da82e743370caf2ed0928be23d7fdc712f24a7
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 10:05:21 2026 +0200
Ignore unusable JULI Log service providers instead of failing
The private constructor of LogFactory runs during the initialization
of the singleton, so any unchecked exception escaping it leaves the
class permanently unusable: every later call of getLog() or
getFactory() fails with NoClassDefFoundError, which takes down the
whole container because nearly every class holds a static Log. Two
cases reached such an exception when a provider of the Log service
was discovered through the ServiceLoader: a provider class without a
public constructor taking the name, for which the code threw a raw
Error, and a provider naming a class that cannot be loaded, for
which the ServiceLoader itself throws a ServiceConfigurationError.
Both cases now simply mark the provider as unusable. Discovery
continues with the next provider and falls back to DirectJDKLog when
no usable one is found, as documented by getFactory() all along.
Providers are now inspected through the ServiceLoader stream API
instead of instantiated objects, so the constructors of providers
that are examined but not chosen are no longer run as a side effect.
Added regression tests that run the discovery in an isolated class
loader with a temporary services file, covering providers without a
name constructor, missing provider classes, malformed services file
lines, a usable provider behind unusable ones and the two unchanged
cases of a single usable provider and no provider at all.
---
java/org/apache/juli/logging/LogFactory.java | 26 +++-
test/org/apache/juli/logging/TestLogFactory.java | 165 +++++++++++++++++++++
.../juli/logging/TesterLogNoStringConstructor.java | 117 +++++++++++++++
.../juli/logging/TesterLogStringConstructor.java | 125 ++++++++++++++++
4 files changed, 425 insertions(+), 8 deletions(-)
diff --git a/java/org/apache/juli/logging/LogFactory.java b/java/org/apache/juli/logging/LogFactory.java
index 4df17a1f0a..3b6bc5e2b0 100644
--- a/java/org/apache/juli/logging/LogFactory.java
+++ b/java/org/apache/juli/logging/LogFactory.java
@@ -18,6 +18,8 @@ package org.apache.juli.logging;
import java.lang.reflect.Constructor;
import java.nio.file.FileSystems;
+import java.util.Iterator;
+import java.util.ServiceConfigurationError;
import java.util.ServiceLoader;
import java.util.logging.LogManager;
@@ -70,16 +72,23 @@ public class LogFactory {
FileSystems.getDefault();
// Look via a ServiceLoader for a Log implementation that has a
- // constructor taking the String name.
+ // constructor taking the String name. Providers that cannot be
+ // loaded or that do not expose the required constructor are
+ // ignored, so a malformed provider does not prevent the discovery
+ // of a usable one, nor the fall back to DirectJDKLog. Note that,
+ // as the ServiceLoader requires, a provider also needs a public
+ // no-argument constructor to be considered at all.
ServiceLoader<Log> logLoader = ServiceLoader.load(Log.class);
Constructor<? extends Log> m = null;
- for (Log log : logLoader) {
- Class<? extends Log> c = log.getClass();
+ Iterator<ServiceLoader.Provider<Log>> providers = logLoader.stream().iterator();
+ while (m == null) {
try {
- m = c.getConstructor(String.class);
- break;
- } catch (NoSuchMethodException | SecurityException e) {
- throw new Error(e);
+ if (!providers.hasNext()) {
+ break;
+ }
+ m = providers.next().type().getConstructor(String.class);
+ } catch (ReflectiveOperationException | SecurityException | ServiceConfigurationError e) {
+ // Not a usable provider. Ignore it and look at the next one.
}
}
discoveredLogConstructor = m;
@@ -145,7 +154,8 @@ public class LogFactory {
* <p>
* Return the singleton <code>LogFactory</code> instance. The {@link Log} implementation is discovered once, using
* a {@link java.util.ServiceLoader} lookup for providers of the <code>Log</code> interface, falling back to
- * {@link DirectJDKLog} if no suitable provider is found.
+ * {@link DirectJDKLog} if no suitable provider is found. Providers that cannot be loaded or that do not expose a
+ * public constructor taking the log name as its only argument are ignored.
* </p>
*
* @return The singleton LogFactory instance
diff --git a/test/org/apache/juli/logging/TestLogFactory.java b/test/org/apache/juli/logging/TestLogFactory.java
new file mode 100644
index 0000000000..69d595ff79
--- /dev/null
+++ b/test/org/apache/juli/logging/TestLogFactory.java
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.juli.logging;
+
+import java.io.File;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestLogFactory {
+
+ private static final String NO_STRING_CONSTRUCTOR = TesterLogNoStringConstructor.class.getName();
+ private static final String STRING_CONSTRUCTOR = TesterLogStringConstructor.class.getName();
+ private static final String MISSING = "org.apache.juli.logging.TesterMissingProvider";
+ private static final String DIRECT_JDK_LOG = DirectJDKLog.class.getName();
+
+ /*
+ * A provider without the constructor taking the log name cannot be used. It must be
+ * ignored, with the discovery falling back to DirectJDKLog. Before the fix a raw Error
+ * was thrown from the singleton initialization, so all later calls failed with
+ * NoClassDefFoundError.
+ */
+ @Test
+ public void testProviderWithoutStringConstructorIsIgnored() throws Exception {
+ Object log = getLogFromIsolatedFactory(NO_STRING_CONSTRUCTOR);
+ Assert.assertEquals(DIRECT_JDK_LOG, log.getClass().getName());
+ }
+
+
+ /*
+ * A provider that names a class that does not exist makes the ServiceLoader throw a
+ * ServiceConfigurationError while iterating. That must not prevent discovery either.
+ */
+ @Test
+ public void testMissingProviderClassIsIgnored() throws Exception {
+ Object log = getLogFromIsolatedFactory(MISSING);
+ Assert.assertEquals(DIRECT_JDK_LOG, log.getClass().getName());
+ }
+
+
+ /*
+ * A syntactically invalid line in a services file also makes the ServiceLoader throw
+ * a ServiceConfigurationError. The factory must fall back instead of failing. Note
+ * that the JDK abandons the whole file in this case, so a usable provider in the same
+ * file is not found and DirectJDKLog is used.
+ */
+ @Test
+ public void testMalformedProviderLineIsIgnored() throws Exception {
+ Object log = getLogFromIsolatedFactory("not a class name!!");
+ Assert.assertEquals(DIRECT_JDK_LOG, log.getClass().getName());
+ }
+
+
+ /*
+ * Discovery must continue past unusable providers and use the first usable one.
+ */
+ @Test
+ public void testUsableProviderAfterUnusableOnesIsDiscovered() throws Exception {
+ Object log = getLogFromIsolatedFactory(MISSING, NO_STRING_CONSTRUCTOR, STRING_CONSTRUCTOR);
+ Assert.assertEquals(STRING_CONSTRUCTOR, log.getClass().getName());
+ }
+
+
+ /*
+ * The regular case: a usable provider is discovered and used. Calling getLog() twice
+ * also pins that the singleton initialization does not fail on first use.
+ */
+ @Test
+ public void testUsableProviderIsDiscovered() throws Exception {
+ Object log = getLogFromIsolatedFactory(STRING_CONSTRUCTOR);
+ Assert.assertEquals(STRING_CONSTRUCTOR, log.getClass().getName());
+ }
+
+
+ /*
+ * Without any provider on the class path the fallback to DirectJDKLog must work.
+ */
+ @Test
+ public void testNoProviderFallsBackToDirectJDKLog() throws Exception {
+ Object log = getLogFromIsolatedFactory();
+ Assert.assertEquals(DIRECT_JDK_LOG, log.getClass().getName());
+ }
+
+
+ /*
+ * Loads a fresh copy of the LogFactory in an isolated class loader in which the given
+ * names are registered as the only providers of the Log service, calls getLog() twice
+ * and returns the class name of the resulting Log instance. The returned object is of
+ * a Log type defined by the isolated class loader, so it can only be inspected as an
+ * Object.
+ */
+ private Object getLogFromIsolatedFactory(String... providerNames) throws Exception {
+ Path tempDir = Files.createTempDirectory("juli-logfactory-test");
+ try {
+ Path servicesDir = Files.createDirectories(tempDir.resolve("META-INF/services"));
+ StringBuilder servicesFile = new StringBuilder();
+ for (String providerName : providerNames) {
+ servicesFile.append(providerName).append(System.lineSeparator());
+ }
+ Files.write(servicesDir.resolve(Log.class.getName()),
+ servicesFile.toString().getBytes(StandardCharsets.UTF_8));
+
+ // The platform class loader as parent keeps the Log interface and the
+ // LogFactory of this class loader out of the way, so the isolated copy
+ // of the factory really runs its own discovery
+ URL[] urls = { codeSourceOf(LogFactory.class), codeSourceOf(TestLogFactory.class),
+ tempDir.toUri().toURL() };
+ try (URLClassLoader isolatedLoader = new URLClassLoader(urls, ClassLoader.getPlatformClassLoader())) {
+ Thread currentThread = Thread.currentThread();
+ ClassLoader originalLoader = currentThread.getContextClassLoader();
+ // The ServiceLoader used by LogFactory looks at the context class loader
+ currentThread.setContextClassLoader(isolatedLoader);
+ try {
+ Class<?> isolatedFactory = isolatedLoader.loadClass(LogFactory.class.getName());
+ Object result = null;
+ // Call twice, the second call must not fail with NoClassDefFoundError
+ for (int i = 0; i < 2; i++) {
+ result = isolatedFactory.getMethod("getLog", String.class).invoke(null, "tester");
+ Assert.assertNotNull(result);
+ }
+ return result;
+ } finally {
+ currentThread.setContextClassLoader(originalLoader);
+ }
+ }
+ } finally {
+ deleteRecursively(tempDir.toFile());
+ }
+ }
+
+
+ private static URL codeSourceOf(Class<?> clazz) throws Exception {
+ return clazz.getProtectionDomain().getCodeSource().getLocation();
+ }
+
+
+ private static void deleteRecursively(File file) {
+ File[] children = file.listFiles();
+ if (children != null) {
+ for (File child : children) {
+ deleteRecursively(child);
+ }
+ }
+ Assert.assertTrue("Failed to delete [" + file + "]", file.delete());
+ }
+}
diff --git a/test/org/apache/juli/logging/TesterLogNoStringConstructor.java b/test/org/apache/juli/logging/TesterLogNoStringConstructor.java
new file mode 100644
index 0000000000..454703102b
--- /dev/null
+++ b/test/org/apache/juli/logging/TesterLogNoStringConstructor.java
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.juli.logging;
+
+/*
+ * Service provider for the tests of LogFactory that offers only the no-argument
+ * constructor required by the ServiceLoader itself, but not the constructor
+ * taking the log name, so LogFactory must ignore it.
+ */
+public class TesterLogNoStringConstructor implements Log {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ public boolean isDebugEnabled() {
+ return false;
+ }
+
+ @Override
+ public boolean isErrorEnabled() {
+ return false;
+ }
+
+ @Override
+ public boolean isFatalEnabled() {
+ return false;
+ }
+
+ @Override
+ public boolean isInfoEnabled() {
+ return false;
+ }
+
+ @Override
+ public boolean isTraceEnabled() {
+ return false;
+ }
+
+ @Override
+ public boolean isWarnEnabled() {
+ return false;
+ }
+
+ @Override
+ public void trace(Object message) {
+ // NO-OP
+ }
+
+ @Override
+ public void trace(Object message, Throwable t) {
+ // NO-OP
+ }
+
+ @Override
+ public void debug(Object message) {
+ // NO-OP
+ }
+
+ @Override
+ public void debug(Object message, Throwable t) {
+ // NO-OP
+ }
+
+ @Override
+ public void info(Object message) {
+ // NO-OP
+ }
+
+ @Override
+ public void info(Object message, Throwable t) {
+ // NO-OP
+ }
+
+ @Override
+ public void warn(Object message) {
+ // NO-OP
+ }
+
+ @Override
+ public void warn(Object message, Throwable t) {
+ // NO-OP
+ }
+
+ @Override
+ public void error(Object message) {
+ // NO-OP
+ }
+
+ @Override
+ public void error(Object message, Throwable t) {
+ // NO-OP
+ }
+
+ @Override
+ public void fatal(Object message) {
+ // NO-OP
+ }
+
+ @Override
+ public void fatal(Object message, Throwable t) {
+ // NO-OP
+ }
+}
diff --git a/test/org/apache/juli/logging/TesterLogStringConstructor.java b/test/org/apache/juli/logging/TesterLogStringConstructor.java
new file mode 100644
index 0000000000..50b0628721
--- /dev/null
+++ b/test/org/apache/juli/logging/TesterLogStringConstructor.java
@@ -0,0 +1,125 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.juli.logging;
+
+/*
+ * Service provider for the tests of LogFactory that offers both the
+ * no-argument constructor required by the ServiceLoader itself and the
+ * constructor taking the log name, so LogFactory must discover and use it.
+ */
+public class TesterLogStringConstructor implements Log {
+
+ private static final long serialVersionUID = 1L;
+
+ public TesterLogStringConstructor() {
+ // Required no-argument constructor for the ServiceLoader
+ }
+
+ public TesterLogStringConstructor(String name) {
+ // Used by LogFactory to create the instance for the given name
+ }
+
+ @Override
+ public boolean isDebugEnabled() {
+ return false;
+ }
+
+ @Override
+ public boolean isErrorEnabled() {
+ return false;
+ }
+
+ @Override
+ public boolean isFatalEnabled() {
+ return false;
+ }
+
+ @Override
+ public boolean isInfoEnabled() {
+ return false;
+ }
+
+ @Override
+ public boolean isTraceEnabled() {
+ return false;
+ }
+
+ @Override
+ public boolean isWarnEnabled() {
+ return false;
+ }
+
+ @Override
+ public void trace(Object message) {
+ // NO-OP
+ }
+
+ @Override
+ public void trace(Object message, Throwable t) {
+ // NO-OP
+ }
+
+ @Override
+ public void debug(Object message) {
+ // NO-OP
+ }
+
+ @Override
+ public void debug(Object message, Throwable t) {
+ // NO-OP
+ }
+
+ @Override
+ public void info(Object message) {
+ // NO-OP
+ }
+
+ @Override
+ public void info(Object message, Throwable t) {
+ // NO-OP
+ }
+
+ @Override
+ public void warn(Object message) {
+ // NO-OP
+ }
+
+ @Override
+ public void warn(Object message, Throwable t) {
+ // NO-OP
+ }
+
+ @Override
+ public void error(Object message) {
+ // NO-OP
+ }
+
+ @Override
+ public void error(Object message, Throwable t) {
+ // NO-OP
+ }
+
+ @Override
+ public void fatal(Object message) {
+ // NO-OP
+ }
+
+ @Override
+ public void fatal(Object message, Throwable t) {
+ // NO-OP
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 01/22: Fix infinite loop in JsonFormatter for cyclic cause chains
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 013d1eeb4f17997d66a4187509803b95068b68d7
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 08:53:00 2026 +0200
Fix infinite loop in JsonFormatter for cyclic cause chains
When a LogRecord throwable has a cyclic cause chain, the do/while loop
in format() that walks getCause() never terminates, appending to the
StringBuilder until the JVM runs out of memory, or hanging the writing
thread indefinitely on a large heap. Throwable.initCause() only rejects
a direct self-reference, so a cycle of length two or more is legal,
plain application code. With AsyncFileHandler the single writer thread
would be permanently lost, taking down all async logging.
The walk of the chain now tracks the visited exceptions in an
identity-based set and stops at the first repeated throwable, emitting
it as a circular reference entry, in the manner of the output of
Throwable.printStackTrace(). Non-cyclic chains produce byte-identical
output. Added a regression test which, without the fix, fails with an
OutOfMemoryError.
---
java/org/apache/juli/JsonFormatter.java | 12 ++++++++++++
test/org/apache/juli/TestJsonFormatter.java | 25 +++++++++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/java/org/apache/juli/JsonFormatter.java b/java/org/apache/juli/JsonFormatter.java
index d954df7664..757fe61795 100644
--- a/java/org/apache/juli/JsonFormatter.java
+++ b/java/org/apache/juli/JsonFormatter.java
@@ -16,6 +16,9 @@
*/
package org.apache.juli;
+import java.util.Collections;
+import java.util.IdentityHashMap;
+import java.util.Set;
import java.util.logging.LogManager;
import java.util.logging.LogRecord;
@@ -93,12 +96,21 @@ public class JsonFormatter extends OneLineFormatter {
// Stack trace
sb.append("\"throwable\": [");
boolean first = true;
+ // A cause chain may legally contain cycles (Throwable.initCause() only
+ // rejects a direct self-reference) so track the visited exceptions and
+ // stop the walk at the first repeated throwable, reporting it as a
+ // circular reference in the manner of Throwable.printStackTrace().
+ Set<Throwable> seen = Collections.newSetFromMap(new IdentityHashMap<>());
do {
if (!first) {
sb.append(',');
} else {
first = false;
}
+ if (!seen.add(t)) {
+ sb.append("\"[Circular reference] ").append(JSONFilter.escape(t.toString())).append('\"');
+ break;
+ }
sb.append('\"').append(JSONFilter.escape(t.toString())).append('\"');
for (StackTraceElement element : t.getStackTrace()) {
sb.append(',').append('\"').append(' ').append(JSONFilter.escape(element.toString())).append('\"');
diff --git a/test/org/apache/juli/TestJsonFormatter.java b/test/org/apache/juli/TestJsonFormatter.java
index cf8938bd52..9640b4a005 100644
--- a/test/org/apache/juli/TestJsonFormatter.java
+++ b/test/org/apache/juli/TestJsonFormatter.java
@@ -55,6 +55,31 @@ public class TestJsonFormatter {
Assert.assertEquals(trace.get(0), "java.lang.IllegalStateException: Bad state");
}
+ @Test
+ public void testCircularCause() throws Exception {
+
+ Exception a = new Exception("A");
+ Exception b = new Exception("B");
+ a.initCause(b);
+ b.initCause(a);
+
+ LogRecord logRecord = new LogRecord(Level.FINE, "Test log message");
+ logRecord.setThrown(a);
+
+ Formatter formatter = new JsonFormatter();
+
+ // Without cycle detection this call would never complete
+ String result = formatter.format(logRecord);
+
+ // Verify JSON content
+ Assert.assertTrue(result.startsWith("{"));
+ JSONParser parser = new JSONParser(new StringReader(result));
+ LinkedHashMap<String,Object> json = parser.object();
+ @SuppressWarnings("unchecked")
+ ArrayList<Object> trace = (ArrayList<Object>) json.get("throwable");
+ Assert.assertTrue(String.valueOf(trace.get(trace.size() - 1)).startsWith("[Circular reference]"));
+ }
+
@Test
public void testEscaping() {
String[][] tests = new String[][] {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 07/22: Release the log handlers of the given class loader in JULI
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 00390ed5d43e3cc423e129e97dea200715d49270
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 10:26:37 2026 +0200
Release the log handlers of the given class loader in JULI
LogFactory.release() never used its class loader argument. Since the
JULI log manager resolves the configuration to reset through the
context class loader of the current thread, the handlers of the
stopping application were only closed when the thread happened to run
with that class loader as the context one, and the handlers of
whatever other class loader was current closed instead, leaking the
open log files of the application until the VM exited.
The release path of the container binds its thread to the class
loader to stop beforehand, so there the argument and the context
class loader already agree and the reset is a plain reset as before.
In the other cases, the class loader to release is now made the
context class loader for the duration of the reset and the previous
one restored afterwards. The argument is documented as required and
as briefly creating a missing configuration to release it, matching
what the reset does internally.
---
java/org/apache/juli/logging/LogFactory.java | 34 +++++++++++++++++++++-------
1 file changed, 26 insertions(+), 8 deletions(-)
diff --git a/java/org/apache/juli/logging/LogFactory.java b/java/org/apache/juli/logging/LogFactory.java
index 3b6bc5e2b0..285b5672f0 100644
--- a/java/org/apache/juli/logging/LogFactory.java
+++ b/java/org/apache/juli/logging/LogFactory.java
@@ -197,18 +197,36 @@ public class LogFactory {
/**
- * Reset the {@link java.util.logging.LogManager}, unless the standard <code>java.util.logging.LogManager</code>
- * implementation is in use. The given class loader is not used: the JULI log manager associates its configuration
- * with the current thread's context class loader.
+ * Release the log handlers that the JULI {@link java.util.logging.LogManager} has associated with the given class
+ * loader, so a stopped application no longer keeps its log files open. If the class loader has no logging
+ * configuration yet, one is briefly created for it and released again. This is a NO-OP when the standard
+ * <code>java.util.logging.LogManager</code> implementation is in use: that one has a single global configuration
+ * and resetting it would discard the logging of the whole JVM. The JULI log manager associates its configuration
+ * with the current thread's context class loader, so the given class loader is made current for the duration of
+ * the reset.
*
- * @param classLoader ClassLoader for which to release the LogFactory. Not used by this implementation.
+ * @param classLoader ClassLoader for which to release the LogFactory. Must not be null.
*/
public static void release(ClassLoader classLoader) {
- // JULI's log manager looks at the current classLoader so there is no
- // need to use the passed in classLoader, the default implementation
- // does not so call reset in that case will break things
+ // JULI's log manager looks at the current classLoader so the
+ // classLoader to release has to be the current one, the default
+ // implementation does not so calling reset in that case would reset
+ // the global configuration and break the logging of the whole JVM
if (!LogManager.getLogManager().getClass().getName().equals("java.util.logging.LogManager")) {
- LogManager.getLogManager().reset();
+ Thread currentThread = Thread.currentThread();
+ ClassLoader originalClassLoader = currentThread.getContextClassLoader();
+ if (originalClassLoader == classLoader) {
+ // The class loader to release is already the current one, as on the
+ // regular container stop path that binds its thread beforehand
+ LogManager.getLogManager().reset();
+ } else {
+ try {
+ currentThread.setContextClassLoader(classLoader);
+ LogManager.getLogManager().reset();
+ } finally {
+ currentThread.setContextClassLoader(originalClassLoader);
+ }
+ }
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 21/22: Return null from ResourceLinkFactory when the global naming context is unavailable
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit d50dd8b6983897f96b1cae2c1f6a383eb3e25733
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 13:55:02 2026 +0200
Return null from ResourceLinkFactory when the global naming context is unavailable
ResourceLinkFactory.getObjectInstance() threw a null pointer exception
when a resource link had been registered while the static global naming
context had not yet been set, which can happen with embedded start-up
ordering because registerGlobalResourceAccess() accepts a null global
context.
Replace the NPE with the standard "cannot resolve" behaviour: log a
warning naming the global resource and return null, which results in the
usual namingContext.failResolvingReference NamingException at the lookup
site. Add a regression test that reproduces the original NPE with the
global context set to null.
---
.../apache/naming/factory/LocalStrings.properties | 1 +
.../apache/naming/factory/ResourceLinkFactory.java | 13 +++++-
.../naming/factory/TestResourceLinkFactory.java | 54 ++++++++++++++++++++++
3 files changed, 67 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/naming/factory/LocalStrings.properties b/java/org/apache/naming/factory/LocalStrings.properties
index a9c5074c0e..b46da5b2b8 100644
--- a/java/org/apache/naming/factory/LocalStrings.properties
+++ b/java/org/apache/naming/factory/LocalStrings.properties
@@ -38,6 +38,7 @@ lookupFactory.typeMismatch=The JNDI reference [{0}] was expected to be of type [
resourceFactory.factoryCreationError=Could not create resource factory instance
resourceLinkFactory.invalidGlobalContext=Caller provided invalid global context
+resourceLinkFactory.noGlobalContext=The global naming context is not available. The local resource link that refers to global resource [{0}] can not be resolved
resourceLinkFactory.nullName=The local resource link [{0}] that refers to global resource [{1}] does not specify the required name or global attribute
resourceLinkFactory.nullType=The local resource link [{0}] that refers to global resource [{1}] does not specify the required attribute type
resourceLinkFactory.unknownType=The local resource link [{0}] that refers to global resource [{1}] specified the unknown type [{2}]
diff --git a/java/org/apache/naming/factory/ResourceLinkFactory.java b/java/org/apache/naming/factory/ResourceLinkFactory.java
index 40f8ad4e57..4a959fcc27 100644
--- a/java/org/apache/naming/factory/ResourceLinkFactory.java
+++ b/java/org/apache/naming/factory/ResourceLinkFactory.java
@@ -26,6 +26,8 @@ import javax.naming.NamingException;
import javax.naming.RefAddr;
import javax.naming.spi.ObjectFactory;
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
import org.apache.naming.ResourceLinkRef;
import org.apache.naming.StringManager;
@@ -48,6 +50,8 @@ public class ResourceLinkFactory implements ObjectFactory {
*/
protected static final StringManager sm = StringManager.getManager(ResourceLinkFactory.class);
+ private static final Log log = LogFactory.getLog(ResourceLinkFactory.class);
+
/**
* Global naming context.
*/
@@ -148,7 +152,7 @@ public class ResourceLinkFactory implements ObjectFactory {
* @param nameCtx unused
* @param environment unused
*
- * @return the object instance
+ * @return the object instance or {@code null} if the reference cannot be resolved
*
* @throws IllegalArgumentException if the type of the reference is missing or does not match the type of the
* global resource
@@ -174,6 +178,13 @@ public class ResourceLinkFactory implements ObjectFactory {
if (!validateGlobalResourceAccess(globalName)) {
return null;
}
+ // The global context may be missing if the web application was
+ // registered before the server created its naming context, which
+ // can happen with embedded start-up ordering
+ if (globalContext == null) {
+ log.warn(sm.getString("resourceLinkFactory.noGlobalContext", globalName));
+ return null;
+ }
Object result = globalContext.lookup(globalName);
if (result == null) {
return null;
diff --git a/test/org/apache/naming/factory/TestResourceLinkFactory.java b/test/org/apache/naming/factory/TestResourceLinkFactory.java
new file mode 100644
index 0000000000..8ef35d7ca7
--- /dev/null
+++ b/test/org/apache/naming/factory/TestResourceLinkFactory.java
@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.naming.factory;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.naming.ResourceLinkRef;
+
+public class TestResourceLinkFactory {
+
+ /*
+ * Regression test for the case where a resource link has been registered
+ * before the global context has been set, e.g. when an embedded Tomcat
+ * initialises the container before the naming context for the server is
+ * created. Prior to the fix, this resulted in a null pointer exception.
+ */
+ @Test
+ public void testGetResourceLinkWithoutGlobalContext() throws Exception {
+ // Arrange: no global context is available and the link is registered
+ ResourceLinkFactory.setGlobalContext(null);
+ ResourceLinkFactory.registerGlobalResourceAccess(null, "link", "global");
+ ResourceLinkRef ref = new ResourceLinkRef("type", "global", null, null);
+ ResourceLinkFactory factory = new ResourceLinkFactory();
+ // Act
+ Object result = factory.getObjectInstance(ref, null, null, null);
+ // Assert
+ // Since the global context is not available, the link can not be
+ // resolved
+ Assert.assertNull(result);
+ }
+
+ @After
+ public void tearDown() {
+ // Remove the registration made by the test so that other tests in the
+ // same JVM are not affected by it
+ ResourceLinkFactory.deregisterGlobalResourceAccess(null, "link");
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 10/22: Fix the class loader used to check the type of a JNDI lookup reference
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit aab838c6d1dd1d9fd51817c155a01ca9a57be256
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 11:35:44 2026 +0200
Fix the class loader used to check the type of a JNDI lookup reference
LookupFactory loaded the class named as the type of the reference with
the one-argument Class.forName. That uses the class loader which loaded
the factory itself. The factory ships in catalina.jar and is therefore
loaded by a class loader which is a parent of the class loaders of the
web applications, so it cannot see any class which is local to a web
application. Every reference which declared such a class as its type
failed with a ClassNotFoundException even when the looked up object was
of the expected type. This includes the type of a field annotated with
Resource if that type is a class of the web application, which is the
normal case.
The type is the type the application expects the object to have, so it
is now loaded with the class loader of the application, which is the
context class loader on the threads of a web application. This is also
how the factory class of the reference is loaded a few lines above and
how the sibling ResourceLinkFactory loads the type it checks against.
If the class cannot be loaded, a NamingException with a message naming
the class and the reference now reports the failure instead of letting
the ClassNotFoundException propagate into the generic wrapping of the
JNDI machinery. A reference which declares no type at all now skips the
type check; previously, the null type led to a NullPointerException and
the empty type to a ClassNotFoundException.
With this, the type of a descriptor also has to name the expected type.
For an EJB reference, the type element of the descriptor is the type of
the reference itself, which is "Entity" or "Session" and never a class
name, so NamingContextListener.addEjb now uses the bean interface which
the application expects - the remote interface or, if the descriptor
names none, the home interface - as the type of a lookup reference. If
the descriptor names neither, the type of the looked up object is left
unchecked. Previously, every EJB reference with a lookup-name failed
because the class "Session" or "Entity" cannot be loaded.
Add a regression test which compiles a class into the web application
so that only its own class loader can see it, pinning both the type
load with the class loader of the application and the use of the remote
interface as the type of an EJB reference.
---
.../catalina/core/NamingContextListener.java | 12 +-
.../apache/naming/factory/LocalStrings.properties | 1 +
java/org/apache/naming/factory/LookupFactory.java | 46 +++--
.../apache/naming/factory/TestLookupFactory.java | 230 +++++++++++++++++++++
4 files changed, 275 insertions(+), 14 deletions(-)
diff --git a/java/org/apache/catalina/core/NamingContextListener.java b/java/org/apache/catalina/core/NamingContextListener.java
index 8ea24c293c..ea94bcb2d7 100644
--- a/java/org/apache/catalina/core/NamingContextListener.java
+++ b/java/org/apache/catalina/core/NamingContextListener.java
@@ -649,7 +649,17 @@ public class NamingContextListener implements LifecycleListener, PropertyChangeL
*/
public void addEjb(ContextEjb ejb) {
- Reference ref = lookForLookupRef(ejb);
+ // The type of an EJB reference descriptor is the type of the reference
+ // itself, which is "Entity" or "Session" and never a class name, so the
+ // interface which the application expects is used as the type of a
+ // lookup reference. No type is given when the descriptor names no
+ // interface, which leaves the type of the looked up object unchecked.
+ String lookupName = ejb.getLookupName();
+ Reference ref = null;
+ if (lookupName != null && !lookupName.isEmpty()) {
+ String beanType = ejb.getRemote() != null ? ejb.getRemote() : ejb.getHome();
+ ref = new LookupRef(beanType, lookupName);
+ }
if (ref == null) {
// Create a reference to the EJB.
diff --git a/java/org/apache/naming/factory/LocalStrings.properties b/java/org/apache/naming/factory/LocalStrings.properties
index fe61bc5d2e..4a54e94849 100644
--- a/java/org/apache/naming/factory/LocalStrings.properties
+++ b/java/org/apache/naming/factory/LocalStrings.properties
@@ -29,6 +29,7 @@ factoryBase.factoryCreationError=Could not create resource factory instance
factoryBase.instanceCreationError=Could not create resource instance
lookupFactory.circularReference=Found a circular reference involving [{0}]
+lookupFactory.classNotFound=Could not load the class [{0}] which is declared as the type of the JNDI reference [{1}]
lookupFactory.createFailed=Could not create instance of JNDI lookup factory class
lookupFactory.loadFailed=Could not load JNDI lookup factory class
lookupFactory.typeMismatch=The JNDI reference [{0}] was expected to be of type [{1}] but the lookup [{2}] return an object of type [{3}]
diff --git a/java/org/apache/naming/factory/LookupFactory.java b/java/org/apache/naming/factory/LookupFactory.java
index e6c447f60c..8f0d9d14b0 100644
--- a/java/org/apache/naming/factory/LookupFactory.java
+++ b/java/org/apache/naming/factory/LookupFactory.java
@@ -129,21 +129,41 @@ public class LookupFactory implements ObjectFactory {
}
}
- Class<?> clazz = Class.forName(ref.getClassName());
- if (result != null && !clazz.isAssignableFrom(result.getClass())) {
- String msg = sm.getString("lookupFactory.typeMismatch", name, ref.getClassName(), lookupName,
- result.getClass().getName());
- NamingException ne = new NamingException(msg);
- log.warn(msg, ne);
- // Close the resource we no longer need if we know how to do so
- if (result instanceof AutoCloseable) {
- try {
- ((AutoCloseable) result).close();
- } catch (Exception e) {
- // Ignore
+ String className = ref.getClassName();
+ if (className != null && !className.isEmpty()) {
+ // The type is the type of the object the application
+ // expects, so it is loaded with the class loader of the
+ // application rather than with the one which loaded this
+ // factory.
+ Class<?> clazz;
+ ClassLoader tcl = Thread.currentThread().getContextClassLoader();
+ try {
+ if (tcl != null) {
+ clazz = tcl.loadClass(className);
+ } else {
+ clazz = Class.forName(className);
}
+ } catch (ClassNotFoundException e) {
+ NamingException ex = new NamingException(
+ sm.getString("lookupFactory.classNotFound", className, name));
+ ex.initCause(e);
+ throw ex;
+ }
+ if (result != null && !clazz.isAssignableFrom(result.getClass())) {
+ String msg = sm.getString("lookupFactory.typeMismatch", name, className, lookupName,
+ result.getClass().getName());
+ NamingException ne = new NamingException(msg);
+ log.warn(msg, ne);
+ // Close the resource we no longer need if we know how to do so
+ if (result instanceof AutoCloseable) {
+ try {
+ ((AutoCloseable) result).close();
+ } catch (Exception e) {
+ // Ignore
+ }
+ }
+ throw ne;
}
- throw ne;
}
} finally {
names.get().remove(lookupName);
diff --git a/test/org/apache/naming/factory/TestLookupFactory.java b/test/org/apache/naming/factory/TestLookupFactory.java
new file mode 100644
index 0000000000..4af5e7ebeb
--- /dev/null
+++ b/test/org/apache/naming/factory/TestLookupFactory.java
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.naming.factory;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import javax.naming.Context;
+import javax.tools.JavaCompiler;
+import javax.tools.ToolProvider;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Assume;
+import org.junit.Test;
+
+import org.apache.catalina.LifecycleListener;
+import org.apache.catalina.core.NamingContextListener;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.naming.ContextAccessController;
+import org.apache.naming.ContextBindings;
+import org.apache.naming.LookupRef;
+import org.apache.tomcat.util.descriptor.web.ContextEjb;
+
+/*
+ * The type of a lookup reference is the type which the application expects, so
+ * it has to be loaded with the class loader of the application. The factory
+ * itself is loaded by a class loader which does not see the classes of the
+ * web application, so using the class loader of the factory left the type
+ * unloadable and the lookup failed for every type which was local to the web
+ * application, which includes the type of a field annotated with Resource.
+ */
+public class TestLookupFactory extends TomcatBaseTest {
+
+ private static final String MARKER_CLASS = "tester.Marker";
+
+ private ClassLoader originalContextClassLoader;
+
+
+ @Test
+ public void testTypeOfWebappLocalClassIsLoaded() throws Exception {
+ TesterWebapp webapp = startWebapp();
+
+ Context container = ContextBindings.getClassLoader();
+ container.bind("comp/env/ref", new LookupRef(MARKER_CLASS, "java:comp/env/target"));
+
+ Object found = new javax.naming.InitialContext().lookup("java:comp/env/ref");
+ Assert.assertSame(webapp.marker(), found);
+ }
+
+
+ @Test
+ public void testEjbLookupRefIsCheckedAgainstTheBeanInterface() throws Exception {
+ TesterWebapp webapp = startWebapp();
+
+ ContextEjb ejb = new ContextEjb();
+ ejb.setName("ejb");
+ // The type of an EJB reference descriptor is the type of the
+ // reference, which is neither a class name nor a type the looked up
+ // object can have
+ ejb.setType("Session");
+ ejb.setLookupName("java:comp/env/target");
+ ejb.setRemote(MARKER_CLASS);
+ webapp.listener().addEjb(ejb);
+
+ Object found = new javax.naming.InitialContext().lookup("java:comp/env/ejb");
+ Assert.assertSame(webapp.marker(), found);
+ }
+
+
+ @Test
+ public void testTypeMismatchIsStillReported() throws Exception {
+ TesterWebapp webapp = startWebapp();
+
+ Context container = ContextBindings.getClassLoader();
+ container.bind("comp/env/ref", new LookupRef(String.class.getName(), "java:comp/env/target"));
+
+ try {
+ Object found = new javax.naming.InitialContext().lookup("java:comp/env/ref");
+ Assert.fail("The reference of the wrong type was returned: " + found);
+ } catch (javax.naming.NamingException e) {
+ Assert.assertTrue("The wrong failure was reported: " + e.getMessage(),
+ e.getMessage().contains("expected to be of type"));
+ }
+ }
+
+
+ /*
+ * The type of an EJB reference descriptor names the type of the reference
+ * itself and is never loadable, so the remote interface has to be the type
+ * which the looked up object is checked against.
+ */
+ @Test
+ public void testEjbLookupRefIsCheckedAgainstTheRemoteInterface() throws Exception {
+ TesterWebapp webapp = startWebapp();
+
+ ContextEjb ejb = new ContextEjb();
+ ejb.setName("ejb");
+ ejb.setType("Session");
+ ejb.setLookupName("java:comp/env/target");
+ // The object bound as the target is not an instance of this interface
+ ejb.setRemote(String.class.getName());
+ webapp.listener().addEjb(ejb);
+
+ try {
+ Object found = new javax.naming.InitialContext().lookup("java:comp/env/ejb");
+ Assert.fail("The reference of the wrong type was returned: " + found);
+ } catch (javax.naming.NamingException e) {
+ Assert.assertTrue("The wrong failure was reported: " + e.getMessage(),
+ e.getMessage().contains("expected to be of type"));
+ }
+ }
+
+
+ @Test
+ public void testTypeWhichCannotBeLoadedIsReported() throws Exception {
+ startWebapp();
+
+ String missingClass = MARKER_CLASS + "Missing";
+ Context container = ContextBindings.getClassLoader();
+ container.bind("comp/env/ref", new LookupRef(missingClass, "java:comp/env/target"));
+
+ try {
+ Object found = new javax.naming.InitialContext().lookup("java:comp/env/ref");
+ Assert.fail("The reference of an unloadable type was returned: " + found);
+ } catch (javax.naming.NamingException e) {
+ Assert.assertTrue("The wrong failure was reported: " + e.getMessage(),
+ e.getMessage().contains("Could not load the class"));
+ Assert.assertTrue("The class which cannot be loaded is not named: " + e.getMessage(),
+ e.getMessage().contains(missingClass));
+ Assert.assertTrue("The reason for the failure is not reported",
+ e.getCause() instanceof ClassNotFoundException);
+ }
+ }
+
+
+ /**
+ * Starts a web application with a class which only its own class loader
+ * can see and binds an instance of it as the target of the lookup
+ * references. The context class loader is set to the one of the web
+ * application, as it is for the threads of a web application, and restored
+ * after the test.
+ *
+ * @return the listener of the naming resources and the instance of the class of the web application
+ *
+ * @throws Exception if the web application cannot be started
+ */
+ private TesterWebapp startWebapp() throws Exception {
+ JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
+ Assume.assumeTrue("The compiler of the JDK is not available", compiler != null);
+
+ File docBase = Files.createTempDirectory(getTemporaryDirectory().toPath(), "docBase").toFile();
+ Path classes = new File(docBase, "WEB-INF/classes/").toPath();
+ Files.createDirectories(classes);
+ Path source = new File(docBase, "tester/Marker.java").toPath();
+ Files.createDirectories(source.getParent());
+ Files.write(source, ("package tester;\npublic class Marker {\n}\n".getBytes(StandardCharsets.UTF_8)));
+ ByteArrayOutputStream compilerOutput = new ByteArrayOutputStream();
+ int result = compiler.run(null, compilerOutput, compilerOutput, "-d", classes.toString(),
+ source.toString());
+ Assert.assertEquals("The class of the web application could not be compiled: " + compilerOutput, 0, result);
+
+ Tomcat tomcat = getTomcatInstance();
+ tomcat.enableNaming();
+ org.apache.catalina.Context ctx = tomcat.addContext("", docBase.getAbsolutePath());
+ tomcat.start();
+
+ // The context of the web application is read only once it is started
+ NamingContextListener listener = null;
+ for (LifecycleListener lifecycleListener : ctx.findLifecycleListeners()) {
+ if (lifecycleListener instanceof NamingContextListener namingContextListener) {
+ listener = namingContextListener;
+ }
+ }
+ Assert.assertNotNull(listener);
+ ContextAccessController.setWritable(listener.getName(), ctx.getNamingToken());
+
+ ClassLoader webappLoader = ctx.getLoader().getClassLoader();
+ // The naming context of a web application is bound to its class loader
+ originalContextClassLoader = Thread.currentThread().getContextClassLoader();
+ Thread.currentThread().setContextClassLoader(webappLoader);
+
+ Object marker = webappLoader.loadClass(MARKER_CLASS).getDeclaredConstructor().newInstance();
+ // The target of the references is bound in the context of the web
+ // application, which is the context a lookup resolves names in
+ Context container = ContextBindings.getClassLoader();
+ container.bind("comp/env/target", marker);
+
+ return new TesterWebapp(listener, marker);
+ }
+
+
+ /*
+ * The lookup is done on the test thread with the class loader of the web
+ * application as the context class loader, as it is on the threads of a
+ * web application. It is restored here, which runs before the tear down of
+ * the base class stops the web application, so that neither the stopping
+ * nor the other tests run with the class loader of the web application.
+ */
+ @After
+ public void restoreContextClassLoader() {
+ Thread.currentThread().setContextClassLoader(originalContextClassLoader);
+ }
+
+
+ /**
+ * The listener which binds the naming resources of the web application and
+ * the instance of the class which only that web application can see.
+ */
+ private record TesterWebapp(NamingContextListener listener, Object marker) {
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 04/22: Avoid NPE in juli formatters for records with no message
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit c605292fab19196636b0e3cee33e0c15742fcc04
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 09:31:10 2026 +0200
Avoid NPE in juli formatters for records with no message
A LogRecord with a null message is legal, and Formatter.formatMessage()
then returns null. The message was passed unguarded to LogUtil.escape()
by OneLineFormatter and JdkLoggerFormatter and to
JsonFormatter.JSONFilter.escape() by JsonFormatter, so all three
formatters threw a NullPointerException instead of formatting the
record. When used with a handler that catches the exception, such as
FileHandler, the record was silently lost. The formatters are public
API that can be used with any handler, where the exception propagated
out of the logging call of the application.
Both escaping utilities now handle a null input as an empty string and
the handling is documented in their Javadoc. Records with no message
are formatted with an empty message, consistently in all three
formatters. Added regression tests for the two utilities and for each
formatter.
---
java/org/apache/juli/JsonFormatter.java | 5 ++-
java/org/apache/juli/LogUtil.java | 5 ++-
test/org/apache/juli/TestJdkLoggerFormatter.java | 45 ++++++++++++++++++++++++
test/org/apache/juli/TestJsonFormatter.java | 16 +++++++++
test/org/apache/juli/TestLogUtil.java | 6 ++++
test/org/apache/juli/TestOneLineFormatter.java | 44 +++++++++++++++++++++++
6 files changed, 119 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/juli/JsonFormatter.java b/java/org/apache/juli/JsonFormatter.java
index 757fe61795..d1b23a2ca2 100644
--- a/java/org/apache/juli/JsonFormatter.java
+++ b/java/org/apache/juli/JsonFormatter.java
@@ -139,11 +139,14 @@ public class JsonFormatter extends OneLineFormatter {
/**
* Escape the given string.
*
- * @param input the string
+ * @param input the string. A null value is handled as an empty string.
*
* @return the escaped string
*/
public static String escape(String input) {
+ if (input == null) {
+ return "";
+ }
return escape(input, 0, input.length()).toString();
}
diff --git a/java/org/apache/juli/LogUtil.java b/java/org/apache/juli/LogUtil.java
index 6f14afc916..f0458baab6 100644
--- a/java/org/apache/juli/LogUtil.java
+++ b/java/org/apache/juli/LogUtil.java
@@ -33,12 +33,15 @@ public class LogUtil {
* All control characters are escaped apart from horizontal tab (\\u0009), new line (\\u000a) and carriage return
* (\\u000d).
*
- * @param input The string to escape
+ * @param input The string to escape. A null value is handled as an empty string.
*
* @return The escaped form of the input string
*/
@SuppressWarnings("null") // sb is not null when used
public static String escape(final String input) {
+ if (input == null) {
+ return "";
+ }
final int len = input.length();
int i = 0;
int lastControl = -1;
diff --git a/test/org/apache/juli/TestJdkLoggerFormatter.java b/test/org/apache/juli/TestJdkLoggerFormatter.java
new file mode 100644
index 0000000000..06de6f8a00
--- /dev/null
+++ b/test/org/apache/juli/TestJdkLoggerFormatter.java
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.juli;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestJdkLoggerFormatter {
+
+ /*
+ * A record with no message is legal. The formatter must render it, with the message as an empty string, rather
+ * than throw a NullPointerException.
+ */
+ @Test
+ public void testNullMessage() {
+ JdkLoggerFormatter formatter = new JdkLoggerFormatter();
+ LogRecord logRecord = new LogRecord(Level.FINE, null);
+ logRecord.setLoggerName("test");
+
+ String result = formatter.format(logRecord);
+
+ // With an empty message the line has nothing after the padding of the logger name, in particular the
+ // string "null" must not appear in place of the message
+ Assert.assertTrue("Unexpected result [" + result + "]",
+ result.endsWith(System.lineSeparator()));
+ Assert.assertFalse("Unexpected result [" + result + "]", result.contains("null"));
+ }
+}
diff --git a/test/org/apache/juli/TestJsonFormatter.java b/test/org/apache/juli/TestJsonFormatter.java
index 9640b4a005..5b78d48e08 100644
--- a/test/org/apache/juli/TestJsonFormatter.java
+++ b/test/org/apache/juli/TestJsonFormatter.java
@@ -55,6 +55,22 @@ public class TestJsonFormatter {
Assert.assertEquals(trace.get(0), "java.lang.IllegalStateException: Bad state");
}
+ @Test
+ public void testNullMessage() throws Exception {
+
+ Formatter formatter = new JsonFormatter();
+ LogRecord logRecord = new LogRecord(Level.FINE, null);
+ logRecord.setLoggerName("test");
+
+ String result = formatter.format(logRecord);
+
+ // Verify JSON content
+ Assert.assertTrue(result.startsWith("{"));
+ JSONParser parser = new JSONParser(new StringReader(result));
+ LinkedHashMap<String,Object> json = parser.object();
+ Assert.assertEquals("", json.get("message"));
+ }
+
@Test
public void testCircularCause() throws Exception {
diff --git a/test/org/apache/juli/TestLogUtil.java b/test/org/apache/juli/TestLogUtil.java
index 12360c2212..7296092896 100644
--- a/test/org/apache/juli/TestLogUtil.java
+++ b/test/org/apache/juli/TestLogUtil.java
@@ -21,6 +21,12 @@ import org.junit.Test;
public class TestLogUtil {
+ @Test
+ public void testEscapeForLoggingNull() {
+ doTestEscapeForLogging(null, "");
+ }
+
+
@Test
public void testEscapeForLoggingEmptyString() {
doTestEscapeForLogging("");
diff --git a/test/org/apache/juli/TestOneLineFormatter.java b/test/org/apache/juli/TestOneLineFormatter.java
new file mode 100644
index 0000000000..2b093d8ed5
--- /dev/null
+++ b/test/org/apache/juli/TestOneLineFormatter.java
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.juli;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestOneLineFormatter {
+
+ /*
+ * A record with no message is legal. The formatter must render it, with the message as an empty string, rather
+ * than throw a NullPointerException.
+ */
+ @Test
+ public void testNullMessage() {
+ OneLineFormatter formatter = new OneLineFormatter();
+ LogRecord logRecord = new LogRecord(Level.FINE, null);
+ logRecord.setLoggerName("test");
+
+ String result = formatter.format(logRecord);
+
+ // With an empty message the output ends with the separator between the source and the message followed by
+ // the line separator for the next record
+ Assert.assertTrue("Unexpected result [" + result + "]",
+ result.endsWith(" " + System.lineSeparator()));
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 03/22: Open the current log file when no date is set
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit a2b46ac588f85a75fdf4ba92e2a5b6ad7f8d24ca
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 09:03:54 2026 +0200
Open the current log file when no date is set
FileHandler builds the name of the log file from the prefix, the as-of
date of the currently open file and the suffix. The date is normally
set by publish() just before it opens the writer, but a direct call to
open(), the public API used to (re-)open a handler, ran with no date
set, so the concatenation rendered the null date as the literal text
null and created files such as catalina.null.log. Such a file never
matches the pattern used for rotation and cleanup, so it lingered in
the log directory. The reopen path of AsyncFileHandler, which opens a
previously closed handler, was equally affected.
The writer now opens the file of the current date when no date is set,
computing the date in the same way as publish(), whose date logic was
extracted to a shared helper. The computed date only becomes the
handler's as-of date once the writer has been created successfully, so
a failed open can still be retried by a subsequent publish. Added a
regression test that opens a handler before any record was published,
verifies the file of the current date is used for the record published
afterwards, and that reopening a closed handler does not create files
with an unset date.
---
java/org/apache/juli/FileHandler.java | 32 +++++++++++++++++++----
test/org/apache/juli/TestFileHandler.java | 43 +++++++++++++++++++++++++++++++
2 files changed, 70 insertions(+), 5 deletions(-)
diff --git a/java/org/apache/juli/FileHandler.java b/java/org/apache/juli/FileHandler.java
index 0ff54b0e4c..56de314fe3 100644
--- a/java/org/apache/juli/FileHandler.java
+++ b/java/org/apache/juli/FileHandler.java
@@ -234,9 +234,7 @@ public class FileHandler extends Handler {
final String tsDate;
if (rotatable.booleanValue()) {
- // Construct the timestamp we will use
- Timestamp ts = new Timestamp(System.currentTimeMillis());
- tsDate = ts.toString().substring(0, 10);
+ tsDate = obtainCurrentDate();
} else {
tsDate = "";
}
@@ -451,7 +449,8 @@ public class FileHandler extends Handler {
/**
- * Open the new log file for the date specified by <code>date</code>.
+ * Open the new log file. If the handler is rotatable and no date has been set yet, the log file for the current
+ * date is opened.
*/
public void open() {
openWriter();
@@ -475,7 +474,16 @@ public class FileHandler extends Handler {
FileOutputStream fos = null;
OutputStream os = null;
try {
- File pathname = new File(dir.getAbsoluteFile(), prefix + (rotatable.booleanValue() ? date : "") + suffix);
+ // If no date has been set, typically because the handler has been opened before any record was
+ // published, use the current date, as publish() does when it rotates the log file, so the file
+ // name always matches the pattern used for rotation and cleanup
+ String currentDate = date;
+ boolean dateComputed = rotatable.booleanValue() && currentDate == null;
+ if (dateComputed) {
+ currentDate = obtainCurrentDate();
+ }
+ File pathname = new File(dir.getAbsoluteFile(),
+ prefix + (rotatable.booleanValue() ? currentDate : "") + suffix);
File parent = pathname.getParentFile();
if (!parent.mkdirs() && !parent.isDirectory()) {
reportError("Unable to create [" + parent + "]", null, ErrorManager.OPEN_FAILURE);
@@ -488,6 +496,9 @@ public class FileHandler extends Handler {
writer = new PrintWriter(
(encoding != null) ? new OutputStreamWriter(os, encoding) : new OutputStreamWriter(os), false);
writer.write(getFormatter().getHead(this));
+ if (dateComputed) {
+ date = currentDate;
+ }
} catch (Exception e) {
reportError(null, e, ErrorManager.OPEN_FAILURE);
writer = null;
@@ -510,6 +521,17 @@ public class FileHandler extends Handler {
}
}
+ /**
+ * Obtain the date string used as the date part of the names of the rotatable log files.
+ *
+ * @return The current date, formatted as <code>YYYY-MM-DD</code>
+ */
+ private static String obtainCurrentDate() {
+ // Construct the timestamp we will use
+ Timestamp ts = new Timestamp(System.currentTimeMillis());
+ return ts.toString().substring(0, 10);
+ }
+
private void clean() {
if (maxDays.intValue() <= 0 || Files.notExists(getDirectoryAsPath())) {
return;
diff --git a/test/org/apache/juli/TestFileHandler.java b/test/org/apache/juli/TestFileHandler.java
index b6f4f2cdea..7b7d9725c5 100644
--- a/test/org/apache/juli/TestFileHandler.java
+++ b/test/org/apache/juli/TestFileHandler.java
@@ -23,6 +23,8 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.time.LocalDate;
import java.time.LocalDateTime;
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
import org.junit.After;
import org.junit.Assert;
@@ -123,6 +125,47 @@ public class TestFileHandler {
fh1.close();
}
+ /*
+ * Opening a rotatable handler before any record has been published must open the log file for the current date,
+ * rather than a file whose name contains the string representation of the unset date. The same applies when a
+ * closed handler is opened again.
+ */
+ @Test
+ public void testOpenUsesCurrentDate() throws Exception {
+ String prefix = "opendate.";
+ String suffix = ".log";
+
+ FileHandler fh = new FileHandler(logsDir.getAbsolutePath(), prefix, suffix);
+ fh.open();
+
+ String today = LocalDate.now().toString();
+ File expected = new File(logsDir, prefix + today + suffix);
+ Assert.assertTrue("Expected file [" + expected + "] was not created", expected.isFile());
+
+ // Publishing after the open must use the file that was opened, not rotate to a new one
+ fh.publish(new LogRecord(Level.INFO, "hello"));
+ Assert.assertTrue("The published record was not written to [" + expected + "]",
+ Files.readString(expected.toPath()).contains("hello"));
+ Assert.assertEquals(1, countFiles(prefix));
+
+ // Reopening a closed handler must not create a file with an unset date either
+ fh.close();
+ fh.open();
+ Assert.assertEquals(1, countFiles(prefix));
+
+ fh.close();
+ }
+
+ private int countFiles(String prefix) {
+ int count = 0;
+ for (File file : logsDir.listFiles()) {
+ if (file.getName().startsWith(prefix)) {
+ count++;
+ }
+ }
+ return count;
+ }
+
private void generateLogFiles(File dir, String prefix, String suffix, int amount)
throws IOException {
for (int i = 0; i < amount; i++) {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 13/22: Create the package directory when generating the reflection less code
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 9956636f60a40ec478442fb956335d74144c1639
Author: opencode <re...@apache.org>
AuthorDate: Tue Oct 20 12:58:25 2026 +0200
Create the package directory when generating the reflection less code
The code generator wrote XReflectionIntrospectionUtils.java into the
package sub-directory derived from the package name without ever
creating it, so standalone use against a fresh output directory died
with an unhandled FileNotFoundException. Only the Ant
embed-programmatic-jar target was unaffected because it pre-copies the
whole java/ tree. Create the sub-directory with mkdirs() before
generating and fail with a clear IOException when it cannot be
created, and update the Javadoc accordingly.
---
.../util/xreflection/ObjectReflectionPropertyInspector.java | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/tomcat/util/xreflection/ObjectReflectionPropertyInspector.java b/java/org/apache/tomcat/util/xreflection/ObjectReflectionPropertyInspector.java
index 1ab83537df..43a0416d5d 100644
--- a/java/org/apache/tomcat/util/xreflection/ObjectReflectionPropertyInspector.java
+++ b/java/org/apache/tomcat/util/xreflection/ObjectReflectionPropertyInspector.java
@@ -18,6 +18,7 @@ package org.apache.tomcat.util.xreflection;
import java.io.File;
+import java.io.IOException;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
@@ -119,8 +120,8 @@ public final class ObjectReflectionPropertyInspector {
*
* @param baseClasses The classes to process
* @param packageName The package name for generated code
- * @param location The output directory (the package sub-directory derived from <code>packageName</code> must
- * already exist)
+ * @param location The output directory (the package sub-directory derived from <code>packageName</code> is
+ * created if it does not exist)
* @param className The class name for generated code
* @throws Exception if an error occurs
*/
@@ -128,6 +129,9 @@ public final class ObjectReflectionPropertyInspector {
String className) throws Exception {
String packageDirectory = packageName.replace('.', '/');
File sourceFileLocation = new File(location, packageDirectory);
+ if (!sourceFileLocation.exists() && !sourceFileLocation.mkdirs()) {
+ throw new IOException("Unable to create directory [" + sourceFileLocation.getAbsolutePath() + "]");
+ }
ReflectionLessCodeGenerator.generateCode(sourceFileLocation, className, packageName, baseClasses);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org