servlet: force always sending end_stream in trailer frame (Fixes #10124)

Signed-off-by: hypnoce <hypnoce@donarproject.org>
grpc · May 3, 2023 · c96be19 · c96be19
1 parent fbc8679
commit c96be19
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 28 deletions.
diff --git a/core/src/test/java/io/grpc/internal/AbstractTransportTest.java b/core/src/test/java/io/grpc/internal/AbstractTransportTest.java
@@ -1098,11 +1098,11 @@ public void earlyServerClose_noServerHeaders() throws Exception {
         clientStreamListener.trailers.get(TIMEOUT_MS, TimeUnit.MILLISECONDS);
     checkClientStatus(status, clientStreamStatus);
     assertEquals(
-        Lists.newArrayList(trailers.getAll(asciiKey)),
-        Lists.newArrayList(clientStreamTrailers.getAll(asciiKey)));
+        String.join(",", trailers.getAll(asciiKey)),
+        String.join(",", clientStreamTrailers.getAll(asciiKey)));
     assertEquals(
-        Lists.newArrayList(trailers.getAll(binaryKey)),
-        Lists.newArrayList(clientStreamTrailers.getAll(binaryKey)));
+        String.join(",", trailers.getAll(binaryKey)),
+        String.join(",", clientStreamTrailers.getAll(binaryKey)));
     assertTrue(clientStreamTracer1.getOutboundHeaders());
     assertSame(clientStreamTrailers, clientStreamTracer1.getInboundTrailers());
     assertSame(clientStreamStatus, clientStreamTracer1.getStatus());

diff --git a/servlet/src/main/java/io/grpc/servlet/ServletServerStream.java b/servlet/src/main/java/io/grpc/servlet/ServletServerStream.java
@@ -276,20 +276,33 @@ public void writeTrailers(Metadata trailers, boolean headersSent, Status status)
             new Object[] {logId, trailers, headersSent, status});
       }
       if (!headersSent) {
-        writeHeadersToServletResponse(trailers);
+        /*
+          Some servlet containers do not support sending trailers only (Tomcat).
+          They send an empty data frame with an end_stream flag.
+          This is not supported by gRPC as is expects end_stream flag in trailer or trailer-only frame
+          To avoid this empty data frame, force the servlet container to either
+            - send a header frame, an empty data frame and a trailer frame with end_stream (Tomcat)
+            - send a header frame and a trailer frame with end_stream (Jetty, Undertow)
+         */
+        writeHeadersToServletResponse(new Metadata());
+        resp.setTrailerFields(trailerSupplier);
+        serializeTrailers(trailers);
       } else {
-        byte[][] serializedHeaders = TransportFrameUtil.toHttp2Headers(trailers);
-        for (int i = 0; i < serializedHeaders.length; i += 2) {
-          String key = new String(serializedHeaders[i], StandardCharsets.US_ASCII);
-          String newValue = new String(serializedHeaders[i + 1], StandardCharsets.US_ASCII);
-          trailerSupplier.get().computeIfPresent(key, (k, v) -> v + "," + newValue);
-          trailerSupplier.get().putIfAbsent(key, newValue);
-        }
+        serializeTrailers(trailers);
       }
-
       writer.complete();
     }
 
+    private void serializeTrailers(Metadata trailers) {
+      byte[][] serializedHeaders = TransportFrameUtil.toHttp2Headers(trailers);
+      for (int i = 0; i < serializedHeaders.length; i += 2) {
+        String key = new String(serializedHeaders[i], StandardCharsets.US_ASCII);
+        String newValue = new String(serializedHeaders[i + 1], StandardCharsets.US_ASCII);
+        trailerSupplier.get().computeIfPresent(key, (k, v) -> v + "," + newValue);
+        trailerSupplier.get().putIfAbsent(key, newValue);
+      }
+    }
+
     @Override
     public void cancel(Status status) {
       if (resp.isCommitted() && Code.DEADLINE_EXCEEDED == status.getCode()) {

diff --git a/servlet/src/tomcatTest/java/io/grpc/servlet/TomcatTransportTest.java b/servlet/src/tomcatTest/java/io/grpc/servlet/TomcatTransportTest.java
@@ -248,21 +248,6 @@ public void interactionsAfterClientStreamCancelAreNoops() {}
   @Test
   public void clientCancel() {}
 
-  @Override
-  @Ignore("Tomcat does not support trailers only")
-  @Test
-  public void earlyServerClose_noServerHeaders() {}
-
-  @Override
-  @Ignore("Tomcat does not support trailers only")
-  @Test
-  public void earlyServerClose_serverFailure() {}
-
-  @Override
-  @Ignore("Tomcat does not support trailers only")
-  @Test
-  public void earlyServerClose_serverFailure_withClientCancelOnListenerClosed() {}
-
   @Override
   @Ignore("regression since bumping grpc v1.46 to v1.53")
   @Test