diff --git a/core/src/test/java/io/grpc/internal/AbstractTransportTest.java b/core/src/test/java/io/grpc/internal/AbstractTransportTest.java index a1c00d7dca29..392369764da6 100644 --- 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/jettyTest/java/io/grpc/servlet/JettyTransportTest.java b/servlet/src/jettyTest/java/io/grpc/servlet/JettyTransportTest.java index 7941afc9b4d3..94ccd8e1a7cc 100644 --- a/servlet/src/jettyTest/java/io/grpc/servlet/JettyTransportTest.java +++ b/servlet/src/jettyTest/java/io/grpc/servlet/JettyTransportTest.java @@ -69,7 +69,7 @@ public void start(ServerListener listener) throws IOException { listener.transportCreated(new ServletServerBuilder.ServerTransportImpl(scheduler)); ServletAdapter adapter = new ServletAdapter(serverTransportListener, streamTracerFactories, - Integer.MAX_VALUE); + Integer.MAX_VALUE, false); GrpcServlet grpcServlet = new GrpcServlet(adapter); jettyServer = new Server(0); diff --git a/servlet/src/main/java/io/grpc/servlet/ServletAdapter.java b/servlet/src/main/java/io/grpc/servlet/ServletAdapter.java index 5a567916f997..da401a488730 100644 --- a/servlet/src/main/java/io/grpc/servlet/ServletAdapter.java +++ b/servlet/src/main/java/io/grpc/servlet/ServletAdapter.java @@ -77,14 +77,17 @@ public final class ServletAdapter { private final List streamTracerFactories; private final int maxInboundMessageSize; private final Attributes attributes; + private final boolean forceTrailers; ServletAdapter( ServerTransportListener transportListener, List streamTracerFactories, - int maxInboundMessageSize) { + int maxInboundMessageSize, + boolean forceTrailers) { this.transportListener = transportListener; this.streamTracerFactories = streamTracerFactories; this.maxInboundMessageSize = maxInboundMessageSize; + this.forceTrailers = forceTrailers; attributes = transportListener.transportReady(Attributes.EMPTY); } @@ -148,7 +151,8 @@ public void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOEx new InetSocketAddress(req.getLocalAddr(), req.getLocalPort())) .build(), getAuthority(req), - logId); + logId, + forceTrailers); transportListener.streamCreated(stream, method, headers); stream.transportState().runOnTransportThread(stream.transportState()::onStreamAllocated); diff --git a/servlet/src/main/java/io/grpc/servlet/ServletServerBuilder.java b/servlet/src/main/java/io/grpc/servlet/ServletServerBuilder.java index 3e852ea3c090..f7638a0621e3 100644 --- a/servlet/src/main/java/io/grpc/servlet/ServletServerBuilder.java +++ b/servlet/src/main/java/io/grpc/servlet/ServletServerBuilder.java @@ -72,6 +72,7 @@ public final class ServletServerBuilder extends ForwardingServerBuilder v + "," + newValue); - trailerSupplier.get().putIfAbsent(key, newValue); + if (forceTrailers) { + writeHeadersToServletResponse(new Metadata()); + resp.setTrailerFields(trailerSupplier); + serializeTrailers(trailers); + } else { + writeHeadersToServletResponse(trailers); } + } else { + 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/TomcatInteropTest.java b/servlet/src/tomcatTest/java/io/grpc/servlet/TomcatInteropTest.java index 1422b5388fd7..786a3a10fd27 100644 --- a/servlet/src/tomcatTest/java/io/grpc/servlet/TomcatInteropTest.java +++ b/servlet/src/tomcatTest/java/io/grpc/servlet/TomcatInteropTest.java @@ -60,7 +60,9 @@ public static void cleanUp() throws Exception { @Override protected ServerBuilder getServerBuilder() { - return new ServletServerBuilder().maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE); + return new ServletServerBuilder() + .forceTrailers(true) + .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE); } @Override @@ -112,28 +114,4 @@ protected boolean metricsExpected() { @Ignore("Tomcat is broken on client GOAWAY") @Test public void gracefulShutdown() {} - - // FIXME - @Override - @Ignore("Tomcat is not able to send trailer only") - @Test - public void specialStatusMessage() {} - - // FIXME - @Override - @Ignore("Tomcat is not able to send trailer only") - @Test - public void unimplementedMethod() {} - - // FIXME - @Override - @Ignore("Tomcat is not able to send trailer only") - @Test - public void statusCodeAndMessage() {} - - // FIXME - @Override - @Ignore("Tomcat is not able to send trailer only") - @Test - public void emptyStream() {} } diff --git a/servlet/src/tomcatTest/java/io/grpc/servlet/TomcatTransportTest.java b/servlet/src/tomcatTest/java/io/grpc/servlet/TomcatTransportTest.java index 43c69e13fdda..12e01efd93b0 100644 --- a/servlet/src/tomcatTest/java/io/grpc/servlet/TomcatTransportTest.java +++ b/servlet/src/tomcatTest/java/io/grpc/servlet/TomcatTransportTest.java @@ -81,7 +81,8 @@ public void start(ServerListener listener) throws IOException { ServerTransportListener serverTransportListener = listener.transportCreated(new ServerTransportImpl(scheduler)); ServletAdapter adapter = - new ServletAdapter(serverTransportListener, streamTracerFactories, Integer.MAX_VALUE); + new ServletAdapter(serverTransportListener, streamTracerFactories, Integer.MAX_VALUE, + true); GrpcServlet grpcServlet = new GrpcServlet(adapter); tomcatServer = new Tomcat(); @@ -248,21 +249,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 diff --git a/servlet/src/undertowTest/java/io/grpc/servlet/UndertowTransportTest.java b/servlet/src/undertowTest/java/io/grpc/servlet/UndertowTransportTest.java index 9d894b5e3f24..9210c0a4bc16 100644 --- a/servlet/src/undertowTest/java/io/grpc/servlet/UndertowTransportTest.java +++ b/servlet/src/undertowTest/java/io/grpc/servlet/UndertowTransportTest.java @@ -100,7 +100,8 @@ public void start(ServerListener listener) throws IOException { ServerTransportListener serverTransportListener = listener.transportCreated(new ServerTransportImpl(scheduler)); ServletAdapter adapter = - new ServletAdapter(serverTransportListener, streamTracerFactories, Integer.MAX_VALUE); + new ServletAdapter(serverTransportListener, streamTracerFactories, Integer.MAX_VALUE, + false); GrpcServlet grpcServlet = new GrpcServlet(adapter); InstanceFactory instanceFactory = () -> new ImmediateInstanceHandle<>(grpcServlet);