Skip to content

Commit 9ebca03

Browse files
committed
Move writeHeaders up to HttpStream and test HTTP/3 request trailers
Address review comments: pull the shared writeHeaders(MultiMap, boolean) declaration up from HttpClientStream/HttpServerStream to HttpStream, drop redundant comments, and mirror the HTTP/2 request trailer tests in Http3ServerTest.
1 parent 4b4f6a6 commit 9ebca03

6 files changed

Lines changed: 49 additions & 13 deletions

File tree

vertx-core/src/main/java/io/vertx/core/http/impl/HttpClientStream.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,6 @@ public interface HttpClientStream extends HttpStream {
2828

2929
Future<Void> writeHead(HttpRequestHead request, boolean chunked, Buffer buf, boolean end, StreamPriority priority, boolean connect);
3030

31-
/**
32-
* Writes a trailing header block, mirroring {@link HttpServerStream#writeHeaders(MultiMap, boolean)}.
33-
* Only called after the request body has been written with {@code end == false}.
34-
*/
35-
Future<Void> writeHeaders(MultiMap headers, boolean end);
36-
3731
HttpClientStream headHandler(Handler<HttpResponseHead> handler);
3832
HttpClientStream resetHandler(Handler<Long> handler);
3933
HttpClientStream exceptionHandler(Handler<Throwable> handler);

vertx-core/src/main/java/io/vertx/core/http/impl/HttpServerRequestImpl.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,6 @@ public void handleData(Buffer data) {
190190
public void handleTrailers(MultiMap trailers) {
191191
HttpEventHandler handler;
192192
synchronized (connection) {
193-
// Setting the trailers must not race with trailers(), where the field can escape:
194-
// if the user already obtained the empty map, update it in place instead.
195193
if (this.trailers == null) {
196194
this.trailers = trailers;
197195
} else if (this.trailers != trailers) {

vertx-core/src/main/java/io/vertx/core/http/impl/HttpServerStream.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ public interface HttpServerStream extends HttpStream {
3434
HttpServerConnection connection();
3535

3636
Future<Void> writeHead(HttpResponseHead head, Buffer chunk, boolean end);
37-
Future<Void> writeHeaders(MultiMap headers, boolean end);
3837

3938
Future<HttpServerStream> sendPush(HostAndPort authority, HttpMethod method, MultiMap headers, String path, StreamPriority priority);
4039

vertx-core/src/main/java/io/vertx/core/http/impl/HttpStream.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ public interface HttpStream {
4242
ContextInternal context();
4343

4444
Future<Void> writeChunk(Buffer buf, boolean end);
45+
Future<Void> writeHeaders(MultiMap headers, boolean end);
4546
Future<Void> writeFrame(int type, int flags, Buffer payload);
4647
Future<Void> writeReset(long code);
4748

vertx-core/src/main/java/io/vertx/core/http/impl/http1/Http1ServerRequest.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,6 @@ protected void handleMessage(Object elt) {
106106
if (elt == InboundBuffer.END_SENTINEL) {
107107
onEnd();
108108
} else if (elt instanceof MultiMap) {
109-
// Trailers travel through the queue so they cannot be observed before the
110-
// preceding data has been delivered.
111109
setTrailers((MultiMap) elt);
112110
onEnd();
113111
} else {
@@ -185,8 +183,6 @@ private void handleEnd(InboundMessageQueue<Object> queue, Object end) {
185183

186184
private void setTrailers(MultiMap trailers) {
187185
synchronized (conn) {
188-
// Must not race with trailers(), where the field can escape: if the user already
189-
// obtained the empty map, update it in place instead of replacing it.
190186
if (this.trailers == null) {
191187
this.trailers = trailers;
192188
} else if (this.trailers != trailers) {

vertx-core/src/test/java/io/vertx/tests/http/http3/Http3ServerTest.java

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,54 @@ public void testTrailers() throws Exception{
142142
Assert.assertEquals("chunk", new String(stream.responseBody()));
143143
}
144144

145+
@Test
146+
public void testRequestTrailers() throws Exception{
147+
148+
server.requestHandler(req -> {
149+
req.handler(buff -> {});
150+
req.endHandler(v -> {
151+
Assert.assertEquals("chunky", req.getTrailer("x-trailer"));
152+
Assert.assertEquals("other", req.getTrailer("x-other"));
153+
Assert.assertEquals(2, req.trailers().size());
154+
req.response().end("done");
155+
});
156+
});
157+
158+
server.listen(8443, "localhost").await();
159+
160+
Http3TestClient.Client.Connection connection = client.connect(new InetSocketAddress(NetUtil.LOCALHOST4, 8443));
161+
Http3TestClient.Client.Stream stream = connection.stream();
162+
163+
stream.write(new DefaultHttp3Headers().method("POST").path("/"));
164+
stream.write("hello".getBytes(StandardCharsets.UTF_8));
165+
stream.end(new DefaultHttp3Headers().set("x-trailer", "chunky").set("x-other", "other"));
166+
167+
Assert.assertEquals("done", new String(stream.responseBody()));
168+
}
169+
170+
@Test
171+
public void testRequestNoTrailers() throws Exception{
172+
173+
server.requestHandler(req -> {
174+
req.handler(buff -> {});
175+
req.endHandler(v -> {
176+
Assert.assertTrue(req.trailers().isEmpty());
177+
Assert.assertNull(req.getTrailer("x-trailer"));
178+
req.response().end("done");
179+
});
180+
});
181+
182+
server.listen(8443, "localhost").await();
183+
184+
Http3TestClient.Client.Connection connection = client.connect(new InetSocketAddress(NetUtil.LOCALHOST4, 8443));
185+
Http3TestClient.Client.Stream stream = connection.stream();
186+
187+
stream.write(new DefaultHttp3Headers().method("POST").path("/"));
188+
stream.end("hello".getBytes(StandardCharsets.UTF_8));
189+
190+
Assert.assertEquals("done", new String(stream.responseBody()));
191+
}
192+
145193
@Test
146194
public void testUnknownFrame() throws Exception{
147195

0 commit comments

Comments
 (0)