Expect:100-continue fixes for Netty

Signed-off-by: Maxim Nesen <[email protected]>
eclipse-ee4j · Oct 3, 2023 · 3339510 · 3339510
1 parent 8a63706
commit 3339510
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 12 deletions.
diff --git a/...c/main/java/org/glassfish/jersey/netty/connector/Expect100ContinueConnectorExtension.java b/...c/main/java/org/glassfish/jersey/netty/connector/Expect100ContinueConnectorExtension.java
@@ -19,6 +19,7 @@
 import io.netty.handler.codec.http.HttpHeaderNames;
 import io.netty.handler.codec.http.HttpHeaderValues;
 import io.netty.handler.codec.http.HttpRequest;
+import io.netty.handler.codec.http.HttpVersion;
 import org.glassfish.jersey.client.ClientProperties;
 import org.glassfish.jersey.client.ClientRequest;
 import org.glassfish.jersey.client.RequestEntityProcessing;
@@ -47,9 +48,10 @@ public void invoke(ClientRequest request, HttpRequest extensionParam) {
         final boolean allowStreaming = length > expectContinueSizeThreshold
                 || entityProcessing == RequestEntityProcessing.CHUNKED;
 
-        if (!Boolean.TRUE.equals(expectContinueActivated)
+        if (extensionParam.protocolVersion().equals(HttpVersion.HTTP_1_1)
+                && (!Boolean.TRUE.equals(expectContinueActivated)
                 || !(HttpMethod.POST.equals(request.getMethod()) || HttpMethod.PUT.equals(request.getMethod()))
-                || !allowStreaming) {
+                || !allowStreaming)) {
             return;
         }
         extensionParam.headers().add(HttpHeaderNames.EXPECT, HttpHeaderValues.CONTINUE);

diff --git a/...rs/netty-connector/src/main/java/org/glassfish/jersey/netty/connector/NettyConnector.java b/...rs/netty-connector/src/main/java/org/glassfish/jersey/netty/connector/NettyConnector.java
@@ -423,7 +423,13 @@ public void operationComplete(io.netty.util.concurrent.Future<? super Void> futu
 
                 if (HttpUtil.is100ContinueExpected(rq)) {
                     expect100ContinueListener =
-                            future -> ch.pipeline().writeAndFlush(nettyRequest);
+                            future -> {
+                        if (future.isSuccess()) {
+                            entityWriter.writeAndFlush(nettyRequest);
+                        } else {
+                            future.channel().pipeline().fireExceptionCaught(future.cause());
+                        }
+                    };
                     expect100ContinueFuture = ch.pipeline().writeAndFlush(rq).sync().awaitUninterruptibly()
                     .addListener(expect100ContinueListener);
                 } else {

diff --git a/...rs/netty-http/src/main/java/org/glassfish/jersey/netty/httpserver/HttpVersionChooser.java b/...rs/netty-http/src/main/java/org/glassfish/jersey/netty/httpserver/HttpVersionChooser.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2019 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2023 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0, which is available at
@@ -20,6 +20,7 @@
 
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.handler.codec.http.HttpServerCodec;
+import io.netty.handler.codec.http.HttpServerExpectContinueHandler;
 import io.netty.handler.codec.http2.Http2MultiplexCodecBuilder;
 import io.netty.handler.ssl.ApplicationProtocolNames;
 import io.netty.handler.ssl.ApplicationProtocolNegotiationHandler;
@@ -55,6 +56,7 @@ protected void configurePipeline(ChannelHandlerContext ctx, String protocol) thr
 
         if (ApplicationProtocolNames.HTTP_1_1.equals(protocol)) {
             ctx.pipeline().addLast(new HttpServerCodec(),
+                                   new HttpServerExpectContinueHandler(),
                                    new ChunkedWriteHandler(),
                                    new JerseyServerHandler(baseUri, container, resourceConfig));
             return;

diff --git a/...s/netty-http/src/main/java/org/glassfish/jersey/netty/httpserver/JerseyServerHandler.java b/...s/netty-http/src/main/java/org/glassfish/jersey/netty/httpserver/JerseyServerHandler.java
@@ -76,10 +76,6 @@ public void channelRead(final ChannelHandlerContext ctx, Object msg) {
         if (msg instanceof HttpRequest) {
             final HttpRequest req = (HttpRequest) msg;
 
-            if (HttpUtil.is100ContinueExpected(req)) {
-                ctx.write(new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.CONTINUE));
-            }
-
             nettyInputStream.clear(); // clearing the content - possible leftover from previous request processing.
             final ContainerRequest requestContext = createContainerRequest(ctx, req);
 

diff --git a/...test/java/org/glassfish/jersey/tests/e2e/client/nettyconnector/Expect100ContinueTest.java b/...test/java/org/glassfish/jersey/tests/e2e/client/nettyconnector/Expect100ContinueTest.java
@@ -23,14 +23,20 @@
 import org.glassfish.jersey.netty.connector.NettyConnectorProvider;
 import org.glassfish.jersey.server.ResourceConfig;
 import org.glassfish.jersey.test.JerseyTest;
+import org.glassfish.jersey.test.grizzly.GrizzlyWebTestContainerFactory;
+import org.glassfish.jersey.test.netty.NettyTestContainerFactory;
+import org.glassfish.jersey.test.spi.TestContainerException;
+import org.glassfish.jersey.test.spi.TestContainerFactory;
 import org.junit.jupiter.api.Test;
 
+import javax.ws.rs.Consumes;
 import javax.ws.rs.HeaderParam;
 import javax.ws.rs.POST;
 import javax.ws.rs.Path;
 import javax.ws.rs.client.Entity;
 import javax.ws.rs.core.Application;
 import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -56,6 +62,11 @@ public Response publishResource(@HeaderParam("Expect") String expect) {
 
     }
 
+    @Override
+    protected TestContainerFactory getTestContainerFactory() throws TestContainerException {
+        return new NettyTestContainerFactory();
+    }
+
     @Override
     protected Application configure() {
         return new ResourceConfig(Expect100ContinueResource.class);
@@ -86,15 +97,15 @@ public void testExpect100ContinueBuffered() {
                .property(ClientProperties.REQUEST_ENTITY_PROCESSING,
                RequestEntityProcessing.BUFFERED).request().header(HttpHeaders.CONTENT_LENGTH, 67000L)
                .post(Entity.text(ENTITY_STRING));
-       assertEquals(100, response.getStatus(), "Expected 100"); //Expect header sent - No Content response
+       assertEquals(204, response.getStatus(), "Expected 204"); //Expect header sent - No Content response
     }
 
     @Test
     public void testExpect100ContinueCustomLength() {
        final Response response =  target(RESOURCE_PATH).register(Expect100ContinueFeature.withCustomThreshold(100L))
-               .request().header(HttpHeaders.CONTENT_LENGTH, 101L)
+               .request().header(HttpHeaders.CONTENT_LENGTH, Integer.MAX_VALUE)
                .post(Entity.text(ENTITY_STRING));
-       assertEquals(100, response.getStatus(), "Expected 100"); //Expect header sent - No Content response
+       assertEquals(204, response.getStatus(), "Expected 204"); //Expect header sent - No Content response
     }
 
     @Test
@@ -123,6 +134,6 @@ public void testExpect100ContinueRegisterViaCustomProperty() {
                .property(ClientProperties.EXPECT_100_CONTINUE, Boolean.TRUE)
                .request().header(HttpHeaders.CONTENT_LENGTH, 44L)
                .post(Entity.text(ENTITY_STRING));
-       assertEquals(100, response.getStatus(), "Expected 100"); //Expect header sent - No Content response
+       assertEquals(204, response.getStatus(), "Expected 204"); //Expect header sent - No Content response
     }
 }