Improved handling of 100 Continue

jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/ContinueProtocolHandler.java

@@ -39,10 +39,14 @@ public String getName()
     @Override
     public boolean accept(Request request, Response response)
     {
-        boolean is100 = response.getStatus() == HttpStatus.CONTINUE_100;
-        boolean expect100 = request.getHeaders().contains(HttpHeader.EXPECT, HttpHeaderValue.CONTINUE.asString());
         boolean handled100 = request.getAttributes().containsKey(ATTRIBUTE);
-        return (is100 || expect100) && !handled100;
+        if (handled100)
+            return false;
+        boolean is100 = response.getStatus() == HttpStatus.CONTINUE_100;
+        if (is100)
+            return true;
+        // Also handle non-100 responses, because we need to complete the request to complete the whole exchange.
+        return request.getHeaders().contains(HttpHeader.EXPECT, HttpHeaderValue.CONTINUE.asString());
     }
 
     @Override
@@ -52,8 +56,9 @@ public Response.Listener getResponseListener()
         return new ContinueListener();
     }
 
-    protected void onContinue(Request request)
+    protected Runnable onContinue(Request request)
     {
+        return null;
     }
 
     protected class ContinueListener extends BufferingResponseListener
@@ -79,8 +84,10 @@ public void onSuccess(Response response)
             {
                 // All good, continue.
                 exchange.resetResponse();
-                exchange.proceed(null);
-                onContinue(request);
+                Runnable proceedAction = onContinue(request);
+                // Pass the proceed action to be executed
+                // by the sender, not here by the receiver.
+                exchange.proceed(proceedAction, null);
             }
             else
             {
@@ -90,7 +97,7 @@ public void onSuccess(Response response)
                 ResponseListeners listeners = exchange.getResponseListeners();
                 HttpContentResponse contentResponse = new HttpContentResponse(response, getContent(), getMediaType(), getEncoding());
                 listeners.emitSuccess(contentResponse);
-                exchange.proceed(new HttpRequestException("Expectation failed", request));
+                exchange.proceed(null, new HttpRequestException("Expectation failed", request));
             }
         }
 

jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpChannel.java

@@ -146,9 +146,9 @@ public void send()
 
     public abstract void release();
 
-    public void proceed(HttpExchange exchange, Throwable failure)
+    public void proceed(HttpExchange exchange, Runnable proceedAction, Throwable failure)
     {
-        getHttpSender().proceed(exchange, failure);
+        getHttpSender().proceed(exchange, proceedAction, failure);
     }
 
     public void abort(HttpExchange exchange, Throwable requestFailure, Throwable responseFailure, Promise<Boolean> promise)

jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpConnection.java

@@ -20,7 +20,6 @@
 
 import org.eclipse.jetty.client.Authentication;
 import org.eclipse.jetty.client.AuthenticationStore;
-import org.eclipse.jetty.client.BytesRequestContent;
 import org.eclipse.jetty.client.HttpClient;
 import org.eclipse.jetty.client.HttpProxy;
 import org.eclipse.jetty.client.HttpRequestException;
@@ -146,7 +145,7 @@ protected void normalizeRequest(HttpRequest request)
 
         // Make sure the path is there
         String path = request.getPath();
-        if (path.trim().length() == 0)
+        if (path.trim().isEmpty())
         {
             path = "/";
             request.path(path);
@@ -191,22 +190,15 @@ protected void normalizeRequest(HttpRequest request)
 
         // Add content headers.
         Request.Content content = request.getBody();
-        if (content == null)
-        {
-            request.body(new BytesRequestContent());
-        }
-        else
+        if (content != null)
         {
             if (!headers.contains(HttpHeader.CONTENT_TYPE))
             {
                 String contentType = content.getContentType();
                 if (contentType == null)
                     contentType = getHttpClient().getDefaultRequestContentType();
                 if (contentType != null)
-                {
-                    HttpField field = new HttpField(HttpHeader.CONTENT_TYPE, contentType);
-                    request.addHeader(field);
-                }
+                    request.addHeader(new HttpField(HttpHeader.CONTENT_TYPE, contentType));
             }
             long contentLength = content.getLength();
             if (contentLength >= 0)
@@ -215,6 +207,9 @@ protected void normalizeRequest(HttpRequest request)
                     request.addHeader(new HttpField.LongValueHttpField(HttpHeader.CONTENT_LENGTH, contentLength));
             }
         }
+        // RFC 9110, section 10.1.1.
+        if (content == null || content.getLength() == 0)
+            request.headers(h -> h.remove(HttpHeader.EXPECT));
 
         // Cookies.
         StringBuilder cookies = convertCookies(request.getCookies(), null);
@@ -243,7 +238,7 @@ private StringBuilder convertCookies(List<HttpCookie> cookies, StringBuilder bui
         {
             if (builder == null)
                 builder = new StringBuilder();
-            if (builder.length() > 0)
+            if (!builder.isEmpty())
                 builder.append("; ");
             builder.append(cookie.getName()).append("=").append(cookie.getValue());
         }

jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpExchange.java

@@ -317,11 +317,11 @@ public void resetResponse()
         }
     }
 
