Skip to content

Commit

Permalink
Expect:100-continue fixes for Netty
Browse files Browse the repository at this point in the history 
Signed-off-by: Maxim Nesen <[email protected]>
  • Loading branch information
@senivam
senivam committed Oct 6, 2023
1 parent ccf434d commit 6158e32
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 24 deletions.
47 changes: 29 additions & 18 deletions ...ector/src/main/java/org/glassfish/jersey/netty/connector/JerseyExpectContinueHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@
import io.netty.handler.codec.http.HttpResponseStatus;
import io.netty.handler.codec.http.HttpUtil;
import org.glassfish.jersey.client.ClientRequest;
import org.glassfish.jersey.netty.connector.internal.NettyEntityWriter;

import javax.ws.rs.ProcessingException;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
Expand All @@ -37,13 +39,19 @@ public class JerseyExpectContinueHandler extends ChannelInboundHandlerAdapter {

private boolean isExpected;

private static final List<HttpResponseStatus> statusesToBeConsidered = Arrays.asList(HttpResponseStatus.CONTINUE,
HttpResponseStatus.UNAUTHORIZED, HttpResponseStatus.EXPECTATION_FAILED,
HttpResponseStatus.METHOD_NOT_ALLOWED, HttpResponseStatus.REQUEST_ENTITY_TOO_LARGE);

private CompletableFuture<HttpResponseStatus> expectedFuture = new CompletableFuture<>();

@Override
public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception {
if (isExpected && msg instanceof HttpResponse) {
final HttpResponse response = (HttpResponse) msg;
expectedFuture.complete(response.status());
if (statusesToBeConsidered.contains(response.status())) {
expectedFuture.complete(response.status());
}
if (!HttpResponseStatus.CONTINUE.equals(response.status())) {
ctx.fireChannelRead(msg); //bypass the message to the next handler in line
}
Expand All @@ -58,7 +66,6 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
CompletableFuture<HttpResponseStatus> processExpect100ContinueRequest(HttpRequest nettyRequest,
ClientRequest jerseyRequest,
Channel ch,
NettyEntityWriter entityWriter,
Integer timeout)
throws InterruptedException, ExecutionException, TimeoutException {
//check for 100-Continue presence/availability
Expand All @@ -81,26 +88,30 @@ CompletableFuture<HttpResponseStatus> processExpect100ContinueRequest(HttpReques
if (!isExpected) {
ch.pipeline().remove(this);
} else {
HttpResponseStatus status = expectedFuture
final HttpResponseStatus status = expectedFuture
.get(timeout, TimeUnit.MILLISECONDS);
if (!expectedFuture.isDone()) { //still not complete, continue without expectations
throw new TimeoutException();
}
switch (status.code()) {
case 417:
entityWriter.writeAndFlush(nettyRequest);
break;
case 100:
break;
case 413:
case 401:
case 405:
throw new ExecutionException("Unexpected value: " + status.code(), null);
}

processExpectationStatus(status);
}
return expectedFuture;
}

private void processExpectationStatus(HttpResponseStatus status)
throws TimeoutException {
if (!statusesToBeConsidered.contains(status)) {
throw new ProcessingException(LocalizationMessages
.UNEXPECTED_VALUE_FOR_EXPECT_100_CONTINUE_STATUSES(status.code()), null);
}
if (!expectedFuture.isDone() || HttpResponseStatus.EXPECTATION_FAILED.equals(status)) {
isExpected = false;
throw new TimeoutException(); // continue without expectations
}
if (!HttpResponseStatus.CONTINUE.equals(status)) {
throw new ProcessingException(LocalizationMessages
.UNEXPECTED_VALUE_FOR_EXPECT_100_CONTINUE_STATUSES(status.code()), null);
}
}

boolean isExpected() {
return isExpected;
}
Expand Down
6 changes: 4 additions & 2 deletions ...rs/netty-connector/src/main/java/org/glassfish/jersey/netty/connector/NettyConnector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,13 +416,15 @@ public void operationComplete(io.netty.util.concurrent.Future<? super Void> futu
}
try {
expect100ContinueHandler.processExpect100ContinueRequest(nettyRequest, jerseyRequest,
ch, entityWriter, expect100ContinueTimeout);
ch, expect100ContinueTimeout);
} catch (ExecutionException e) {
responseDone.completeExceptionally(e);
} catch (TimeoutException e) {
//Expect:100-continue allows timeouts by the spec
//just removing the pipeline from processing
ch.pipeline().remove(EXPECT_100_CONTINUE_HANDLER);
if (ch.pipeline().context(JerseyExpectContinueHandler.class) != null) {
ch.pipeline().remove(EXPECT_100_CONTINUE_HANDLER);
}
}

if (!expect100ContinueHandler.isExpected()) {
Expand Down
1 change: 1 addition & 0 deletions ...connector/src/main/resources/org/glassfish/jersey/netty/connector/localization.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@ redirect.no.location="Received redirect that does not contain a location or the
redirect.error.determining.location="Error determining redirect location: ({0})."
redirect.infinite.loop="Infinite loop in chained redirects detected."
redirect.limit.reached="Max chained redirect limit ({0}) exceeded."
unexpected.value.for.expect.100.continue.statuses=Unexpected value: ("{0}").
13 changes: 9 additions & 4 deletions ...test/java/org/glassfish/jersey/tests/e2e/client/nettyconnector/Expect100ContinueTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ public void testExpect100ContinueUnauthorized() {
assertThrows(ProcessingException.class, () -> target(RESOURCE_PATH_UNAUTHORIZED)
.property(ClientProperties.EXPECT_100_CONTINUE_THRESHOLD_SIZE, 43L)
.property(ClientProperties.EXPECT_100_CONTINUE, Boolean.TRUE)
.property(ClientProperties.EXPECT_100_CONTINUE_TIMEOUT, 10000)
.request().header(HttpHeaders.CONTENT_LENGTH, 44L)
.post(Entity.text(ENTITY_STRING)));
}
Expand All @@ -185,6 +186,7 @@ public void testExpect100ContinuePayloadTooLarge() {
assertThrows(ProcessingException.class, () -> target(RESOURCE_PATH_PAYLOAD_TOO_LARGE)
.property(ClientProperties.EXPECT_100_CONTINUE_THRESHOLD_SIZE, 43L)
.property(ClientProperties.EXPECT_100_CONTINUE, Boolean.TRUE)
.property(ClientProperties.EXPECT_100_CONTINUE_TIMEOUT, 10000)
.request().header(HttpHeaders.CONTENT_LENGTH, 44L)
.post(Entity.text(ENTITY_STRING)));
}
Expand All @@ -194,6 +196,7 @@ public void testExpect100ContinueMethodNotSupported() {
assertThrows(ProcessingException.class, () -> target(RESOURCE_PATH_METHOD_NOT_SUPPORTED)
.property(ClientProperties.EXPECT_100_CONTINUE_THRESHOLD_SIZE, 43L)
.property(ClientProperties.EXPECT_100_CONTINUE, Boolean.TRUE)
.property(ClientProperties.EXPECT_100_CONTINUE_TIMEOUT, 10000)
.request().header(HttpHeaders.CONTENT_LENGTH, 44L)
.post(Entity.text(ENTITY_STRING)));
}
Expand All @@ -205,23 +208,25 @@ public void handle(String target,
HttpServletRequest request,
HttpServletResponse response) throws IOException {
boolean expected = request.getHeader("Expect") != null;
boolean failed = false;
if (target.equals("/" + RESOURCE_PATH_NOT_SUPPORTED)) {
response.sendError(417);
failed = true;
}
if (target.equals("/" + RESOURCE_PATH_UNAUTHORIZED)) {
response.sendError(401);
failed = true;
}
if (target.equals("/" + RESOURCE_PATH_PAYLOAD_TOO_LARGE)) {
response.sendError(413);
failed = true;
}
if (target.equals("/" + RESOURCE_PATH_METHOD_NOT_SUPPORTED)) {
response.sendError(405);
failed = true;
}
if (expected) {
if (expected && !failed) {
System.out.println("Expect:100-continue found, sending response header");
}

if (expected) {
response.setStatus(204);
}
response.getWriter().println();
Expand Down

0 comments on commit 6158e32

Please sign in to comment.