Bump reactor-netty to 2022.0.7 (#437)

* Bump reactor-netty to 2022.0.7 * fix flaky tests * test: fix MaxHeaderSizeTest See reactor/reactor-netty#2534 * test: fix ConnectionPoolTest * now we should expect Bad Request with capital R * also, minor refactor in some tests while trying to reproduce failings test: working on CacheContentLengthLimitTest * ci: add Surefire JUnit report collection apply some suggestions from #404 --------- Co-authored-by: paolo.venturi <paolo.venturi@diennea.com> Co-authored-by: Niccolò Maltoni <niccolo.maltoni@diennea.com>
diennea · May 23, 2023 · e33acbf · e33acbf
1 parent 154ab29
commit e33acbf
Show file tree

Hide file tree

Showing 7 changed files with 113 additions and 82 deletions.
diff --git a/.github/workflows/pr-validation.yml b/.github/workflows/pr-validation.yml
@@ -16,27 +16,43 @@
 name: Java CI - Code tests
 on: [pull_request]
 env:
-  MAVEN_OPTS: -Dmaven.artifact.threads=4 -Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn  -Dmaven.wagon.httpconnectionManager.ttlSeconds=25 -Dmaven.wagon.http.retryHandler.count=3
+  MAVEN_OPTS: >-
+    -Dmaven.artifact.threads=4 
+    -Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn 
+    -Dmaven.wagon.httpconnectionManager.ttlSeconds=25 
+    -Dmaven.wagon.http.retryHandler.count=3
+  MAVEN_CLI_OPTS: >-
+    --batch-mode 
+    -Pproduction 
+    -Dmaven.test.redirectTestOutputToFile=true 
+    -Dsurefire.rerunFailingTestsCount=3
+    --update-snapshots
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}
 jobs:
   build:
     runs-on: ubuntu-latest
     steps:
-      - name: Cancel Previous Runs
-        uses: styfle/cancel-workflow-action@0.6.0
-        with:
-          access_token: ${{ github.token }}
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
       - name: 'Set up JDK 17'
-        uses: actions/setup-java@v1
+        uses: actions/setup-java@v3
         with:
-          java-version: '17.0.3'
+          java-version: '17'
+          distribution: 'temurin'
       - name: 'Cache Maven packages'
-        uses: actions/cache@v2
+        uses: actions/cache@v3
         with:
           path: ~/.m2
           key: 'cache'
           restore-keys: 'cache'
