You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/11/20 13:24:49 UTC
[tomcat] branch 8.5.x updated (e92da8e -> 85ba8b6)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git.
from e92da8e Additional debug logging to investigate bug 63859
new 52c6412 Fix SpotBugs warnings in JDBC pool module
new 065c5d1 Fix SpotBugs warnings for unit tests
new c4c1420 Fix SpotBugs issues in JDBC pool tests
new 85ba8b6 One missed false positive
The 4 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:
.../apache/tomcat/jdbc/pool/ConnectionPool.java | 5 +-
.../apache/tomcat/jdbc/pool/PoolProperties.java | 2 +-
.../apache/tomcat/jdbc/pool/PooledConnection.java | 4 +-
.../jdbc/pool/interceptor/StatementCache.java | 3 +-
.../interceptor/StatementDecoratorInterceptor.java | 2 +-
.../java/org/apache/tomcat/jdbc/bugs/Bug53367.java | 6 +-
.../apache/tomcat/jdbc/test/ConnectCountTest.java | 4 +-
.../org/apache/tomcat/jdbc/test/FairnessTest.java | 4 +-
.../apache/tomcat/jdbc/test/JmxPasswordTest.java | 2 +-
.../apache/tomcat/jdbc/test/MultipleCloseTest.java | 4 +-
.../apache/tomcat/jdbc/test/StarvationTest.java | 2 +
.../org/apache/tomcat/jdbc/test/TestException.java | 12 +-
.../tomcat/jdbc/test/TestStatementCache.java | 2 +
.../org/apache/tomcat/jdbc/test/TestTimeout.java | 26 +-
.../jdbc/test/TestValidationQueryTimeout.java | 16 +-
res/findbugs/filter-false-positives.xml | 315 ++++++++++++++++++++-
.../TesterDigestAuthenticatorPerformance.java | 2 +-
.../apache/catalina/connector/TestSendFile.java | 2 +-
.../apache/catalina/core/TestAsyncContextImpl.java | 18 +-
...TestWebappClassLoaderThreadLocalMemoryLeak.java | 5 +-
.../catalina/nonblocking/TestNonBlockingAPI.java | 37 ++-
.../tribes/test/transport/SocketNioReceive.java | 8 +-
.../valves/TestLoadBalancerDrainingValve.java | 6 +-
.../apache/coyote/http11/upgrade/TestUpgrade.java | 9 +-
.../http11/upgrade/TestUpgradeInternalHandler.java | 7 +-
test/org/apache/coyote/http2/Http2TestBase.java | 2 +-
test/org/apache/coyote/http2/TestAsync.java | 7 +-
test/org/apache/jasper/TestJspC.java | 2 +-
.../apache/jasper/TestJspCompilationContext.java | 3 +-
test/org/apache/juli/TestFileHandler.java | 13 +-
30 files changed, 442 insertions(+), 88 deletions(-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 03/04: Fix SpotBugs issues in JDBC pool tests
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
View the commit online:
https://github.com/apache/tomcat/commit/c4c14207035ac95c99f7c2da1dc326e3e989efa6
commit c4c14207035ac95c99f7c2da1dc326e3e989efa6
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Nov 20 13:21:51 2019 +0000
Fix SpotBugs issues in JDBC pool tests
---
.../java/org/apache/tomcat/jdbc/bugs/Bug53367.java | 6 +--
.../apache/tomcat/jdbc/test/ConnectCountTest.java | 4 +-
.../org/apache/tomcat/jdbc/test/FairnessTest.java | 4 +-
.../apache/tomcat/jdbc/test/JmxPasswordTest.java | 2 +-
.../apache/tomcat/jdbc/test/MultipleCloseTest.java | 4 +-
.../apache/tomcat/jdbc/test/StarvationTest.java | 2 +
.../org/apache/tomcat/jdbc/test/TestException.java | 12 ++---
.../tomcat/jdbc/test/TestStatementCache.java | 2 +
.../org/apache/tomcat/jdbc/test/TestTimeout.java | 26 +++++++----
.../jdbc/test/TestValidationQueryTimeout.java | 16 +++----
res/findbugs/filter-false-positives.xml | 51 ++++++++++++++++++++++
11 files changed, 99 insertions(+), 30 deletions(-)
diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/bugs/Bug53367.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/bugs/Bug53367.java
index 1e534e0..6c0984e 100644
--- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/bugs/Bug53367.java
+++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/bugs/Bug53367.java
@@ -107,8 +107,8 @@ public class Bug53367 {
Thread thread = new Thread(new Runnable() {
@Override
public void run() {
- try {
- ds.getConnection();
+ // Expected to fail
+ try (Connection c = ds.getConnection()) {
} catch (Exception e) {
System.err.println("Step 2:"+e.getMessage());
}
@@ -174,4 +174,4 @@ public class Bug53367 {
Assert.assertEquals(0, pool.getActive());
Assert.assertEquals(threadsCount, pool.getSize());
}
-}
\ No newline at end of file
+}
diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ConnectCountTest.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ConnectCountTest.java
index 5a0a71f..8e240f9 100644
--- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ConnectCountTest.java
+++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ConnectCountTest.java
@@ -17,7 +17,9 @@
package org.apache.tomcat.jdbc.test;
import java.sql.Connection;
+import java.sql.SQLException;
import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
@@ -259,7 +261,7 @@ public class ConnectCountTest extends DefaultTestCase {
totalruntime+=(System.nanoTime()-start);
}
- } catch (Exception x) {
+ } catch (RuntimeException | SQLException | ExecutionException | InterruptedException x) {
x.printStackTrace();
} finally {
ConnectCountTest.this.latch.countDown();
diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/FairnessTest.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/FairnessTest.java
index 1c282df..bb2cc8b 100644
--- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/FairnessTest.java
+++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/FairnessTest.java
@@ -17,7 +17,9 @@
package org.apache.tomcat.jdbc.test;
import java.sql.Connection;
+import java.sql.SQLException;
import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
@@ -241,7 +243,7 @@ public class FairnessTest extends DefaultTestCase {
totalruntime+=(System.nanoTime()-start);
}
- } catch (Exception x) {
+ } catch (RuntimeException | SQLException | ExecutionException | InterruptedException x) {
x.printStackTrace();
} finally {
FairnessTest.this.latch.countDown();
diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/JmxPasswordTest.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/JmxPasswordTest.java
index 2074447..6c05c00 100644
--- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/JmxPasswordTest.java
+++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/JmxPasswordTest.java
@@ -36,7 +36,7 @@ import org.apache.tomcat.jdbc.test.driver.Driver;
public class JmxPasswordTest extends DefaultTestCase{
public static final String password = "password";
public static final String username = "username";
- public static ObjectName oname = null;
+ public ObjectName oname = null;
@Before
public void setUp() throws Exception {
diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/MultipleCloseTest.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/MultipleCloseTest.java
index cd867d3..4180c51 100644
--- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/MultipleCloseTest.java
+++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/MultipleCloseTest.java
@@ -61,9 +61,11 @@ public class MultipleCloseTest extends DefaultTestCase {
Assert.assertTrue(con1.isClosed());
// Open a new connection (This will re-use the previous pooled connection)
- datasource.getConnection();
+ Connection con2 = datasource.getConnection();
// A connection, once closed, should stay closed
Assert.assertTrue(con1.isClosed());
+
+ con2.close();
}
}
diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/StarvationTest.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/StarvationTest.java
index 0c1a667..1c551ed 100644
--- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/StarvationTest.java
+++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/StarvationTest.java
@@ -82,6 +82,7 @@ public class StarvationTest extends DefaultTestCase {
}finally {
if (con2!=null) con2.close();
}
+ con1.close();
}
@Test
@@ -104,5 +105,6 @@ public class StarvationTest extends DefaultTestCase {
}finally {
if (con2!=null) con2.close();
}
+ con1.close();
}
}
diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestException.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestException.java
index be06e38..fe4d20d 100644
--- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestException.java
+++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestException.java
@@ -17,7 +17,9 @@
package org.apache.tomcat.jdbc.test;
import java.sql.Connection;
+import java.sql.Statement;
+import org.junit.Assert;
import org.junit.Test;
import org.apache.tomcat.jdbc.pool.ConnectionPool;
@@ -30,11 +32,11 @@ public class TestException extends DefaultTestCase {
public void testException() throws Exception {
datasource.getPoolProperties().setJdbcInterceptors(TestInterceptor.class.getName());
Connection con = datasource.getConnection();
- try {
- con.createStatement();
- }catch (Exception x) {
- // Ignore
+ try (Statement s = con.createStatement()){
+ } catch (Exception x) {
+ Assert.fail();
}
+ con.close();
}
@@ -42,7 +44,7 @@ public class TestException extends DefaultTestCase {
@Override
public void reset(ConnectionPool parent, PooledConnection con) {
- // TODO Auto-generated method stub
+ // NO-OP
}
}
}
diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestStatementCache.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestStatementCache.java
index 3a8778a..a80962c 100644
--- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestStatementCache.java
+++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestStatementCache.java
@@ -88,6 +88,7 @@ public class TestStatementCache extends DefaultTestCase {
ps3.close();
Assert.assertTrue(ps3.isClosed());
Assert.assertEquals(1,interceptor.getCacheSize().get());
+ con.close();
}
@Test
@@ -108,6 +109,7 @@ public class TestStatementCache extends DefaultTestCase {
ps3.close();
Assert.assertTrue(ps3.isClosed());
Assert.assertEquals(0,interceptor.getCacheSize().get());
+ con.close();
}
@Test
diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestTimeout.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestTimeout.java
index 9b9fbde..aec85b6 100644
--- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestTimeout.java
+++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestTimeout.java
@@ -16,17 +16,18 @@
*/
package org.apache.tomcat.jdbc.test;
-import java.util.concurrent.atomic.AtomicInteger;
+import java.sql.Connection;
+import java.util.HashSet;
+import java.util.Set;
import org.junit.Assert;
import org.junit.Test;
public class TestTimeout extends DefaultTestCase {
- AtomicInteger counter = new AtomicInteger(0);
-
@Test
public void testCheckoutTimeout() throws Exception {
+ Set<Connection> cons = new HashSet<>();
try {
this.datasource.getPoolProperties().setTestWhileIdle(true);
this.datasource.getPoolProperties().setTestOnBorrow(false);
@@ -42,20 +43,24 @@ public class TestTimeout extends DefaultTestCase {
System.out.println("About to test connection pool:"+datasource);
for (int i = 0; i < 21; i++) {
long now = System.currentTimeMillis();
- this.datasource.getConnection();
+ cons.add(this.datasource.getConnection());
long delta = System.currentTimeMillis()-now;
System.out.println("Got connection #"+i+" in "+delta+" ms.");
}
- Assert.assertTrue(false);
+ Assert.fail();
} catch ( Exception x ) {
- Assert.assertTrue(true);
+ // Expected on 21st checkout
}finally {
Thread.sleep(2000);
}
+ for (Connection c : cons) {
+ c.close();
+ }
}
@Test
public void testCheckoutTimeoutFair() throws Exception {
+ Set<Connection> cons = new HashSet<>();
try {
this.datasource.getPoolProperties().setFairQueue(true);
this.datasource.getPoolProperties().setTestWhileIdle(true);
@@ -72,15 +77,18 @@ public class TestTimeout extends DefaultTestCase {
System.out.println("About to test connection pool:"+datasource);
for (int i = 0; i < 21; i++) {
long now = System.currentTimeMillis();
- this.datasource.getConnection();
+ cons.add(this.datasource.getConnection());
long delta = System.currentTimeMillis()-now;
System.out.println("Got connection #"+i+" in "+delta+" ms.");
}
- Assert.assertTrue(false);
+ Assert.fail();
} catch ( Exception x ) {
- Assert.assertTrue(true);
+ // Expected on 21st checkout
}finally {
Thread.sleep(2000);
}
+ for (Connection c : cons) {
+ c.close();
+ }
}
}
diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestValidationQueryTimeout.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestValidationQueryTimeout.java
index 79e2fdc..054df27 100644
--- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestValidationQueryTimeout.java
+++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestValidationQueryTimeout.java
@@ -35,7 +35,7 @@ import org.apache.tomcat.jdbc.pool.interceptor.QueryTimeoutInterceptor;
public class TestValidationQueryTimeout extends DefaultTestCase {
- private static int TIMEOUT = 10;
+ private static final int TIMEOUT = 10;
private static boolean isTimeoutSet;
private static final String longQuery = "select * from test as A, test as B, test as C, test as D, test as E";
@@ -54,7 +54,6 @@ public class TestValidationQueryTimeout extends DefaultTestCase {
this.datasource.setValidationQuery("SELECT 1");
this.datasource.setValidationQueryTimeout(TIMEOUT);
- TIMEOUT = 10;
isTimeoutSet = false;
}
@@ -67,8 +66,9 @@ public class TestValidationQueryTimeout extends DefaultTestCase {
@Test
public void testValidationQueryTimeoutEnabled() throws Exception {
// because testOnBorrow is true, this triggers the validation query
- this.datasource.getConnection();
+ Connection con = this.datasource.getConnection();
Assert.assertTrue(isTimeoutSet);
+ con.close();
}
@Test
@@ -76,8 +76,9 @@ public class TestValidationQueryTimeout extends DefaultTestCase {
this.datasource.setValidationQueryTimeout(-1);
// because testOnBorrow is true, this triggers the validation query
- this.datasource.getConnection();
+ Connection con = this.datasource.getConnection();
Assert.assertFalse(isTimeoutSet);
+ con.close();
}
@Test
@@ -91,9 +92,6 @@ public class TestValidationQueryTimeout extends DefaultTestCase {
Connection con = this.datasource.getConnection();
Assert.assertTrue(isTimeoutSet);
- // increase the expected timeout to 30, which is what we set for the interceptor
- TIMEOUT = 30;
-
// now create a statement, make sure the query timeout is set by the interceptor
Statement st = con.createStatement();
Assert.assertEquals(interceptorTimeout, st.getQueryTimeout());
@@ -107,10 +105,10 @@ public class TestValidationQueryTimeout extends DefaultTestCase {
con.close();
// pull another connection and check it
- TIMEOUT = 10;
isTimeoutSet = false;
- this.datasource.getConnection();
+ Connection con2 = this.datasource.getConnection();
Assert.assertTrue(isTimeoutSet);
+ con2.close();
}
// this test depends on the execution time of the validation query
diff --git a/res/findbugs/filter-false-positives.xml b/res/findbugs/filter-false-positives.xml
index 73443d6..06f104e 100644
--- a/res/findbugs/filter-false-positives.xml
+++ b/res/findbugs/filter-false-positives.xml
@@ -1898,6 +1898,15 @@
<Bug code="ST" />
</Match>
<Match>
+ <!-- The name shadowing is deliberate -->
+ <Or>
+ <Class name="org.apache.tomcat.jdbc.test.driver.Connection" />
+ <Class name="org.apache.tomcat.jdbc.test.driver.Driver" />
+ <Class name="org.apache.tomcat.jdbc.test.driver.ResultSet" />
+ </Or>
+ <Bug pattern="NM_SAME_SIMPLE_NAME_AS_INTERFACE" />
+ </Match>
+ <Match>
<!-- The call with the ignored return value is used to ensure the pool -->
<!-- thinks the connection is being used. -->
<Class name="org.apache.tomcat.jdbc.test.AbandonPercentageTest" />
@@ -1905,6 +1914,32 @@
<Bug pattern="RV_RETURN_VALUE_IGNORED" />
</Match>
<Match>
+ <!-- A number of the tests incude performance tests -->
+ <Class name="org.apache.tomcat.jdbc.test.DefaultTestCase" />
+ <Method name="tearDown" />
+ <Bug pattern="DM_GC" />
+ </Match>
+ <Match>
+ <!-- Test does not explicitly close statement deliberately -->
+ <Class name="org.apache.tomcat.jdbc.test.StatementFinalizerTest" />
+ <Method name="testStatementFinalization" />
+ <Bug pattern="ODR_OPEN_DATABASE_RESOURCE"/>
+ </Match>
+ <Match>
+ <!-- Choice of name is deliberate -->
+ <Class name="org.apache.tomcat.jdbc.test.TestException" />
+ <Bug pattern="NM_CLASS_NOT_EXCEPTION" />
+ </Match>
+ <Match>
+ <!-- Testing auto-close so connections not explicitly closed -->
+ <Class name="org.apache.tomcat.jdbc.test.TestGCClose" />
+ <Or>
+ <Method name="testGCStop" />
+ <Method name="testClose" />
+ </Or>
+ <Bug pattern="ODR_OPEN_DATABASE_RESOURCE" />
+ </Match>
+ <Match>
<!-- SQL is from config so is considered safe -->
<Class name="org.apache.tomcat.jdbc.test.TestSlowQueryReport" />
<Method name="testFastSql" />
@@ -1923,6 +1958,22 @@
<Bug pattern="SQL_PREPARED_STATEMENT_GENERATED_FROM_NONCONSTANT_STRING" />
</Match>
<Match>
+ <!-- Tests throw exceptions so connections are never created -->
+ <Class name="org.apache.tomcat.jdbc.test.TestValidationQueryTimeout" />
+ <Or>
+ <Method name="testValidationQueryTimeoutOnConnection" />
+ <Method name="testValidationInvalidOnConnection" />
+ <Method name="testValidationQueryTimeoutOnBorrow" />
+ </Or>
+ <Bug pattern="ODR_OPEN_DATABASE_RESOURCE" />
+ </Match>
+ <Match>
+ <!-- Statics used to work around API limitations -->
+ <Class name="org.apache.tomcat.jdbc.test.TestValidationQueryTimeout" />
+ <Field name="isTimeoutSet" />
+ <Bug pattern="ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD" />
+ </Match>
+ <Match>
<Class name="org.apache.tomcat.jdbc.test.TwoDataSources" />
<Method name="testTwoDataSources" />
<Or>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 01/04: Fix SpotBugs warnings in JDBC pool module
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
View the commit online:
https://github.com/apache/tomcat/commit/52c6412892def19fdd9a0f341f8c3f772839610d
commit 52c6412892def19fdd9a0f341f8c3f772839610d
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Nov 20 12:23:21 2019 +0000
Fix SpotBugs warnings in JDBC pool module
---
.../org/apache/tomcat/jdbc/pool/ConnectionPool.java | 5 ++++-
.../org/apache/tomcat/jdbc/pool/PoolProperties.java | 2 +-
.../org/apache/tomcat/jdbc/pool/PooledConnection.java | 4 ++--
.../tomcat/jdbc/pool/interceptor/StatementCache.java | 3 ++-
.../interceptor/StatementDecoratorInterceptor.java | 2 +-
res/findbugs/filter-false-positives.xml | 19 +++++++++++++++++++
6 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java
index 7763a98..cbb1615 100644
--- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java
+++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java
@@ -622,7 +622,10 @@ public class ConnectionPool {
// we could have threads stuck in idle.poll(timeout) that will never be
// notified
if (waitcount.get() > 0) {
- idle.offer(create(true));
+ if (!idle.offer(create(true))) {
+ log.warn("Failed to add a new connection to the pool after releasing a connection " +
+ "when at least one thread was waiting for a connection.");
+ }
}
}
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 3b09eb0..2d995d9 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
@@ -54,7 +54,7 @@ public class PoolProperties implements PoolConfiguration, Cloneable, Serializabl
private volatile String validationQuery;
private volatile int validationQueryTimeout = -1;
private volatile String validatorClassName;
- private volatile Validator validator;
+ private transient volatile Validator validator;
private volatile boolean testOnBorrow = false;
private volatile boolean testOnReturn = false;
private volatile boolean testWhileIdle = false;
diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java
index 69cf1f0..8978810 100644
--- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java
+++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java
@@ -202,9 +202,9 @@ public class PooledConnection implements PooledConnectionMBean {
log.debug("Unable to disconnect previous connection.", x);
} //catch
} //end if
- if (poolProperties.getDataSource()==null && poolProperties.getDataSourceJNDI()!=null) {
+ //if (poolProperties.getDataSource()==null && poolProperties.getDataSourceJNDI()!=null) {
//TODO lookup JNDI name
- }
+ //}
if (poolProperties.getDataSource()!=null) {
connectUsingDataSource();
diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java
index 03b7f84..a5b4d36 100644
--- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java
+++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java
@@ -20,6 +20,7 @@ import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
+import java.sql.SQLException;
import java.util.Arrays;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
@@ -305,7 +306,7 @@ public class StatementCache extends StatementDecoratorInterceptor implements Sta
proxy.cached = true;
shouldClose = false;
}
- } catch (Exception x) {
+ } catch (RuntimeException | ReflectiveOperationException | SQLException x) {
removeStatement(proxy);
}
}
diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementDecoratorInterceptor.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementDecoratorInterceptor.java
index 51ae9df..3066ef2 100644
--- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementDecoratorInterceptor.java
+++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementDecoratorInterceptor.java
@@ -49,7 +49,7 @@ public class StatementDecoratorInterceptor extends AbstractCreateStatementInterc
/**
* the constructor to create the resultSet proxies
*/
- protected static Constructor<?> resultSetConstructor = null;
+ protected static volatile Constructor<?> resultSetConstructor = null;
@Override
public void closeInvoked() {
diff --git a/res/findbugs/filter-false-positives.xml b/res/findbugs/filter-false-positives.xml
index 4578e0b..5d63303 100644
--- a/res/findbugs/filter-false-positives.xml
+++ b/res/findbugs/filter-false-positives.xml
@@ -1104,6 +1104,14 @@
<Bug code="RV" />
</Match>
<Match>
+ <!-- Name shadowing is deliberate -->
+ <Or>
+ <Class name="org.apache.tomcat.jdbc.pool.DataSource" />
+ <Class name="org.apache.tomcat.jdbc.pool.XADataSource" />
+ </Or>
+ <Bug pattern="NM_SAME_SIMPLE_NAME_AS_INTERFACE" />
+ </Match>
+ <Match>
<!-- Lock is released -->
<Class name="org.apache.tomcat.jdbc.pool.FairBlockingQueue" />
<Method name="poll" />
@@ -1128,6 +1136,17 @@
<Bug pattern="SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE" />
</Match>
<Match>
+ <!-- Array elements are not mutated -->
+ <Class name="org.apache.tomcat.jdbc.pool.PoolProperties" />
+ <Field name="interceptors" />
+ <Bug pattern="VO_VOLATILE_REFERENCE_TO_ARRAY" />
+ </Match>
+ <Match>
+ <!-- The name isn't great but it is part of the public API now -->
+ <Class name="org.apache.tomcat.jdbc.pool.TrapException" />
+ <Bug pattern="NM_CLASS_NOT_EXCEPTION" />
+ </Match>
+ <Match>
<!-- Lack of thread-safety is accepted in return for better performance. -->
<Class name="org.apache.tomcat.jdbc.pool.interceptor.SlowQueryReport$QueryStats" />
<Or>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 02/04: Fix SpotBugs warnings for unit tests
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
View the commit online:
https://github.com/apache/tomcat/commit/065c5d1ca88f00075ffc9d9b15e295ea69733f13
commit 065c5d1ca88f00075ffc9d9b15e295ea69733f13
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Nov 20 13:09:55 2019 +0000
Fix SpotBugs warnings for unit tests
---
res/findbugs/filter-false-positives.xml | 239 ++++++++++++++++++++-
.../TesterDigestAuthenticatorPerformance.java | 2 +-
.../apache/catalina/connector/TestSendFile.java | 2 +-
.../apache/catalina/core/TestAsyncContextImpl.java | 18 +-
...TestWebappClassLoaderThreadLocalMemoryLeak.java | 5 +-
.../catalina/nonblocking/TestNonBlockingAPI.java | 37 ++--
.../tribes/test/transport/SocketNioReceive.java | 8 +-
.../valves/TestLoadBalancerDrainingValve.java | 6 +-
.../apache/coyote/http11/upgrade/TestUpgrade.java | 9 +-
.../http11/upgrade/TestUpgradeInternalHandler.java | 7 +-
test/org/apache/coyote/http2/Http2TestBase.java | 2 +-
test/org/apache/coyote/http2/TestAsync.java | 7 +-
test/org/apache/jasper/TestJspC.java | 2 +-
.../apache/jasper/TestJspCompilationContext.java | 3 +-
test/org/apache/juli/TestFileHandler.java | 13 +-
15 files changed, 308 insertions(+), 52 deletions(-)
diff --git a/res/findbugs/filter-false-positives.xml b/res/findbugs/filter-false-positives.xml
index 5d63303..73443d6 100644
--- a/res/findbugs/filter-false-positives.xml
+++ b/res/findbugs/filter-false-positives.xml
@@ -1551,6 +1551,12 @@
<!-- Test code -->
<Match>
<!-- Code is deliberately unused -->
+ <Class name="javax.el.TestImportHandler" />
+ <Method name="testImportPackage01_57574"/>
+ <Bug pattern="UC_USELESS_OBJECT"/>
+ </Match>
+ <Match>
+ <!-- Code is deliberately unused -->
<Or>
<Class name="javax.servlet.http.TestCookie" />
<Class name="javax.servlet.http.TestCookieStrict" />
@@ -1558,12 +1564,26 @@
<Bug pattern="DLS_DEAD_LOCAL_STORE"/>
</Match>
<Match>
+ <!-- Name is consistent in context -->
+ <Class name="javax.servlet.http.TestHttpServletResponseSendError$ErrorServletStaticException" />
+ <Bug pattern="NM_CLASS_NOT_EXCEPTION"/>
+ </Match>
+ <Match>
<!-- Code is intentionally unused -->
<Class name="org.apache.catalina.authenticator.TestBasicAuthParser"/>
- <Method name="testAuthMethodBadMethod"/>
+ <Or>
+ <Method name="testAuthMethodBadMethod"/>
+ <Method name="testBadBase64Char"/>
+ </Or>
<Bug pattern="DLS_DEAD_LOCAL_STORE"/>
</Match>
<Match>
+ <!-- Deliberate hack for the purposes of the test -->
+ <Class name="org.apache.catalina.connector.TestCoyoteAdapter$AsyncServlet"/>
+ <Field name="t"/>
+ <Bug pattern="MSF_MUTABLE_SERVLET_FIELD"/>
+ </Match>
+ <Match>
<Class name="org.apache.catalina.core.TestApplicationSessionCookieConfig$CustomContext" />
<Method name="getState"/>
<Bug code="UG" />
@@ -1575,12 +1595,101 @@
<Bug pattern="RV_RETURN_VALUE_IGNORED"/>
</Match>
<Match>
+ <!-- Dead store is deliberate -->
+ <Or>
+ <Class name="org.apache.catalina.core.TestAsyncContextImpl$AsyncDispatchUrlWithSpacesServlet"/>
+ <Class name="org.apache.catalina.core.TestAsyncContextImpl$ForwardDispatchUrlWithSpacesServlet"/>
+ </Or>
+ <Method name="doGet"/>
+ <Bug pattern="DLS_DEAD_LOCAL_STORE"/>
+ </Match>
+ <Match>
+ <!-- Deliberate hack for the purposes of the test -->
+ <Or>
+ <Class name="org.apache.catalina.core.TestAsyncContextImpl$Bug49528Servlet"/>
+ <Class name="org.apache.catalina.core.TestAsyncContextImpl$Bug49567Servlet"/>
+ </Or>
+ <Field name="result"/>
+ <Bug pattern="MSF_MUTABLE_SERVLET_FIELD"/>
+ </Match>
+ <Match>
+ <!-- Deliberate hack for the purposes of the test -->
+ <Class name="org.apache.catalina.core.TestAsyncContextImpl$Bug53843ServletA"/>
+ <Field name="isAsyncWhenExpected"/>
+ <Bug pattern="MSF_MUTABLE_SERVLET_FIELD"/>
+ </Match>
+ <Match>
+ <!-- Deliberate hack for the purposes of the test -->
+ <Class name="org.apache.catalina.core.TestAsyncContextImpl$AsyncIoEndServlet"/>
+ <Field name="asyncIoEndWriteListener"/>
+ <Bug pattern="MSF_MUTABLE_SERVLET_FIELD"/>
+ </Match>
+ <Match>
+ <!-- Deliberate hack for the purposes of the test -->
+ <Class name="org.apache.catalina.core.TestAsyncContextImpl$AsyncISEServlet"/>
+ <Field name="asyncContext"/>
+ <Bug pattern="MSF_MUTABLE_SERVLET_FIELD"/>
+ </Match>
+ <Match>
+ <!-- Deliberate use of run() for the purposes of the test -->
+ <Class name="org.apache.catalina.core.TestAsyncContextStateChanges$AsyncServlet"/>
+ <Method name="doGet"/>
+ <Bug pattern="RU_INVOKE_RUN"/>
+ </Match>
+ <Match>
+ <!-- Hard-coded absolute path is intentional -->
+ <Class name="org.apache.catalina.core.TestStandardContext"/>
+ <Method name="testBug57556b"/>
+ <Bug pattern="DMI_HARDCODED_ABSOLUTE_FILENAME"/>
+ </Match>
+ <Match>
+ <!-- Deliberate hack for the purposes of the test -->
+ <Class name="org.apache.catalina.core.TestStandardContext$Bug51376Servlet"/>
+ <Field name="destroyOk"/>
+ <Bug pattern="MSF_MUTABLE_SERVLET_FIELD"/>
+ </Match>
+ <Match>
+ <!-- Deliberate hack for the purposes of the test -->
+ <Class name="org.apache.catalina.filters.TestRemoteIpFilter$MockHttpServlet"/>
+ <Field name="request"/>
+ <Bug pattern="MSF_MUTABLE_SERVLET_FIELD"/>
+ </Match>
+ <Match>
<!-- Return value of latch is intentionally ignored -->
<Class name="org.apache.catalina.nonblocking.TestNonBlockingAPI"/>
<Method name="testDelayedNBWrite"/>
<Bug pattern="RV_RETURN_VALUE_IGNORED"/>
</Match>
<Match>
+ <!-- Deliberate hack for the purposes of the test -->
+ <Class name="org.apache.catalina.nonblocking.TestNonBlockingAPI$NBReadServlet"/>
+ <Filed name="listener"/>
+ <Bug pattern="MSF_MUTABLE_SERVLET_FIELD"/>
+ </Match>
+ <Match>
+ <!-- Return value of read is intentionally ignored -->
+ <Class name="org.apache.catalina.nonblocking.TestNonBlockingAPI$NBReadWithDispatchServlet$1"/>
+ <Method name="onDataAvailable"/>
+ <Bug pattern="RR_NOT_CHECKED"/>
+ </Match>
+ <Match>
+ <!-- Deliberate hack for the purposes of the test -->
+ <Class name="org.apache.catalina.startup.TesterServletWithAnnotations"/>
+ <Or>
+ <Field name="envEntry2"/>
+ <Field name="envEntry3"/>
+ <Field name="envEntry4"/>
+ <Field name="envEntry6"/>
+ </Or>
+ <Bug pattern="MSF_MUTABLE_SERVLET_FIELD"/>
+ </Match>
+ <Match>
+ <!-- Deliberate hack for the purposes of the test -->
+ <Class name="org.apache.catalina.startup.TesterServletWithLifeCycleMethods"/>
+ <Field name="result"/>
+ <Bug pattern="MSF_MUTABLE_SERVLET_FIELD"/>
+ </Match>
+ <Match>
<Or>
<Class name="org.apache.catalina.startup.TestListener$SCL" />
<Class name="org.apache.catalina.startup.TestListener$SCL3" />
@@ -1593,6 +1702,12 @@
<Bug code="Se"/>
</Match>
<Match>
+ <!-- Deliberate hack for the purposes of the test -->
+ <Class name="org.apache.catalina.startup.TestTomcat$CustomContextConfig"/>
+ <Field name="used"/>
+ <Bug pattern="ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD"/>
+ </Match>
+ <Match>
<Class name="org.apache.catalina.tribes.demos.EchoRpcTest" />
<Method name="run"/>
<Bug code="REC" />
@@ -1705,6 +1820,60 @@
<Bug pattern="DLS_DEAD_LOCAL_STORE"/>
</Match>
<Match>
+ <!-- Return value ignored as this is a performance test -->
+ <Class name="org.apache.catalina.webresources.TestAbstractFileResourceSetPerformance" />
+ <Method name="testFileNameFiltering" />
+ <Bug pattern="RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT"/>
+ </Match>
+ <Match>
+ <!-- Use of hard-coded path is deliberate -->
+ <Class name="org.apache.catalina.webresources.TestStandardRoot" />
+ <Method name="<clinit>" />
+ <Bug pattern="DMI_HARDCODED_ABSOLUTE_FILENAME"/>
+ </Match>
+ <Match>
+ <!-- Deliberate hack for the purposes of the test -->
+ <Class name="org.apache.coyote.http11.filters.TestChunkedInputFilter$BodyReadServlet"/>
+ <Or>
+ <Field name="countRead"/>
+ <Field name="exceptionDuringRead"/>
+ </Or>
+ <Bug pattern="MSF_MUTABLE_SERVLET_FIELD"/>
+ </Match>
+ <Match>
+ <!-- Deliberate hack for the purposes of the test -->
+ <Class name="org.apache.coyote.http11.filters.TestChunkedInputFilter$EchoHeaderServlet"/>
+ <Field name="exceptionDuringRead"/>
+ <Bug pattern="MSF_MUTABLE_SERVLET_FIELD"/>
+ </Match>
+ <Match>
+ <!-- Deliberate hack for the purposes of the test -->
+ <Class name="org.apache.coyote.http11.TestHttp11Processor"/>
+ <Field name="bug55772IsSecondRequest"/>
+ <Bug pattern="ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD"/>
+ </Match>
+ <Match>
+ <!-- Result is negated to compare result when order is reversed -->
+ <Class name="org.apache.el.TestELEvaluation" />
+ <Method name="compareBoth" />
+ <Bug pattern="RV_NEGATING_RESULT_OF_COMPARETO" />
+ </Match>
+ <Match>
+ <!-- Performance test so results ignored -->
+ <Class name="org.apache.jasper.compiler.TesterValidator" />
+ <Method name="doTestBug53867" />
+ <Bug pattern="RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT" />
+ </Match>
+ <Match>
+ <!-- Test is single threaded. Syncs not required. -->
+ <Class name="org.apache.jasper.util.FastRemovalDequeue" />
+ <Or>
+ <Field name="first" />
+ <Field name="last" />
+ </Or>
+ <Bug pattern="IS2_INCONSISTENT_SYNC" />
+ </Match>
+ <Match>
<!-- Field set via injection-->
<Class name="org.apache.naming.TesterInjectionServlet" />
<Or>
@@ -1714,6 +1883,15 @@
<Bug pattern="UWF_NULL_FIELD" />
</Match>
<Match>
+ <!-- Deliberate hack for the purposes of the test -->
+ <Class name="org.apache.naming.TesterInjectionServlet" />
+ <Or>
+ <Field name="property2"/>
+ <Field name="property2a"/>
+ </Or>
+ <Bug pattern="MSF_MUTABLE_SERVLET_FIELD"/>
+ </Match>
+ <Match>
<!-- Use of statics is unavoidable in all cases -->
<!-- Better to use it consistently rather than only where necessary -->
<Class name="org.apache.tomcat.jdbc.pool.interceptor.TestInterceptor" />
@@ -1760,6 +1938,12 @@
<Bug pattern="XSS_REQUEST_PARAMETER_TO_SERVLET_WRITER"/>
</Match>
<Match>
+ <!-- No performance issue as there is no DNS resolution -->
+ <Class name="org.apache.tomcat.util.bcel.TesterPerformance" />
+ <Method name="testClassParserPerformance" />
+ <Bug pattern="DMI_COLLECTION_OF_URLS" />
+ </Match>
+ <Match>
<Class name="org.apache.tomcat.util.net.TestSsl" />
<Or>
<Method name="testRenegotiateFail" />
@@ -1768,6 +1952,29 @@
<Bug code="RR" />
</Match>
<Match>
+ <!-- Path is designed to test edge cases and does not have to exist-->
+ <Class name="org.apache.tomcat.util.buf.TesterUriUtilBase" />
+ <Or>
+ <Method name="testBuildJarUrl01"/>
+ <Method name="testBuildJarUrl02"/>
+ <Method name="testBuildJarUrl03"/>
+ <Method name="performanceTestBuildJarUrl"/>
+ </Or>
+ <Bug pattern="DMI_HARDCODED_ABSOLUTE_FILENAME"/>
+ </Match>
+ <Match>
+ <!-- Object not used as this is a performance test -->
+ <Class name="org.apache.tomcat.util.http.TesterParametersPerformance" />
+ <Method name="doCreateString" />
+ <Bug pattern="UC_USELESS_OBJECT" />
+ </Match>
+ <Match>
+ <!-- Return value ignored because an exception is expected -->
+ <Class name="org.apache.tomcat.util.net.TestTLSClientHelloExtractor" />
+ <Method name="doTestInputMalformed" />
+ <Bug pattern="RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT" />
+ </Match>
+ <Match>
<Class name="org.apache.tomcat.util.threads.TestLimitLatch" />
<Or>
<Method name="waitForThreadToStop" />
@@ -1799,13 +2006,41 @@
<Bug pattern="ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD"/>
</Match>
<Match>
+ <!-- Statics are used deliberately as they are simpler -->
+ <Class name="org.apache.tomcat.websocket.TestWsSubprotocols$SubProtocolsEndpoint" />
+ <Field name="subprotocols" />
+ <Bug pattern="ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD"/>
+ </Match>
+ <Match>
+ <!-- Statics are used deliberately as they are simpler -->
+ <Class name="org.apache.tomcat.websocket.TestWsWebSocketContainer$ConstantTxEndpoint" />
+ <Or>
+ <Field name="exception" />
+ <Field name="running" />
+ <Field name="timeout" />
+ </Or>
+ <Bug pattern="ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD"/>
+ </Match>
+ <Match>
<!-- Code is deliberately unused -->
<Class name="org.apache.tomcat.websocket.server.TestUriTemplate" />
<Or>
<Method name="testBasicPrefix" />
- <Method name="testQuote2" />
<Method name="testDuplicate01" />
+ <Method name="testEgMailingList04" />
+ <Method name="testEgMailingList05" />
+ <Method name="testQuote2" />
</Or>
<Bug pattern="DLS_DEAD_LOCAL_STORE"/>
</Match>
+ <Match>
+ <!-- Exception thrown so return value ignored -->
+ <Class name="org.apache.tomcat.websocket.server.TestUriTemplate" />
+ <Or>
+ <Method name="testPrefixOneOfTwo" />
+ <Method name="testPrefixTwoOfTwo" />
+ <Method name="testQuote1" />
+ </Or>
+ <Bug pattern="RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT"/>
+ </Match>
</FindBugsFilter>
diff --git a/test/org/apache/catalina/authenticator/TesterDigestAuthenticatorPerformance.java b/test/org/apache/catalina/authenticator/TesterDigestAuthenticatorPerformance.java
index 991dda5..4ff8335 100644
--- a/test/org/apache/catalina/authenticator/TesterDigestAuthenticatorPerformance.java
+++ b/test/org/apache/catalina/authenticator/TesterDigestAuthenticatorPerformance.java
@@ -98,7 +98,7 @@ public class TesterDigestAuthenticatorPerformance {
System.out.println("Average time per request (wall): " +
wallTime/(threadCount * requestCount));
- Assert.assertEquals(requestCount * threadCount, totalSuccess);
+ Assert.assertEquals(((long)requestCount) * threadCount, totalSuccess);
}
@Before
diff --git a/test/org/apache/catalina/connector/TestSendFile.java b/test/org/apache/catalina/connector/TestSendFile.java
index a8aafbc..3fca72f 100644
--- a/test/org/apache/catalina/connector/TestSendFile.java
+++ b/test/org/apache/catalina/connector/TestSendFile.java
@@ -80,7 +80,7 @@ public class TestSendFile extends TomcatBaseTest {
Assert.assertEquals(HttpServletResponse.SC_OK, rc);
System.out.println("Client received " + bc.getLength() + " bytes in "
+ (System.currentTimeMillis() - start) + " ms.");
- Assert.assertEquals(EXPECTED_CONTENT_LENGTH * (i + 1), bc.getLength());
+ Assert.assertEquals(EXPECTED_CONTENT_LENGTH * (i + 1L), bc.getLength());
bc.recycle();
}
diff --git a/test/org/apache/catalina/core/TestAsyncContextImpl.java b/test/org/apache/catalina/core/TestAsyncContextImpl.java
index d305cbd..31942f5 100644
--- a/test/org/apache/catalina/core/TestAsyncContextImpl.java
+++ b/test/org/apache/catalina/core/TestAsyncContextImpl.java
@@ -564,7 +564,7 @@ public class TestAsyncContextImpl extends TomcatBaseTest {
private static final long serialVersionUID = 1L;
private final Boolean completeOnTimeout;
- private final TrackingListener trackingListener;
+ private final transient TrackingListener trackingListener;
public static final long ASYNC_TIMEOUT = 100;
@@ -694,7 +694,7 @@ public class TestAsyncContextImpl extends TomcatBaseTest {
private static final long serialVersionUID = 1L;
private static final String ITER_PARAM = "iter";
private static final String DISPATCH_CHECK = "check";
- private final TrackingListener trackingListener;
+ private final transient TrackingListener trackingListener;
public DispatchingServlet(boolean addTrackingListener,
boolean completeOnError) {
@@ -1719,7 +1719,7 @@ public class TestAsyncContextImpl extends TomcatBaseTest {
REQUEST_TIME);
}
- private static enum ErrorPageAsyncMode {
+ private enum ErrorPageAsyncMode {
NO_COMPLETE,
COMPLETE,
DISPATCH
@@ -1905,7 +1905,7 @@ public class TestAsyncContextImpl extends TomcatBaseTest {
private static final long serialVersionUID = 1L;
- private final TrackingListener trackingListener = new TrackingListener(true, false, "/async");
+ private final transient TrackingListener trackingListener = new TrackingListener(true, false, "/async");
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
@@ -2756,7 +2756,7 @@ public class TestAsyncContextImpl extends TomcatBaseTest {
private final boolean useThread;
private final boolean useComplete;
- private AsyncIoEndWriteListener asyncIoEndWriteListener;
+ private transient AsyncIoEndWriteListener asyncIoEndWriteListener;
public AsyncIoEndServlet(boolean useThread, boolean useComplete) {
this.useThread = useThread;
@@ -2775,7 +2775,7 @@ public class TestAsyncContextImpl extends TomcatBaseTest {
}
AsyncContext ac = req.startAsync();
ServletOutputStream sos = resp.getOutputStream();
- asyncIoEndWriteListener= new AsyncIoEndWriteListener(ac, useThread, useComplete);
+ asyncIoEndWriteListener = new AsyncIoEndWriteListener(ac, useThread, useComplete);
sos.setWriteListener(asyncIoEndWriteListener);
}
@@ -2930,9 +2930,9 @@ public class TestAsyncContextImpl extends TomcatBaseTest {
private static final long serialVersionUID = 1L;
- private final CountDownLatch doGetLatch;
- private final CountDownLatch clientCloseLatch;
- private final CountDownLatch threadCompleteLatch;
+ private final transient CountDownLatch doGetLatch;
+ private final transient CountDownLatch clientCloseLatch;
+ private final transient CountDownLatch threadCompleteLatch;
private final AtomicBoolean ise;
public Bug63816Servlet(CountDownLatch doGetLatch, CountDownLatch clientCloseLatch,
diff --git a/test/org/apache/catalina/loader/TestWebappClassLoaderThreadLocalMemoryLeak.java b/test/org/apache/catalina/loader/TestWebappClassLoaderThreadLocalMemoryLeak.java
index 34e4d6e..a1c948b 100644
--- a/test/org/apache/catalina/loader/TestWebappClassLoaderThreadLocalMemoryLeak.java
+++ b/test/org/apache/catalina/loader/TestWebappClassLoaderThreadLocalMemoryLeak.java
@@ -196,8 +196,7 @@ public class TestWebappClassLoaderThreadLocalMemoryLeak extends TomcatBaseTest {
"org.apache.tomcat.unittest." + name, classBytes, 0,
offset, cl.getClass().getProtectionDomain());
// Make sure we can create an instance
- Object obj = lpClass.getConstructor().newInstance();
- obj.toString();
+ lpClass.getConstructor().newInstance();
}
}
-}
\ No newline at end of file
+}
diff --git a/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java b/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
index a1a9a4a..3e0597c 100644
--- a/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
+++ b/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
@@ -129,6 +129,9 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
Assert.assertEquals(HttpServletResponse.SC_OK, rc);
if (async) {
Assert.assertEquals(2000000 * 8, servlet.listener.body.length());
+ TestAsyncReadListener listener = (TestAsyncReadListener) servlet.listener;
+ Assert.assertTrue(Math.abs(listener.containerThreadCount.get() - listener.notReadyCount.get()) <= 1);
+ Assert.assertEquals(listener.isReadyCount.get(), listener.nonContainerThreadCount.get());
} else {
Assert.assertEquals(5 * 8, servlet.listener.body.length());
}
@@ -174,7 +177,13 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
"Host: localhost:" + getPort() + "\r\n" +
"\r\n").getBytes(StandardCharsets.ISO_8859_1));
os.flush();
- is.read(buffer);
+ // Make sure the entire response has been read.
+ int read = is.read(buffer);
+ // The response should end with CRLFCRLF
+ Assert.assertEquals(buffer[read - 4], '\r');
+ Assert.assertEquals(buffer[read - 3], '\n');
+ Assert.assertEquals(buffer[read - 2], '\r');
+ Assert.assertEquals(buffer[read - 1], '\n');
}
os.write(("GET / HTTP/1.1\r\n" +
"Host: localhost:" + getPort() + "\r\n" +
@@ -186,11 +195,15 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
int readSinceLastPause = 0;
while (read != -1) {
read = is.read(buffer);
+ if (readSinceLastPause == 0) {
+ log.info("Reading data");
+ }
if (read > 0) {
result.append(buffer, 0, read);
}
readSinceLastPause += read;
if (readSinceLastPause > WRITE_SIZE / 16) {
+ log.info("Read " + readSinceLastPause + " bytes, pause 500ms");
readSinceLastPause = 0;
Thread.sleep(500);
}
@@ -428,8 +441,8 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
int max = 5;
int count = 0;
long delay = 0;
- byte[] b = "WANTMORE".getBytes();
- byte[] f = "FINISHED".getBytes();
+ byte[] b = "WANTMORE".getBytes(StandardCharsets.ISO_8859_1);
+ byte[] f = "FINISHED".getBytes(StandardCharsets.ISO_8859_1);
public DataWriter(long delay, int max) {
this.delay = delay;
@@ -472,7 +485,7 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
}
@WebServlet(asyncSupported = true)
- public class NBReadServlet extends TesterServlet {
+ public static class NBReadServlet extends TesterServlet {
private static final long serialVersionUID = 1L;
private final boolean async;
private final boolean ignoreIsReady;
@@ -526,10 +539,10 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
}
@WebServlet(asyncSupported = true)
- public class NBWriteServlet extends TesterServlet {
+ public static class NBWriteServlet extends TesterServlet {
private static final long serialVersionUID = 1L;
- public volatile TestWriteListener wlistener;
- public volatile TestReadListener rlistener;
+ public transient volatile TestWriteListener wlistener;
+ public transient volatile TestReadListener rlistener;
@Override
protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
@@ -572,9 +585,9 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
}
@WebServlet(asyncSupported = true)
- public class NBReadWriteServlet extends TesterServlet {
+ public static class NBReadWriteServlet extends TesterServlet {
private static final long serialVersionUID = 1L;
- public volatile TestReadWriteListener rwlistener;
+ public transient volatile TestReadWriteListener rwlistener;
@Override
protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
@@ -714,7 +727,7 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
}
}
- private class TestWriteListener implements WriteListener {
+ private static class TestWriteListener implements WriteListener {
AsyncContext ctx;
int written = 0;
public volatile boolean onErrorInvoked = false;
@@ -758,7 +771,7 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
}
- private class TestReadWriteListener implements ReadListener {
+ private static class TestReadWriteListener implements ReadListener {
AsyncContext ctx;
private final StringBuilder body = new StringBuilder();
@@ -1028,7 +1041,7 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
@WebServlet(asyncSupported = true)
- private final class NBReadWithDispatchServlet extends TesterServlet {
+ private static final class NBReadWithDispatchServlet extends TesterServlet {
private static final long serialVersionUID = 1L;
diff --git a/test/org/apache/catalina/tribes/test/transport/SocketNioReceive.java b/test/org/apache/catalina/tribes/test/transport/SocketNioReceive.java
index 5ce21fe..763d7b2 100644
--- a/test/org/apache/catalina/tribes/test/transport/SocketNioReceive.java
+++ b/test/org/apache/catalina/tribes/test/transport/SocketNioReceive.java
@@ -53,11 +53,9 @@ public class SocketNioReceive {
System.out.println("Listening on 9999");
while (true) {
try {
- synchronized (mutex) {
- mutex.wait(5000);
- if ( start != 0 ) {
- System.out.println("Throughput " + df.format(mb / seconds) + " MB/seconds, messages "+count+" accepts "+accept+", total "+mb+" MB.");
- }
+ Thread.sleep(5000);
+ if ( start != 0 ) {
+ System.out.println("Throughput " + df.format(mb / seconds) + " MB/seconds, messages "+count+" accepts "+accept+", total "+mb+" MB.");
}
}catch (Throwable x) {
x.printStackTrace();
diff --git a/test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java b/test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java
index 2acf8ed..5953499 100644
--- a/test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java
+++ b/test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java
@@ -129,9 +129,11 @@ public class TestLoadBalancerDrainingValve {
@Override
public boolean equals(Object o) {
- if(null == o) return false;
- MyCookie mc = (MyCookie)o;
+ if(!(o instanceof MyCookie)) {
+ return false;
+ }
+ MyCookie mc = (MyCookie)o;
return mc.getName().equals(this.getName())
&& mc.getPath().equals(this.getPath())
&& mc.getValue().equals(this.getValue())
diff --git a/test/org/apache/coyote/http11/upgrade/TestUpgrade.java b/test/org/apache/coyote/http11/upgrade/TestUpgrade.java
index 93c3edf..c5180d6 100644
--- a/test/org/apache/coyote/http11/upgrade/TestUpgrade.java
+++ b/test/org/apache/coyote/http11/upgrade/TestUpgrade.java
@@ -26,6 +26,7 @@ import java.io.PrintWriter;
import java.io.Reader;
import java.io.Writer;
import java.net.Socket;
+import java.nio.charset.StandardCharsets;
import javax.net.SocketFactory;
import javax.servlet.ReadListener;
@@ -147,6 +148,7 @@ public class TestUpgrade extends TomcatBaseTest {
Assert.assertEquals(MESSAGE, response);
uc.shutdownInput();
+ pw.close();
}
@@ -227,8 +229,9 @@ public class TestUpgrade extends TomcatBaseTest {
throw new IllegalArgumentException(ioe);
}
- BufferedReader reader = new BufferedReader(new InputStreamReader(is));
- Writer writer = new OutputStreamWriter(os);
+ BufferedReader reader =
+ new BufferedReader(new InputStreamReader(is, StandardCharsets.UTF_8));
+ Writer writer = new OutputStreamWriter(os, StandardCharsets.UTF_8);
this.writer = writer;
this.reader = reader;
@@ -302,7 +305,7 @@ public class TestUpgrade extends TomcatBaseTest {
}
- private class EchoListener implements ReadListener, WriteListener {
+ private static class EchoListener implements ReadListener, WriteListener {
private final ServletInputStream sis;
private final ServletOutputStream sos;
diff --git a/test/org/apache/coyote/http11/upgrade/TestUpgradeInternalHandler.java b/test/org/apache/coyote/http11/upgrade/TestUpgradeInternalHandler.java
index 84b64ae..babab0d 100644
--- a/test/org/apache/coyote/http11/upgrade/TestUpgradeInternalHandler.java
+++ b/test/org/apache/coyote/http11/upgrade/TestUpgradeInternalHandler.java
@@ -27,6 +27,7 @@ import java.io.Writer;
import java.net.Socket;
import java.nio.ByteBuffer;
import java.nio.channels.CompletionHandler;
+import java.nio.charset.StandardCharsets;
import java.util.concurrent.TimeUnit;
import javax.net.SocketFactory;
@@ -79,6 +80,7 @@ public class TestUpgradeInternalHandler extends TomcatBaseTest {
Assert.assertEquals(MESSAGE, response);
uc.shutdownInput();
+ pw.close();
}
private UpgradeConnection doUpgrade(
@@ -158,8 +160,9 @@ public class TestUpgradeInternalHandler extends TomcatBaseTest {
throw new IllegalArgumentException(ioe);
}
- BufferedReader reader = new BufferedReader(new InputStreamReader(is));
- Writer writer = new OutputStreamWriter(os);
+ BufferedReader reader =
+ new BufferedReader(new InputStreamReader(is, StandardCharsets.UTF_8));
+ Writer writer = new OutputStreamWriter(os, StandardCharsets.UTF_8);
this.writer = writer;
this.reader = reader;
diff --git a/test/org/apache/coyote/http2/Http2TestBase.java b/test/org/apache/coyote/http2/Http2TestBase.java
index 7f08fcb..3e611c5 100644
--- a/test/org/apache/coyote/http2/Http2TestBase.java
+++ b/test/org/apache/coyote/http2/Http2TestBase.java
@@ -1248,7 +1248,7 @@ public abstract class Http2TestBase extends TomcatBaseTest {
int count = 128 * 1024;
// Two bytes per entry
- resp.setContentLengthLong(count * 2);
+ resp.setContentLengthLong(count * 2L);
OutputStream os = resp.getOutputStream();
byte[] data = new byte[2];
diff --git a/test/org/apache/coyote/http2/TestAsync.java b/test/org/apache/coyote/http2/TestAsync.java
index 83c46ab..e01bb7e 100644
--- a/test/org/apache/coyote/http2/TestAsync.java
+++ b/test/org/apache/coyote/http2/TestAsync.java
@@ -153,7 +153,6 @@ public class TestAsync extends Http2TestBase {
// Body
if (!connectionUnlimited || !streamUnlimited) {
-
while (output.getBytesRead() < startingWindowSize) {
parser.readFrame(true);
}
@@ -193,7 +192,7 @@ public class TestAsync extends Http2TestBase {
}
// Check that the right number of bytes were received
- Assert.assertEquals(blockCount * BLOCK_SIZE, output.getBytesRead());
+ Assert.assertEquals((long) blockCount * BLOCK_SIZE, output.getBytesRead());
}
@@ -203,8 +202,8 @@ public class TestAsync extends Http2TestBase {
private final int blockLimit;
private final boolean useNonContainerThreadForWrite;
- private final ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1);
- private volatile Future<?> future;
+ private final transient ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1);
+ private transient volatile Future<?> future;
public AsyncServlet(int blockLimit, boolean useNonContainerThreadForWrite) {
this.blockLimit = blockLimit;
diff --git a/test/org/apache/jasper/TestJspC.java b/test/org/apache/jasper/TestJspC.java
index 620f572..f5d28e9 100644
--- a/test/org/apache/jasper/TestJspC.java
+++ b/test/org/apache/jasper/TestJspC.java
@@ -106,7 +106,7 @@ public class TestJspC {
private void precompile(File appDir, File webappOut) throws IOException {
remove(webappOut);
- webappOut.mkdirs();
+ Assert.assertTrue("Failed to create [" + webappOut + "]", webappOut.mkdirs());
jspc.setUriroot(appDir.toString());
jspc.setOutputDir(webappOut.toString());
jspc.setValidateTld(false);
diff --git a/test/org/apache/jasper/TestJspCompilationContext.java b/test/org/apache/jasper/TestJspCompilationContext.java
index 7a9d36c..7c1b786 100644
--- a/test/org/apache/jasper/TestJspCompilationContext.java
+++ b/test/org/apache/jasper/TestJspCompilationContext.java
@@ -58,7 +58,8 @@ public class TestJspCompilationContext extends TomcatBaseTest {
Assert.assertTrue(body.toString().contains("00 - OK"));
File jsp = new File("test/webapp/jsp/tagFileInJar.jsp");
- jsp.setLastModified(jsp.lastModified() + 10000);
+ Assert.assertTrue("Failed to set last modified for [" + jsp + "]",
+ jsp.setLastModified(jsp.lastModified() + 10000));
// This test requires that modificationTestInterval is set to zero in
// web.xml. If not, a sleep longer that modificationTestInterval is
diff --git a/test/org/apache/juli/TestFileHandler.java b/test/org/apache/juli/TestFileHandler.java
index e265884..c6c779a 100644
--- a/test/org/apache/juli/TestFileHandler.java
+++ b/test/org/apache/juli/TestFileHandler.java
@@ -59,8 +59,9 @@ public class TestFileHandler {
date.add(Calendar.DAY_OF_MONTH, -3);
SimpleDateFormat formatter = new SimpleDateFormat("yyyy-MM-dd'T'HH-mm-ss.SSS");
File file = new File(logsDir, PREFIX_1 + formatter.format(date.getTime()) + SUFIX_1);
- file.createNewFile();
-
+ if (!file.createNewFile()) {
+ Assert.fail("Unable to create " + file.getAbsolutePath());
+ }
}
@After
@@ -68,9 +69,9 @@ public class TestFileHandler {
File[] files = logsDir.listFiles();
if (files != null) {
for (File file : files) {
- file.delete();
+ Assert.assertTrue("Failed to delete [" + file + "]", file.delete());
}
- logsDir.delete();
+ Assert.assertTrue("Failed to create [" + logsDir + "]", logsDir.delete());
}
}
@@ -126,7 +127,9 @@ public class TestFileHandler {
for (int i = 0; i < amount; i++) {
cal.add(Calendar.DAY_OF_MONTH, -1);
File file = new File(dir, prefix + formatter.format(cal.getTime()) + sufix);
- file.createNewFile();
+ if (!file.createNewFile()) {
+ Assert.fail("Unable to create " + file.getAbsolutePath());
+ }
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 04/04: One missed false positive
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
View the commit online:
https://github.com/apache/tomcat/commit/85ba8b6836e07727def00c4845081eacc7f0f9c8
commit 85ba8b6836e07727def00c4845081eacc7f0f9c8
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Nov 20 13:22:58 2019 +0000
One missed false positive
---
res/findbugs/filter-false-positives.xml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/res/findbugs/filter-false-positives.xml b/res/findbugs/filter-false-positives.xml
index 06f104e..b45c160 100644
--- a/res/findbugs/filter-false-positives.xml
+++ b/res/findbugs/filter-false-positives.xml
@@ -1708,6 +1708,12 @@
<Bug pattern="ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD"/>
</Match>
<Match>
+ <!-- Test code - array is safe -->
+ <Class name="org.apache.catalina.startup.TomcatBaseTest"/>
+ <Field name="booleans"/>
+ <Bug pattern="MS_MUTABLE_ARRAY"/>
+ </Match>
+ <Match>
<Class name="org.apache.catalina.tribes.demos.EchoRpcTest" />
<Method name="run"/>
<Bug code="REC" />
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org