diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/Exchange.java b/src/java.net.http/share/classes/jdk/internal/net/http/Exchange.java index ec3e6f0a5438ae55a9200822fb9013a39e68666e..c2f8390e461e16a2567b929b003a4c5b26c05293 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/Exchange.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/Exchange.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2021, 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 @@ -602,7 +602,7 @@ final class Exchange<T> { .thenCompose((Http2Connection c) -> { boolean cached = c.offerConnection(); if (cached) connectionAborter.disable(); - Stream<T> s = c.getStream(1); + Stream<T> s = c.getInitialStream(); if (s == null) { // s can be null if an exception occurred diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java b/src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java index 0a89faa4dff0d8b3fc86ab8c762f90817752e9ac..c216538b8e748506b55c3cdcf4d0f4feeddac3c6 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java @@ -53,12 +53,14 @@ import java.net.http.HttpHeaders; 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; @@ -279,6 +281,7 @@ class Http2Connection { final ConnectionWindowUpdateSender windowUpdater; private volatile Throwable cause; private volatile Supplier<ByteBuffer> initial; + private volatile Stream<?> initialStream; static final int DEFAULT_FRAME_SIZE = 16 * 1024; @@ -332,6 +335,7 @@ class Http2Connection { 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"); @@ -765,7 +769,7 @@ class Http2Connection { 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) { @@ -863,7 +867,7 @@ class Http2Connection { // 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()); @@ -875,7 +879,7 @@ class Http2Connection { 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, @@ -1122,6 +1126,21 @@ class Http2Connection { 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 */ @@ -1439,76 +1458,6 @@ class Http2Connection { + 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; diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/Stream.java b/src/java.net.http/share/classes/jdk/internal/net/http/Stream.java index 80639537f64c4d2f5e8246737e437d060b472200..e781467db6b4df1b8acbd048e402d262481a5b82 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/Stream.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/Stream.java @@ -460,7 +460,7 @@ class Stream<T> extends ExchangeImpl<T> { // 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 { @@ -1504,9 +1504,10 @@ class Stream<T> extends ExchangeImpl<T> { 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."); @@ -1516,15 +1517,28 @@ class Stream<T> extends ExchangeImpl<T> { 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); + } } private static final VarHandle STREAM_STATE; diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/common/HeaderDecoder.java b/src/java.net.http/share/classes/jdk/internal/net/http/common/HeaderDecoder.java new file mode 100644 index 0000000000000000000000000000000000000000..62d03844d2e82f2acfb3e30f869a3e66976a9376 --- /dev/null +++ b/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(); + } +} diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java b/src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java index a3f6a11aa3ef4b3d88f304dc4c13baa9dc6b2039..0e0c9a3eb9e1707e698c74309f13e1c7c5825e3c 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java @@ -392,16 +392,21 @@ public final class Utils { } + 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++) { @@ -410,6 +415,16 @@ public final class Utils { 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 a RFC 7230 field-name. */ diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/common/ValidatingHeadersConsumer.java b/src/java.net.http/share/classes/jdk/internal/net/http/common/ValidatingHeadersConsumer.java new file mode 100644 index 0000000000000000000000000000000000000000..db873cdb05e18fbeba924d6d0192a9499a59404e --- /dev/null +++ b/src/java.net.http/share/classes/jdk/internal/net/http/common/ValidatingHeadersConsumer.java @@ -0,0 +1,89 @@ +/* + * 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.io.IOException; +import java.io.UncheckedIOException; +import java.util.Set; + +/* + * Checks RFC 9113 rules (relaxed) compliance regarding pseudo-headers. + */ +public class ValidatingHeadersConsumer { + + 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. + */ + public void reset() { + pseudoHeadersEnded = false; + } + + /** + * Called when a header field (name, value) pair has been decoded + * @param name the decoded name + * @param value the decoded value + * @throws UncheckedIOException if the name or value are illegal + */ + 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; + // RFC-9113, section 8.2.1 for HTTP/2 and RFC-9114, section 4.2 state that + // header name MUST be lowercase (and allowed characters) + if (!Utils.isValidLowerCaseName(n)) { + throw newException("Bad header name '%s'", n); + } + } + String v = value.toString(); + if (!Utils.isValidValue(v)) { + throw newException("Bad header value '%s'", v); + } + } + + protected String formatMessage(String message, String header) { + return String.format(message, header); + } + + protected UncheckedIOException newException(String message, String header) + { + return new UncheckedIOException( + new IOException(formatMessage(message, header))); + } +} diff --git a/test/jdk/java/net/httpclient/http2/BadHeadersTest.java b/test/jdk/java/net/httpclient/http2/BadHeadersTest.java index 0179317d6e52b533d435ebf42bcae9b4593d36fb..062b4c89e09b71d8f75d1a7cf4b33bedde80ea45 100644 --- a/test/jdk/java/net/httpclient/http2/BadHeadersTest.java +++ b/test/jdk/java/net/httpclient/http2/BadHeadersTest.java @@ -23,6 +23,7 @@ /* * @test + * @bug 8303965 * @library /test/lib /test/jdk/java/net/httpclient/lib * @build jdk.httpclient.test.lib.http2.Http2TestServer jdk.test.lib.net.SimpleSSLContext * @run testng/othervm -Djdk.internal.httpclient.debug=true BadHeadersTest @@ -202,20 +203,26 @@ public class BadHeadersTest { // Assertions based on implementation specific detail messages. Keep in // sync with implementation. static void assertDetailMessage(Throwable throwable, int iterationIndex) { - assertTrue(throwable instanceof IOException, - "Expected IOException, got, " + throwable); - assertTrue(throwable.getMessage().contains("protocol error"), - "Expected \"protocol error\" in: " + throwable.getMessage()); - - if (iterationIndex == 0) { // unknown - assertTrue(throwable.getMessage().contains("Unknown pseudo-header"), - "Expected \"Unknown pseudo-header\" in: " + throwable.getMessage()); - } else if (iterationIndex == 4) { // unexpected - assertTrue(throwable.getMessage().contains(" Unexpected pseudo-header"), - "Expected \" Unexpected pseudo-header\" in: " + throwable.getMessage()); - } else { - assertTrue(throwable.getMessage().contains("Bad header"), - "Expected \"Bad header\" in: " + throwable.getMessage()); + try { + assertTrue(throwable instanceof IOException, + "Expected IOException, got, " + throwable); + assertTrue(throwable.getMessage().contains("malformed response"), + "Expected \"malformed response\" in: " + throwable.getMessage()); + + if (iterationIndex == 0) { // unknown + assertTrue(throwable.getMessage().contains("Unknown pseudo-header"), + "Expected \"Unknown pseudo-header\" in: " + throwable.getMessage()); + } else if (iterationIndex == 4) { // unexpected + assertTrue(throwable.getMessage().contains(" Unexpected pseudo-header"), + "Expected \" Unexpected pseudo-header\" in: " + throwable.getMessage()); + } else { + assertTrue(throwable.getMessage().contains("Bad header"), + "Expected \"Bad header\" in: " + throwable.getMessage()); + } + } catch (AssertionError e) { + System.out.println("Exception does not match expectation: " + throwable); + throwable.printStackTrace(System.out); + throw e; } }