-    public void proceed(Throwable failure)
+    public void proceed(Runnable proceedAction, Throwable failure)
     {
         HttpChannel channel = getHttpChannel();
         if (channel != null)
-            channel.proceed(this, failure);
+            channel.proceed(this, proceedAction, failure);
     }
 
     @Override

jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpSender.java

@@ -317,12 +317,15 @@ protected void dispose()
     {
     }
 
-    public void proceed(HttpExchange exchange, Throwable failure)
+    public void proceed(HttpExchange exchange, Runnable proceedAction, Throwable failure)
     {
-        // Received a 100 Continue, although Expect header was not sent.
+        // Received a 100 Continue, although the Expect header was not sent.
         if (!contentSender.expect100)
             return;
 
+        // Write the fields in this order, since the reader of
+        // these fields will read them in the opposite order.
+        contentSender.proceedAction = proceedAction;
         contentSender.expect100 = false;
         if (failure == null)
         {
@@ -462,32 +465,39 @@ private enum RequestState
 
     private class ContentSender extends IteratingCallback
     {
-        private HttpExchange exchange;
+        // Fields that are set externally.
+        private volatile HttpExchange exchange;
+        private volatile Runnable proceedAction;
+        private volatile boolean expect100;
+        // Fields only used internally.
         private Content.Chunk chunk;
         private ByteBuffer contentBuffer;
-        private boolean expect100;
         private boolean committed;
         private boolean success;
         private boolean complete;
         private Promise<Boolean> abort;
+        private boolean demanded;
 
         @Override
         public boolean reset()
         {
             exchange = null;
+            proceedAction = null;
+            expect100 = false;
             chunk = null;
             contentBuffer = null;
-            expect100 = false;
             committed = false;
             success = false;
             complete = false;
             abort = null;
+            demanded = false;
             return super.reset();
         }
 
         @Override
         protected Action process() throws Throwable
         {
+            HttpExchange exchange = this.exchange;
             if (complete)
             {
                 if (success)
@@ -498,16 +508,27 @@ protected Action process() throws Throwable
             HttpRequest request = exchange.getRequest();
             Content.Source content = request.getBody();
 
+            boolean expect100 = this.expect100;
             if (expect100)
             {
+                // If the request was sent already, wait for
+                // the 100 response before sending the content.
                 if (committed)
                     return Action.IDLE;
-                else
-                    chunk = null;
+                // Do not send any content yet.
+                chunk = null;
             }
             else
             {
-                chunk = content.read();
+                // Run the proceed action first, which likely will provide
+                // content after having received the 100 Continue response.
+                Runnable action = proceedAction;
+                proceedAction = null;
+                if (action != null)
+                    action.run();
+
+                // Read the request content.
+                chunk = content != null ? content.read() : Content.Chunk.EOF;
             }
             if (LOG.isDebugEnabled())
                 LOG.debug("Content {} for {}", chunk, request);
@@ -516,11 +537,15 @@ protected Action process() throws Throwable
             {
                 if (committed)
                 {
-                    content.demand(this::iterate);
-                    return Action.IDLE;
+                    // No content after the headers, demand.
+                    demanded = true;
+                    assert content != null;
+                    content.demand(this::succeeded);
+                    return Action.SCHEDULED;
                 }
                 else
                 {
+                    // Normalize to avoid null checks.
                     chunk = Content.Chunk.EMPTY;
                 }
             }
@@ -545,49 +570,50 @@ protected Action process() throws Throwable
         @Override
         protected void onSuccess()
         {
-            boolean proceed = true;
-            if (committed)
+            if (demanded)
             {
-                if (contentBuffer.hasRemaining())
-                    proceed = someToContent(exchange, contentBuffer);
+                // Content is now available, reset
+                // the demand and iterate again.
+                demanded = false;
             }
             else
             {
-                committed = true;
-                if (headersToCommit(exchange))
+                boolean proceed = true;
+                if (committed)
                 {
-                    // Was any content sent while committing?
                     if (contentBuffer.hasRemaining())
                         proceed = someToContent(exchange, contentBuffer);
                 }
                 else
                 {
-                    proceed = false;
+                    committed = true;
+                    proceed = headersToCommit(exchange);
+                    if (proceed)
+                    {
+                        // Was any content sent while committing?
+                        if (contentBuffer.hasRemaining())
+                            proceed = someToContent(exchange, contentBuffer);
+                    }
                 }
-            }
 
-            boolean last = chunk.isLast();
-            chunk.release();
-            chunk = null;
+                boolean last = chunk.isLast();
+                chunk.release();
+                chunk = null;
 
-            if (proceed)
-            {
-                if (last)
+                if (proceed)
                 {
-                    success = true;
-                    complete = true;
+                    if (last)
+                    {
+                        success = true;
+                        complete = true;
+                    }
                 }
-                else if (expect100)
+                else
                 {
-                    if (LOG.isDebugEnabled())
-                        LOG.debug("Expecting 100 Continue for {}", exchange.getRequest());
+                    // There was some concurrent error, terminate.
+                    complete = true;
                 }
             }
-            else
-            {
-                // There was some concurrent error, terminate.
-                complete = true;
-            }
         }
 
         @Override

jetty-core/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ProxyHandler.java

@@ -393,11 +393,12 @@ protected void addForwardedHeader(Request clientToProxyRequest, org.eclipse.jett
 
     private boolean hasContent(Request clientToProxyRequest)
     {
-        if (clientToProxyRequest.getLength() > 0)
+        long contentLength = clientToProxyRequest.getLength();
+        if (contentLength == 0)
+            return false;
+        if (contentLength > 0)
             return true;
-        HttpFields headers = clientToProxyRequest.getHeaders();
-        return headers.get(HttpHeader.CONTENT_TYPE) != null ||
-               headers.get(HttpHeader.TRANSFER_ENCODING) != null;
+        return clientToProxyRequest.getHeaders().get(HttpHeader.TRANSFER_ENCODING) != null;
     }
 
     private boolean expects100Continue(Request clientToProxyRequest)
@@ -444,12 +445,11 @@ protected void onServerToProxyResponseFailure(Request clientToProxyRequest, org.
         Response.writeError(clientToProxyRequest, proxyToClientResponse, callback, status);
     }
 
-    protected void onServerToProxyResponse100Continue(Request clientToProxyRequest, org.eclipse.jetty.client.Request proxyToServerRequest)
+    protected Runnable onServerToProxyResponse100Continue(Request clientToProxyRequest, org.eclipse.jetty.client.Request proxyToServerRequest)
     {
         if (LOG.isDebugEnabled())
             LOG.debug("{} P2C 100 continue response", requestId(clientToProxyRequest));
-        Runnable action = (Runnable)proxyToServerRequest.getAttributes().get(PROXY_TO_SERVER_CONTINUE_ATTRIBUTE);
-        action.run();
+        return (Runnable)proxyToServerRequest.getAttributes().get(PROXY_TO_SERVER_CONTINUE_ATTRIBUTE);
     }
 
     protected void onServerToProxyResponse102Processing(Request clientToProxyRequest, org.eclipse.jetty.client.Request proxyToServerRequest, HttpFields serverToProxyResponseHeaders, Response proxyToClientResponse)
@@ -776,13 +776,12 @@ public InvocationType getInvocationType()
     private class ProxyContinueProtocolHandler extends ContinueProtocolHandler
     {
         @Override
-        protected void onContinue(org.eclipse.jetty.client.Request proxyToServerRequest)
+        protected Runnable onContinue(org.eclipse.jetty.client.Request proxyToServerRequest)
         {
-            super.onContinue(proxyToServerRequest);
             var clientToProxyRequest = (Request)proxyToServerRequest.getAttributes().get(CLIENT_TO_PROXY_REQUEST_ATTRIBUTE);
             if (LOG.isDebugEnabled())
                 LOG.debug("{} S2P received 100 Continue", requestId(clientToProxyRequest));
-            onServerToProxyResponse100Continue(clientToProxyRequest, proxyToServerRequest);
+            return onServerToProxyResponse100Continue(clientToProxyRequest, proxyToServerRequest);
         }
     }
 

jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java

@@ -1117,8 +1117,6 @@ public String toString()
      */
     public static class ChannelResponse implements Response, Callback
     {
-        private static final CompletableFuture<Void> UNEXPECTED_100_CONTINUE = CompletableFuture.failedFuture(new IllegalStateException("100 not expected"));
-        private static final CompletableFuture<Void> COMMITTED_100_CONTINUE = CompletableFuture.failedFuture(new IllegalStateException("Committed"));
         private final ChannelRequest _request;
         private final ResponseHttpFields _httpFields;
         protected int _status;
@@ -1408,12 +1406,14 @@ public CompletableFuture<Void> writeInterim(int status, HttpFields headers)
                 if (status == HttpStatus.CONTINUE_100)
                 {
                     if (!httpChannelState._expects100Continue)
-                        return UNEXPECTED_100_CONTINUE;
+                        return CompletableFuture.failedFuture(new IllegalStateException("100 not expected"));
+                    if (_request.getLength() == 0)
+                        return CompletableFuture.completedFuture(null);
                     httpChannelState._expects100Continue = false;
                 }
 
                 if (_httpFields.isCommitted())
-                    return status == HttpStatus.CONTINUE_100 ? COMMITTED_100_CONTINUE : CompletableFuture.failedFuture(new IllegalStateException("Committed"));
+                    return CompletableFuture.failedFuture(new IllegalStateException("Committed"));
                 if (_writeCallback != null)
                     return CompletableFuture.failedFuture(new WritePendingException());