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 2020/10/16 16:16:00 UTC

[tomcat] branch master updated (66df2af -> fcd87ed)

This is an automated email from the ASF dual-hosted git repository.

markt pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git.


    from 66df2af  SpotBugs - Additional false positives when running with 4.1.4
     new 5c77665  SpotBugs - fix various exception class related issues
     new fcd87ed  Remaining false positives

The 2 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:
 res/checkstyle/checkstyle.xml                             |  5 +++--
 res/findbugs/filter-false-positives.xml                   | 15 +++++++++++++++
 test/jakarta/el/TestBeanELResolver.java                   |  7 +++++--
 .../catalina/authenticator/TestBasicAuthParser.java       | 14 +++-----------
 .../apache/catalina/core/TestSwallowAbortedUploads.java   | 11 +++++++----
 test/org/apache/catalina/realm/TestJNDIRealm.java         |  7 +++++--
 .../startup/TestHostConfigAutomaticDeployment.java        |  5 ++++-
 test/org/apache/catalina/startup/TestTomcat.java          |  7 +++++--
 .../apache/catalina/startup/TestWebappServiceLoader.java  |  7 +++++--
 test/org/apache/catalina/tribes/io/TestXByteBuffer.java   |  5 ++++-
 .../apache/jasper/compiler/TestELInterpreterFactory.java  | 14 ++++++++------
 11 files changed, 64 insertions(+), 33 deletions(-)


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 01/02: SpotBugs - fix various exception class related issues

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 5c77665d560be3aa0216eb658089c56815a3a499
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Oct 16 17:06:16 2020 +0100

    SpotBugs - fix various exception class related issues
    
    I'd prefer not to use static imports at all but JUnit uses them
    extensively and avoiding them in these instances means unnecessarily
    verbose code.
---
 res/checkstyle/checkstyle.xml                              |  5 +++--
 test/jakarta/el/TestBeanELResolver.java                    |  7 +++++--
 .../apache/catalina/authenticator/TestBasicAuthParser.java | 14 +++-----------
 .../apache/catalina/core/TestSwallowAbortedUploads.java    | 11 +++++++----
 test/org/apache/catalina/realm/TestJNDIRealm.java          |  7 +++++--
 .../startup/TestHostConfigAutomaticDeployment.java         |  5 ++++-
 test/org/apache/catalina/startup/TestTomcat.java           |  7 +++++--
 .../apache/catalina/startup/TestWebappServiceLoader.java   |  7 +++++--
 test/org/apache/catalina/tribes/io/TestXByteBuffer.java    |  5 ++++-
 .../apache/jasper/compiler/TestELInterpreterFactory.java   | 14 ++++++++------
 10 files changed, 49 insertions(+), 33 deletions(-)

diff --git a/res/checkstyle/checkstyle.xml b/res/checkstyle/checkstyle.xml
index 6fb7231..b52dc27 100644
--- a/res/checkstyle/checkstyle.xml
+++ b/res/checkstyle/checkstyle.xml
@@ -63,8 +63,9 @@
     <!-- Imports -->
     <module name="AvoidStarImport"/>
     <module name="AvoidStaticImport">
-        <property name="excludes"
-                  value="org.apache.catalina.startup.SimpleHttpClient.CRLF"/>
+        <property name="excludes" value="org.apache.catalina.startup.SimpleHttpClient.CRLF"/>
+        <property name="excludes" value="org.hamcrest.MatcherAssert.*"/>
+        <property name="excludes" value="org.hamcrest.core.IsInstanceOf.*"/>
     </module>
     <module name="IllegalImport">
         <property name="illegalPkgs" value="sun,junit.framework"/>
diff --git a/test/jakarta/el/TestBeanELResolver.java b/test/jakarta/el/TestBeanELResolver.java
index a1bf5a3..312a5a4 100644
--- a/test/jakarta/el/TestBeanELResolver.java
+++ b/test/jakarta/el/TestBeanELResolver.java
@@ -21,6 +21,9 @@ import java.beans.PropertyDescriptor;
 import java.util.ArrayList;
 import java.util.Iterator;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
+
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -57,9 +60,9 @@ public class TestBeanELResolver {
         } catch (PropertyNotFoundException pnfe) {
             e = pnfe;
         }
-        Assert.assertTrue("Wrong exception type",
-                e instanceof PropertyNotFoundException);
+        assertThat("Wrong exception type", e, instanceOf(PropertyNotFoundException.class));
         String type = Bean.class.getName();
