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 9c31d5b64268a5de40c55b8eac297f01f368d25a Author: opencode <[email protected]> AuthorDate: Thu Nov 5 11:30:46 2026 +0100 Do not log configured property values at TRACE The generic JNDI resource factory logged the full setProperty call, including the configured value, when TRACE was enabled. A password or other secret bean property was therefore written verbatim to the log. Log only the target class and property name at TRACE and keep the value out of the message. Add a regression test that applies a secret value through a String setter and checks that no captured log record contains it. --- .../jdbc/naming/GenericNamingResourcesFactory.java | 13 ++++--- .../tomcat/jdbc/naming/TestNamingFactory.java | 44 ++++++++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/naming/GenericNamingResourcesFactory.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/naming/GenericNamingResourcesFactory.java index 4b0df56c69..ef53408515 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/naming/GenericNamingResourcesFactory.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/naming/GenericNamingResourcesFactory.java @@ -76,12 +76,16 @@ import org.apache.tomcat.jdbc.pool.util.MessageFactory; * {@code false}, ignoring case and surrounding whitespace - not the * anything-but-{@code true}-is-{@code false} coercion of * {@code org.apache.naming.factory.BeanFactory} - and an address carrying no - * content is applied to nothing: missing content is not a configured value. + * content is applied to nothing: missing content is not a configured value + * and, like any other property that cannot be applied, it fails the lookup. * When no {@code setFoo} setter exists at all, a two-argument * {@code setProperty(name, value)} property bag is consulted; when one * exists but its type cannot be built from the configured value, the bag is * not consulted - what the bean's own setter rejects must not be silently - * absorbed there. + * absorbed there. If the bean has both a boolean-returning and a + * void-returning bag, the boolean one is tried first and a {@code false} + * answer means the property was not applied; the void bag is not consulted + * in that case. * <p> * A property that cannot be applied - no usable setter, a value the setter * type cannot be built from, or a setter that throws - fails the lookup with @@ -197,7 +201,8 @@ public class GenericNamingResourcesFactory implements ObjectFactory { private static SetResult setProperty(Object o, String name, String value) { if (log.isTraceEnabled()) { - log.trace("setProperty(" + o.getClass() + " " + name + "=" + value + ")"); + // Do not log the value: it may be a credential. + log.trace("setProperty(" + o.getClass() + " " + name + ")"); } // Missing content is not a configured value: no setter - String or @@ -214,7 +219,6 @@ public class GenericNamingResourcesFactory implements ObjectFactory { Method setPropertyMethodVoid = null; Method setPropertyMethodBool = null; - // First, the ideal case - a setFoo( String ) method for (int i = 0; i < methods.length; i++) { Class<?>[] paramT = methods[i].getParameterTypes(); if (setter.equals(methods[i].getName()) && paramT.length == 1 && @@ -281,7 +285,6 @@ public class GenericNamingResourcesFactory implements ObjectFactory { } } - // Ok, no setXXX found, try a setProperty("name", "value") // A setXXX matched but no supported overload converted the value: // consulting the bag here would silently absorb the value the // bean's own setter rejected and report success, leaving the real diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/naming/TestNamingFactory.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/naming/TestNamingFactory.java index 041fcbe1f3..6935969d66 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/naming/TestNamingFactory.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/naming/TestNamingFactory.java @@ -777,6 +777,50 @@ public class TestNamingFactory extends JdbcPoolBaseTest { "the default must survive an address with no content"); } + @Test + void traceLoggingDoesNotIncludeThePropertyValue() throws Exception { + Logger logger = Logger.getLogger( + GenericNamingResourcesFactory.class.getName()); + Level originalLevel = logger.getLevel(); + final List<LogRecord> records = new ArrayList<>(); + Handler handler = new Handler() { + @Override + public void publish(LogRecord record) { + records.add(record); + } + @Override + public void flush() { + // no buffering + } + @Override + public void close() { + // nothing to release + } + }; + logger.addHandler(handler); + logger.setLevel(Level.FINEST); + try { + Reference ref = new Reference(NamingTarget.class.getName(), + null, null); + ref.add(new StringRefAddr("name", "s3cr3t-value")); + + Object result = new GenericNamingResourcesFactory().getObjectInstance( + ref, new CompositeName("test/trace"), null, null); + Assertions.assertEquals("s3cr3t-value", + ((NamingTarget) result).getName(), + "the property must still be applied"); + } finally { + logger.removeHandler(handler); + logger.setLevel(originalLevel); + } + for (LogRecord record : records) { + Assertions.assertFalse(record.getMessage() != null && + record.getMessage().contains("s3cr3t-value"), + "the trace line must not log the configured value: " + + record.getMessage()); + } + } + /** Target with the numeric and character setters BeanFactory supports. */ public static class WideTarget { private short small; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
