Candidate fix for an issue with HTTP 1xx responses.

We were returning the origin connection to the pool after getting a 1xx HTTP response, which was incorrect – those are responses that indicate a further response was coming. In some cases, the origin's eventual (real) response could actually go to the wrong client in these cases.

This also fixes an issue where the client -> proxy connection explicitly handled HTTP 100 (Continue), but not any other 1xx response. The others aren't common, but we should handle them anyway.

Some Claude code in here, fwiw, but I've reviewed enough that I'm to blame for mistakes :)

Author: matthoffman

zuul-core/src/main/java/com/netflix/netty/common/HttpClientLifecycleChannelHandler.java

@@ -23,6 +23,7 @@
 import io.netty.channel.ChannelPromise;
 import io.netty.handler.codec.http.HttpRequest;
 import io.netty.handler.codec.http.HttpResponse;
+import io.netty.handler.codec.http.HttpStatusClass;
 import io.netty.handler.codec.http.LastHttpContent;
 import io.netty.util.AttributeKey;
 
@@ -48,7 +49,12 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
                 super.channelRead(ctx, msg);
             } finally {
                 if (msg instanceof LastHttpContent) {
-                    fireCompleteEventIfNotAlready(ctx, CompleteReason.SESSION_COMPLETE);
+                    HttpResponse resp = ctx.channel().attr(ATTR_HTTP_RESP).get();
+                    boolean isInformational =
+                            resp != null && resp.status().codeClass() == HttpStatusClass.INFORMATIONAL;
+                    if (!isInformational) {
+                        fireCompleteEventIfNotAlready(ctx, CompleteReason.SESSION_COMPLETE);
+                    }
                 }
             }
         }

zuul-core/src/main/java/com/netflix/netty/common/HttpServerLifecycleChannelHandler.java

@@ -23,10 +23,9 @@
 import io.netty.channel.ChannelPromise;
 import io.netty.handler.codec.http.HttpRequest;
 import io.netty.handler.codec.http.HttpResponse;
-import io.netty.handler.codec.http.HttpResponseStatus;
+import io.netty.handler.codec.http.HttpStatusClass;
 import io.netty.handler.codec.http.LastHttpContent;
 import io.netty.util.ReferenceCountUtil;
-import java.util.Objects;
 
 /**
  * @author michaels
@@ -68,19 +67,21 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)
             } finally {
                 if (msg instanceof LastHttpContent) {
 
-                    boolean dontFireCompleteYet = false;
+                    // Handle case of 100 CONTINUE (or other 1xx responses), where server sends an initial 100 status
+                    // response to indicate to client that it can continue sending the initial request body.
+                    // i.e. in this case we don't want to consider the state to be COMPLETE until after the 2nd
+                    // response.
+                    HttpResponse resp;
                     if (msg instanceof HttpResponse httpResponse) {
-                        // Handle case of 100 CONTINUE, where server sends an initial 100 status response to indicate to
-                        // client
-                        // that it can continue sending the initial request body.
-                        // ie. in this case we don't want to consider the state to be COMPLETE until after the 2nd
-                        // response.
-                        if (Objects.equals(httpResponse.status(), HttpResponseStatus.CONTINUE)) {
-                            dontFireCompleteYet = true;
-                        }
+                        resp = httpResponse;
+                    } else {
+                        // 1xx responses forwarded from the origin are often forwarded as two separate pipeline events
+                        // (the actual 1xx response and an empty LastHttpContent). In that case, httpResponse is null.
+                        resp = ctx.channel().attr(ATTR_HTTP_RESP).get();
                     }
-
-                    if (!dontFireCompleteYet) {
+                    boolean isInformational =
+                            resp != null && resp.status().codeClass() == HttpStatusClass.INFORMATIONAL;
+                    if (!isInformational) {
                         if (promise.isDone()) {
                             fireCompleteEventIfNotAlready(ctx, CompleteReason.SESSION_COMPLETE);
                         } else {

zuul-core/src/main/java/com/netflix/zuul/filters/endpoint/ProxyEndpoint.java

@@ -82,6 +82,7 @@
 import io.netty.handler.codec.http.DefaultLastHttpContent;
 import io.netty.handler.codec.http.HttpContent;
 import io.netty.handler.codec.http.HttpResponse;
+import io.netty.handler.codec.http.HttpStatusClass;
 import io.netty.handler.codec.http.LastHttpContent;
 import io.netty.util.ReferenceCountUtil;
 import io.netty.util.concurrent.Future;
@@ -137,6 +138,7 @@ public class ProxyEndpoint extends SyncZuulFilterAdapter<HttpRequestMessage, Htt
     protected MethodBinding<?> methodBinding;
     protected HttpResponseMessage zuulResponse;
     protected boolean startedSendingResponseToClient;
+    private boolean pendingInterimResponse;
     protected Duration timeLeftForAttempt;
 
     /* Individual retry related state */
@@ -432,7 +434,13 @@ private void filterResponseChunk(HttpContent chunk) {
         }
 
         if (chunk instanceof LastHttpContent) {
-            unlinkFromOrigin();
+            if (pendingInterimResponse) {
+                // This empty LastHttpContent is the tail of a 1xx interim response; the origin connection
+                // must stay open for the real final response that follows.
+                pendingInterimResponse = false;
+            } else {
+                unlinkFromOrigin();
+            }
         }
 
         if (responseFilters != null) {
@@ -838,13 +846,30 @@ public void responseFromOrigin(HttpResponse originResponse) {
     }
 
     private void processResponseFromOrigin(HttpResponse originResponse) {
-        if (originResponse.status().code() >= 500) {
+        if (originResponse.status().codeClass() == HttpStatusClass.INFORMATIONAL) {
+            handleInterimResponse(originResponse);
+        } else if (originResponse.status().code() >= 500) {
             handleOriginNonSuccessResponse(originResponse, chosenServer.get());
         } else {
             handleOriginSuccessResponse(originResponse, chosenServer.get());
         }
     }
 
+    private void handleInterimResponse(HttpResponse originResponse) {
+        int respStatus = originResponse.status().code();
+        HttpResponseMessage interimZuulResponse = new HttpResponseMessageImpl(context, zuulRequest, respStatus);
+        Headers respHeaders = interimZuulResponse.getHeaders();
+        for (Map.Entry<String, String> entry : originResponse.headers()) {
+            respHeaders.add(entry.getKey(), entry.getValue());
+        }
+        // Track that we are mid-interim-response so filterResponseChunk does not call unlinkFromOrigin()
+        // on the empty LastHttpContent that Netty emits after every 1xx.
+        // Do NOT set startedSendingResponseToClient and do NOT record final metrics or response status.
+        pendingInterimResponse = true;
+        zuulResponse = interimZuulResponse;
+        invokeNext(interimZuulResponse);
+    }
+
     protected void handleOriginSuccessResponse(HttpResponse originResponse, DiscoveryResult chosenServer) {
         origin.recordSuccessResponse();
         if (originConn != null) {