+        @SuppressWarnings("null") // Not possible due to test above
         String msg = e.getMessage();
         Assert.assertTrue("No reference to type [" + type +
                 "] where property cannot be found in [" + msg + "]",
diff --git a/test/org/apache/catalina/authenticator/TestBasicAuthParser.java b/test/org/apache/catalina/authenticator/TestBasicAuthParser.java
index 93e874d..d1dfe31 100644
--- a/test/org/apache/catalina/authenticator/TestBasicAuthParser.java
+++ b/test/org/apache/catalina/authenticator/TestBasicAuthParser.java
@@ -165,22 +165,14 @@ public class TestBasicAuthParser {
     /*
      * Confirm the Basic parser rejects an invalid authentication method.
      */
-    @Test
+    @Test(expected = IllegalArgumentException.class)
     public void testAuthMethodBadMethod() throws Exception {
         final String METHOD = "BadMethod";
         final BasicAuthHeader AUTH_HEADER =
                 new BasicAuthHeader(METHOD, USER_NAME, PASSWORD);
         @SuppressWarnings("unused")
-        BasicAuthenticator.BasicCredentials credentials = null;
-        try {
-            credentials = new BasicAuthenticator.BasicCredentials(
-                AUTH_HEADER.getHeader(), StandardCharsets.UTF_8, true);
-            Assert.fail("IllegalArgumentException expected");
-        }
-        catch (Exception e) {
-            Assert.assertTrue(e instanceof IllegalArgumentException);
-            Assert.assertTrue(e.getMessage().contains("header method"));
-        }
+        BasicAuthenticator.BasicCredentials credentials =
+                new BasicAuthenticator.BasicCredentials(AUTH_HEADER.getHeader(), StandardCharsets.UTF_8, true);
     }
 
     /*
diff --git a/test/org/apache/catalina/core/TestSwallowAbortedUploads.java b/test/org/apache/catalina/core/TestSwallowAbortedUploads.java
index 1d0c077..e9c855a 100644
--- a/test/org/apache/catalina/core/TestSwallowAbortedUploads.java
+++ b/test/org/apache/catalina/core/TestSwallowAbortedUploads.java
@@ -35,6 +35,9 @@ import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
 import jakarta.servlet.http.Part;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
+
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -128,8 +131,8 @@ public class TestSwallowAbortedUploads extends TomcatBaseTest {
         log.info("Limited, swallow disabled");
         AbortedUploadClient client = new AbortedUploadClient();
         Exception ex = doAbortedUploadTest(client, true, false);
-        Assert.assertTrue("Limited upload with swallow disabled does not generate client exception",
-                   ex instanceof java.net.SocketException);
+        assertThat("Limited upload with swallow disabled does not generate client exception",
+                   ex, instanceOf(java.net.SocketException.class));
         client.reset();
     }
 
@@ -174,8 +177,8 @@ public class TestSwallowAbortedUploads extends TomcatBaseTest {
         log.info("Aborted (413), swallow disabled");
         AbortedPOSTClient client = new AbortedPOSTClient();
         Exception ex = doAbortedPOSTTest(client, HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE, false);
-        Assert.assertTrue("Limited upload with swallow disabled does not generate client exception",
-                   ex instanceof java.net.SocketException);
+        assertThat("Limited upload with swallow disabled does not generate client exception",
+                ex, instanceOf(java.net.SocketException.class));
         client.reset();
     }
 
diff --git a/test/org/apache/catalina/realm/TestJNDIRealm.java b/test/org/apache/catalina/realm/TestJNDIRealm.java
index c729be8..845bc61 100644
--- a/test/org/apache/catalina/realm/TestJNDIRealm.java
+++ b/test/org/apache/catalina/realm/TestJNDIRealm.java
@@ -29,6 +29,9 @@ import javax.naming.directory.InitialDirContext;
 import javax.naming.directory.SearchControls;
 import javax.naming.directory.SearchResult;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
+
 import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -87,7 +90,7 @@ public class TestJNDIRealm {
                 realm.authenticate(USER, expectedResponse, NONCE, null, null, null, REALM, HA2);
 
         // THEN
-        Assert.assertTrue(principal instanceof GenericPrincipal);
+        assertThat(principal, instanceOf(GenericPrincipal.class));
         Assert.assertEquals(USER, principal.getName());
     }
 
@@ -105,7 +108,7 @@ public class TestJNDIRealm {
                 realm.authenticate(USER, expectedResponse, NONCE, null, null, null, REALM, HA2);
 
         // THEN
-        Assert.assertTrue(principal instanceof GenericPrincipal);
+        assertThat(principal, instanceOf(GenericPrincipal.class));
         Assert.assertEquals(USER, principal.getName());
     }
 
diff --git a/test/org/apache/catalina/startup/TestHostConfigAutomaticDeployment.java b/test/org/apache/catalina/startup/TestHostConfigAutomaticDeployment.java
index 9769f77..cfe5c46 100644
--- a/test/org/apache/catalina/startup/TestHostConfigAutomaticDeployment.java
+++ b/test/org/apache/catalina/startup/TestHostConfigAutomaticDeployment.java
@@ -22,6 +22,9 @@ import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
+
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -1884,7 +1887,7 @@ public class TestHostConfigAutomaticDeployment extends TomcatBaseTest {
         // Check the Context class
         Context ctxt = (Context) host.findChild(APP_NAME.getName());
 
-        Assert.assertTrue(ctxt instanceof TesterContext);
+        assertThat(ctxt, instanceOf(TesterContext.class));
     }
 
 
diff --git a/test/org/apache/catalina/startup/TestTomcat.java b/test/org/apache/catalina/startup/TestTomcat.java
index faa7e4c..81c0151 100644
--- a/test/org/apache/catalina/startup/TestTomcat.java
+++ b/test/org/apache/catalina/startup/TestTomcat.java
@@ -33,6 +33,9 @@ import jakarta.servlet.http.HttpServlet;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
+
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -572,7 +575,7 @@ public class TestTomcat extends TomcatBaseTest {
             tomcat.start();
             Assert.fail();
         } catch (Throwable t) {
-            Assert.assertTrue(getRootCause(t) instanceof LifecycleException);
+            assertThat(getRootCause(t), instanceOf(LifecycleException.class));
         }
     }
 
@@ -590,7 +593,7 @@ public class TestTomcat extends TomcatBaseTest {
             tomcat.start();
             Assert.fail();
         } catch (Throwable t) {
-            Assert.assertTrue(getRootCause(t) instanceof MultiThrowable);
+            assertThat(getRootCause(t), instanceOf(MultiThrowable.class));
         }
     }
 
diff --git a/test/org/apache/catalina/startup/TestWebappServiceLoader.java b/test/org/apache/catalina/startup/TestWebappServiceLoader.java
index 3e4d1e2..0efee2d 100644
--- a/test/org/apache/catalina/startup/TestWebappServiceLoader.java
+++ b/test/org/apache/catalina/startup/TestWebappServiceLoader.java
@@ -27,6 +27,9 @@ import java.util.List;
 import jakarta.servlet.ServletContainerInitializer;
 import jakarta.servlet.ServletContext;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
+
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -162,7 +165,7 @@ public class TestWebappServiceLoader {
         try {
             loader.loadServices(ServletContainerInitializer.class, names);
         } catch (IOException e) {
-            Assert.assertTrue(e.getCause() instanceof ClassCastException);
+            assertThat(e.getCause(), instanceOf(ClassCastException.class));
         } finally {
             control.verify();
         }
@@ -181,7 +184,7 @@ public class TestWebappServiceLoader {
         try {
             loader.loadServices(ServletContainerInitializer.class, names);
         } catch (IOException e) {
-            Assert.assertTrue(e.getCause() instanceof ReflectiveOperationException);
+            assertThat(e.getCause(), instanceOf(ReflectiveOperationException.class));
         } finally {
             control.verify();
         }
diff --git a/test/org/apache/catalina/tribes/io/TestXByteBuffer.java b/test/org/apache/catalina/tribes/io/TestXByteBuffer.java
index 5bbb7c2..cf7ff86 100644
--- a/test/org/apache/catalina/tribes/io/TestXByteBuffer.java
+++ b/test/org/apache/catalina/tribes/io/TestXByteBuffer.java
@@ -16,6 +16,9 @@
  */
 package org.apache.catalina.tribes.io;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
+
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -32,7 +35,7 @@ public class TestXByteBuffer {
         String test = "This is as test.";
         byte[] msg = XByteBuffer.serialize(test);
         Object obj = XByteBuffer.deserialize(msg);
-        Assert.assertTrue(obj instanceof String);
+        assertThat(obj, instanceOf(String.class));
         Assert.assertEquals(test, obj);
     }
 }
diff --git a/test/org/apache/jasper/compiler/TestELInterpreterFactory.java b/test/org/apache/jasper/compiler/TestELInterpreterFactory.java
index ea61401..04bb5f8 100644
--- a/test/org/apache/jasper/compiler/TestELInterpreterFactory.java
+++ b/test/org/apache/jasper/compiler/TestELInterpreterFactory.java
@@ -22,6 +22,9 @@ import jakarta.servlet.ServletContext;
 import jakarta.servlet.ServletContextEvent;
 import jakarta.servlet.ServletContextListener;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
+
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -53,10 +56,9 @@ public class TestELInterpreterFactory extends TomcatBaseTest {
 
         ServletContext context = ctx.getServletContext();
 
-        ELInterpreter interpreter =
-                ELInterpreterFactory.getELInterpreter(context);
+        ELInterpreter interpreter = ELInterpreterFactory.getELInterpreter(context);
         Assert.assertNotNull(interpreter);
-        Assert.assertTrue(interpreter instanceof DefaultELInterpreter);
+        assertThat(interpreter, instanceOf(DefaultELInterpreter.class));
 
         context.removeAttribute(ELInterpreter.class.getName());
 
@@ -64,7 +66,7 @@ public class TestELInterpreterFactory extends TomcatBaseTest {
                 SimpleELInterpreter.class.getName());
         interpreter = ELInterpreterFactory.getELInterpreter(context);
         Assert.assertNotNull(interpreter);
-        Assert.assertTrue(interpreter instanceof SimpleELInterpreter);
+        assertThat(interpreter, instanceOf(SimpleELInterpreter.class));
 
         context.removeAttribute(ELInterpreter.class.getName());
 
@@ -72,7 +74,7 @@ public class TestELInterpreterFactory extends TomcatBaseTest {
         context.setAttribute(ELInterpreter.class.getName(), simpleInterpreter);
         interpreter = ELInterpreterFactory.getELInterpreter(context);
         Assert.assertNotNull(interpreter);
-        Assert.assertTrue(interpreter instanceof SimpleELInterpreter);
+        assertThat(interpreter, instanceOf(SimpleELInterpreter.class));
         Assert.assertTrue(interpreter == simpleInterpreter);
 
         context.removeAttribute(ELInterpreter.class.getName());
@@ -83,7 +85,7 @@ public class TestELInterpreterFactory extends TomcatBaseTest {
 
         interpreter = ELInterpreterFactory.getELInterpreter(ctx.getServletContext());
         Assert.assertNotNull(interpreter);
-        Assert.assertTrue(interpreter instanceof SimpleELInterpreter);
+        assertThat(interpreter, instanceOf(SimpleELInterpreter.class));
     }
 
     public static class Bug54239Listener implements ServletContextListener {


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 02/02: Remaining false positives

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit fcd87edac2e2a86108afc4781eb8be1b0388058e
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Oct 16 17:15:36 2020 +0100

    Remaining false positives
---
 res/findbugs/filter-false-positives.xml | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/res/findbugs/filter-false-positives.xml b/res/findbugs/filter-false-positives.xml
index d4e0b03..4c718a3 100644
--- a/res/findbugs/filter-false-positives.xml
+++ b/res/findbugs/filter-false-positives.xml
@@ -1909,6 +1909,15 @@
     <Bug pattern="RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT"/>
   </Match>
   <Match>
+    <!-- Return values are irrelevant as exceptions should be thrown -->
+    <Class name="org.apache.catalina.webresources.TestJarContents"/>
+    <Or>
+      <Method name="testStringOutOfBoundExceptions"/>
+      <Method name="testNullPointerExceptions"/>
+    </Or>
+    <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="&lt;clinit&gt;" />
@@ -2140,6 +2149,12 @@
     </Or>
   </Match>
   <Match>
+    <!-- Array contents is not mutated -->
+    <Class name="org.apache.tomcat.websocket.pojo.TestEncodingDecoding$MsgByte"/>
+    <Field name="data"/>
+    <Bug pattern="VO_VOLATILE_REFERENCE_TO_ARRAY"/>
+  </Match>
+  <Match>
     <!-- Return value of latch is intentionally ignored -->
     <Or>
       <Class name="org.apache.tomcat.websocket.TestWebSocketFrameClient"/>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org