8303965: java.net.http.HttpClient should reset the stream if response…

… headers contain malformed header fields

Reviewed-by: jpai

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -630,7 +630,7 @@ HttpResponse.BodySubscriber<T> ignoreBody(HttpResponse.ResponseInfo hdrs) {
                             if (!cached && connection != null) {
                                 connectionAborter.connection(connection);
                             }
-                            Stream<T> s = c.getStream(1);
+                            Stream<T> s = c.getInitialStream();
 
                             if (s == null) {
                                 // s can be null if an exception occurred

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

@@ -57,12 +57,14 @@
 import jdk.internal.net.http.HttpConnection.HttpPublisher;
 import jdk.internal.net.http.common.FlowTube;
 import jdk.internal.net.http.common.FlowTube.TubeSubscriber;
+import jdk.internal.net.http.common.HeaderDecoder;
 import jdk.internal.net.http.common.HttpHeadersBuilder;
 import jdk.internal.net.http.common.Log;
 import jdk.internal.net.http.common.Logger;
 import jdk.internal.net.http.common.MinimalFuture;
 import jdk.internal.net.http.common.SequentialScheduler;
 import jdk.internal.net.http.common.Utils;
+import jdk.internal.net.http.common.ValidatingHeadersConsumer;
 import jdk.internal.net.http.frame.ContinuationFrame;
 import jdk.internal.net.http.frame.DataFrame;
 import jdk.internal.net.http.frame.ErrorFrame;
@@ -317,6 +319,7 @@ private record PushContinuationState(HeaderDecoder pushContDecoder, PushPromiseF
     final ConnectionWindowUpdateSender windowUpdater;
     private volatile Throwable cause;
     private volatile Supplier<ByteBuffer> initial;
+    private volatile Stream<?> initialStream;
 
     static final int DEFAULT_FRAME_SIZE = 16 * 1024;
 
@@ -370,6 +373,7 @@ private Http2Connection(HttpConnection connection,
 
         Stream<?> initialStream = createStream(exchange);
         boolean opened = initialStream.registerStream(1, true);
+        this.initialStream = initialStream;
         if (debug.on() && !opened) {
             debug.log("Initial stream was cancelled - but connection is maintained: " +
                     "reset frame will need to be sent later");
@@ -808,7 +812,7 @@ void processFrame(Http2Frame frame) throws IOException {
                 if (frame instanceof HeaderFrame) {
                     // always decode the headers as they may affect
                     // connection-level HPACK decoding state
-                    DecodingCallback decoder = new ValidatingHeadersConsumer();
+                    DecodingCallback decoder = new ValidatingHeadersConsumer()::onDecoded;
                     try {
                         decodeHeaders((HeaderFrame) frame, decoder);
                     } catch (UncheckedIOException e) {
@@ -910,7 +914,7 @@ private <T> void handlePushPromise(Stream<T> parent, PushPromiseFrame pp)
         // decoding state
         assert pushContinuationState == null;
         HeaderDecoder decoder = new HeaderDecoder();
-        decodeHeaders(pp, decoder);
+        decodeHeaders(pp, decoder::onDecoded);
         int promisedStreamid = pp.getPromisedStream();
         if (pp.endHeaders()) {
             completePushPromise(promisedStreamid, parent, decoder.headers());
@@ -922,7 +926,7 @@ private <T> void handlePushPromise(Stream<T> parent, PushPromiseFrame pp)
     private <T> void handlePushContinuation(Stream<T> parent, ContinuationFrame cf)
             throws IOException {
         var pcs = pushContinuationState;
-        decodeHeaders(cf, pcs.pushContDecoder);
+        decodeHeaders(cf, pcs.pushContDecoder::onDecoded);
         // if all continuations are sent, set pushWithContinuation to null
         if (cf.endHeaders()) {
             completePushPromise(pcs.pushContFrame.getPromisedStream(), parent,
@@ -1204,6 +1208,21 @@ private void sendConnectionPreface() throws IOException {
         subscriber.onNext(List.of(EMPTY_TRIGGER));
     }
 
+    /**
+     * Called to get the initial stream after a connection upgrade.
+     * If the stream was cancelled, it might no longer be in the
+     * stream map. Therefore - we use the initialStream field
+     * instead, and reset it to null after returning it.
+     * @param <T> the response type
+     * @return the initial stream created during the upgrade.
+     */
+    @SuppressWarnings("unchecked")
+    <T> Stream<T> getInitialStream() {
+         var s = (Stream<T>) initialStream;
+         initialStream = null;
+         return s;
+    }
+
     /**
      * Returns an existing Stream with given id, or null if doesn't exist
      */
@@ -1536,76 +1555,6 @@ final String dbgString() {
                     + connection.getConnectionFlow() + ")";
     }
 
-    static class HeaderDecoder extends ValidatingHeadersConsumer {
-
-        HttpHeadersBuilder headersBuilder;
-
-        HeaderDecoder() {
-            this.headersBuilder = new HttpHeadersBuilder();
-        }
-
-        @Override
-        public void onDecoded(CharSequence name, CharSequence value) {
-            String n = name.toString();
-            String v = value.toString();
-            super.onDecoded(n, v);
-            headersBuilder.addHeader(n, v);
-        }
-
-        HttpHeaders headers() {
-            return headersBuilder.build();
-        }
-    }
-
-    /*
-     * Checks RFC 7540 rules (relaxed) compliance regarding pseudo-headers.
-     */
-    static class ValidatingHeadersConsumer implements DecodingCallback {
-
-        private static final Set<String> PSEUDO_HEADERS =
-                Set.of(":authority", ":method", ":path", ":scheme", ":status");
-
-        /** Used to check that if there are pseudo-headers, they go first */
-        private boolean pseudoHeadersEnded;
-
-        /**
-         * Called when END_HEADERS was received. This consumer may be invoked
-         * again after reset() is called, but for a whole new set of headers.
-         */
-        void reset() {
-            pseudoHeadersEnded = false;
-        }
-
-        @Override
-        public void onDecoded(CharSequence name, CharSequence value)
-                throws UncheckedIOException
-        {
-            String n = name.toString();
-            if (n.startsWith(":")) {
-                if (pseudoHeadersEnded) {
-                    throw newException("Unexpected pseudo-header '%s'", n);
-                } else if (!PSEUDO_HEADERS.contains(n)) {
-                    throw newException("Unknown pseudo-header '%s'", n);
-                }
-            } else {
-                pseudoHeadersEnded = true;
-                if (!Utils.isValidName(n)) {
-                    throw newException("Bad header name '%s'", n);
-                }
-            }
-            String v = value.toString();
-            if (!Utils.isValidValue(v)) {
-                throw newException("Bad header value '%s'", v);
-            }
-        }
-
-        private UncheckedIOException newException(String message, String header)
-        {
-            return new UncheckedIOException(
-                    new IOException(String.format(message, header)));
-        }
-    }
-
     static final class ConnectionWindowUpdateSender extends WindowUpdateSender {
 
         final int initialWindowSize;

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

@@ -500,7 +500,7 @@ void otherFrame(Http2Frame frame) throws IOException {
     // The Hpack decoder decodes into one of these consumers of name,value pairs
 
     DecodingCallback rspHeadersConsumer() {
-        return rspHeadersConsumer;
+        return rspHeadersConsumer::onDecoded;
     }
 
     protected void handleResponse() throws IOException {
@@ -1577,9 +1577,10 @@ final String dbgString() {
         return connection.dbgString() + "/Stream("+streamid+")";
     }
 
-    private class HeadersConsumer extends Http2Connection.ValidatingHeadersConsumer {
+    private class HeadersConsumer extends ValidatingHeadersConsumer {
 
-        void reset() {
+        @Override
+        public void reset() {
             super.reset();
             responseHeadersBuilder.clear();
             debug.log("Response builder cleared, ready to receive new headers.");
@@ -1589,15 +1590,28 @@ void reset() {
         public void onDecoded(CharSequence name, CharSequence value)
             throws UncheckedIOException
         {
-            String n = name.toString();
-            String v = value.toString();
-            super.onDecoded(n, v);
-            responseHeadersBuilder.addHeader(n, v);
-            if (Log.headers() && Log.trace()) {
-                Log.logTrace("RECEIVED HEADER (streamid={0}): {1}: {2}",
-                             streamid, n, v);
+            try {
+                String n = name.toString();
+                String v = value.toString();
+                super.onDecoded(n, v);
+                responseHeadersBuilder.addHeader(n, v);
+                if (Log.headers() && Log.trace()) {
+                    Log.logTrace("RECEIVED HEADER (streamid={0}): {1}: {2}",
+                            streamid, n, v);
+                }
+            } catch (UncheckedIOException uio) {
+                // reset stream: From RFC 9113, section 8.1
+                // Malformed requests or responses that are detected MUST be
+                // treated as a stream error (Section 5.4.2) of type
+                // PROTOCOL_ERROR.
+                onProtocolError(uio.getCause());
             }
         }
+
+        @Override
+        protected String formatMessage(String message, String header) {
+            return "malformed response: " + super.formatMessage(message, header);
+        }
     }
 
     final class Http2StreamResponseSubscriber<U> extends HttpBodySubscriberWrapper<U> {

src/java.net.http/share/classes/jdk/internal/net/http/common/HeaderDecoder.java

@@ -0,0 +1,48 @@
+/*
+ * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package jdk.internal.net.http.common;
+
+import java.net.http.HttpHeaders;
+
+public class HeaderDecoder extends ValidatingHeadersConsumer {
+
+    private final HttpHeadersBuilder headersBuilder;
+
+    public HeaderDecoder() {
+        this.headersBuilder = new HttpHeadersBuilder();
+    }
+
+    @Override
+    public void onDecoded(CharSequence name, CharSequence value) {
+        String n = name.toString();
+        String v = value.toString();
+        super.onDecoded(n, v);
+        headersBuilder.addHeader(n, v);
+    }
+
+    public HttpHeaders headers() {
+        return headersBuilder.build();
+    }
+}

src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java

@@ -431,16 +431,21 @@ public static URLPermission permissionForServer(URI uri,
         return new URLPermission(urlString, actionStringBuilder.toString());
     }
 
+    private static final boolean[] LOWER_CASE_CHARS = new boolean[128];
+
     // ABNF primitives defined in RFC 7230
     private static final boolean[] tchar      = new boolean[256];
     private static final boolean[] fieldvchar = new boolean[256];
 
     static {
-        char[] allowedTokenChars =
-                ("!#$%&'*+-.^_`|~0123456789" +
-                 "abcdefghijklmnopqrstuvwxyz" +
-                 "ABCDEFGHIJKLMNOPQRSTUVWXYZ").toCharArray();
-        for (char c : allowedTokenChars) {
+        char[] lcase = ("!#$%&'*+-.^_`|~0123456789" +
+                "abcdefghijklmnopqrstuvwxyz").toCharArray();
+        for (char c : lcase) {
+            tchar[c] = true;
+            LOWER_CASE_CHARS[c] = true;
+        }
+        char[] ucase = ("ABCDEFGHIJKLMNOPQRSTUVWXYZ").toCharArray();
+        for (char c : ucase) {
             tchar[c] = true;
         }
         for (char c = 0x21; c <= 0xFF; c++) {
@@ -449,6 +454,16 @@ public static URLPermission permissionForServer(URI uri,
         fieldvchar[0x7F] = false; // a little hole (DEL) in the range
     }
 
+    public static boolean isValidLowerCaseName(String token) {
+        for (int i = 0; i < token.length(); i++) {
+            char c = token.charAt(i);
+            if (c > 255 || !LOWER_CASE_CHARS[c]) {
+                return false;
+            }
+        }
+        return !token.isEmpty();
+    }
+
     /*
      * Validates an RFC 7230 field-name.
      */