This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 11.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit ebdea780e1d27bb10bad7d9b96cf2a484b0c3111 Author: opencode <[email protected]> AuthorDate: Wed Nov 4 15:17:25 2026 +0100 Put the pool's own JMX MBean's registrations, credentials and wording in order A campaign pass over the pool's own JMX MBean, verified finding by finding against the current source, fixed by fresh-session reviews and run through the jdbc-pool testsuite throughout. Registration lifecycle: the server calls preRegister() before it looks the requested name up in its repository, and it reports a refusal only through postRegister(false) - a registration it refused it never made, so it runs no postDeregister(). The wrapper stored the name in preRegister() and ignored postRegister(), so a registration the server refused left an MBean that was never published reporting itself as registered under a name some other MBean owns: its notifications attributed themselves to that stranger, and every physical connection offered itself to the server below that foreign name - logged as an error, or, the child name being free, registered where this pool has no claim and no release. The MBean now drops the name on a refused registration, as the DataSource that publishes it already does for its own handle. Credential masking: Url and ConnectionProperties were served raw, so a JDBC URL or a connectionProperties pair carrying a password came back in cleartext over the shortest read path, while the neighbouring Password and DbProperties attributes exist to withhold exactly that. They now publish the configuration's own best-effort mask (maskCredentials becomes public), and JdbcInterceptors - the last credential-capable attribute, whose definition parameters reach a third-party interceptor through setProperties() as the other two reach the driver - got the dump's maskInterceptorCredentials, public for it the same way. The writes keep taking the raw strings: the read/write asymmetry the password attribute has always had. This is hardening of the project's own masking policy, not a security fix: an authenticated JMX principal is trusted in the first place. Credential rotation: writing Password or Username over the MBean reaches new connections and, since the borrow compares the identity a session carries with the identity configured now, borrowed ones too - re-established in place. A connection the pool renews on its own account still signs on with its cached credential and is retired when the database refuses it, so a rotation converges without operator action. The setter javadocs said none of this correctly - setPassword claimed the opposite of the borrowed path, setUsername said nothing - and now record both paths and point at purge and purgeOnReturn as the way to stop presenting a superseded credential at once. The remaining wording followed the same review: five setters that refuse their write on a running pool were commented "No-op" while throwing - they now carry javadoc and exception messages saying the property must be set before the pool is started; the notification ordering promise is qualified to the per-producer order that actually holds (sequence records construction, concurrent producers may arrive either way); a notifier worker killed by an Error is replaced by the executor, not "on the next notification"; the unregistered-source fallback now claims only to name the pool; and comments that restated code or their own javadoc were dropped. Tests: TestJmxRegistration gained aRefusedPoolMBeanRegistrationLeavesThePoolMBeanWithoutAName, TestJmxCredentialAttributes (new) pins the masked attributes, the raw write paths and a credential-carrying URL that still connects, and TestJmxCredentialWrites gained aPasswordRotatedThroughThePoolMBeanReachesTheConnectionsAlreadyInServ ice with a driver that refuses superseded credentials the way a rotated database does. Javadoc runs warning-free; the suite is green. --- .../apache/tomcat/jdbc/pool/PoolProperties.java | 4 +- .../tomcat/jdbc/pool/jmx/ConnectionPool.java | 179 ++++++++--- .../jdbc/pool/jmx/TestJmxCredentialAttributes.java | 357 +++++++++++++++++++++ .../jdbc/pool/jmx/TestJmxCredentialWrites.java | 145 ++++++++- .../tomcat/jdbc/pool/jmx/TestJmxRegistration.java | 89 +++++ 5 files changed, 715 insertions(+), 59 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolProperties.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolProperties.java index 0e4b238b4b..5c961fdd9d 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolProperties.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PoolProperties.java @@ -1103,7 +1103,7 @@ public class PoolProperties implements PoolConfiguration, Cloneable, Serializabl * @return the string with every recognised credential replaced by the mask, * or the string unchanged when it holds none */ - static String maskCredentials(String value) { + public static String maskCredentials(String value) { if (value == null) { return null; } @@ -1204,7 +1204,7 @@ public class PoolProperties implements PoolConfiguration, Cloneable, Serializabl * @return the string with every recognised credential value replaced by * the mask, or the string unchanged when it holds none */ - private static String maskInterceptorCredentials(String value) { + public static String maskInterceptorCredentials(String value) { if (value == null) { return null; } diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/jmx/ConnectionPool.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/jmx/ConnectionPool.java index 858ee273bc..6084782d5f 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/jmx/ConnectionPool.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/jmx/ConnectionPool.java @@ -34,6 +34,7 @@ import javax.management.ObjectName; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; import org.apache.tomcat.jdbc.pool.PoolConfiguration; +import org.apache.tomcat.jdbc.pool.PoolProperties; import org.apache.tomcat.jdbc.pool.PoolProperties.InterceptorDefinition; import org.apache.tomcat.jdbc.pool.PoolUtilities; import org.apache.tomcat.jdbc.pool.Validator; @@ -47,9 +48,6 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co private static final Log log = LogFactory.getLog(ConnectionPool.class); - /** - * the connection pool - */ protected org.apache.tomcat.jdbc.pool.ConnectionPool pool = null; /** * sequence for JMX notifications @@ -73,12 +71,15 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co * The single thread that pool notifications are delivered on. Delivery goes through here rather than running on the * thread that reports the condition, because several of the pool's notifications are raised from the connection * lifecycle - some of them while holding the lock of the connection concerned - and a monitoring listener that is - * slow or blocked must not stall those paths. One worker keeps the notifications in the order the pool raised them; - * the bounded queue and the drop on overflow keep a listener that stops draining from growing the queue without - * limit. The thread is a daemon, is started on the first notification and dies when it has been idle, so a pool - * that never notifies costs nothing, and it never holds up the JVM on shutdown. Shared by every pool MBean in the - * JVM: notifications of one pool can queue behind those of another, but a blocked listener then only starves - * delivery - the drop above is what a pool's operations feel, not the block. + * slow or blocked must not stall those paths. One worker keeps the notifications one reporting thread raised in + * the order that thread raised them: the sequence number is stamped on the reporting thread before the hand-off, + * so it records the order the notifications were built in, and two threads reporting at the same time can hand off + * - and so be delivered - in either order. The bounded queue and the drop on overflow keep a listener that stops + * draining from growing the queue without limit. The thread is a daemon, is started on the first notification and + * dies when it has been idle, so a pool that never notifies costs nothing, and it never holds up the JVM on + * shutdown. Shared by every pool MBean in the JVM: notifications of one pool can queue behind those of another, + * but a blocked listener then only starves delivery - the drop above is what a pool's operations feel, not the + * block. */ private static final ThreadPoolExecutor NOTIFIER = new ThreadPoolExecutor(0, 1, 60L, TimeUnit.SECONDS, new LinkedBlockingQueue<>(NOTIFY_QUEUE_CAPACITY), r -> { @@ -88,10 +89,11 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co }); /** - * the ObjectName of this pool, or {@code null} while this MBean is not registered - before its registration and - * after its deregistration. Written by the threads that register and deregister the MBean and read by the pool's - * own threads, so it is volatile - as it already is on the {@link org.apache.tomcat.jdbc.pool.DataSource} that - * publishes this MBean - to make those writes visible to the threads that raise notifications. + * the ObjectName of this pool, or {@code null} while this MBean is not registered - before its registration, after + * one the server has refused, and after its deregistration. Written by the threads that register and deregister the + * MBean and read by the pool's own threads, so it is volatile - as it already is on the + * {@link org.apache.tomcat.jdbc.pool.DataSource} that publishes this MBean - to make those writes visible to the + * threads that raise notifications. */ private volatile ObjectName oname = null; @@ -137,8 +139,8 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co /** * Returns the JMX ObjectName under which this MBean is registered. * - * @return the JMX ObjectName, or {@code null} while this MBean is not registered - before its registration and - * after its deregistration + * @return the JMX ObjectName, or {@code null} while this MBean is not registered - before its registration, after + * one the server has refused, and after its deregistration */ public ObjectName getObjectName() { return oname; @@ -150,8 +152,34 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co return name; } + /** + * Drops the registration name when the MBean server did not complete this MBean's registration. + * <p> + * The server calls {@link #preRegister(MBeanServer, ObjectName)} before it has looked the requested name up in its + * repository, so the name stored there can still be refused - the usual reason being another MBean that already + * holds it, most often the leftover of a pool that died without deregistering. After refusing a registration the + * server runs no {@link #postDeregister()}: per the {@link MBeanRegistration} contract a {@code false} + * {@code registrationDone} reports that the registration phase failed, which leaves this callback as the only + * chance to undo what {@code preRegister} wrote. + * <p> + * A name kept through a refusal is a name this MBean never acquired, and both ways it is used then act on a + * publication that never happened. The pool's notifications - the pool keeps running - attribute themselves to the + * MBean that does answer for that name, while {@link #getNotificationSource()} names the pool as a string + * precisely so that an MBean nobody published points at nobody else's name. And the connections, which publish + * themselves below {@link #getObjectName()} as soon as their physical session is up, are offered to the server + * below a stranger's name: the publish either collides with the children of the MBean that owns that name - a + * stale pool MBean holds the very same child names - or, the child name being free, publishes a connection of this + * pool into a subtree this pool has no claim to and no release for, at one error log per connection. The name is + * also what {@link org.apache.tomcat.jdbc.pool.ConnectionPool#registerConnectionMBeans()} waits for, so the gate + * that keeps the connections of an unpublished pool unpublished opens for a name this MBean was refused. The + * {@link org.apache.tomcat.jdbc.pool.DataSource} that publishes this MBean gives up its own side of a refused + * registration the same way. + */ @Override public void postRegister(Boolean registrationDone) { + if (!Boolean.TRUE.equals(registrationDone)) { + this.oname = null; + } } @Override @@ -215,9 +243,11 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co * and a monitoring listener - or a filter - that is slow or blocked must not reach those paths. The notification * itself is still built on the calling thread, so it carries the time, sequence number and serializable source of * the condition it reports - see {@link #getNotificationSource()} - and, since the notifier is a single thread and - * delivers a notification to all its listeners in one go, every listener observes the notifications in the order - * the pool reported them and a listener registered both through JMX and locally is reached through JMX before it - * is reached locally, as it was when delivery was synchronous. + * delivers a notification to all its listeners in one go, every listener observes the notifications one reporting + * thread raised in the order that thread raised them: the sequence number stamped on the calling thread records + * that construction order, and notifications two threads raise at the same time may be delivered in either order. + * A listener registered both through JMX and locally is reached through JMX before it is reached locally, as it + * was when delivery was synchronous. * * @param type The notification type * @param message The message @@ -236,12 +266,9 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co // then the single gate that accepts or refuses it, so the result // and the drop log below stay true to what was scheduled - a // refusal never means the notification partly reached its - // listeners, it means nothing was queued at all. On the notifier - // thread the JMX listeners are reached first - the constructor - // runs their hand-offs inline, on this thread - and the local - // ones next, so a listener registered on both sides is reached - // through JMX before it is reached locally, as it was when - // delivery was synchronous. + // listeners, it means nothing was queued at all. The order the + // two halves of the delivery run in is the one the javadoc above + // promises. NOTIFIER.execute(() -> { try { sendNotification(n); @@ -279,7 +306,8 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co * may not kill the notifier thread that every pool in the JVM delivers its notifications on, and it may not * suppress the delivery of that notification to the listeners registered behind the thrower either. An * {@link Error} is not contained - as it is not on the JMX half of the delivery, where the JDK broadcaster catches - * only what a listener throws - and takes the notifier thread down; the next notification starts a new one. + * only what a listener throws - and takes the notifier thread down; the pool replaces the worker as soon as the + * executor notices it is gone, so the next notification is delivered by a live worker. * * @param n the notification to deliver * @param type the notification type, for the failure log @@ -289,13 +317,6 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co try { listener.handleNotification(n, this); } catch (Exception x) { - // Contained at the listener that threw: the notification - // still reaches the listeners behind it and the notifier - // thread goes on to the next notification. An Error is left - // alone, as on the JMX half of the delivery - the JDK - // broadcaster's hand-off catches only a listener's exceptions - // - and takes this thread down, to be replaced when the next - // notification arrives if (log.isDebugEnabled()) { log.debug("Local notification listener failed. Type=" + type, x); } @@ -311,7 +332,7 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co * serializable. This MBean is not, and neither is the pool it wraps, so the source is the {@link ObjectName} the * MBean is registered under, which names the pool and is serializable. A pool MBean that is not registered - one * that has never been registered, or one the server has taken back while its pool keeps running - has no such - * name: the source is then a string of the shape the name would have had. + * name: the source is then a string that names the pool. * * @return the registered ObjectName, or a string naming the pool if the MBean is not registered */ @@ -434,9 +455,15 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co } + /** + * Returns the connection properties with any credential they carry masked out, so that a read of this attribute + * does not hand back in cleartext the password {@link #getPassword()} exists to withhold. The write is not masked: + * {@link #setConnectionProperties(String)} takes the string as it is meant to reach the driver, which is the same + * read/write asymmetry the password attribute has. {@inheritDoc} + */ @Override public String getConnectionProperties() { - return getPoolProperties().getConnectionProperties(); + return PoolProperties.maskCredentials(getPoolProperties().getConnectionProperties()); } /** @@ -480,9 +507,16 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co return getPoolProperties().getInitSQL(); } + /** + * Returns the interceptor configuration with any credential its definitions carry masked out, so that a read of + * this attribute does not hand back in cleartext the password {@link #getPassword()} exists to withhold - the + * parameter list of a definition reaches a third-party interceptor as configured, so it is one of the raw strings + * an operator may type a credential into. There is no write to mask: {@link #setJdbcInterceptors(String)} refuses + * the change on a running pool at all. {@inheritDoc} + */ @Override public String getJdbcInterceptors() { - return getPoolProperties().getJdbcInterceptors(); + return PoolProperties.maskInterceptorCredentials(getPoolProperties().getJdbcInterceptors()); } @Override @@ -544,9 +578,15 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co return getPoolProperties().getTimeBetweenEvictionRunsMillis(); } + /** + * Returns the JDBC URL with any credential it carries masked out, so that a read of this attribute does not hand + * back in cleartext the password {@link #getPassword()} exists to withhold. The write is not masked: + * {@link #setUrl(String)} takes the URL as it is meant to reach the driver, which is the same read/write asymmetry + * the password attribute has. {@inheritDoc} + */ @Override public String getUrl() { - return getPoolProperties().getUrl(); + return PoolProperties.maskCredentials(getPoolProperties().getUrl()); } @Override @@ -736,18 +776,23 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co } + /** + * @throws UnsupportedOperationException always - whether borrow waits are queued fairly is decided when the pool + * builds its wait queue; set the property before starting the pool + */ @Override public void setFairQueue(boolean fairQueue) { - // No-op - this pool is already running - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException("fairQueue must be set before the pool is started"); } + /** + * @throws UnsupportedOperationException always - the initial size is what the pool pre-loads with when it is + * created; set the property before starting the pool + */ @Override public void setInitialSize(int initialSize) { - // No-op - this pool is already running - throw new UnsupportedOperationException(); - + throw new UnsupportedOperationException("initialSize must be set before the pool is started"); } @@ -758,17 +803,24 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co } + /** + * @throws UnsupportedOperationException always - the interceptor chain is built when the pool starts and for + * every connection it establishes; set the property before starting the pool + */ @Override public void setJdbcInterceptors(String jdbcInterceptors) { - // No-op - this pool is already running - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException("jdbcInterceptors must be set before the pool is started"); } + /** + * @throws UnsupportedOperationException always - whether the pool registers this MBean at all is decided when the + * pool is created, and a client reaching this setter has reached an MBean whose registration is that + * decision; set the property before starting the pool + */ @Override public void setJmxEnabled(boolean jmxEnabled) { - // No-op - this pool is already running and obviously jmx enabled - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException("jmxEnabled must be set before the pool is started"); } @@ -822,8 +874,24 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co * Sets the password on the running pool's configuration. The write carries the same meaning as * {@link org.apache.tomcat.jdbc.pool.DataSourceProxy#setPassword(String)}: the credential also lives among the * database properties the driver is given, and a {@code null} password takes it away from there as well, so that - * clearing it here really does stop new connections authenticating with it. Connections the pool had already - * established reconnect with the credential they were first established with. + * clearing it here really does stop new connections authenticating with it. + * <p> + * No session is signed on again by the write itself, so what a rotation does to the connections the pool had + * already established depends on who takes such a connection up again. A borrowed one takes the new password up: + * the borrow compares the identity the session carries with the identity configured now and, finding them + * different, re-establishes that same connection under the password written here, which is the identity it keeps + * from then on. A connection the pool takes up on its own account - an idle one whose + * {@link PoolConfiguration#getMaxAge() maxAge} has expired - signs on again with the credential it cached for + * itself at its first sign-on rather than with this one, so wherever the two differ the database may refuse that + * reconnect; the pool retires such a connection and the next borrow is served by one established under the + * password set here. A rotation therefore converges without the operator doing anything, at the price of a + * refused sign-on or two on the renewals the pool runs by itself, and it does not leave sessions in service under + * a superseded credential. + * <p> + * To stop presenting the superseded credential now rather than connection by connection, expire the sessions that + * still carry it: {@link #purge()} closes the idle connections at once and the busy ones as they come back, while + * {@link #purgeOnReturn()} waits for each connection to come back, an idle one no sooner than it has been borrowed + * and returned. Whatever the pool establishes after either authenticates with the password set here. * * @param password the password, or {@code null} to take the credential away */ @@ -892,7 +960,6 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co * task that write armed or leave running a task that write made unnecessary. */ private void poolCleanerAttributeUpdated() { - // Make sure pool cleaner starts/stops when it should pool.reconcilePoolCleaner(getPoolProperties()); } @@ -916,8 +983,6 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co * from a reading this writer took, is recorded at poolCleanerAttributeUpdated. */ private void poolConfigurationUpdated() { - // Make sure the pool is properly configured, and then start, stop or re-arm - // the cleaner to match what that correction left the configuration asking for. pool.checkPoolConfiguration(getPoolProperties()); poolCleanerAttributeUpdated(); } @@ -944,6 +1009,13 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co * Sets the user name on the running pool's configuration, with the same meaning as * {@link org.apache.tomcat.jdbc.pool.DataSourceProxy#setUsername(String)}: a {@code null} user name takes the * credential away from the database properties as well as from the configuration. + * <p> + * What {@link #setPassword(String)} records about connections the pool had already established holds here in + * full, because the identity a session serves under is this user name together with that password: the borrow + * compares the pair the session carries with the pair configured now and re-establishes the connection under the + * new one, while a connection the pool renews on its own account signs on again with the pair it cached at its + * first sign-on and is retired when the database refuses it. {@link #purge()} and {@link #purgeOnReturn()} expire + * the sessions that still carry the superseded user name, on demand. * * @param username the user name, or {@code null} to take the credential away */ @@ -1092,10 +1164,13 @@ public class ConnectionPool extends NotificationBroadcasterSupport implements Co return getPoolProperties().isIgnoreExceptionOnPreLoad(); } + /** + * @throws UnsupportedOperationException always - the flag governs the preload of initial connections the pool + * runs when it is created; set the property before starting the pool + */ @Override public void setIgnoreExceptionOnPreLoad(boolean ignoreExceptionOnPreLoad) { - // No-op - this pool is already running - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException("ignoreExceptionOnPreLoad must be set before the pool is started"); } /** diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxCredentialAttributes.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxCredentialAttributes.java new file mode 100644 index 0000000000..dc7e1295a1 --- /dev/null +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxCredentialAttributes.java @@ -0,0 +1,357 @@ +/* + * 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.tomcat.jdbc.pool.jmx; + +import java.lang.management.ManagementFactory; +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.Statement; +import java.util.HashSet; +import java.util.Set; + +import javax.management.Attribute; +import javax.management.AttributeList; +import javax.management.MBeanServer; +import javax.management.MalformedObjectNameException; +import javax.management.ObjectName; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import org.apache.tomcat.jdbc.JdbcPoolBaseTest; +import org.apache.tomcat.jdbc.pool.DataSource; +import org.apache.tomcat.jdbc.pool.PoolProperties.InterceptorDefinition; + +/** + * The three raw configuration strings of the pool - the JDBC URL, the + * connection properties and the interceptor definitions - are published as + * attributes of the pool's own MBean, and any of them is a place an operator + * may type a credential: a URL of the form + * {@code jdbc:mysql://host/db?password=xxx} carries its own, so does a + * {@code connectionProperties} line, and so does the parameter list of an + * interceptor definition, whose pairs reach a third-party interceptor through + * {@code setProperties()} exactly as the pairs of the other two reach the + * driver. + * <p> + * The MBean already refuses to say the word for the credential held in the + * dedicated property: {@link ConnectionPool#getPassword()} answers with a fixed + * sentence rather than the password, and {@link ConnectionPool#getDbProperties()} + * answers with a copy that has the password entry taken out. Both exist for + * exactly one reason - a monitor that only reads must be able to look at a live + * pool's configuration - and that reason does not stop at one attribute: while + * the neighbouring pair kept the secret, these two handed it back verbatim, so + * the shortest read path to a credential was the one attribute an operator reads + * when the password attribute tells them nothing. The masking the configuration + * applies to its own dump and to the pool's log lines never reached here. + * </p> + * <p> + * What is pinned below is therefore a read/write asymmetry, the same one the + * password attribute has: a write through the MBean takes the raw string, since + * a configuration written scrubbed is a pool that cannot connect, while the read + * gives back the pool's own best-effort masking, so the JMX read, the + * configuration dump and the log all say the same thing about a configuration + * that carries its credential inline. + * </p> + */ +class TestJmxCredentialAttributes extends JdbcPoolBaseTest { + + /* + * The mask the configuration dump and the pool's log lines put over a + * recognised credential, so that a monitor cannot tell from the mask which + * surface it read. + */ + private static final String MASK = "********"; + + private static final String URL_SECRET = "url-inline-secret"; + private static final String PROPERTIES_SECRET = "properties-inline-secret"; + private static final String INTERCEPTOR_SECRET = "interceptor-inline-secret"; + + private static MBeanServer server() { + return ManagementFactory.getPlatformMBeanServer(); + } + + /* A name no other test of the suite uses, so nothing collides by accident. */ + private static ObjectName uniqueName(String role) throws MalformedObjectNameException { + return new ObjectName("tomcat:type=JmxCredentialAttributesTest,role=" + role + + ",uid=" + System.nanoTime()); + } + + /* + * One container registration: the name the MBean server was asked to publish + * the data source under, and the name the pool MBean - what the attributes + * below are read from - actually answers for. + */ + private static final class Registration { + private final DataSource ds; + private final ObjectName dsName; + private final ObjectName poolOname; + + private Registration(DataSource ds, ObjectName dsName, ObjectName poolOname) { + this.ds = ds; + this.dsName = dsName; + this.poolOname = poolOname; + } + } + + /* + * Everything a registration published: the pool's own MBean and anything + * published below it. The uid is carried down from the data source's name + * into every name derived from it, so it identifies this registration's + * publications and nothing else. + */ + private static Set<ObjectName> publishedFor(MBeanServer server, ObjectName dsName) throws Exception { + String uid = dsName.getKeyProperty("uid"); + Set<ObjectName> found = new HashSet<>(); + for (ObjectName name : server.queryNames(new ObjectName("tomcat.jdbc:*"), null)) { + if (uid.equals(name.getKeyProperty("uid"))) { + found.add(name); + } + } + return found; + } + + /* + * Publishes a pool the way a container does - the data source is not itself + * a JMX compliant MBean, so the pool's own MBean is what gets published, + * from the data source's preRegister() - and checks that the read path this + * class is about really is the attribute read of a registered MBean. + */ + private Registration publish(DataSource ds, String role) throws Exception { + ObjectName dsName = uniqueName(role); + ds.preRegister(server(), dsName); + ObjectName poolOname = ds.createObjectName(dsName); + Assertions.assertTrue(server().isRegistered(poolOname), + "precondition: the container's registration must have published the pool's own MBean"); + return new Registration(ds, dsName, poolOname); + } + + /* + * Takes a registration down the way a closing container does, and leaves + * nothing of it published. + */ + private static void closeAndUnpublish(Registration r) throws Exception { + MBeanServer server = server(); + r.ds.close(); + r.ds.postDeregister(); + for (ObjectName stale : publishedFor(server, r.dsName)) { + server.unregisterMBean(stale); + } + } + + /* + * A pool whose credential is carried inline in both raw strings, the way a + * URL typed into a JNDI resource or a connectionProperties line of a + * container descriptor carries it. No session is established: these tests + * read the published configuration, they do not use it. + */ + private DataSource poolWithInlineCredentials() throws Exception { + DataSource ds = newDataSource(); + ds.setUrl(getUrl() + ";password=" + URL_SECRET); + ds.getPoolProperties().setConnectionProperties("password=" + PROPERTIES_SECRET + ";a=b"); + ds.getPoolProperties().setJmxEnabled(true); + ds.getPoolProperties().setInitialSize(0); + ds.createPool(); + return ds; + } + + @Test + void theUrlAttributeOfThePoolMBeanNeverCarriesTheCredentialTypedIntoTheUrl() throws Exception { + Registration r = publish(poolWithInlineCredentials(), "url-attribute"); + try { + String published = (String) server().getAttribute(r.poolOname, "Url"); + Assertions.assertFalse(published.contains(URL_SECRET), + "a password typed into the JDBC URL must not come back out of the pool's MBean in" + + " cleartext: the password attribute of that very MBean refuses to say the" + + " word at all, and does so precisely so that a monitor which only reads may" + + " look at a live pool's configuration - an attribute that repeats the URL as" + + " it stands takes that protection away from every configuration that carries" + + " its credential in the URL rather than in the dedicated property"); + Assertions.assertEquals(getUrl() + ";password=" + MASK, published, + "what the attribute does publish must be the pool's own masking of the configured URL" + + " - the credential pair under the mask the configuration dump uses, and the" + + " rest of the URL still readable, since the point of the attribute is to tell" + + " an operator which database this pool is pointed at"); + + Assertions.assertEquals(getUrl() + ";password=" + URL_SECRET, r.ds.getPoolProperties().getUrl(), + "masking the read must leave the configuration itself as it was written: the URL is what" + + " the connect path hands the driver, so scrubbing it there turns a display" + + " change into a pool that cannot connect"); + } finally { + closeAndUnpublish(r); + } + } + + @Test + void theConnectionPropertiesAttributeOfThePoolMBeanNeverCarriesTheCredentialTypedIntoThem() + throws Exception { + Registration r = publish(poolWithInlineCredentials(), "properties-attribute"); + try { + String published = (String) server().getAttribute(r.poolOname, "ConnectionProperties"); + Assertions.assertFalse(published.contains(PROPERTIES_SECRET), + "a password carried by the connection properties must not come back out of the pool's" + + " MBean in cleartext: those properties go to the driver as they are typed, so" + + " they are a first place to look for a credential, and the MBean's own password" + + " attribute is masked for the sake of the read-only monitor that would read it" + + " here instead"); + Assertions.assertEquals("password=" + MASK + ";a=b", published, + "the masking must hide the credential and say nothing else about the configuration: the" + + " other pairs are what an operator needs to see to diagnose a pool, and a mask" + + " that swallowed them would be a monitor turned away from the configuration"); + + Assertions.assertEquals("password=" + PROPERTIES_SECRET + ";a=b", + r.ds.getPoolProperties().getConnectionProperties(), + "the configuration the connect path reads must keep the string as it was written: the" + + " pairs behind the mask are the pairs the driver is handed"); + } finally { + closeAndUnpublish(r); + } + } + + @Test + void theJdbcInterceptorsAttributeOfThePoolMBeanNeverCarriesTheCredentialTypedIntoADefinition() + throws Exception { + DataSource ds = newDataSource(); + // An interceptor definition whose parameter list carries a credential, + // the way an interceptor that reports to a monitored endpoint is + // configured in this very string. ResetAbandonedTimer stands in for + // any interceptor that takes its configuration through + // setProperties(): the base JdbcInterceptor stores every pair the + // definition carries rather than only the ones it knows, so the + // credential below really is delivered to the interceptor as typed - + // and the pool start instantiates the configured class and hands it + // these properties, so the configuration is live, not decoration. + ds.setJdbcInterceptors("ResetAbandonedTimer(password=" + INTERCEPTOR_SECRET + ")"); + ds.getPoolProperties().setJmxEnabled(true); + ds.getPoolProperties().setInitialSize(0); + ds.createPool(); + InterceptorDefinition configured = null; + for (InterceptorDefinition definition : ds.getPoolProperties().getJdbcInterceptorsAsArray()) { + if ("ResetAbandonedTimer".equals(definition.getClassName())) { + configured = definition; + } + } + Assertions.assertNotNull(configured, + "precondition: the definition below must really parse into an interceptor definition, not just " + + "read as one"); + Assertions.assertEquals(INTERCEPTOR_SECRET, configured.getProperties().get("password").getValue(), + "precondition: the pair typed into the definition must reach the parsed properties the " + + "interceptor is configured with - the attribute under test has a credential to hide " + + "precisely because the parse keeps it"); + + Registration r = publish(ds, "interceptors-attribute"); + try { + String published = (String) server().getAttribute(r.poolOname, "JdbcInterceptors"); + Assertions.assertFalse(published.contains(INTERCEPTOR_SECRET), + "a password typed into an interceptor definition must not come back out of the pool's MBean" + + " in cleartext: interceptor properties reach a third-party interceptor as typed," + + " so the definition string is as much a place to look for a credential as the URL" + + " and the connection properties are, and the configuration dump over" + + " PoolProperties already masks these pairs for the sake of exactly the read-only" + + " monitor that reads this attribute"); + Assertions.assertEquals("ResetAbandonedTimer(password=" + MASK + ")", published, + "the masking must hide the credential and keep everything else the attribute is read for:" + + " which interceptors are configured and which properties they carry, each pair" + + " named as configured, so a monitor still sees the shape of the chain behind the" + + " mask"); + + Assertions.assertEquals("ResetAbandonedTimer(password=" + INTERCEPTOR_SECRET + ")", + r.ds.getPoolProperties().getJdbcInterceptors(), + "the configuration the interceptor chain is built from must keep the string as it was" + + " written: the pairs behind the mask are the pairs the interceptor is" + + " configured with"); + } finally { + closeAndUnpublish(r); + } + } + + @Test + void aCredentialWrittenThroughThePoolMBeanIsStoredRawAndReadBackMasked() throws Exception { + Registration r = publish(poolWithInlineCredentials(), "write-path"); + try { + String rotatedUrl = getUrl() + ";password=rotated-" + URL_SECRET; + String rotatedProperties = "password=rotated-" + PROPERTIES_SECRET + ";a=b"; + AttributeList writes = new AttributeList(); + writes.add(new Attribute("Url", rotatedUrl)); + writes.add(new Attribute("ConnectionProperties", rotatedProperties)); + server().setAttributes(r.poolOname, writes); + + Assertions.assertEquals(rotatedUrl, r.ds.getPoolProperties().getUrl(), + "an operator who writes the URL through the pool's MBean must have written the URL, the" + + " whole URL: a configuration surface that scrubs on the way in is a surface an" + + " operator cannot configure a pool with, which is why the masked read of the" + + " password attribute has always had an unmasked write beside it"); + Assertions.assertEquals(rotatedProperties, r.ds.getPoolProperties().getConnectionProperties(), + "and the same for the connection properties, which reach the driver pair for pair"); + + Assertions.assertFalse(((String) server().getAttribute(r.poolOname, "Url")) + .contains("rotated-" + URL_SECRET), + "a credential written through the MBean is masked on the way out like one written" + + " anywhere else: an operator who rotates a secret over JMX must not have made" + + " the secret readable over JMX by the rotation"); + Assertions.assertFalse(((String) server().getAttribute(r.poolOname, "ConnectionProperties")) + .contains("rotated-" + PROPERTIES_SECRET), + "and the same for the connection properties"); + } finally { + closeAndUnpublish(r); + } + } + + @Test + void aPoolWhoseUrlCarriesItsCredentialStillBorrowsAWorkingConnection() throws Exception { + DataSource ds = newDataSource(); + // A database of its own, created by the very URL that names its + // credential. H2 authenticates a session against the database it + // connects to, so the credential in the URL below is not decoration but + // what the session is really established with. The password the + // configuration carries is overlaid on the properties handed to the + // driver, and a driver that also finds the password in the URL counts + // that as one property twice, so the URL is the only thing carrying it. + String rawUrl = urlForDatabase("jmxattr_" + System.nanoTime()) + ";password=" + URL_SECRET; + ds.setUrl(rawUrl); + ds.setPassword(null); + ds.getPoolProperties().setJmxEnabled(true); + ds.createPool(); + Registration r = publish(ds, "connect"); + try { + Assertions.assertEquals(1, ds.getPool().getIdle(), + "precondition: the pool established its initial session out of the URL as configured," + + " credential and all, before anything was ever read off the MBean"); + Assertions.assertFalse(((String) server().getAttribute(r.poolOname, "Url")).contains(URL_SECRET), + "the pool that connects with that credential must publish it masked: the masking is a" + + " decision taken where the configuration is read for display, and not a change to" + + " what this pool hands its driver"); + + try (Connection con = ds.getConnection(); + Statement st = con.createStatement(); + ResultSet rs = st.executeQuery("SELECT 1")) { + Assertions.assertTrue(rs.next(), "the borrowed session must answer"); + Assertions.assertEquals(1, rs.getInt(1), "on a statement that ran"); + } + Assertions.assertEquals(rawUrl, ds.getPoolProperties().getUrl(), + "and connecting with the credential must have left the configured URL intact, since a" + + " pool that rewrites its own URL once it is monitored is a pool that cannot" + + " reconnect"); + } finally { + closeAndUnpublish(r); + } + } + + /* The database URL of this test class' server, with a database name of the caller's own. */ + private static String urlForDatabase(String database) { + return getUrl().substring(0, getUrl().indexOf("/mem:")) + "/mem:" + database; + } +} diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxCredentialWrites.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxCredentialWrites.java index 064986fa8b..5ab0c671a0 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxCredentialWrites.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxCredentialWrites.java @@ -21,6 +21,10 @@ import java.sql.Driver; import java.sql.DriverPropertyInfo; import java.sql.SQLException; import java.sql.SQLFeatureNotSupportedException; +import java.sql.Statement; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.Properties; import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Logger; @@ -45,7 +49,9 @@ import org.apache.tomcat.jdbc.pool.PoolUtilities; * The pool here is configured the way the complaint is configured in practice - the credential sits among the * database properties, with no top-level property overriding it. The driver records the properties it is handed and * then connects with the test database's own credentials, so that the assertions are about what the pool offers the - * driver rather than about what the database would have accepted. + * driver rather than about what the database would have accepted. The last test leaves that fixture for one where the + * credential is a top level property and the database refuses a superseded password, because what it pins is how a + * write reaches the connections that were already in service when it was written. */ class TestJmxCredentialWrites extends JdbcPoolBaseTest { @@ -75,10 +81,10 @@ class TestJmxCredentialWrites extends JdbcPoolBaseTest { } /* - * What the next established connection is handed. A pooled connection keeps - * the credential it first connected with, so the connection that answered - * with the old credential has to leave the pool before the new state can be - * observed - which is also the remedy the credential setters document. + * What the next established connection is handed. The purge takes out the connections that signed on before the + * write under test, so that what these assertions look at is a connection established from the configuration as + * it now stands. What happens to a connection that was already in service when the credential was written is the + * subject of the test below, and that one does not purge. */ private static Properties establishFreshConnection(DataSource ds) { ds.purge(); @@ -146,6 +152,135 @@ class TestJmxCredentialWrites extends JdbcPoolBaseTest { + "the moment the top level property stops overriding it"); } + /** + * The rotation reaches the connections that were already in service when it was written, and who takes such a + * connection up again decides how it gets there. A borrowed connection takes the new password up as it is handed + * over: the borrow compares the identity the session carries with the identity configured now, finds them + * different, and re-establishes that same connection under the password written over the MBean, so the borrower is + * served with the rotated credential and nobody has to purge. An idle connection the pool renews on its own + * account - one that max age has expired while it sat idle - signs on again with the credential it cached at its + * first sign-on, which a database that has moved on refuses; the pool retires that connection, and the borrow that + * follows is served by a connection established under the new password. The superseded credential therefore stops + * authenticating either way, and the refusals along the way are the pool's own renewals rather than borrowed + * sessions failing. + */ + @Test + void aPasswordRotatedThroughThePoolMBeanReachesTheConnectionsAlreadyInService() throws Exception { + RotatingDriver.acceptedPassword.set("first-secret"); + RotatingDriver.presented.clear(); + DataSource ds = poolWithRotatingDriver(); + + borrowAndReturn(ds); + Assertions.assertEquals(Collections.singletonList("first-secret"), RotatingDriver.snapshot(), + "precondition: the connection now in service signed on with the first password"); + Assertions.assertEquals(1, poolSessions(), "precondition: one physical session is in service"); + + // The database moves first: that is the situation an operator is + // reacting to when they write the credential through the MBean. + RotatingDriver.acceptedPassword.set("rotated-secret"); + ds.getPool().getJmxPool().setPassword("rotated-secret"); + + // The renewal the pool runs by itself, over a connection nobody has + // borrowed since the write. It presents the credential that connection + // cached, the database refuses it, and the session leaves the pool. + ds.getPoolProperties().setMaxAge(1); + sleep(5); + ds.getPool().getJmxPool().testIdle(); + Assertions.assertEquals("first-secret", RotatingDriver.lastPresented(), + "a connection the pool renews on its own account signs on again with the credential it cached"); + Assertions.assertEquals(0, poolSessions(), + "the refused session leaves the pool rather than staying in service on the old credential"); + + // The next borrow is served by a connection established with the + // password written here. + borrowAndReturn(ds); + Assertions.assertEquals("rotated-secret", RotatingDriver.lastPresented(), + "the connection the pool establishes after the rotation carries the password written through it"); + Assertions.assertEquals(1, poolSessions()); + ds.getPoolProperties().setMaxAge(0); + + // A borrowed connection takes the rotation up in place, without the + // purge the setters offer as the immediate remedy. + long reconnected = ds.getPool().getReconnectedCount(); + RotatingDriver.acceptedPassword.set("second-secret"); + ds.getPool().getJmxPool().setPassword("second-secret"); + borrowAndReturn(ds); + Assertions.assertEquals("second-secret", RotatingDriver.lastPresented(), + "the borrowed session that carried the superseded credential is re-established under the new one"); + Assertions.assertEquals(reconnected + 1, ds.getPool().getReconnectedCount(), + "the connection already in service was re-established rather than replaced by another one"); + Assertions.assertEquals(1, poolSessions(), + "the re-established connection holds one session, not the two the old and the new sign-on would leave"); + + // And it keeps the identity it was re-established with: the following + // borrow is the same session and signs on nobody. + int signedOn = RotatingDriver.snapshot().size(); + borrowAndReturn(ds); + Assertions.assertEquals(reconnected + 1, ds.getPool().getReconnectedCount(), + "a connection that already carries the configured identity is not re-established again"); + Assertions.assertEquals(signedOn, RotatingDriver.snapshot().size(), + "the converged connection is reused as it is, so the rotation does not churn the pool"); + } + + /* A published-MBean pool whose credential is a top level property, so that each connection caches it. */ + private DataSource poolWithRotatingDriver() throws Exception { + DataSource ds = newDataSource(); + ds.setDriverClassName(RotatingDriver.class.getName()); + PoolConfiguration properties = ds.getPoolProperties(); + properties.setJmxEnabled(true); + properties.setInitialSize(0); + properties.setUsername("alice"); + properties.setPassword("first-secret"); + ds.createPool(); + Assertions.assertNotNull(ds.getPool().getJmxPool(), + "precondition: the fixture must have published the pool's own MBean"); + return ds; + } + + /* Takes a connection out and hands it back, so that it sits in the idle queue afterwards. */ + private static void borrowAndReturn(DataSource ds) { + try (Connection con = ds.getConnection(); + Statement st = con.createStatement()) { + st.execute("SELECT 1"); + } catch (SQLException x) { + throw new AssertionError(x); + } + } + + /** + * A driver that, like a database that has just rotated a password, accepts only the credential it is currently + * configured to accept and refuses every other, and that records the password each connect presented. The + * delegate is still reached with the test database's own credentials, so a connection the driver accepts is a real + * session the pool can use. + */ + public static class RotatingDriver extends RecordingDriver { + + static final AtomicReference<String> acceptedPassword = new AtomicReference<>(); + static final List<String> presented = Collections.synchronizedList(new ArrayList<>()); + + static List<String> snapshot() { + synchronized (presented) { + return new ArrayList<>(presented); + } + } + + static String lastPresented() { + List<String> seen = snapshot(); + Assertions.assertFalse(seen.isEmpty(), "precondition: the driver must have been called"); + return seen.get(seen.size() - 1); + } + + @Override + public Connection connect(String url, Properties info) throws SQLException { + String password = info.getProperty(PoolUtilities.PROP_PASSWORD); + presented.add(password); + if (!acceptedPassword.get().equals(password)) { + throw new SQLException("the database does not accept password [" + password + "] any more"); + } + return super.connect(url, info); + } + } + /** * Records the properties each connect was offered and then delegates to the real driver with the test database's * own credentials, so that a test can look at what the pool decided to hand over without the database voting on diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxRegistration.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxRegistration.java index e89eae76cd..3e34e68467 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxRegistration.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxRegistration.java @@ -733,6 +733,95 @@ class TestJmxRegistration extends JdbcPoolBaseTest { } } + @Test + void aRefusedPoolMBeanRegistrationLeavesThePoolMBeanWithoutAName() throws Exception { + MBeanServer server = server(); + DataSource ds = newDataSource(); + ds.getPoolProperties().setJmxEnabled(true); + ds.getPoolProperties().setInitialSize(0); + ds.createPool(); + ConnectionPool jmx = ds.getPool().getJmxPool(); + Assertions.assertNotNull(jmx, "an enabled pool has a JMX wrapper"); + Assertions.assertNull(jmx.getObjectName(), "this pool is not registered yet"); + + // A stale registration holds the name this pool would be published + // under - the leftover of a pool that died without deregistering, or + // another pool that picked the same name. It is live and reporting. + ObjectName dsName = uniqueName("refused-mbean"); + ObjectName poolOname = ds.createObjectName(dsName); + server.registerMBean(new Probe("foreign"), poolOname); + BlockingQueue<Notification> delivered = new LinkedBlockingQueue<>(); + NotificationListener listener = (n, handback) -> delivered.offer(n); + try { + // The container registers the DataSource, and the pool MBean the + // DataSource publishes from its preRegister() is refused that name. + // The MBean server calls the pool MBean's own preRegister() before + // it looks the name up in its repository, so the wrapper has taken + // the name into its registration handle by the time the refusal + // happens, and reports it back through postRegister(FALSE) - the + // one callback a refused registration still gets, since the server + // runs no postDeregister() for a registration it never made. + jmx.addListener(listener); + ds.preRegister(server, dsName); + + Assertions.assertNull(jmx.getObjectName(), + "a pool MBean whose registration the server refused must drop the name its own" + + " preRegister() stored: this MBean never became the MBean behind that" + + " name, and every user of the handle - the source its notifications" + + " carry, and the connections that publish themselves below it - acts" + + " on a registration it does not have"); + Assertions.assertEquals("foreign", server.getAttribute(poolOname, "Marker"), + "a refused registration must have left the MBean that owns the name alone"); + + // The harm, on the pool's own lifecycle: a connection publishes + // itself below whatever name the pool MBean believes it holds, so + // the connections of a pool that was never published turn up in + // the subtree of the stranger that owns the name. + try (Connection con = ds.getConnection()) { + Assertions.assertNotNull(con, + "the pool must keep handing out connections while its own MBean cannot be" + + " published"); + Set<ObjectName> children = publishedFor(server, dsName); + children.remove(poolOname); + Assertions.assertTrue(children.isEmpty(), + "a connection must not be published below a name the pool was refused:" + + " that subtree belongs to the MBean that owns the name, and the" + + " registrations are refused or collide with its own children" + + " while the pool that made them is not monitored at all"); + } + + // And the harm on the notifications: an MBean that is not + // registered names itself with the string fallback, so that a + // listener is not told that a condition of this pool came from the + // MBean that owns the refused name. + Assertions.assertTrue(jmx.notify(ConnectionPool.POOL_EMPTY, + "reported by a pool whose publication was refused"), + "the notification must be accepted for delivery"); + Notification n = delivered.poll(30, TimeUnit.SECONDS); + Assertions.assertNotNull(n, "the notification was handed off for delivery"); + Assertions.assertFalse(poolOname.equals(n.getSource()), + "a notification of a pool MBean whose registration was refused must not" + + " attribute itself to the refused name: listeners take the source as" + + " the address of the MBean that reported the condition, and this MBean" + + " is not the one published there"); + Assertions.assertInstanceOf(String.class, n.getSource(), + "an unregistered pool MBean reports the condition as the string the name would" + + " have had, which is what an MBean that was never published has"); + } finally { + jmx.removeListener(listener); + ds.close(); + ds.postDeregister(); + if (server.isRegistered(poolOname)) { + server.unregisterMBean(poolOname); + } + // test hygiene: take away anything a regression would have left + // published below the name the foreign MBean holds + for (ObjectName stale : publishedFor(server, dsName)) { + server.unregisterMBean(stale); + } + } + } + @Test void aPoolThreadRaisesItsNotificationUnderTheNameTheRegistrationPublished() throws Exception { MBeanServer server = server(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
