javaee / grizzly

Writing scalable server applications in the Java™ programming language has always been difficult. Before the advent of the Java New I/O API (NIO), thread management issues made it impossible for a server to scale to thousands of users. The Grizzly NIO framework has been designed to help developers to take advantage of the Java™ NIO API.
https://javaee.github.io/grizzly/
Other
222 stars 60 forks source link

Response not received if it contains `Upgrade` header #1958

Closed elrodro83 closed 7 years ago

elrodro83 commented 7 years ago

When an http server returns a 200 response with an Upgrade header with value h2 or h2,h2c, the rest of the response is not received, the ahc listener is never called.

I added the following test on ahc to show this behavior. The test fails due to time.

index b10345e..f9dad33 100644
--- a/src/test/java/com/ning/http/client/async/AbstractBasicTest.java
+++ b/src/test/java/com/ning/http/client/async/AbstractBasicTest.java
@@ -92,7 +92,12 @@
                     httpResponse.sendRedirect(httpRequest.getHeader("X-redirect"));
                     return;
                 }
-                httpResponse.addHeader("X-" + param, httpRequest.getHeader(param));
+                
+                if(param.startsWith("Y-")) {
+                   httpResponse.addHeader(param.substring(2), httpRequest.getHeader(param));
+                } else {
+                   httpResponse.addHeader("X-" + param, httpRequest.getHeader(param));
+                }
             }

             Enumeration<?> i = httpRequest.getParameterNames();
diff --git a/src/test/java/com/ning/http/client/async/AsyncProvidersBasicTest.java b/src/test/java/com/ning/http/client/async/AsyncProvidersBasicTest.java
index d9fb7c5..a1ce276 100755
--- a/src/test/java/com/ning/http/client/async/AsyncProvidersBasicTest.java
+++ b/src/test/java/com/ning/http/client/async/AsyncProvidersBasicTest.java
@@ -429,6 +429,31 @@
             }
         }
     }
+    
+    @Test(groups = { "standalone", "default_provider", "async" })
+    public void asyncResponseWithUpgradeAndContentTest() throws Throwable {
+        try (AsyncHttpClient client = getAsyncHttpClient(null)) {
+            final CountDownLatch l = new CountDownLatch(1);
+            FluentCaseInsensitiveStringsMap h = new FluentCaseInsensitiveStringsMap();
+            h.add("Y-Upgrade", "h2,h2c");
+            client.prepareGet(getTargetUrl()).setHeaders(h).execute(new AsyncCompletionHandlerAdapter() {
+
+                @Override
+                public Response onCompleted(Response response) throws Exception {
+                    try {
+                        assertEquals(response.getStatusCode(), 200);
+                        assertEquals(response.getHeader("Upgrade"), "h2,h2c");
+                    } finally {
+                        l.countDown();
+                    }
+                    return response;
+                }
+            }).get();
+            if (!l.await(TIMEOUT, TimeUnit.SECONDS)) {
+                Assert.fail("Timeout out");
+            }
+        }
+    }

     @Test(groups = { "standalone", "default_provider", "async" })
     public void asyncDoGetCookieTest() throws Throwable {

I would expect the onCompleted method to be called regardless of the Upgrade header.

I traced the behavior up to HttpCodecFilter in grizzly, that's why i created this issue in this project, even it i reproduced it with an ahc test.

rlubke commented 7 years ago

Fixed in the AHC project.

elrodro83 commented 7 years ago

Great! con you point me to the issue/commit in AHC? Also, do you know when a version with the fix will be released?

Thanks!

rlubke commented 7 years ago

@elrodro83 https://github.com/javaee/grizzly-ahc/commit/83f78b1089d0d9af37adee8fe714629fc6e4509a

I may be able to do a release next week.