Improve handling of HttpInput.Interceptor behavior

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
jetty · Apr 19, 2021 · 25467f8 · 25467f8
1 parent 4c98990
commit 25467f8
Show file tree

Hide file tree

Showing 3 changed files with 381 additions and 11 deletions.
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/AsyncContentProducer.java b/jetty-server/src/main/java/org/eclipse/jetty/server/AsyncContentProducer.java
@@ -13,6 +13,7 @@
 
 package org.eclipse.jetty.server;
 
+import java.io.IOException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.Condition;
 
@@ -302,12 +303,16 @@ private HttpInput.Content nextTransformedContent()
 
                 // In case the _rawContent was set by consumeAll(), check the httpChannel
                 // to see if it has a more precise error. Otherwise, the exact same
-                // special content will be returned by the httpChannel.
-                HttpInput.Content refreshedRawContent = produceRawContent();
-                if (refreshedRawContent != null)
-                    _rawContent = refreshedRawContent;
+                // special content will be returned by the httpChannel; do not do that
+                // if the _error flag was set, meaning the current error is definitive.
+                if (!_error)
+                {
+                    HttpInput.Content refreshedRawContent = produceRawContent();
+                    if (refreshedRawContent != null)
+                        _rawContent = refreshedRawContent;
+                    _error = _rawContent.getError() != null;
+                }
 
-                _error = _rawContent.getError() != null;
                 if (LOG.isDebugEnabled())
                     LOG.debug("raw content is special (with error = {}), returning it {}", _error, this);
                 return _rawContent;
@@ -317,7 +322,9 @@ private HttpInput.Content nextTransformedContent()
             {
                 if (LOG.isDebugEnabled())
                     LOG.debug("using interceptor to transform raw content {}", this);
-                _transformedContent = _interceptor.readFrom(_rawContent);
+                _transformedContent = intercept();
+                if (_error)
+                    return _rawContent;
             }
             else
             {
@@ -369,6 +376,26 @@ private HttpInput.Content nextTransformedContent()
         return _transformedContent;
     }
 
+    private HttpInput.Content intercept()
+    {
+        try
+        {
+            return _interceptor.readFrom(_rawContent);
+        }
+        catch (Throwable x)
+        {
+            IOException failure = new IOException("Bad content", x);
+            failCurrentContent(failure);
+            // Set the _error flag to mark the error as definitive, i.e.:
+            // do not try to produce new raw content to get a fresher error.
+            _error = true;
+            Response response = _httpChannel.getResponse();
+            if (response.isCommitted())
+                _httpChannel.abort(failure);
+            return null;
+        }
+    }
+
     private HttpInput.Content produceRawContent()
     {
         HttpInput.Content content = _httpChannel.produceContent();

diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java
@@ -298,6 +298,14 @@ else if (filled < 0)
                 }
             }
         }
+        catch (Throwable x)
+        {
+            if (LOG.isDebugEnabled())
+                LOG.debug("{} caught exception {}", this, _channel.getState(), x);
+            BufferUtil.clear(_requestBuffer);
+            releaseRequestBuffer();
+            getEndPoint().close(x);
+        }
         finally
         {
             setCurrentConnection(last);
@@ -331,10 +339,7 @@ void parseAndFillForContent()
     private int fillRequestBuffer()
     {
         if (_contentBufferReferences.get() > 0)
-        {
-            LOG.warn("{} fill with unconsumed content!", this);
-            return 0;
-        }
+            throw new IllegalStateException("fill with unconsumed content on " + this);
 
         if (BufferUtil.isEmpty(_requestBuffer))
         {
@@ -362,7 +367,8 @@ else if (filled < 0)
             }
             catch (IOException e)
             {
-                LOG.debug("Unable to fill from endpoint {}", getEndPoint(), e);
+                if (LOG.isDebugEnabled())
+                    LOG.debug("Unable to fill from endpoint {}", getEndPoint(), e);
                 _parser.atEOF();
                 return -1;
             }