diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java b/src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java index f9442df9ac460..07b705b69847e 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java +++ b/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 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 CompletableFuture readBody(HttpResponse.BodySubscriber p, final CompletableFuture 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 diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/Http3ExchangeImpl.java b/src/java.net.http/share/classes/jdk/internal/net/http/Http3ExchangeImpl.java index 41a4a84958a22..a024b2d6cc482 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/Http3ExchangeImpl.java +++ b/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); } diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/Http3Stream.java b/src/java.net.http/share/classes/jdk/internal/net/http/Http3Stream.java index cdac68b47f1b5..a3bc7c97a4792 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/Http3Stream.java +++ b/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()) { diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java b/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java index ec621f7f95576..b49cb2f31a259 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java +++ b/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 bodyIsPresent(Response r, Exchange exchange) { + + // Read the `Content-Length` header + var resultFuture = new MinimalFuture(); 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 setNewResponse(HttpRequest request, Response r, T body, processAltSvcHeader(r, client(), currentreq); Exchange 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)); 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 4b59a777de298..8bf119a9a8e30 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 @@ -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)); diff --git a/test/jdk/java/net/httpclient/http3/H3MalformedResponseTest.java b/test/jdk/java/net/httpclient/http3/H3MalformedResponseTest.java index 881277da1f82c..097753e15df30 100644 --- a/test/jdk/java/net/httpclient/http3/H3MalformedResponseTest.java +++ b/test/jdk/java/net/httpclient/http3/H3MalformedResponseTest.java @@ -22,410 +22,457 @@ */ import jdk.httpclient.test.lib.common.HttpServerAdapters; -import jdk.httpclient.test.lib.quic.QuicServerConnection; import jdk.httpclient.test.lib.quic.QuicStandaloneServer; -import jdk.internal.net.http.http3.Http3Error; -import jdk.internal.net.http.quic.TerminationCause; +import jdk.internal.net.http.common.Logger; +import jdk.internal.net.http.common.Utils; import jdk.internal.net.quic.QuicVersion; import jdk.test.lib.net.SimpleSSLContext; import jdk.test.lib.net.URIBuilder; -import jdk.test.lib.Utils; -import org.testng.annotations.AfterClass; -import org.testng.annotations.BeforeClass; -import org.testng.annotations.DataProvider; -import org.testng.annotations.Test; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import javax.net.ssl.SSLContext; +import java.io.IOException; import java.io.OutputStream; -import java.net.URI; -import java.net.URISyntaxException; +import java.net.ProtocolException; import java.net.http.HttpClient; import java.net.http.HttpClient.Version; import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.net.http.HttpResponse.BodyHandlers; import java.util.HexFormat; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.BooleanSupplier; -import static java.net.http.HttpOption.Http3DiscoveryMode.HTTP_3_URI_ONLY; +import static java.net.http.HttpClient.Builder.NO_PROXY; import static java.net.http.HttpOption.H3_DISCOVERY; -import static org.testng.Assert.*; +import static java.net.http.HttpOption.Http3DiscoveryMode.HTTP_3_URI_ONLY; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; /* * @test - * @summary Verifies that the HTTP client correctly handles malformed responses - * @library /test/lib /test/jdk/java/net/httpclient/lib - * @library ../access - * @build jdk.test.lib.net.SimpleSSLContext - * jdk.httpclient.test.lib.common.HttpServerAdapters - * @build java.net.http/jdk.internal.net.http.Http3ConnectionAccess - * @run testng/othervm - * -Djdk.internal.httpclient.debug=true - * -Djdk.httpclient.HttpClient.log=requests,responses,errors H3MalformedResponseTest + * @bug 8369595 + * @summary Verifies that the HTTP/3 malformed responses are correctly handled + * @library /test/jdk/java/net/httpclient/lib + * /test/lib + * @run junit H3MalformedResponseTest */ -public class H3MalformedResponseTest implements HttpServerAdapters { - private SSLContext sslContext; - private QuicStandaloneServer server; - private String requestURIBase; +/// Verifies that the HTTP/3 malformed responses are correctly handled. +/// +/// ### HTTP/3 `HEADERS` frame & QPACK Field Section encoding crash course +/// +/// Consider an HTTP/3 `HEADERS` frame that carries: +/// +/// ``` +/// :status: 200 +/// content-length: 2 +/// ``` +/// +/// This will be encoded as the following byte sequence: +/// +/// ``` +/// 01 06 00 00 D9 54 01 32 +/// ``` +/// +/// Let's start with decoding the HTTP/3 frame: +/// +/// - `01`: Frame Type (`01` denotes `HEADERS``) +/// +/// - `06`: Payload length (6 bytes) +/// +/// Figured this is a `HEADERS` frame containing 6 bytes: `00 00 D9 54 01 32`. +/// Let's decode the QPACK Field Section +/// +/// - `00`: Required Insert Count (0) +/// +/// - `00`: Base (0) +/// +/// - `D9`: +/// QPACK has a static table (indexed from 0) and `:status: 200` is at the +/// static-table index 25. +/// +/// ``` +/// D9 = <1101 1001> +/// = <1> (Indexed Field Line) +/// + <1> (Static Table) +/// + <01 1001> (entry index = 25) +/// ``` +/// +/// - `54 01 32`: +/// `content-length: 2` can be encoded as a *literal field line with name +/// reference* using the static name `content-length` (static index 4) and +/// the literal value `2`. +/// +/// ``` +/// 54 01 32 = <0101 0100 0000 0001 0011 0010> +/// = <0> (Literal Field Line) +/// + <1> (Name Reference) +/// + <0100> (entry index = 4) +/// + <0000 0001> (value length = 1 byte) +/// + <0011 0010> (value = ASCII "2" = 0x32 = 50) +/// ``` +/// +/// Note that the `value length` field (i.e., `0000 0001`) follows a variable +/// coding scheme: +/// +/// | Prefix | Total size | Payload size | Max value | +/// | --------------------- | ---------- | ------------ | ------------- | +/// | `00xx xxxx` | 1 byte | 6 bits | 63 | +/// | `01xx xxxx xxxx xxxx` | 2 bytes | 14 bits | 16,383 | +/// | `10xx xxx…` | 4 bytes | 30 bits | 1,073,741,823 | +/// | `11xx xxx…` | 8 bytes | 62 bits | 4.61e18 | +class H3MalformedResponseTest { + + private static final String CLASS_NAME = H3MalformedResponseTest.class.getSimpleName(); + + private static final Logger LOGGER = Utils.getDebugLogger(CLASS_NAME::toString, Utils.DEBUG); + + private static SSLContext SSL_CONTEXT; + + private static QuicStandaloneServer SERVER; + + private static HttpRequest REQUEST; + + @BeforeAll + static void setUp() throws Exception { + + // Obtain an `SSLContext` + SSL_CONTEXT = new SimpleSSLContext().get(); + assertNotNull(SSL_CONTEXT); + + // Create and start the server + SERVER = QuicStandaloneServer.newBuilder() + .availableVersions(new QuicVersion[]{QuicVersion.QUIC_V1}) + .sslContext(SSL_CONTEXT) + .alpn("h3") + .build(); + SERVER.start(); + LOGGER.log("Server is started at {}", SERVER.getAddress()); + + // Create the request + var requestURI = URIBuilder.newBuilder() + .scheme("https") + .loopback() + .port(SERVER.getAddress().getPort()) + .path("/" + CLASS_NAME) + .build(); + REQUEST = HttpRequest.newBuilder(requestURI) + .version(Version.HTTP_3) + .setOption(H3_DISCOVERY, HTTP_3_URI_ONLY) + .build(); + + } + + @AfterAll + static void tearDown() { + close("server", SERVER); + } + + private static void close(String name, AutoCloseable closeable) { + if (closeable != null) { + LOGGER.log("Closing {}", name); + try { + closeable.close(); + } catch (Exception e) { + LOGGER.log("Could not close " + name, e); + } + } + } - // These responses are malformed and should not be accepted by the client, - // but they should not cause connection closure - @DataProvider - public static Object[][] malformedResponse() { + /// Malformed responses that should not be accepted by the client, but + /// should neither cause the connection get closed. + static Object[][] malformedResponsesInvalidatingConnection() { return new Object[][]{ - new Object[] {"empty", HexFormat.of().parseHex( - "" - )}, - new Object[] {"non-final response", HexFormat.of().parseHex( - "01040000"+ // headers, length 4, section prefix - "ff00" // :status:100 - )}, - new Object[] {"uppercase header name", HexFormat.of().parseHex( - "01090000"+ // headers, length 9, section prefix - "d9"+ // :status:200 - "234147450130"+ // AGE:0 - "000100" // data, 1 byte - )}, - new Object[] {"content too long", HexFormat.of().parseHex( - "01040000"+ // headers, length 4, section prefix - "d9"+ // :status:200 - "c4"+ // content-length:0 + {"empty", IOException.class, parseHex("")}, + {"non-final response", IOException.class, parseHex( + "01040000", // headers, length 4, section prefix + "ff00" // :status:100 + )}, + {"uppercase header name", ProtocolException.class, parseHex( + "01090000", // headers, length 9, section prefix + "d9", // :status:200 + "234147450130", // AGE:0 + "000100" // data, 1 byte + )}, + {"content too long", IOException.class, parseHex( + "01040000", // headers, length 4, section prefix + "d9", // :status:200 + "c4", // content-length:0 + "000100" // data, 1 byte + )}, + {"content too short", IOException.class, parseHex( + "01060000", // headers, length 6, section prefix + "d9", // :status:200 + "540132", // content-length:2 + "000100" // data, 1 byte + )}, + {"text in content-length", ProtocolException.class, parseHex( + "01060000" + // headers, length 6, section prefix + "d9" + // :status:200 + "540161" + // content-length:a "000100" // data, 1 byte )}, - new Object[] {"content too short", HexFormat.of().parseHex( - "01060000"+ // headers, length 6, section prefix - "d9"+ // :status:200 - "540132"+ // content-length:2 - "000100" // data, 1 byte - )}, - new Object[] {"text in content-length", HexFormat.of().parseHex( - "01060000"+ // headers, length 6, section prefix - "d9"+ // :status:200 - "540161"+ // content-length:a - "000100" // data, 1 byte - )}, - new Object[] {"connection: close", HexFormat.of().parseHex( - "01150000"+ // headers, length 21, section prefix - "d9"+ // :status:200 - "2703636F6E6E656374696F6E05636C6F7365"+ // connection:close + {"connection: close", ProtocolException.class, parseHex( + "01150000", // headers, length 21, section prefix + "d9", // :status:200 + "2703636F6E6E656374696F6E05636C6F7365" + // connection:close "000100" // data, 1 byte )}, // request pseudo-headers in response - new Object[] {":method in response", HexFormat.of().parseHex( - "01040000"+ // headers, length 4, section prefix - "d9"+ // :status:200 - "d1"+ // :method:get - "000100" // data, 1 byte - )}, - new Object[] {":authority in response", HexFormat.of().parseHex( - "01100000"+ // headers, length 16, section prefix - "d9"+ // :status:200 - "508b089d5c0b8170dc702fbce7"+ // :authority - "000100" // data, 1 byte - )}, - new Object[] {":path in response", HexFormat.of().parseHex( - "010a0000"+ // headers, length 10, section prefix - "d9"+ // :status:200 - "51856272d141ff"+ // :path - "000100" // data, 1 byte - )}, - new Object[] {":scheme in response", HexFormat.of().parseHex( - "01040000"+ // headers, length 4, section prefix - "d9"+ // :status:200 - "d7"+ // :scheme:https - "000100" // data, 1 byte - )}, - new Object[] {"undefined pseudo-header", HexFormat.of().parseHex( - "01080000"+ // headers, length 8, section prefix - "d9"+ // :status:200 - "223A6D0130"+ // :m:0 - "000100" // data, 1 byte - )}, - new Object[] {"pseudo-header after regular", HexFormat.of().parseHex( - "011a0000"+ // headers, length 26, section prefix - "5f5094ca3ee35a74a6b589418b5258132b1aa496ca8747"+ //user-agent - "d9"+ // :status:200 - "000100" // data, 1 byte - )}, - new Object[] {"trailer", HexFormat.of().parseHex( + {":method in response", ProtocolException.class, parseHex( + "01040000", // headers, length 4, section prefix + "d9", // :status:200 + "d1", // :method:get + "000100" // data, 1 byte + )}, + {":authority in response", ProtocolException.class, parseHex( + "01100000", // headers, length 16, section prefix + "d9", // :status:200 + "508b089d5c0b8170dc702fbce7", // :authority + "000100" // data, 1 byte + )}, + {":path in response", ProtocolException.class, parseHex( + "010a0000", // headers, length 10, section prefix + "d9", // :status:200 + "51856272d141ff", // :path + "000100" // data, 1 byte + )}, + {":scheme in response", ProtocolException.class, parseHex( + "01040000", // headers, length 4, section prefix + "d9", // :status:200 + "d7", // :scheme:https + "000100" // data, 1 byte + )}, + {"undefined pseudo-header", ProtocolException.class, parseHex( + "01080000", // headers, length 8, section prefix + "d9", // :status:200 + "223A6D0130", // :m:0 + "000100" // data, 1 byte + )}, + {"pseudo-header after regular", ProtocolException.class, parseHex( + "011a0000", // headers, length 26, section prefix + "5f5094ca3ee35a74a6b589418b5258132b1aa496ca8747", //user-agent + "d9", // :status:200 + "000100" // data, 1 byte + )}, + {"trailer", IOException.class, parseHex( "01020000" // headers, length 2, section prefix )}, - new Object[] {"trailer+data", HexFormat.of().parseHex( - "01020000"+ // headers, length 2, section prefix - "000100" // data, 1 byte + {"trailer+data", IOException.class, parseHex( + "01020000", // headers, length 2, section prefix + "000100" // data, 1 byte )}, // valid characters include \t, 0x20-0x7e, 0x80-0xff (RFC 9110, section 5.5) - new Object[] {"invalid character in field value 00", HexFormat.of().parseHex( - "01060000"+ // headers, length 6, section prefix - "d9"+ // :status:200 - "570100"+ // etag:\0 - "000100" // data, 1 byte - )}, - new Object[] {"invalid character in field value 0a", HexFormat.of().parseHex( - "01060000"+ // headers, length 6, section prefix - "d9"+ // :status:200 - "57010a"+ // etag:\n - "000100" // data, 1 byte - )}, - new Object[] {"invalid character in field value 0d", HexFormat.of().parseHex( - "01060000"+ // headers, length 6, section prefix - "d9"+ // :status:200 - "57010d"+ // etag:\r - "000100" // data, 1 byte - )}, - new Object[] {"invalid character in field value 7f", HexFormat.of().parseHex( - "01060000"+ // headers, length 6, section prefix - "d9"+ // :status:200 - "57017f"+ // etag: 0x7f - "000100" // data, 1 byte + {"invalid character in field value 00", ProtocolException.class, parseHex( + "01060000", // headers, length 6, section prefix + "d9", // :status:200 + "570100", // etag:\0 + "000100" // data, 1 byte + )}, + {"invalid character in field value 0a", ProtocolException.class, parseHex( + "01060000", // headers, length 6, section prefix + "d9", // :status:200 + "57010a", // etag:\n + "000100" // data, 1 byte + )}, + {"invalid character in field value 0d", ProtocolException.class, parseHex( + "01060000", // headers, length 6, section prefix + "d9", // :status:200 + "57010d", // etag:\r + "000100" // data, 1 byte + )}, + {"invalid character in field value 7f", ProtocolException.class, parseHex( + "01060000", // headers, length 6, section prefix + "d9", // :status:200 + "57017f", // etag: 0x7f + "000100" // data, 1 byte )}, }; } - // These responses are malformed and should not be accepted by the client. - // They might or might not cause connection closure (H3_FRAME_UNEXPECTED) - @DataProvider - public static Object[][] malformedResponse2() { + /// Malformed responses that should not be accepted by the client. + /// They might or might not cause the connection to get closed (`H3_FRAME_UNEXPECTED`). + static Object[][] malformedResponses() { // data before headers is covered by H3ErrorHandlingTest return new Object[][]{ - new Object[] {"100+data", HexFormat.of().parseHex( - "01040000"+ // headers, length 4, section prefix - "ff00"+ // :status:100 - "000100" // data, 1 byte - )}, - new Object[] {"100+data+200", HexFormat.of().parseHex( - "01040000"+ // headers, length 4, section prefix - "ff00"+ // :status:100 - "000100"+ // data, 1 byte - "01030000"+ // headers, length 3, section prefix - "d9" // :status:200 - )}, - new Object[] {"200+data+200", HexFormat.of().parseHex( - "01030000"+ // headers, length 3, section prefix - "d9"+ // :status:200 - "000100"+ // data, 1 byte - "01030000"+ // headers, length 3, section prefix - "d9" // :status:200 - )}, - new Object[] {"200+data+100", HexFormat.of().parseHex( - "01030000"+ // headers, length 3, section prefix - "d9"+ // :status:200 - "000100"+ // data, 1 byte - "01040000"+ // headers, length 4, section prefix - "ff00" // :status:100 - )}, - new Object[] {"200+data+trailers+data", HexFormat.of().parseHex( - "01030000"+ // headers, length 3, section prefix - "d9"+ // :status:200 - "000100"+ // data, 1 byte - "01020000"+ // trailers, length 2, section prefix - "000100" // data, 1 byte - )}, - new Object[] {"200+trailers+data", HexFormat.of().parseHex( - "01030000"+ // headers, length 3, section prefix - "d9"+ // :status:200 - "01020000"+ // trailers, length 2, section prefix - "000100" // data, 1 byte - )}, - new Object[] {"200+200", HexFormat.of().parseHex( - "01030000"+ // headers, length 3, section prefix - "d9"+ // :status:200 - "01030000"+ // headers, length 3, section prefix - "d9" // :status:200 - )}, - new Object[] {"200+100", HexFormat.of().parseHex( - "01030000"+ // headers, length 3, section prefix - "d9"+ // :status:200 - "01040000"+ // headers, length 4, section prefix - "ff00" // :status:100 + {"100+data", IOException.class, parseHex( + "01040000", // headers, length 4, section prefix + "ff00", // :status:100 + "000100" // data, 1 byte + )}, + {"100+data+200", IOException.class, parseHex( + "01040000", // headers, length 4, section prefix + "ff00", // :status:100 + "000100", // data, 1 byte + "01030000", // headers, length 3, section prefix + "d9" // :status:200 + )}, + {"200+data+200", IOException.class, parseHex( + "01030000", // headers, length 3, section prefix + "d9", // :status:200 + "000100", // data, 1 byte + "01030000", // headers, length 3, section prefix + "d9" // :status:200 + )}, + {"200+data+100", IOException.class, parseHex( + "01030000", // headers, length 3, section prefix + "d9", // :status:200 + "000100", // data, 1 byte + "01040000", // headers, length 4, section prefix + "ff00" // :status:100 + )}, + {"200+data+trailers+data", ProtocolException.class, parseHex( + "01030000", // headers, length 3, section prefix + "d9", // :status:200 + "000100", // data, 1 byte + "01020000", // trailers, length 2, section prefix + "000100" // data, 1 byte + )}, + {"200+trailers+data", IOException.class, parseHex( + "01030000", // headers, length 3, section prefix + "d9", // :status:200 + "01020000", // trailers, length 2, section prefix + "000100" // data, 1 byte + )}, + {"200+200", IOException.class, parseHex( + "01030000", // headers, length 3, section prefix + "d9", // :status:200 + "01030000", // headers, length 3, section prefix + "d9" // :status:200 + )}, + {"200+100", IOException.class, parseHex( + "01030000", // headers, length 3, section prefix + "d9", // :status:200 + "01040000", // headers, length 4, section prefix + "ff00" // :status:100 )}, }; } - @DataProvider - public static Object[][] wellformedResponse() { + /// Well-formed responses that should be accepted by the client. + static Object[][] wellFormedResponses() { return new Object[][]{ - new Object[] {"100+200+data+reserved", HexFormat.of().parseHex( - "01040000"+ // headers, length 4, section prefix - "ff00"+ // :status:100 - "01030000"+ // headers, length 3, section prefix - "d9"+ // :status:200 - "000100"+ // data, 1 byte - "210100" // reserved, 1 byte - )}, - new Object[] {"200+data+reserved", HexFormat.of().parseHex( - "01030000"+ // headers, length 3, section prefix - "d9"+ // :status:200 - "000100"+ // data, 1 byte - "210100" // reserved, 1 byte - )}, - new Object[] {"200+data", HexFormat.of().parseHex( - "01030000"+ // headers, length 3, section prefix - "d9"+ // :status:200 - "000100" // data, 1 byte - )}, - new Object[] {"200+user-agent+data", HexFormat.of().parseHex( - "011a0000"+ // headers, length 26, section prefix - "d9"+ // :status:200 - "5f5094ca3ee35a74a6b589418b5258132b1aa496ca8747"+ //user-agent - "000100" // data, 1 byte - )}, - new Object[] {"200", HexFormat.of().parseHex( - "01030000"+ // headers, length 3, section prefix - "d9" // :status:200 - )}, - new Object[] {"200+data+data", HexFormat.of().parseHex( - "01030000"+ // headers, length 3, section prefix - "d9"+ // :status:200 - "000100"+ // data, 1 byte - "000100" // data, 1 byte - )}, - new Object[] {"200+data+trailers", HexFormat.of().parseHex( - "01030000"+ // headers, length 3, section prefix - "d9"+ // :status:200 - "000100"+ // data, 1 byte - "01020000" // trailers, length 2, section prefix - )}, - new Object[] {"200+trailers", HexFormat.of().parseHex( - "01030000"+ // headers, length 3, section prefix - "d9"+ // :status:200 - "01020000" // trailers, length 2, section prefix + {"100+200+data+reserved", parseHex( + "01040000", // headers, length 4, section prefix + "ff00", // :status:100 + "01030000", // headers, length 3, section prefix + "d9", // :status:200 + "000100", // data, 1 byte + "210100" // reserved, 1 byte + )}, + {"200+data+reserved", parseHex( + "01030000", // headers, length 3, section prefix + "d9", // :status:200 + "000100", // data, 1 byte + "210100" // reserved, 1 byte + )}, + {"200+data", parseHex( + "01030000", // headers, length 3, section prefix + "d9", // :status:200 + "000100" // data, 1 byte + )}, + {"200+user-agent+data", parseHex( + "011a0000", // headers, length 26, section prefix + "d9", // :status:200 + "5f5094ca3ee35a74a6b589418b5258132b1aa496ca8747", //user-agent + "000100" // data, 1 byte + )}, + {"200", parseHex( + "01030000", // headers, length 3, section prefix + "d9" // :status:200 + )}, + {"200+data+data", parseHex( + "01030000", // headers, length 3, section prefix + "d9", // :status:200 + "000100", // data, 1 byte + "000100" // data, 1 byte + )}, + {"200+data+trailers", parseHex( + "01030000", // headers, length 3, section prefix + "d9", // :status:200 + "000100", // data, 1 byte + "01020000" // trailers, length 2, section prefix + )}, + {"200+trailers", parseHex( + "01030000", // headers, length 3, section prefix + "d9", // :status:200 + "01020000" // trailers, length 2, section prefix )}, }; } - @BeforeClass - public void beforeClass() throws Exception { - sslContext = new SimpleSSLContext().get(); - if (sslContext == null) { - throw new AssertionError("Unexpected null sslContext"); - } - server = QuicStandaloneServer.newBuilder() - .availableVersions(new QuicVersion[]{QuicVersion.QUIC_V1}) - .sslContext(sslContext) - .alpn("h3") - .build(); - server.start(); - System.out.println("Server started at " + server.getAddress()); - requestURIBase = URIBuilder.newBuilder().scheme("https").loopback() - .port(server.getAddress().getPort()).build().toString(); - } - - @AfterClass - public void afterClass() throws Exception { - if (server != null) { - System.out.println("Stopping server " + server.getAddress()); - server.close(); + private static byte[] parseHex(String... strings) { + var buffer = new StringBuilder(); + for (String string : strings) { + buffer.append(string); } + return HexFormat.of().parseHex(buffer.toString()); } - /** - * Server sends a well-formed response - */ - @Test(dataProvider = "wellformedResponse") - public void testWellFormedResponse(String desc, byte[] response) throws Exception { - CompletableFuture errorCF = new CompletableFuture<>(); - server.addHandler((c,s)-> { - try (OutputStream outputStream = s.outputStream()) { - outputStream.write(response); - } - // verify that the connection stays open - completeUponTermination(c, errorCF); - }); - HttpClient client = getHttpClient(); - try { - HttpRequest request = getRequest(); - final HttpResponse response1 = client.send( - request, - BodyHandlers.discarding()); - assertEquals(response1.statusCode(), 200); - assertFalse(errorCF.isDone(), "Expected the connection to be open"); - } finally { - client.shutdownNow(); + @ParameterizedTest + @MethodSource("wellFormedResponses") + void testWellFormedResponse(String desc, byte[] serverResponseBytes) throws Exception { + var terminated = configureServerResponse(serverResponseBytes); + try (var client = createClient()) { + final HttpResponse response = client.send(REQUEST, BodyHandlers.discarding()); + assertEquals(200, response.statusCode()); + assertFalse(terminated.getAsBoolean(), "Expected the connection to be open"); } } - - /** - * Server sends a malformed response that should not close connection - */ - @Test(dataProvider = "malformedResponse") - public void testMalformedResponse(String desc, byte[] response) throws Exception { - CompletableFuture errorCF = new CompletableFuture<>(); - server.addHandler((c,s)-> { - try (OutputStream outputStream = s.outputStream()) { - outputStream.write(response); - } - // verify that the connection stays open - completeUponTermination(c, errorCF); - }); - HttpClient client = getHttpClient(); - try { - HttpRequest request = getRequest(); - final HttpResponse response1 = client.send( - request, - BodyHandlers.discarding()); - fail("Expected the request to fail, got " + response1); - } catch (Exception e) { - System.out.println("Got expected exception: " +e); - e.printStackTrace(); - assertFalse(errorCF.isDone(), "Expected the connection to be open"); - } finally { - client.shutdownNow(); + @ParameterizedTest + @MethodSource("malformedResponsesInvalidatingConnection") + void testMalformedResponseInvalidatingConnection( + String desc, + Class exceptionClass, + byte[] serverResponseBytes) { + var terminated = configureServerResponse(serverResponseBytes); + try (var client = createClient()) { + var exception = assertThrows(exceptionClass, () -> client.send(REQUEST, BodyHandlers.discarding())); + LOGGER.log("Got expected exception for: " + desc, exception); + assertFalse(terminated.getAsBoolean(), "Expected the connection to be open"); } } - /** - * Server sends a malformed response that might close connection - */ - @Test(dataProvider = "malformedResponse2") - public void testMalformedResponse2(String desc, byte[] response) throws Exception { - server.addHandler((c,s)-> { - try (OutputStream outputStream = s.outputStream()) { - outputStream.write(response); - } - }); - HttpClient client = getHttpClient(); - try { - HttpRequest request = getRequest(); - final HttpResponse response1 = client.send( - request, - BodyHandlers.discarding()); - fail("Expected the request to fail, got " + response1); - } catch (Exception e) { - System.out.println("Got expected exception: " +e); - e.printStackTrace(); - } finally { - client.shutdownNow(); + @ParameterizedTest + @MethodSource("malformedResponses") + void testMalformedResponse( + String desc, + Class exceptionClass, + byte[] serverResponseBytes) { + configureServerResponse(serverResponseBytes); + try (var client = createClient()) { + var exception = assertThrows(exceptionClass, () -> client.send(REQUEST, BodyHandlers.discarding())); + LOGGER.log("Got expected exception for: " + desc, exception); } } - private HttpRequest getRequest() throws URISyntaxException { - final URI reqURI = new URI(requestURIBase + "/hello"); - final HttpRequest.Builder reqBuilder = HttpRequest.newBuilder(reqURI) - .version(Version.HTTP_3) - .setOption(H3_DISCOVERY, HTTP_3_URI_ONLY); - return reqBuilder.build(); - } - - private HttpClient getHttpClient() { - final HttpClient client = newClientBuilderForH3() - .proxy(HttpClient.Builder.NO_PROXY) + private static HttpClient createClient() { + return HttpServerAdapters.createClientBuilderForH3() + .proxy(NO_PROXY) .version(Version.HTTP_3) - .sslContext(sslContext).build(); - return client; + .sslContext(SSL_CONTEXT) + .build(); } - private static String toHexString(final Http3Error error) { - return error.name() + "(0x" + Long.toHexString(error.code()) + ")"; + private static BooleanSupplier configureServerResponse(byte[] serverResponseBytes) { + var terminated = new AtomicBoolean(); + SERVER.addHandler((c, s)-> { + try (OutputStream outputStream = s.outputStream()) { + outputStream.write(serverResponseBytes); + } + c.futureTerminationCause().handle((_, _) -> { + terminated.set(true); + return true; + }); + }); + return terminated::get; } - private static void completeUponTermination(final QuicServerConnection serverConnection, - final CompletableFuture cf) { - serverConnection.futureTerminationCause().handle( - (r,t) -> t != null ? cf.completeExceptionally(t) : cf.complete(r)); - } }