-      - name: 'Build with Maven'
-        run: mvn verify -Pproduction -Dmaven.test.redirectTestOutputToFile=true -Dsurefire.rerunFailingTestsCount=3
-      - name: 'Remove Snapshots Before Caching'
-        run: find ~/.m2 -name '*SNAPSHOT' | xargs rm -Rf
+      - name: 'Build and test with Maven'
+        run: mvn verify
+      - name: 'Report test results'
+        uses: dorny/test-reporter@v1
+        if: always()
+        with:
+          name: Maven Tests
+          path: carapace-*/target/surefire-reports/*.xml
+          reporter: java-junit
+          fail-on-error: true
diff --git a/carapace-server/src/test/java/org/carapaceproxy/RawClientTest.java b/carapace-server/src/test/java/org/carapaceproxy/RawClientTest.java
@@ -363,9 +363,13 @@ public void testServerRequestContinue() throws Exception {
                                 failed.set(true);
                                 return;
                             }
-                            resp = consumeHttpResponseInput(socket.getInputStream()).getBodyString();
-                            System.out.println("### RESP client1: " + resp);
-                            if (!resp.contains("resp=client1")) {
+                            int count = 0;
+                            do {
+                                Thread.sleep(5_000);
+                                resp = consumeHttpResponseInput(socket.getInputStream()).getBodyString();
+                                System.out.println("### RESP client1: " + resp);
+                            } while (!resp.contains("resp=client1") && count++ < 10);
+                            if (count >= 10) {
                                 failed.set(true);
                             }
                         } catch (Exception e) {
@@ -661,9 +665,13 @@ public void testMaxConnectionsAndBorrowTimeout() throws Exception {
                                 failed.set(true);
                                 return;
                             }
-                            resp = consumeHttpResponseInput(socket.getInputStream()).getBodyString();
-                            System.out.println("### RESP client1: " + resp);
-                            if (!resp.contains("resp=client1")) {
+                            int count = 0;
+                            do {
+                                Thread.sleep(5_000);
+                                resp = consumeHttpResponseInput(socket.getInputStream()).getBodyString();
+                                System.out.println("### RESP client1: " + resp);
+                            } while (!resp.contains("resp=client1") && count++ < 10);
+                            if (count >= 10) {
                                 failed.set(true);
                             }
                         } catch (Exception e) {

diff --git a/carapace-server/src/test/java/org/carapaceproxy/backends/ConnectionPoolTest.java b/carapace-server/src/test/java/org/carapaceproxy/backends/ConnectionPoolTest.java
@@ -23,8 +23,10 @@
 import static com.github.tomakehurst.wiremock.client.WireMock.get;
 import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
 import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.*;
@@ -70,8 +72,6 @@ public class ConnectionPoolTest extends UseAdminServer {
     @Rule
     public WireMockRule wireMockRule = new WireMockRule(0);
 
-    private Properties config;
-
     private void configureAndStartServer() throws Exception {
 
         HttpTestUtils.overrideJvmWideHttpsVerifier();
@@ -80,10 +80,10 @@ private void configureAndStartServer() throws Exception {
                 .willReturn(aResponse()
                         .withStatus(200)
                         .withHeader("Content-Type", "text/html")
-                        .withHeader("Content-Length", "it <b>works</b> !!".length() + "")
+                        .withHeader("Content-Length", String.valueOf("it <b>works</b> !!".length()))
                         .withBody("it <b>works</b> !!")));
 
-        config = new Properties(HTTP_ADMIN_SERVER_CONFIG);
+        final Properties config = new Properties(HTTP_ADMIN_SERVER_CONFIG);
         config.put("config.type", "database");
         config.put("db.jdbc.url", "jdbc:herddb:localhost");
         config.put("db.server.base.dir", tmpDir.newFolder().getAbsolutePath());
@@ -107,12 +107,12 @@ private void configureAndStartServer() throws Exception {
         config.put("backend.1.id", "localhost");
         config.put("backend.1.enabled", "true");
         config.put("backend.1.host", "localhost");
-        config.put("backend.1.port", wireMockRule.port() + "");
+        config.put("backend.1.port", String.valueOf(wireMockRule.port()));
 
         config.put("backend.2.id", "localhost2");
         config.put("backend.2.enabled", "true");
         config.put("backend.2.host", "localhost2");
-        config.put("backend.2.port", wireMockRule.port() + "");
+        config.put("backend.2.port", String.valueOf(wireMockRule.port()));
 
         // Default director
         config.put("director.1.id", "*");
@@ -172,55 +172,55 @@ public void hostHeaderTest() throws Exception {
 
         //No Http header host
         String response = sendRequest(false, hostname, port, path, null);
-        assertTrue(response.contains("Bad request"));
+        assertThat(response, containsString("Bad request"));
 
         //Add Http header host
         String response2 = sendRequest(true, hostname, port, path, "localhost");
-        assertTrue(response2.contains("it <b>works</b> !!"));
+        assertThat(response2, containsString("it <b>works</b> !!"));
 
         //Empty header host
         String response3 = sendRequest(true, hostname, port, path, "");
-        assertTrue(response3.contains("Bad request"));
+        assertThat(response3, containsString("Bad request"));
 
         //Empty header host
         String response4 = sendRequest(true, hostname, port, path, " ");
-        assertTrue(response4.contains("Bad request"));
+        assertThat(response4, containsString("Bad request"));
 
         //Header host with special character
         String response5 = sendRequest(true, hostname, port, path, "local%&&host");
-        assertTrue(response5.contains("Bad request"));
+        assertThat(response5, containsString("Bad request"));
 
         //Header host with multiple domain
         String response6 = sendRequest(true, hostname, port, path, "localhost1,localhost2");
-        assertTrue(response6.contains("Bad request"));
+        assertThat(response6, containsString("Bad request"));
 
         //Ip address as host header
         String response7 = sendRequest(true, hostname, port, path, "127.0.0.1");
-        assertTrue(response7.contains("it <b>works</b> !!"));
+        assertThat(response7, containsString("it <b>works</b> !!"));
 
         //Invalid ip address
         String response8 = sendRequest(true, hostname, port, path, "256.0.0.0");
-        assertTrue(response8.contains("Bad request"));
+        assertThat(response8, containsString("Bad request"));
 
         //Ip address as host header with port
         String response9 = sendRequest(true, hostname, port, path, "127.0.0.1:8080");
-        assertTrue(response9.contains("it <b>works</b> !!"));
+        assertThat(response9, containsString("it <b>works</b> !!"));
 
         //Hostname as host header with port
         String response10 = sendRequest(true, hostname, port, path, "localhost:8080");
-        assertTrue(response10.contains("it <b>works</b> !!"));
+        assertThat(response10, containsString("it <b>works</b> !!"));
 
         //Hostname as host header with port
         String response11 = sendRequest(true, hostname, port, path, "localhost:8080&");
-        assertTrue(response11.contains("Bad request"));
+        assertThat(response11, containsString("Bad Request"));
 
         //Invalid hostname
         String response12 = sendRequest(true, hostname, port, path, "::::8080::9999");
-        assertTrue(response12.contains("Bad request"));
+        assertThat(response12, containsString("Bad Request"));
 
         //Add Http header host
         String response13 = sendRequest(true, hostname, port, path, "localhost3");
-        assertTrue(response13.contains("it <b>works</b> !!"));
+        assertThat(response13, containsString("it <b>works</b> !!"));
     }
 
     public String sendRequest(boolean addHeaderHost, String hostname, int port, String path, String headerHost) throws IOException {
@@ -271,8 +271,9 @@ public void test() throws Exception {
             ConnectionProvider provider = connectionPools.get(defaultPool);
             assertThat(provider, not(nullValue()));
             Map<SocketAddress, Integer> maxConnectionsPerHost = provider.maxConnectionsPerHost();
+            assertThat(maxConnectionsPerHost, is(notNullValue(Map.class)));
             assertThat(maxConnectionsPerHost.size(), is(2));
-            maxConnectionsPerHost.values().stream().allMatch(e -> e == 10);
+            assertTrue(maxConnectionsPerHost.values().stream().allMatch(e -> e == 10));
         }
 
         // pool with defaults
@@ -283,8 +284,9 @@ public void test() throws Exception {
             ConnectionProvider provider = connectionPools.get(poolWithDefaults);
             assertThat(provider, not(nullValue()));
             Map<SocketAddress, Integer> maxConnectionsPerHost = provider.maxConnectionsPerHost();
+            assertThat(maxConnectionsPerHost, is(notNullValue(Map.class)));
             assertThat(maxConnectionsPerHost.size(), is(2));
-            maxConnectionsPerHost.values().stream().allMatch(e -> e == 10);
+            assertTrue(maxConnectionsPerHost.values().stream().allMatch(e -> e == 10));
         }
 
         // custom pool
@@ -295,8 +297,9 @@ public void test() throws Exception {
             ConnectionProvider provider = connectionPools.get(customPool);
             assertThat(provider, not(nullValue()));
             Map<SocketAddress, Integer> maxConnectionsPerHost = provider.maxConnectionsPerHost();
+            assertThat(maxConnectionsPerHost, is(notNullValue(Map.class)));
             assertThat(maxConnectionsPerHost.size(), is(2));
-            maxConnectionsPerHost.values().stream().allMatch(e -> e == 20);
+            assertTrue(maxConnectionsPerHost.values().stream().allMatch(e -> e == 20));
         }
 
         // connection pool selection
@@ -313,8 +316,9 @@ public void test() throws Exception {
             assertThat(res.getKey(), is(defaultPool));
             ConnectionProvider provider = res.getValue();
             Map<SocketAddress, Integer> maxConnectionsPerHost = provider.maxConnectionsPerHost();
+            assertThat(maxConnectionsPerHost, is(notNullValue(Map.class)));
             assertThat(maxConnectionsPerHost.size(), is(2));
-            maxConnectionsPerHost.values().stream().allMatch(e -> e == 10);
+            assertTrue(maxConnectionsPerHost.values().stream().allMatch(e -> e == 10));
 
             try (RawHttpClient client = new RawHttpClient("localhost", port)) {
                 RawHttpClient.HttpResponse resp = client.executeRequest("GET /index.html HTTP/1.1\r\n" + HttpHeaderNames.HOST + ": localhostx" + "\r\n\r\n");
@@ -337,8 +341,9 @@ public void test() throws Exception {
             assertThat(res.getKey(), is(poolWithDefaults));
             ConnectionProvider provider = res.getValue();
             Map<SocketAddress, Integer> maxConnectionsPerHost = provider.maxConnectionsPerHost();
+            assertThat(maxConnectionsPerHost, is(notNullValue(Map.class)));
             assertThat(maxConnectionsPerHost.size(), is(2));
-            maxConnectionsPerHost.values().stream().allMatch(e -> e == 10);
+            assertTrue(maxConnectionsPerHost.values().stream().allMatch(e -> e == 10));
 
             try (RawHttpClient client = new RawHttpClient("localhost", port)) {
                 RawHttpClient.HttpResponse resp = client.executeRequest("GET /index.html HTTP/1.1\r\n" + HttpHeaderNames.HOST + ": localhost" + "\r\n\r\n");
@@ -361,8 +366,9 @@ public void test() throws Exception {
             assertThat(res.getKey(), is(customPool));
             ConnectionProvider provider = res.getValue();
             Map<SocketAddress, Integer> maxConnectionsPerHost = provider.maxConnectionsPerHost();
+            assertThat(maxConnectionsPerHost, is(notNullValue(Map.class)));
             assertThat(maxConnectionsPerHost.size(), is(2));
-            maxConnectionsPerHost.values().stream().allMatch(e -> e == 20);
+            assertTrue(maxConnectionsPerHost.values().stream().allMatch(e -> e == 20));
 
             try (RawHttpClient client = new RawHttpClient("localhost", port)) {
                 RawHttpClient.HttpResponse resp = client.executeRequest("GET /index.html HTTP/1.1\r\n" + HttpHeaderNames.HOST + ": localhost3" + "\r\n\r\n");

diff --git a/carapace-server/src/test/java/org/carapaceproxy/core/MaxHeaderSizeTest.java b/carapace-server/src/test/java/org/carapaceproxy/core/MaxHeaderSizeTest.java
@@ -99,6 +99,6 @@ public void test() throws Exception {
 
         HttpResponse<String> response2 = httpClient2.send(request, HttpResponse.BodyHandlers.ofString());
 
-        assertEquals(413, response2.statusCode());
+        assertEquals(431, response2.statusCode());
     }
 }
diff --git a/...pace-server/src/test/java/org/carapaceproxy/server/cache/CacheContentLengthLimitTest.java b/...pace-server/src/test/java/org/carapaceproxy/server/cache/CacheContentLengthLimitTest.java
@@ -51,22 +51,22 @@ public class CacheContentLengthLimitTest {
     public TemporaryFolder tmpDir = new TemporaryFolder();
 
     @Test
-    public void testWithContentLenghtHeader() throws Exception {
+    public void testWithContentLengthHeader() throws Exception {
 
         String body = "01234567890123456789";
 
         stubFor(get(urlEqualTo("/index.html"))
                 .willReturn(aResponse()
                         .withStatus(200)
                         .withHeader("Content-Type", "text/html")
-                        .withHeader("Content-Length", body.length() + "")
+                        .withHeader("Content-Length", String.valueOf(body.length()))
                         .withBody(body)));
 
         testFileSizeCache(body, false);
     }
 
     @Test
-    public void testWithoutContentLenghtHeader() throws Exception {
+    public void testWithoutContentLengthHeader() throws Exception {
 
         String body = "01234567890123456789";
 
@@ -86,7 +86,7 @@ private void testFileSizeCache(String body, boolean chunked) throws Exception {
 
         // No size checking
         {
-            try (HttpProxyServer server = HttpProxyServer.buildForTests("localhost", 0, mapper, tmpDir.newFolder());) {
+            try (HttpProxyServer server = HttpProxyServer.buildForTests("localhost", 0, mapper, tmpDir.newFolder())) {
                 server.getCurrentConfiguration().setCacheMaxFileSize(0);
                 server.getCurrentConfiguration().setRequestCompressionEnabled(false);
                 server.getCache().reloadConfiguration(server.getCurrentConfiguration());
@@ -102,7 +102,7 @@ private void testFileSizeCache(String body, boolean chunked) throws Exception {
 
         // Max size set to current content size
         {
-            try (HttpProxyServer server = HttpProxyServer.buildForTests("localhost", 0, mapper, tmpDir.newFolder());) {
+            try (HttpProxyServer server = HttpProxyServer.buildForTests("localhost", 0, mapper, tmpDir.newFolder())) {
                 server.getCurrentConfiguration().setCacheMaxFileSize(body.length());
                 server.getCurrentConfiguration().setRequestCompressionEnabled(false);
                 server.getCache().reloadConfiguration(server.getCurrentConfiguration());
@@ -118,7 +118,7 @@ private void testFileSizeCache(String body, boolean chunked) throws Exception {
 
         // Max size set to drop current content
         {
-            try (HttpProxyServer server = HttpProxyServer.buildForTests("localhost", 0, mapper, tmpDir.newFolder());) {
+            try (HttpProxyServer server = HttpProxyServer.buildForTests("localhost", 0, mapper, tmpDir.newFolder())) {
                 server.getCurrentConfiguration().setCacheMaxFileSize(body.length() - 1);
                 server.getCurrentConfiguration().setRequestCompressionEnabled(false);
                 server.getCache().reloadConfiguration(server.getCurrentConfiguration());