Implemented server-side reset of the stream using error code NO_ERROR…

… in case the request content is not read. Signed-off-by: Simone Bordet <[email protected]>
jetty · Dec 15, 2023 · c5e40f5 · c5e40f5
1 parent de5dffe
commit c5e40f5
Show file tree

Hide file tree

Showing 7 changed files with 19 additions and 15 deletions.
diff --git a/...re/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java b/...re/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java
@@ -335,11 +335,12 @@ public void onPriority(PriorityFrame frame)
     @Override
     public void onReset(ResetFrame frame)
     {
-        if (LOG.isDebugEnabled())
-            LOG.debug("Received {} on {}", frame, this);
-
         int streamId = frame.getStreamId();
         HTTP2Stream stream = getStream(streamId);
+
+        if (LOG.isDebugEnabled())
+            LOG.debug("Received {} for {} on {}", frame, stream, this);
+
         if (stream != null)
         {
             stream.process(frame, new OnResetCallback());

diff --git a/...ore/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java b/...ore/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java
@@ -766,6 +766,8 @@ public int updateRecvWindow(int delta)
     @Override
     public void close()
     {
+        if (LOG.isDebugEnabled())
+            LOG.debug("Close for {}", this);
         CloseState oldState = closeState.getAndSet(CloseState.CLOSED);
         if (oldState != CloseState.CLOSED)
         {

diff --git a/...tp2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HttpStreamOverHTTP2.java b/...tp2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HttpStreamOverHTTP2.java
@@ -609,7 +609,7 @@ public void succeeded()
                 // Send a reset to the other end so that it stops sending data.
                 if (LOG.isDebugEnabled())
                     LOG.debug("HTTP2 response #{}/{}: unconsumed request content, resetting stream", _stream.getId(), Integer.toHexString(_stream.getSession().hashCode()));
-                _stream.reset(new ResetFrame(_stream.getId(), ErrorCode.CANCEL_STREAM_ERROR.code), Callback.NOOP);
+                _stream.reset(new ResetFrame(_stream.getId(), ErrorCode.NO_ERROR.code), Callback.NOOP);
             }
         }
         _httpChannel.recycle();
@@ -619,9 +619,10 @@ public void succeeded()
     @Override
     public void failed(Throwable x)
     {
+        ErrorCode errorCode = x == HttpStream.CONTENT_NOT_CONSUMED ? ErrorCode.NO_ERROR : ErrorCode.CANCEL_STREAM_ERROR;
         if (LOG.isDebugEnabled())
-            LOG.debug("HTTP2 response #{}/{} aborted", _stream.getId(), Integer.toHexString(_stream.getSession().hashCode()));
-        _stream.reset(new ResetFrame(_stream.getId(), ErrorCode.CANCEL_STREAM_ERROR.code), Callback.NOOP);
+            LOG.debug("HTTP2 response #{}/{} failed {}", _stream.getId(), Integer.toHexString(_stream.getSession().hashCode()), errorCode, x);
+        _stream.reset(new ResetFrame(_stream.getId(), errorCode.code), Callback.NOOP);
     }
 
     private class SendTrailers extends Callback.Nested

diff --git a/...tp3-server/src/main/java/org/eclipse/jetty/http3/server/internal/HttpStreamOverHTTP3.java b/...tp3-server/src/main/java/org/eclipse/jetty/http3/server/internal/HttpStreamOverHTTP3.java
@@ -13,7 +13,6 @@
 
 package org.eclipse.jetty.http3.server.internal;
 
-import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.TimeoutException;
@@ -494,16 +493,17 @@ public void succeeded()
         {
             if (LOG.isDebugEnabled())
                 LOG.debug("HTTP3 Response #{}/{}: unconsumed request content, resetting stream", stream.getId(), Integer.toHexString(stream.getSession().hashCode()));
-            stream.reset(HTTP3ErrorCode.REQUEST_CANCELLED_ERROR.code(), new IOException("unconsumed content"));
+            stream.reset(HTTP3ErrorCode.NO_ERROR.code(), CONTENT_NOT_CONSUMED);
         }
     }
 
     @Override
     public void failed(Throwable x)
     {
+        HTTP3ErrorCode errorCode = x == HttpStream.CONTENT_NOT_CONSUMED ? HTTP3ErrorCode.NO_ERROR : HTTP3ErrorCode.REQUEST_CANCELLED_ERROR;
         if (LOG.isDebugEnabled())
-            LOG.debug("HTTP3 Response #{}/{} aborted", stream.getId(), Integer.toHexString(stream.getSession().hashCode()));
-        stream.reset(HTTP3ErrorCode.REQUEST_CANCELLED_ERROR.code(), x);
+            LOG.debug("HTTP3 Response #{}/{} failed {}", stream.getId(), Integer.toHexString(stream.getSession().hashCode()), errorCode, x);
+        stream.reset(errorCode.code(), x);
     }
 
     public void onIdleTimeout(TimeoutException failure, BiConsumer<Runnable, Boolean> consumer)

diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpStream.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpStream.java
@@ -31,7 +31,7 @@
  */
 public interface HttpStream extends Callback
 {
-    Exception CONTENT_NOT_CONSUMED = new StaticException("Content not consumed");
+    Exception CONTENT_NOT_CONSUMED = new StaticException("Unconsumed request content");
 
     /**
      * <p>Attribute name to be used as a {@link Request} attribute to store/retrieve
@@ -119,7 +119,7 @@ static Throwable consumeAvailable(HttpStream stream, HttpConfiguration httpConfi
 
             // if we cannot read to EOF then fail the stream rather than wait for unconsumed content
             if (content == null)
-                return CONTENT_NOT_CONSUMED;
+                break;
 
             // Always release any returned content. This is a noop for EOF and Error content.
             content.release();

diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpChannelTest.java
@@ -793,7 +793,7 @@ public boolean handle(Request request, Response response, Callback callback)
 
         assertThat(stream.isComplete(), is(true));
         assertThat(stream.getFailure(), notNullValue());
-        assertThat(stream.getFailure().getMessage(), containsString("Content not consumed"));
+        assertThat(stream.getFailure().getMessage(), containsString("Unconsumed request content"));
         assertThat(stream.getResponse(), notNullValue());
         assertThat(stream.getResponse().getStatus(), equalTo(200));
         assertThat(stream.getResponseHeaders().get(HttpHeader.CONTENT_TYPE), equalTo(MimeTypes.Type.TEXT_PLAIN_UTF_8.asString()));

diff --git a/...ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/AsyncContentProducer.java b/...ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/AsyncContentProducer.java
@@ -19,6 +19,7 @@
 
 import org.eclipse.jetty.http.BadMessageException;
 import org.eclipse.jetty.http.HttpStatus;
+import org.eclipse.jetty.server.HttpStream;
 import org.eclipse.jetty.util.NanoTime;
 import org.eclipse.jetty.util.StaticException;
 import org.eclipse.jetty.util.component.Destroyable;
@@ -34,7 +35,6 @@ class AsyncContentProducer implements ContentProducer
 {
     private static final Logger LOG = LoggerFactory.getLogger(AsyncContentProducer.class);
     private static final HttpInput.ErrorContent RECYCLED_ERROR_CONTENT = new HttpInput.ErrorContent(new StaticException("ContentProducer has been recycled"));
-    private static final Throwable UNCONSUMED_CONTENT_EXCEPTION = new StaticException("Unconsumed content");
 
     private final AutoLock _lock = new AutoLock();
     private final HttpChannel _httpChannel;
@@ -182,7 +182,7 @@ public long getRawContentArrived()
     public boolean consumeAll()
     {
         assertLocked();
-        Throwable x = UNCONSUMED_CONTENT_EXCEPTION;
+        Throwable x = HttpStream.CONTENT_NOT_CONSUMED;
         if (LOG.isTraceEnabled())
         {
             x = new StaticException("Unconsumed content", true);