From 7ac49cd43c58778a7f8eeaea50966cacb4ec1184 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Mon, 15 May 2023 09:50:36 +0200 Subject: [PATCH] H3: Fix racy read from stream-less channel (#9761) * #9655 introduce new Stream.Client.Listener.onNewStream() method to allow setting the channel's stream before sending any data to the network Signed-off-by: Ludovic Orban --- .../client/internal/HTTP3SessionClient.java | 1 + .../client/internal/HTTP3StreamClient.java | 19 +++++++++++++++++++ .../org/eclipse/jetty/http3/api/Stream.java | 10 ++++++++++ .../http/internal/HttpReceiverOverHTTP3.java | 17 ++++++++++++++++- .../http/internal/HttpSenderOverHTTP3.java | 1 - .../test/resources/jetty-logging.properties | 1 + 6 files changed, 47 insertions(+), 2 deletions(-) diff --git a/jetty-http3/http3-client/src/main/java/org/eclipse/jetty/http3/client/internal/HTTP3SessionClient.java b/jetty-http3/http3-client/src/main/java/org/eclipse/jetty/http3/client/internal/HTTP3SessionClient.java index 54454b429f6..f701e2dde20 100644 --- a/jetty-http3/http3-client/src/main/java/org/eclipse/jetty/http3/client/internal/HTTP3SessionClient.java +++ b/jetty-http3/http3-client/src/main/java/org/eclipse/jetty/http3/client/internal/HTTP3SessionClient.java @@ -106,6 +106,7 @@ public class HTTP3SessionClient extends HTTP3Session implements Session.Client return promise; stream.setListener(listener); + stream.onOpen(); stream.writeFrame(frame) .whenComplete((r, x) -> diff --git a/jetty-http3/http3-client/src/main/java/org/eclipse/jetty/http3/client/internal/HTTP3StreamClient.java b/jetty-http3/http3-client/src/main/java/org/eclipse/jetty/http3/client/internal/HTTP3StreamClient.java index fa6fe973cc0..d48b7698878 100644 --- a/jetty-http3/http3-client/src/main/java/org/eclipse/jetty/http3/client/internal/HTTP3StreamClient.java +++ b/jetty-http3/http3-client/src/main/java/org/eclipse/jetty/http3/client/internal/HTTP3StreamClient.java @@ -42,6 +42,11 @@ public class HTTP3StreamClient extends HTTP3Stream implements Stream.Client return listener; } + public void onOpen() + { + notifyNewStream(); + } + public void setListener(Stream.Client.Listener listener) { this.listener = listener; @@ -65,6 +70,20 @@ public class HTTP3StreamClient extends HTTP3Stream implements Stream.Client } } + private void notifyNewStream() + { + Stream.Client.Listener listener = getListener(); + try + { + if (listener != null) + listener.onNewStream(this); + } + catch (Throwable x) + { + LOG.info("Failure while notifying listener {}", listener, x); + } + } + private void notifyResponse(HeadersFrame frame) { Stream.Client.Listener listener = getListener(); diff --git a/jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/api/Stream.java b/jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/api/Stream.java index c4f0e178d53..7f99667812d 100644 --- a/jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/api/Stream.java +++ b/jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/api/Stream.java @@ -138,6 +138,16 @@ public interface Stream */ public interface Listener { + /** + *

Callback method invoked when a stream is created locally by + * {@link Session.Client#newRequest(HeadersFrame, Listener)}.

+ * + * @param stream the newly created stream + */ + public default void onNewStream(Stream.Client stream) + { + } + /** *

Callback method invoked when a response is received.

*

To read response content, applications should call diff --git a/jetty-http3/http3-http-client-transport/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpReceiverOverHTTP3.java b/jetty-http3/http3-http-client-transport/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpReceiverOverHTTP3.java index 841f8c59393..4c726342eb4 100644 --- a/jetty-http3/http3-http-client-transport/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpReceiverOverHTTP3.java +++ b/jetty-http3/http3-http-client-transport/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpReceiverOverHTTP3.java @@ -59,9 +59,24 @@ public class HttpReceiverOverHTTP3 extends HttpReceiver implements Stream.Client return; if (notifySuccess) + { responseSuccess(exchange); + } else - getHttpChannel().getStream().demand(); + { + Stream stream = getHttpChannel().getStream(); + if (LOG.isDebugEnabled()) + LOG.debug("Demanding from {} in {}", stream, this); + if (stream == null) + return; + stream.demand(); + } + } + + @Override + public void onNewStream(Stream.Client stream) + { + getHttpChannel().setStream(stream); } @Override diff --git a/jetty-http3/http3-http-client-transport/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpSenderOverHTTP3.java b/jetty-http3/http3-http-client-transport/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpSenderOverHTTP3.java index 0612c46eec8..f5fa7dd1e23 100644 --- a/jetty-http3/http3-http-client-transport/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpSenderOverHTTP3.java +++ b/jetty-http3/http3-http-client-transport/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpSenderOverHTTP3.java @@ -138,7 +138,6 @@ public class HttpSenderOverHTTP3 extends HttpSender private Stream onNewStream(Stream stream, HttpRequest request) { - getHttpChannel().setStream(stream); long idleTimeout = request.getIdleTimeout(); if (idleTimeout > 0) ((HTTP3Stream)stream).setIdleTimeout(idleTimeout); diff --git a/tests/test-http-client-transport/src/test/resources/jetty-logging.properties b/tests/test-http-client-transport/src/test/resources/jetty-logging.properties index 7ba9dbc3b1f..4fdadbafe69 100644 --- a/tests/test-http-client-transport/src/test/resources/jetty-logging.properties +++ b/tests/test-http-client-transport/src/test/resources/jetty-logging.properties @@ -7,6 +7,7 @@ org.eclipse.jetty.jmx.LEVEL=INFO org.eclipse.jetty.http2.hpack.LEVEL=INFO #org.eclipse.jetty.http2.client.LEVEL=DEBUG #org.eclipse.jetty.http3.LEVEL=DEBUG +#org.eclipse.jetty.http3.client.LEVEL=DEBUG org.eclipse.jetty.http3.qpack.LEVEL=INFO #org.eclipse.jetty.quic.LEVEL=DEBUG org.eclipse.jetty.quic.quiche.LEVEL=INFO