8303965: java.net.http.HttpClient should reset the stream if response headers contain malformed header fields

Reviewed-by: jpai
This commit is contained in:
Daniel Fuchs 2023-03-13 14:24:56 +00:00
parent 431e702b67
commit 466ffebcae
7 changed files with 225 additions and 103 deletions
src/java.net.http/share/classes/jdk/internal/net/http
test/jdk/java/net/httpclient/http2

@ -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 @@ final class Exchange<T> {
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

@ -57,12 +57,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;
@ -317,6 +319,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;
@ -370,6 +373,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");
@ -808,7 +812,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) {
@ -910,7 +914,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());
@ -922,7 +926,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,
@ -1204,6 +1208,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
*/
@ -1536,76 +1555,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;

@ -500,7 +500,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 {
@ -1577,9 +1577,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.");
@ -1589,15 +1590,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);
}
}
final class Http2StreamResponseSubscriber<U> extends HttpBodySubscriberWrapper<U> {

@ -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();
}
}

@ -431,16 +431,21 @@ public final class Utils {
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 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 an RFC 7230 field-name.
*/

@ -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)));
}
}

@ -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());
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());
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;
}
}