8369595: HttpClient: HttpHeaders.firstValueAsLong failures should be converted to ProtocolException

src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java

@@ -29,6 +29,7 @@
 import java.lang.System.Logger.Level;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.VarHandle;
+import java.net.ProtocolException;
 import java.net.http.HttpResponse.BodySubscriber;
 import java.nio.ByteBuffer;
 import java.util.concurrent.CompletableFuture;
@@ -275,13 +276,25 @@ long fixupContentLen(long clen) {
      * Read up to MAX_IGNORE bytes discarding
      */
     public CompletableFuture<Void> ignoreBody(Executor executor) {
-        int clen = (int)headers.firstValueAsLong("Content-Length").orElse(-1);
+
+        // Read the `Content-Length` header
+        int clen;
+        var clenK = "Content-Length";
+        try {
+            clen = (int) headers.firstValueAsLong(clenK).orElse(-1);
+        } catch (NumberFormatException nfe) {
+            var pe = new ProtocolException("Illegal value in header " + clenK);
+            pe.initCause(nfe);
+            return MinimalFuture.failedFuture(pe);
+        }
+
         if (clen == -1 || clen > MAX_IGNORE) {
             connection.close();
             return MinimalFuture.completedFuture(null); // not treating as error
         } else {
             return readBody(discarding(), !request.isWebSocket(), executor);
         }
+
     }
 
     // Used for those response codes that have no body associated
@@ -311,13 +324,26 @@ public <U> CompletableFuture<U> readBody(HttpResponse.BodySubscriber<U> p,
 
         final CompletableFuture<U> cf = new MinimalFuture<>();
 
-        long clen0 = headers.firstValueAsLong("Content-Length").orElse(-1L);
-        final long clen = fixupContentLen(clen0);
+        // Read the `Content-Length` header
+        var clenK = "Content-Length";
+        long clen;
+        try {
+            long clen0 = headers.firstValueAsLong(clenK).orElse(-1L);
+            clen = fixupContentLen(clen0);
+        } catch (NumberFormatException nfe) {
+            var pe = new ProtocolException("Invalid value in header " + clenK);
+            pe.initCause(nfe);
+            cf.completeExceptionally(pe);
+            return cf;
+        }
 
         // expect-continue reads headers and body twice.
         // if we reach here, we must reset the headersReader state.
-        asyncReceiver.unsubscribe(headersReader);
-        headersReader.reset();
+        finally {
+            asyncReceiver.unsubscribe(headersReader);
+            headersReader.reset();
+        }
+
         ClientRefCountTracker refCountTracker = new ClientRefCountTracker(connection.client(), debug);
 
         // We need to keep hold on the client facade until the

src/java.net.http/share/classes/jdk/internal/net/http/Http3ExchangeImpl.java

@@ -1281,10 +1281,21 @@ protected void handlePromise(PushHeadersConsumer consumer) throws IOException {
         if (Set.of("PUT", "DELETE", "OPTIONS", "TRACE").contains(method)) {
             throw new ProtocolException("push method not allowed pushId=" + pushId);
         }
-        long clen = promiseHeaders.firstValueAsLong("Content-Length").orElse(-1);
+
+        // Read & validate `Content-Length`
+        var clenK = "Content-Length";
+        long clen;
+        try {
+            clen = promiseHeaders.firstValueAsLong(clenK).orElse(-1);
+        } catch (NumberFormatException nfe) {
+            var pe = new ProtocolException("push headers contain illegal " + clenK);
+            pe.initCause(nfe);
+            throw pe;
+        }
         if (clen > 0) {
-            throw new ProtocolException("push headers contain non-zero Content-Length for pushId=" + pushId);
+            throw new ProtocolException("push headers contain non-zero " + clenK + " for pushId=" + pushId);
         }
+
         if (promiseHeaders.firstValue("Transfer-Encoding").isPresent()) {
             throw new ProtocolException("push headers contain Transfer-Encoding for pushId=" + pushId);
         }

src/java.net.http/share/classes/jdk/internal/net/http/Http3Stream.java

@@ -31,7 +31,6 @@
 import java.net.http.HttpHeaders;
 import java.nio.ByteBuffer;
 import java.util.List;
-import java.util.OptionalLong;
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -608,6 +607,7 @@ void handleResponse(HttpHeadersBuilder responseHeadersBuilder,
          int responseCode;
          boolean finalResponse = false;
          try {
+             responseHeaders.firstValue(":status");
              responseCode = (int) responseHeaders
                      .firstValueAsLong(":status")
                      .orElseThrow(() -> new IOException("no statuscode in response"));
@@ -653,23 +653,28 @@ void handleResponse(HttpHeadersBuilder responseHeadersBuilder,
                      responseHeaders);
          }
 
-         try {
-             OptionalLong cl = responseHeaders.firstValueAsLong("content-length");
-             if (finalResponse && cl.isPresent()) {
-                 long cll = cl.getAsLong();
-                 if (cll < 0) {
-                     cancelImpl(new IOException("Invalid content-length value "+cll), Http3Error.H3_MESSAGE_ERROR);
-                     return;
-                 }
-                 if (!(exchange.request().method().equalsIgnoreCase("HEAD") || responseCode == HTTP_NOT_MODIFIED)) {
-                     // HEAD response and 304 response might have a content-length header,
-                     // but it carries no meaning
-                     contentLength = cll;
-                 }
+         var clK = "content-length";
+         var clS = responseHeaders.firstValue(clK).orElse(null);
+         if (finalResponse && clS != null) {
+             long cl;
+             try {
+                 cl = Long.parseLong(clS);
+             } catch (NumberFormatException nfe) {
+                 var pe = new ProtocolException("Invalid " + clK + " value: " + clS);
+                 pe.initCause(nfe);
+                 cancelImpl(pe, Http3Error.H3_MESSAGE_ERROR);
+                 return;
+             }
+             if (cl < 0) {
+                 var pe = new ProtocolException("Invalid " + clK + " value: " + cl);
+                 cancelImpl(pe, Http3Error.H3_MESSAGE_ERROR);
+                 return;
+             }
+             if (!(exchange.request().method().equalsIgnoreCase("HEAD") || responseCode == HTTP_NOT_MODIFIED)) {
+                 // HEAD response and 304 response might have a content-length header,
+                 // but it carries no meaning
+                 contentLength = cl;
              }
-         } catch (NumberFormatException nfe) {
-             cancelImpl(nfe, Http3Error.H3_MESSAGE_ERROR);
-             return;
          }
 
          if (Log.headers() || debug.on()) {

src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java

@@ -25,10 +25,10 @@
 
 package jdk.internal.net.http;
 
-import java.io.IOError;
 import java.io.IOException;
 import java.lang.ref.WeakReference;
 import java.net.ConnectException;
+import java.net.ProtocolException;
 import java.net.http.HttpClient.Version;
 import java.net.http.HttpConnectTimeoutException;
 import java.net.http.StreamLimitException;
@@ -359,13 +359,28 @@ private static boolean bodyNotPermitted(Response r) {
         return r.statusCode == 204;
     }
 
-    private boolean bodyIsPresent(Response r) {
+    private CompletionStage<Boolean> bodyIsPresent(Response r, Exchange<T> exchange) {
+
+        // Read the `Content-Length` header
+        var resultFuture = new MinimalFuture<Boolean>();
         HttpHeaders headers = r.headers();
-        if (headers.firstValueAsLong("Content-length").orElse(0L) != 0L)
-            return true;
-        if (headers.firstValue("Transfer-encoding").isPresent())
-            return true;
-        return false;
+        var clK = "Content-length";
+        long cl;
+        try {
+            cl = headers.firstValueAsLong(clK).orElse(0L);
+        } catch (NumberFormatException nfe) {
+            var pe = new ProtocolException("Invalid value in header " + clK);
+            pe.initCause(nfe);
+            exchange.cancel(pe);
+            resultFuture.completeExceptionally(pe);
+            return resultFuture;
+        }
+
+        // Perform the check
+        var result = cl != 0L || headers.firstValue("Transfer-encoding").isPresent();
+        resultFuture.complete(result);
+        return resultFuture;
+
     }
 
     // Call the user's body handler to get an empty body object
@@ -404,13 +419,15 @@ private HttpResponse<T> setNewResponse(HttpRequest request, Response r, T body,
                         processAltSvcHeader(r, client(), currentreq);
                         Exchange<T> exch = getExchange();
                         if (bodyNotPermitted(r)) {
-                            if (bodyIsPresent(r)) {
-                                IOException ioe = new IOException(
-                                    "unexpected content length header with 204 response");
-                                exch.cancel(ioe);
-                                return MinimalFuture.failedFuture(ioe);
-                            } else
-                                return handleNoBody(r, exch);
+                            return bodyIsPresent(r, exch).thenCompose(bodyIsPresent -> {
+                                if (bodyIsPresent) {
+                                    IOException ioe = new IOException(
+                                            "unexpected content length header with 204 response");
+                                    exch.cancel(ioe);
+                                    return MinimalFuture.failedFuture(ioe);
+                                } else
+                                    return handleNoBody(r, exch);
+                            });
                         }
                         return exch.readBodyAsync(responseHandler)
                             .thenApply((T body) -> setNewResponse(r.request, r, body, exch));

src/java.net.http/share/classes/jdk/internal/net/http/Stream.java

@@ -620,11 +620,21 @@ protected void handleResponse(HeaderFrame hf) throws IOException {
 
         if (!finalResponseCodeReceived) {
             try {
-                responseCode = (int) responseHeaders
-                        .firstValueAsLong(":status")
-                        .orElseThrow(() -> new ProtocolException(String.format(
-                                "Stream %s PROTOCOL_ERROR: no status code in response",
-                                streamid)));
+                var responseCodeString = responseHeaders.firstValue(":status").orElse(null);
+                if (responseCodeString == null) {
+                    throw new ProtocolException(String.format(
+                            "Stream %s PROTOCOL_ERROR: no status code in response",
+                            streamid));
+                }
+                try {
+                    responseCode = Integer.parseInt(responseCodeString);
+                } catch (NumberFormatException nfe) {
+                    var pe = new ProtocolException(String.format(
+                            "Stream %s PROTOCOL_ERROR: invalid status code in response",
+                            streamid));
+                    pe.initCause(nfe);
+                    throw pe;
+                }
             } catch (ProtocolException cause) {
                 cancelImpl(cause, ResetFrame.PROTOCOL_ERROR);
                 rspHeadersConsumer.reset();
@@ -660,12 +670,6 @@ protected void handleResponse(HeaderFrame hf) throws IOException {
                     request, exchange, responseHeaders, connection(),
                     responseCode, HttpClient.Version.HTTP_2);
 
-            /* TODO: review if needs to be removed
-               the value is not used, but in case `content-length` doesn't parse as
-               long, there will be NumberFormatException. If left as is, make sure
-               code up the stack handles NFE correctly. */
-            responseHeaders.firstValueAsLong("content-length");
-
             if (Log.headers()) {
                 StringBuilder sb = new StringBuilder("RESPONSE HEADERS (streamid=%s):\n".formatted(streamid));
                 sb.append("  %s %s %s\n".formatted(request.method(), request.uri(), responseCode));
@@ -1755,21 +1759,28 @@ protected void handleResponse(HeaderFrame hf) {
             HttpHeaders responseHeaders = responseHeadersBuilder.build();
 
             if (!finalPushResponseCodeReceived) {
-                responseCode = (int)responseHeaders
-                    .firstValueAsLong(":status")
-                    .orElse(-1);
-
-                if (responseCode == -1) {
-                    cancelImpl(new ProtocolException("No status code"), ResetFrame.PROTOCOL_ERROR);
+                try {
+                    var responseCodeString = responseHeaders.firstValue(":status").orElse(null);
+                    if (responseCodeString == null) {
+                        throw new ProtocolException("No status code");
+                    }
+                    try {
+                        responseCode = Integer.parseInt(responseCodeString);
+                    } catch (NumberFormatException nfe) {
+                        var pe = new ProtocolException("Invalid status code: " + responseCodeString);
+                        pe.initCause(nfe);
+                        throw pe;
+                    }
+                    if (responseCode >= 100 && responseCode < 200) {
+                        String protocolErrorMsg = checkInterimResponseCountExceeded();
+                        if (protocolErrorMsg != null) {
+                            throw new ProtocolException(protocolErrorMsg);
+                        }
+                    }
+                } catch (ProtocolException pe) {
+                    cancelImpl(pe, ResetFrame.PROTOCOL_ERROR);
                     rspHeadersConsumer.reset();
                     return;
-                } else if (responseCode >= 100 && responseCode < 200) {
-                    String protocolErrorMsg = checkInterimResponseCountExceeded();
-                    if (protocolErrorMsg != null) {
-                        cancelImpl(new ProtocolException(protocolErrorMsg), ResetFrame.PROTOCOL_ERROR);
-                        rspHeadersConsumer.reset();
-                        return;
-                    }
                 }
 
                 this.finalPushResponseCodeReceived = true;
@@ -1778,12 +1789,6 @@ protected void handleResponse(HeaderFrame hf) {
                         pushReq, exchange, responseHeaders, connection(),
                         responseCode, HttpClient.Version.HTTP_2);
 
-                /* TODO: review if needs to be removed
-                   the value is not used, but in case `content-length` doesn't parse
-                   as long, there will be NumberFormatException. If left as is, make
-                   sure code up the stack handles NFE correctly. */
-                responseHeaders.firstValueAsLong("content-length");
-
                 if (Log.headers()) {
                     StringBuilder sb = new StringBuilder("RESPONSE HEADERS (streamid=%s):\n".formatted(streamid));
                     sb.append("  %s %s %s\n".formatted(request.method(), request.uri(), responseCode));