You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2021/03/15 19:47:44 UTC

[tomcat] branch 8.5.x updated (4f105d7 -> 14adbe2)

This is an automated email from the ASF dual-hosted git repository.

markt pushed a change to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git.


    from 4f105d7  Make fields final after new approach to recycling
     new 26a5bfc  Rename Output.swallowedPadding to onSwallowedDataFramePayload
     new 8f40712  Rename swallow() -> swallowPayload()
     new 6a52ef0  Add frameTypeId parameter to swallowedPayload()
     new adeb053  Expose the actual frameTypeId for unknown frames
     new f32ecf4  Fix BZ 65179 - avoid flow control window exhaustion
     new 8a7407e  Fix failing test after flow control updates
     new 554bbd9  Fix failing test
     new 14adbe2  Fix Javadoc errors

The 8 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../apache/coyote/http2/AbstractNonZeroStream.java |  16 +++
 java/org/apache/coyote/http2/FrameType.java        |   5 +
 java/org/apache/coyote/http2/Http2Parser.java      | 136 +++++++++++++++------
 .../apache/coyote/http2/Http2UpgradeHandler.java   |  24 ++--
 java/org/apache/coyote/http2/RecycledStream.java   |  30 ++++-
 java/org/apache/coyote/http2/Stream.java           |  42 +++++--
 test/org/apache/coyote/http2/Http2TestBase.java    |  13 +-
 .../apache/coyote/http2/TestCancelledUpload.java   |  73 ++++-------
 test/org/apache/coyote/http2/TestFlowControl.java  | 131 ++++++++++++++++++++
 webapps/docs/changelog.xml                         |   6 +
 10 files changed, 355 insertions(+), 121 deletions(-)
 create mode 100644 test/org/apache/coyote/http2/TestFlowControl.java


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 04/08: Expose the actual frameTypeId for unknown frames

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit adeb0533b3f67d172df73692344b51fe69fb6a16
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Mar 15 18:57:37 2021 +0000

    Expose the actual frameTypeId for unknown frames
---
 java/org/apache/coyote/http2/Http2Parser.java      | 24 ++++++++++++++++------
 .../apache/coyote/http2/Http2UpgradeHandler.java   |  2 +-
 test/org/apache/coyote/http2/Http2TestBase.java    |  4 ++--
 3 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/java/org/apache/coyote/http2/Http2Parser.java b/java/org/apache/coyote/http2/Http2Parser.java
index 42647c8..1cf6f77 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -122,7 +122,7 @@ class Http2Parser {
             readContinuationFrame(streamId, flags, payloadSize);
             break;
         case UNKNOWN:
-            readUnknownFrame(streamId, frameType, flags, payloadSize);
+            readUnknownFrame(streamId, frameTypeId, flags, payloadSize);
         }
 
         return true;
@@ -459,15 +459,15 @@ class Http2Parser {
     }
 
 
-    private void readUnknownFrame(int streamId, FrameType frameType, int flags, int payloadSize)
+    protected void readUnknownFrame(int streamId, int frameTypeId, int flags, int payloadSize)
             throws IOException {
         try {
-            swallowPayload(streamId, frameType.getId(), payloadSize, false);
+            swallowPayload(streamId, frameTypeId, payloadSize, false);
         } catch (ConnectionException e) {
             // Will never happen because swallow() is called with mustBeZero set
             // to false
         }
-        output.swallowed(streamId, frameType, flags, payloadSize);
+        output.onSwallowedUnknownFrame(streamId, frameTypeId, flags, payloadSize);
     }
 
 
@@ -697,7 +697,19 @@ class Http2Parser {
         // Window size
         void incrementWindowSize(int streamId, int increment) throws Http2Exception;
 
-        // Testing
-        void swallowed(int streamId, FrameType frameType, int flags, int size) throws IOException;
+        /**
+         * Notification triggered when the parser swallows the payload of an
+         * unknown frame.
+         *
+         * @param streamId      The stream on which the swallowed frame was
+         *                      received
+         * @param frameTypeId   The (unrecognised) type of swallowed frame
+         * @param flags         The flags set in the header of the swallowed
+         *                      frame
+         * @param size          The payload size of the swallowed frame
+         *
+         * @throws IOException
+         */
+        void onSwallowedUnknownFrame(int streamId, int frameTypeId, int flags, int size) throws IOException;
     }
 }
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 02e6b9d..3c82718 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -1875,7 +1875,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
 
 
     @Override
-    public void swallowed(int streamId, FrameType frameType, int flags, int size)
+    public void onSwallowedUnknownFrame(int streamId, int frameTypeId, int flags, int size)
             throws IOException {
         // NO-OP.
     }
diff --git a/test/org/apache/coyote/http2/Http2TestBase.java b/test/org/apache/coyote/http2/Http2TestBase.java
index bb3f145..f459a40 100644
--- a/test/org/apache/coyote/http2/Http2TestBase.java
+++ b/test/org/apache/coyote/http2/Http2TestBase.java
@@ -1184,10 +1184,10 @@ public abstract class Http2TestBase extends TomcatBaseTest {
 
 
         @Override
-        public void swallowed(int streamId, FrameType frameType, int flags, int size) {
+        public void onSwallowedUnknownFrame(int streamId, int frameTypeId, int flags, int size) {
             trace.append(streamId);
             trace.append(",");
-            trace.append(frameType);
+            trace.append(frameTypeId);
             trace.append(",");
             trace.append(flags);
             trace.append(",");


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 05/08: Fix BZ 65179 - avoid flow control window exhaustion

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit f32ecf47dde6a452559065ad999db7a75eb7ecd4
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Mar 15 17:57:41 2021 +0000

    Fix BZ 65179 - avoid flow control window exhaustion
    
    Ensure that the connection level flow control window from the client to
    the server is updated when handling DATA frames received for completed
    streams else the flow control window  may become exhausted
    
    https://bz.apache.org/bugzilla/show_bug.cgi?id=65179
---
 .../apache/coyote/http2/AbstractNonZeroStream.java |  16 +++
 java/org/apache/coyote/http2/Http2Parser.java      |  66 +++++++----
 .../apache/coyote/http2/Http2UpgradeHandler.java   |  22 ++--
 java/org/apache/coyote/http2/RecycledStream.java   |  30 ++++-
 java/org/apache/coyote/http2/Stream.java           |  42 +++++--
 test/org/apache/coyote/http2/Http2TestBase.java    |   7 +-
 test/org/apache/coyote/http2/TestFlowControl.java  | 131 +++++++++++++++++++++
 webapps/docs/changelog.xml                         |   6 +
 8 files changed, 264 insertions(+), 56 deletions(-)

diff --git a/java/org/apache/coyote/http2/AbstractNonZeroStream.java b/java/org/apache/coyote/http2/AbstractNonZeroStream.java
index 38761ad..0368c4f 100644
--- a/java/org/apache/coyote/http2/AbstractNonZeroStream.java
+++ b/java/org/apache/coyote/http2/AbstractNonZeroStream.java
@@ -16,6 +16,7 @@
  */
 package org.apache.coyote.http2;
 
+import java.nio.ByteBuffer;
 import java.util.Iterator;
 
 import org.apache.juli.logging.Log;
@@ -31,6 +32,8 @@ abstract class AbstractNonZeroStream extends AbstractStream {
     private static final Log log = LogFactory.getLog(AbstractNonZeroStream.class);
     private static final StringManager sm = StringManager.getManager(AbstractNonZeroStream.class);
 
+    protected static final ByteBuffer ZERO_LENGTH_BYTEBUFFER = ByteBuffer.allocate(0);
+
     protected final StreamStateMachine state;
 
     private volatile int weight = Constants.DEFAULT_WEIGHT;
@@ -140,4 +143,17 @@ abstract class AbstractNonZeroStream extends AbstractStream {
     final void checkState(FrameType frameType) throws Http2Exception {
         state.checkFrameType(frameType);
     }
+
+
+    /**
+     * Obtain the ByteBuffer to store DATA frame payload data for this stream
+     * that has been received from the client.
+     *
+     * @return {@code null} if the DATA frame payload can be swallowed, or a
+     *         ByteBuffer with at least enough space remaining for the current
+     *         flow control window for stream data from the client.
+     */
+    abstract ByteBuffer getInputByteBuffer();
+
+    abstract void receivedData(int payloadSize) throws Http2Exception;
 }
diff --git a/java/org/apache/coyote/http2/Http2Parser.java b/java/org/apache/coyote/http2/Http2Parser.java
index 1cf6f77..3f0e1b8 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -170,7 +170,7 @@ class Http2Parser {
             swallowPayload(streamId, FrameType.DATA.getId(), dataLength, false);
             // Process padding before sending any notifications in case padding
             // is invalid.
-            if (padLength > 0) {
+            if (Flags.hasPadding(flags)) {
                 swallowPayload(streamId, FrameType.DATA.getId(), padLength, true);
             }
             if (endOfStream) {
@@ -178,16 +178,19 @@ class Http2Parser {
             }
         } else {
             synchronized (dest) {
-                if (dest.remaining() < dataLength) {
-                    swallowPayload(streamId, FrameType.DATA.getId(), dataLength, false);
+                if (dest.remaining() < payloadSize) {
                     // Client has sent more data than permitted by Window size
+                    swallowPayload(streamId, FrameType.DATA.getId(), dataLength, false);
+                    if (Flags.hasPadding(flags)) {
+                        swallowPayload(streamId, FrameType.DATA.getId(), padLength, true);
+                    }
                     throw new StreamException(sm.getString("http2Parser.processFrameData.window", connectionId),
                             Http2Error.FLOW_CONTROL_ERROR, streamId);
                 }
                 input.fill(true, dest, dataLength);
                 // Process padding before sending any notifications in case
                 // padding is invalid.
-                if (padLength > 0) {
+                if (Flags.hasPadding(flags)) {
                     swallowPayload(streamId, FrameType.DATA.getId(), padLength, true);
                 }
                 if (endOfStream) {
@@ -196,9 +199,6 @@ class Http2Parser {
                 output.endRequestBodyFrame(streamId);
             }
         }
-        if (Flags.hasPadding(flags)) {
-            output.onSwallowedDataFramePayload(streamId, padLength);
-        }
     }
 
 
@@ -464,10 +464,11 @@ class Http2Parser {
         try {
             swallowPayload(streamId, frameTypeId, payloadSize, false);
         } catch (ConnectionException e) {
-            // Will never happen because swallow() is called with mustBeZero set
+            // Will never happen because swallowPayload() is called with isPadding set
             // to false
+        } finally {
+            output.onSwallowedUnknownFrame(streamId, frameTypeId, flags, payloadSize);
         }
-        output.onSwallowedUnknownFrame(streamId, frameTypeId, flags, payloadSize);
     }
 
 
@@ -491,24 +492,39 @@ class Http2Parser {
             log.debug(sm.getString("http2Parser.swallow.debug", connectionId,
                     Integer.toString(streamId), Integer.toString(len)));
         }
-        if (len == 0) {
-            return;
-        }
         int read = 0;
-        byte[] buffer = new byte[1024];
-        while (read < len) {
-            int thisTime = Math.min(buffer.length, len - read);
-            input.fill(true, buffer, 0, thisTime);
-            if (isPadding) {
-                // Validate the padding is zero since receiving non-zero padding
-                // is a strong indication of either a faulty client or a server
-                // side bug.
-                for (int i = 0; i < thisTime; i++) {
-                    if (buffer[i] != 0) {
-                        throw new ConnectionException(sm.getString("http2Parser.nonZeroPadding",
-                                connectionId, Integer.toString(streamId)), Http2Error.PROTOCOL_ERROR);
+        int thisTime = 0;
+        try {
+            if (len == 0) {
+                return;
+            }
+            byte[] buffer = new byte[1024];
+            while (read < len) {
+                thisTime = Math.min(buffer.length, len - read);
+                input.fill(true, buffer, 0, thisTime);
+                if (isPadding) {
+                    // Validate the padding is zero since receiving non-zero padding
+                    // is a strong indication of either a faulty client or a server
+                    // side bug.
+                    for (int i = 0; i < thisTime; i++) {
+                        if (buffer[i] != 0) {
+                            throw new ConnectionException(sm.getString("http2Parser.nonZeroPadding",
+                                    connectionId, Integer.toString(streamId)), Http2Error.PROTOCOL_ERROR);
+                        }
                     }
                 }
+                read += thisTime;
+            }
+        } finally {
+            if (FrameType.DATA.getIdByte() == frameTypeId) {
+                if (isPadding) {
+                    // Need to add 1 for the padding length bytes that was also
+                    // part of the payload.
+                    len += 1;
+                }
+                if (len > 0) {
+                    output.onSwallowedDataFramePayload(streamId, len);
+                }
             }
             read += thisTime;
         }
@@ -654,7 +670,7 @@ class Http2Parser {
 
         // Data frames
         ByteBuffer startRequestBodyFrame(int streamId, int payloadSize, boolean endOfStream) throws Http2Exception;
-        void endRequestBodyFrame(int streamId) throws Http2Exception;
+        void endRequestBodyFrame(int streamId) throws Http2Exception, IOException;
         void receivedEndOfStream(int streamId) throws ConnectionException;
         /**
          * Notification triggered when the parser swallows some or all of a DATA
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 3c82718..68fe75e 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -1574,20 +1574,14 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
         }
 
         AbstractNonZeroStream abstractNonZeroStream = getAbstractNonZeroStream(streamId, true);
-        if (abstractNonZeroStream instanceof Stream) {
-            Stream stream = (Stream) abstractNonZeroStream;
-            stream.checkState(FrameType.DATA);
-            stream.receivedData(payloadSize);
-            return stream.getInputByteBuffer();
-        } else {
-            abstractNonZeroStream.checkState(FrameType.DATA);
-            return null;
-        }
+        abstractNonZeroStream.checkState(FrameType.DATA);
+        abstractNonZeroStream.receivedData(payloadSize);
+        return abstractNonZeroStream.getInputByteBuffer();
     }
 
 
     @Override
-    public void endRequestBodyFrame(int streamId) throws Http2Exception {
+    public void endRequestBodyFrame(int streamId) throws Http2Exception, IOException {
         AbstractNonZeroStream abstractNonZeroStream = getAbstractNonZeroStream(streamId, true);
         if (abstractNonZeroStream instanceof Stream) {
             ((Stream) abstractNonZeroStream).getInputBuffer().onDataAvailable();
@@ -1610,11 +1604,9 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
 
 
     @Override
-    public void onSwallowedDataFramePayload(int streamId, int swallowedDataBytesCount) throws
-            ConnectionException, IOException {
-        AbstractNonZeroStream abstractNonZeroStream = getAbstractNonZeroStream(streamId, true);
-        // +1 is for the payload byte used to define the padding length
-        writeWindowUpdate(abstractNonZeroStream, swallowedDataBytesCount + 1, false);
+    public void onSwallowedDataFramePayload(int streamId, int swallowedDataBytesCount) throws IOException {
+        AbstractNonZeroStream abstractNonZeroStream = getAbstractNonZeroStream(streamId);
+        writeWindowUpdate(abstractNonZeroStream, swallowedDataBytesCount, false);
     }
 
 
diff --git a/java/org/apache/coyote/http2/RecycledStream.java b/java/org/apache/coyote/http2/RecycledStream.java
index b2b3e32..730936b 100644
--- a/java/org/apache/coyote/http2/RecycledStream.java
+++ b/java/org/apache/coyote/http2/RecycledStream.java
@@ -16,6 +16,8 @@
  */
 package org.apache.coyote.http2;
 
+import java.nio.ByteBuffer;
+
 /**
  * Represents a closed stream in the priority tree. Used in preference to the
  * full {@link Stream} as has much lower memory usage.
@@ -23,10 +25,12 @@ package org.apache.coyote.http2;
 class RecycledStream extends AbstractNonZeroStream {
 
     private final String connectionId;
+    private int remainingFlowControlWindow;
 
-    RecycledStream(String connectionId, Integer identifier, StreamStateMachine state) {
+    RecycledStream(String connectionId, Integer identifier, StreamStateMachine state, int remainingFlowControlWindow) {
         super(identifier, state);
         this.connectionId = connectionId;
+        this.remainingFlowControlWindow = remainingFlowControlWindow;
     }
 
 
@@ -41,5 +45,27 @@ class RecycledStream extends AbstractNonZeroStream {
     void incrementWindowSize(int increment) throws Http2Exception {
         // NO-OP
     }
-}
 
+
+    @Override
+    void receivedData(int payloadSize) throws ConnectionException {
+        remainingFlowControlWindow -= payloadSize;
+    }
+
+
+    /**
+     * {@inheritDoc}
+     * <p>
+     * This implementation will return an zero length ByteBuffer to trigger a
+     * flow control error if more DATA frame payload than the remaining flow
+     * control window is received for this recycled stream.
+     */
+    @Override
+    ByteBuffer getInputByteBuffer() {
+        if (remainingFlowControlWindow < 0) {
+            return ZERO_LENGTH_BYTEBUFFER;
+        } else {
+            return null;
+        }
+    }
+}
diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java
index 788bf27..a0110c0 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -478,9 +478,13 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
     }
 
 
+    @Override
     final ByteBuffer getInputByteBuffer() {
         if (inputBuffer == null) {
-            return null;
+            // This must either be a push or an HTTP upgrade. Either way there
+            // should not be a request body so return a zero length ByteBuffer
+            // to trigger a flow control error.
+            return ZERO_LENGTH_BYTEBUFFER;
         }
         return inputBuffer.getInBuffer();
     }
@@ -508,7 +512,8 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
     }
 
 
-    final void receivedData(int payloadSize) throws ConnectionException {
+    @Override
+    final void receivedData(int payloadSize) throws Http2Exception {
         contentLengthReceived += payloadSize;
         long contentLengthHeader = coyoteRequest.getContentLengthLong();
         if (contentLengthHeader > -1 && contentLengthReceived > contentLengthHeader) {
@@ -609,6 +614,7 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
                 state.sendReset();
                 cancelAllocationRequests();
                 handler.sendStreamReset(se);
+                inputBuffer.swallowUnread();
             } catch (IOException ioe) {
                 ConnectionException ce = new ConnectionException(
                         sm.getString("stream.reset.fail"), Http2Error.PROTOCOL_ERROR, ioe);
@@ -637,7 +643,8 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
         if (log.isDebugEnabled()) {
             log.debug(sm.getString("stream.recycle", getConnectionId(), getIdAsString()));
         }
-        handler.replaceStream(this, new RecycledStream(getConnectionId(), getIdentifier(), state));
+        handler.replaceStream(
+                this, new RecycledStream(getConnectionId(), getIdentifier(), state, getInputByteBuffer().remaining()));
     }
 
 
@@ -987,7 +994,8 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
         // 'write mode'.
         private volatile ByteBuffer inBuffer;
         private volatile boolean readInterest;
-        private boolean resetReceived = false;
+        private volatile boolean closed;
+        private boolean resetReceived;
 
         /**
          * @deprecated Unused. Will be removed in Tomcat 9. Use
@@ -1175,8 +1183,10 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
         /*
          * Called after placing some data in the inBuffer.
          */
-        final synchronized boolean onDataAvailable() {
-            if (readInterest) {
+        final synchronized void onDataAvailable() throws IOException {
+            if (closed) {
+                swallowUnread();
+            } else if (readInterest) {
                 if (log.isDebugEnabled()) {
                     log.debug(sm.getString("stream.inputBuffer.dispatch"));
                 }
@@ -1186,7 +1196,6 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
                 // the incoming connection and streams are processed on their
                 // own.
                 coyoteRequest.action(ActionCode.DISPATCH_EXECUTE, null);
-                return true;
             } else {
                 if (log.isDebugEnabled()) {
                     log.debug(sm.getString("stream.inputBuffer.signal"));
@@ -1194,7 +1203,6 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
                 synchronized (inBuffer) {
                     inBuffer.notifyAll();
                 }
-                return false;
             }
         }
 
@@ -1211,13 +1219,13 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
 
 
         private final void ensureBuffersExist() {
-            if (inBuffer == null) {
+            if (inBuffer == null && !closed) {
                 // The client must obey Tomcat's window size when sending so
                 // this is the initial window size set by Tomcat that the client
                 // uses (i.e. the local setting is required here).
                 int size = handler.getLocalSettings().getInitialWindowSize();
                 synchronized (this) {
-                    if (inBuffer == null) {
+                    if (inBuffer == null && !closed) {
                         inBuffer = ByteBuffer.allocate(size);
                         outBuffer = new byte[size];
                     }
@@ -1242,5 +1250,19 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
                 }
             }
         }
+
+        private final void swallowUnread() throws IOException {
+            if (inBuffer != null) {
+                synchronized (inputBuffer) {
+                    closed = true;
+                    int unreadByteCount = inBuffer.position();
+                    if (unreadByteCount > 0) {
+                        inBuffer.position(0);
+                        inBuffer.limit(inBuffer.limit() - unreadByteCount);
+                        handler.onSwallowedDataFramePayload(getIdAsInt(), unreadByteCount);
+                    }
+                }
+            }
+        }
     }
 }
diff --git a/test/org/apache/coyote/http2/Http2TestBase.java b/test/org/apache/coyote/http2/Http2TestBase.java
index f459a40..a848c55 100644
--- a/test/org/apache/coyote/http2/Http2TestBase.java
+++ b/test/org/apache/coyote/http2/Http2TestBase.java
@@ -1198,10 +1198,9 @@ public abstract class Http2TestBase extends TomcatBaseTest {
 
         @Override
         public void onSwallowedDataFramePayload(int streamId, int swallowedDataBytesCount) {
-            trace.append(streamId);
-            trace.append("-SwallowedDataFramePayload-[");
-            trace.append(swallowedDataBytesCount);
-            trace.append("]\n");
+            // NO-OP
+            // Many tests swallow request bodies which triggers this
+            // notification. It is added to the trace to reduce noise.
         }
 
 
diff --git a/test/org/apache/coyote/http2/TestFlowControl.java b/test/org/apache/coyote/http2/TestFlowControl.java
new file mode 100644
index 0000000..3841a1d
--- /dev/null
+++ b/test/org/apache/coyote/http2/TestFlowControl.java
@@ -0,0 +1,131 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.apache.coyote.http2;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.tomcat.util.http.MimeHeaders;
+
+public class TestFlowControl extends Http2TestBase {
+
+    /*
+     * https://tomcat.markmail.org/thread/lijsebphms7hr3zj
+     */
+    @Test
+    public void testNotFound() throws Exception {
+
+        http2Connect();
+
+        // Connection and per-stream flow control default to 64k-1 bytes
+
+        // Set up a POST request to a non-existent end-point
+        // Generate headers
+        byte[] headersFrameHeader = new byte[9];
+        ByteBuffer headersPayload = ByteBuffer.allocate(128);
+
+        MimeHeaders headers = new MimeHeaders();
+        headers.addValue(":method").setString("POST");
+        headers.addValue(":scheme").setString("http");
+        headers.addValue(":path").setString("/path-does-not-exist");
+        headers.addValue(":authority").setString("localhost:" + getPort());
+        headers.addValue("content-length").setLong(65536);
+        hpackEncoder.encode(headers, headersPayload);
+
+        headersPayload.flip();
+
+        ByteUtil.setThreeBytes(headersFrameHeader, 0, headersPayload.limit());
+        headersFrameHeader[3] = FrameType.HEADERS.getIdByte();
+        // Flags. end of headers (0x04)
+        headersFrameHeader[4] = 0x04;
+        // Stream id
+        ByteUtil.set31Bits(headersFrameHeader, 5, 3);
+
+        writeFrame(headersFrameHeader, headersPayload);
+
+        // Generate body
+        // Max data payload is 16k
+        byte[] dataFrameHeader = new byte[9];
+        ByteBuffer dataPayload = ByteBuffer.allocate(16 * 1024);
+
+        while (dataPayload.hasRemaining()) {
+            dataPayload.put((byte) 'x');
+        }
+        dataPayload.flip();
+
+        // Size
+        ByteUtil.setThreeBytes(dataFrameHeader, 0, dataPayload.limit());
+        ByteUtil.set31Bits(dataFrameHeader, 5, 3);
+
+        // Read the 404 error page
+        // headers
+        parser.readFrame(true);
+        // body
+        parser.readFrame(true);
+        // reset (becasue the request body was not fully read)
+        parser.readFrame(true);
+        // Validate response
+        Assert.assertEquals(
+                "3-HeadersStart\n" +
+                "3-Header-[:status]-[404]\n" +
+                "3-Header-[content-type]-[text/html;charset=utf-8]\n" +
+                "3-Header-[content-language]-[en]\n" +
+                "3-Header-[content-length]-[692]\n" +
+                "3-Header-[date]-[Wed, 11 Nov 2015 19:18:42 GMT]\n" +
+                "3-HeadersEnd\n" +
+                "3-Body-692\n" +
+                "3-EndOfStream\n" +
+                "3-RST-[8]\n", output.getTrace());
+        output.clearTrace();
+
+        // Write 3*16k=48k of request body
+        int count = 0;
+        while (count < 3) {
+            writeFrame(dataFrameHeader, dataPayload);
+            waitForWindowSize(0);
+            dataPayload.position(0);
+            count++;
+        }
+
+        // EOS
+        dataFrameHeader[4] = 0x01;
+        writeFrame(dataFrameHeader, dataPayload);
+        waitForWindowSize(0);
+    }
+
+
+    /*
+     * This might be unnecessary but given the potential for timing differences
+     * across different systems a more robust approach seems prudent.
+     */
+    private void waitForWindowSize(int streamId) throws Http2Exception, IOException {
+        String prefix = streamId + "-WindowSize-";
+        boolean found = false;
+        String trace;
+        do {
+            parser.readFrame(true);
+            trace = output.getTrace();
+            output.clearTrace();
+            System.out.println(trace);
+            found = trace.startsWith(prefix);
+        } while (!found);
+    }
+}
+
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 2be4fd4..d77a083 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -123,6 +123,12 @@
         Simplify the closing on an HTTP/2 stream when an error condition is
         present. (markt)
       </scode>
+      <fix>
+        <bug>65179</bug>: Ensure that the connection level flow control window
+        from the client to the server is updated when handling DATA frames
+        received for completed streams else the flow control window  may become
+        exhausted. (markt)
+      </fix>
     </changelog>
   </subsection>
 </section>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 07/08: Fix failing test

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 554bbd95c43b2ab0251a32c6b01de5d950f60cf7
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Mar 15 19:47:05 2021 +0000

    Fix failing test
---
 test/org/apache/coyote/http2/TestFlowControl.java | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/org/apache/coyote/http2/TestFlowControl.java b/test/org/apache/coyote/http2/TestFlowControl.java
index 3841a1d..f49c13c 100644
--- a/test/org/apache/coyote/http2/TestFlowControl.java
+++ b/test/org/apache/coyote/http2/TestFlowControl.java
@@ -87,10 +87,10 @@ public class TestFlowControl extends Http2TestBase {
                 "3-Header-[:status]-[404]\n" +
                 "3-Header-[content-type]-[text/html;charset=utf-8]\n" +
                 "3-Header-[content-language]-[en]\n" +
-                "3-Header-[content-length]-[692]\n" +
+                "3-Header-[content-length]-[691]\n" +
                 "3-Header-[date]-[Wed, 11 Nov 2015 19:18:42 GMT]\n" +
                 "3-HeadersEnd\n" +
-                "3-Body-692\n" +
+                "3-Body-691\n" +
                 "3-EndOfStream\n" +
                 "3-RST-[8]\n", output.getTrace());
         output.clearTrace();


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 02/08: Rename swallow() -> swallowPayload()

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 8f407126031d8a8b8b0ed3de24ef58f01697139e
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Mar 15 19:29:57 2021 +0000

    Rename swallow() -> swallowPayload()
---
 java/org/apache/coyote/http2/Http2Parser.java | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/java/org/apache/coyote/http2/Http2Parser.java b/java/org/apache/coyote/http2/Http2Parser.java
index 6552d23..2bcfb2a 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -85,7 +85,7 @@ class Http2Parser {
         try {
             validateFrame(expected, frameType, streamId, flags, payloadSize);
         } catch (StreamException se) {
-            swallow(streamId, payloadSize, false);
+            swallowPayload(streamId, payloadSize, false);
             throw se;
         }
 
@@ -166,11 +166,11 @@ class Http2Parser {
 
         ByteBuffer dest = output.startRequestBodyFrame(streamId, payloadSize, endOfStream);
         if (dest == null) {
-            swallow(streamId, dataLength, false);
+            swallowPayload(streamId, dataLength, false);
             // Process padding before sending any notifications in case padding
             // is invalid.
             if (padLength > 0) {
-                swallow(streamId, padLength, true);
+                swallowPayload(streamId, padLength, true);
             }
             if (endOfStream) {
                 output.receivedEndOfStream(streamId);
@@ -178,7 +178,7 @@ class Http2Parser {
         } else {
             synchronized (dest) {
                 if (dest.remaining() < dataLength) {
-                    swallow(streamId, dataLength, false);
+                    swallowPayload(streamId, dataLength, false);
                     // Client has sent more data than permitted by Window size
                     throw new StreamException(sm.getString("http2Parser.processFrameData.window", connectionId),
                             Http2Error.FLOW_CONTROL_ERROR, streamId);
@@ -187,7 +187,7 @@ class Http2Parser {
                 // Process padding before sending any notifications in case
                 // padding is invalid.
                 if (padLength > 0) {
-                    swallow(streamId, padLength, true);
+                    swallowPayload(streamId, padLength, true);
                 }
                 if (endOfStream) {
                     output.receivedEndOfStream(streamId);
@@ -212,7 +212,7 @@ class Http2Parser {
         try {
             hpackDecoder.setHeaderEmitter(output.headersStart(streamId, headersEndStream));
         } catch (StreamException se) {
-            swallow(streamId, payloadSize, false);
+            swallowPayload(streamId, payloadSize, false);
             throw se;
         }
 
@@ -252,7 +252,7 @@ class Http2Parser {
 
         readHeaderPayload(streamId, payloadSize);
 
-        swallow(streamId, padLength, true);
+        swallowPayload(streamId, padLength, true);
 
         if (Flags.isEndOfHeaders(flags)) {
             onHeadersComplete(streamId);
@@ -461,7 +461,7 @@ class Http2Parser {
     private void readUnknownFrame(int streamId, FrameType frameType, int flags, int payloadSize)
             throws IOException {
         try {
-            swallow(streamId, payloadSize, false);
+            swallowPayload(streamId, payloadSize, false);
         } catch (ConnectionException e) {
             // Will never happen because swallow() is called with mustBeZero set
             // to false
@@ -470,7 +470,7 @@ class Http2Parser {
     }
 
 
-    private void swallow(int streamId, int len, boolean mustBeZero)
+    private void swallowPayload(int streamId, int len, boolean mustBeZero)
             throws IOException, ConnectionException {
         if (log.isDebugEnabled()) {
             log.debug(sm.getString("http2Parser.swallow.debug", connectionId,


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 06/08: Fix failing test after flow control updates

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 8a7407e057213dd0a6440633c9027d37bc5e9bc7
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Mar 15 19:24:08 2021 +0000

    Fix failing test after flow control updates
---
 .../apache/coyote/http2/TestCancelledUpload.java   | 73 ++++++++--------------
 1 file changed, 26 insertions(+), 47 deletions(-)

diff --git a/test/org/apache/coyote/http2/TestCancelledUpload.java b/test/org/apache/coyote/http2/TestCancelledUpload.java
index 9964aa4..accd4b3 100644
--- a/test/org/apache/coyote/http2/TestCancelledUpload.java
+++ b/test/org/apache/coyote/http2/TestCancelledUpload.java
@@ -73,26 +73,6 @@ public class TestCancelledUpload extends Http2TestBase {
         // - reset the stream if further DATA frames are received
         parser.readFrame(true);
 
-        // If reset is first frame received end test here
-        if (checkReset()) {
-            return;
-        }
-
-        // Validate any WindowSize frames. Usually arrive in pairs. Depending on
-        // timing, can see a reset rather than than stream update.
-        while (output.getTrace().startsWith("0-WindowSize-[")) {
-            String trace = output.getTrace();
-            int size = Integer.parseInt(trace.substring(14, trace.length() - 2));
-            output.clearTrace();
-            parser.readFrame(true);
-            if (checkReset()) {
-                return;
-            }
-            Assert.assertEquals("3-WindowSize-[" + size + "]\n", output.getTrace());
-            output.clearTrace();
-            parser.readFrame(true);
-        }
-
         // Check for reset and exit if found
         if (checkReset()) {
             return;
@@ -106,58 +86,57 @@ public class TestCancelledUpload extends Http2TestBase {
                 "3-HeadersEnd\n",
                 output.getTrace());
         output.clearTrace();
-
         parser.readFrame(true);
+
         // Check for reset and exit if found
         if (checkReset()) {
             return;
         }
 
-        // Not reset, must be request body
+        // Not window update, not reset, must be the response body
         Assert.assertEquals("3-Body-0\n" +
                 "3-EndOfStream\n",
                 output.getTrace());
         output.clearTrace();
-
-        // There must be a reset. There may be some WindowSize frames
         parser.readFrame(true);
 
-        // Validate any WindowSize frames. Usually arrive in pairs. Depending on
-        // timing, can see a reset rather than than stream update.
-        while (output.getTrace().startsWith("0-WindowSize-[")) {
-            String trace = output.getTrace();
-            int size = Integer.parseInt(trace.substring(14, trace.length() - 2));
-            output.clearTrace();
-            parser.readFrame(true);
-            if (checkReset()) {
-                return;
-            }
-            Assert.assertEquals("3-WindowSize-[" + size + "]\n", output.getTrace());
-            output.clearTrace();
-            parser.readFrame(true);
-        }
-
-        // This should be the reset
-        checkReset();
-        Assert.assertEquals("3-RST-[3]\n", output.getTrace());
+        Assert.assertTrue(checkReset());
 
         // If there are any more frames after this, ignore them
     }
 
 
     /*
-     * Depending on timing, several resets may be sent.
+     * Looking for a RST frame with error type 3 (flow control error).
+     *
+     * If there is a flow control window update for stream 0 it may be followed
+     * by one for stream 3.
+     *
+     * If there is a flow control window update for stream 3 it will always be
+     * preceded by one for stream 0.
      */
     private boolean checkReset() throws IOException, Http2Exception {
+        int lastConnectionFlowControlWindowUpdate = -1;
         while (true) {
-            if (output.getTrace().startsWith("3-RST-[3]\n")) {
+            String trace = output.getTrace();
+            if (trace.startsWith("3-RST-[3]\n")) {
+                // This is the reset we are looking for
                 return true;
-            } else if (output.getTrace().startsWith("3-RST-[")) {
-                output.clearTrace();
-                parser.readFrame(true);
+            } else if (trace.startsWith("3-RST-[")) {
+                // Probably error type 8 (cancel) - ignore
+            } else if (trace.startsWith("0-WindowSize-[")) {
+                // Connection flow control window update
+                lastConnectionFlowControlWindowUpdate = Integer.parseInt(trace.substring(14, trace.length() - 2));
+            } else if (trace.startsWith("3-WindowSize-[")) {
+                // Stream flow control window update
+                // Must be same size as last connection flow control window
+                // update. True for Tomcat anyway.
+                Assert.assertEquals("3-WindowSize-[" + lastConnectionFlowControlWindowUpdate + "]\n", trace);
             } else {
                 return false;
             }
+            output.clearTrace();
+            parser.readFrame(true);
         }
     }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 03/08: Add frameTypeId parameter to swallowedPayload()

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 6a52ef0613c418a9ba0bb74c46ca90ee510a9069
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Mar 15 18:50:29 2021 +0000

    Add frameTypeId parameter to swallowedPayload()
---
 java/org/apache/coyote/http2/FrameType.java   |  5 ++++
 java/org/apache/coyote/http2/Http2Parser.java | 37 +++++++++++++++++++--------
 2 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/java/org/apache/coyote/http2/FrameType.java b/java/org/apache/coyote/http2/FrameType.java
index d671f10..44e5b55 100644
--- a/java/org/apache/coyote/http2/FrameType.java
+++ b/java/org/apache/coyote/http2/FrameType.java
@@ -51,6 +51,11 @@ enum FrameType {
     }
 
 
+    int getId() {
+        return id;
+    }
+
+
     byte getIdByte() {
         return (byte) id;
     }
diff --git a/java/org/apache/coyote/http2/Http2Parser.java b/java/org/apache/coyote/http2/Http2Parser.java
index 2bcfb2a..42647c8 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -78,14 +78,15 @@ class Http2Parser {
         }
 
         int payloadSize = ByteUtil.getThreeBytes(frameHeaderBuffer, 0);
-        FrameType frameType = FrameType.valueOf(ByteUtil.getOneByte(frameHeaderBuffer, 3));
+        int frameTypeId = ByteUtil.getOneByte(frameHeaderBuffer, 3);
+        FrameType frameType = FrameType.valueOf(frameTypeId);
         int flags = ByteUtil.getOneByte(frameHeaderBuffer, 4);
         int streamId = ByteUtil.get31Bits(frameHeaderBuffer, 5);
 
         try {
             validateFrame(expected, frameType, streamId, flags, payloadSize);
         } catch (StreamException se) {
-            swallowPayload(streamId, payloadSize, false);
+            swallowPayload(streamId, frameTypeId, payloadSize, false);
             throw se;
         }
 
@@ -166,11 +167,11 @@ class Http2Parser {
 
         ByteBuffer dest = output.startRequestBodyFrame(streamId, payloadSize, endOfStream);
         if (dest == null) {
-            swallowPayload(streamId, dataLength, false);
+            swallowPayload(streamId, FrameType.DATA.getId(), dataLength, false);
             // Process padding before sending any notifications in case padding
             // is invalid.
             if (padLength > 0) {
-                swallowPayload(streamId, padLength, true);
+                swallowPayload(streamId, FrameType.DATA.getId(), padLength, true);
             }
             if (endOfStream) {
                 output.receivedEndOfStream(streamId);
@@ -178,7 +179,7 @@ class Http2Parser {
         } else {
             synchronized (dest) {
                 if (dest.remaining() < dataLength) {
-                    swallowPayload(streamId, dataLength, false);
+                    swallowPayload(streamId, FrameType.DATA.getId(), dataLength, false);
                     // Client has sent more data than permitted by Window size
                     throw new StreamException(sm.getString("http2Parser.processFrameData.window", connectionId),
                             Http2Error.FLOW_CONTROL_ERROR, streamId);
@@ -187,7 +188,7 @@ class Http2Parser {
                 // Process padding before sending any notifications in case
                 // padding is invalid.
                 if (padLength > 0) {
-                    swallowPayload(streamId, padLength, true);
+                    swallowPayload(streamId, FrameType.DATA.getId(), padLength, true);
                 }
                 if (endOfStream) {
                     output.receivedEndOfStream(streamId);
@@ -212,7 +213,7 @@ class Http2Parser {
         try {
             hpackDecoder.setHeaderEmitter(output.headersStart(streamId, headersEndStream));
         } catch (StreamException se) {
-            swallowPayload(streamId, payloadSize, false);
+            swallowPayload(streamId, FrameType.HEADERS.getId(), payloadSize, false);
             throw se;
         }
 
@@ -252,7 +253,7 @@ class Http2Parser {
 
         readHeaderPayload(streamId, payloadSize);
 
-        swallowPayload(streamId, padLength, true);
+        swallowPayload(streamId, FrameType.HEADERS.getId(), padLength, true);
 
         if (Flags.isEndOfHeaders(flags)) {
             onHeadersComplete(streamId);
@@ -461,7 +462,7 @@ class Http2Parser {
     private void readUnknownFrame(int streamId, FrameType frameType, int flags, int payloadSize)
             throws IOException {
         try {
-            swallowPayload(streamId, payloadSize, false);
+            swallowPayload(streamId, frameType.getId(), payloadSize, false);
         } catch (ConnectionException e) {
             // Will never happen because swallow() is called with mustBeZero set
             // to false
@@ -470,7 +471,21 @@ class Http2Parser {
     }
 
 
-    private void swallowPayload(int streamId, int len, boolean mustBeZero)
+    /**
+     * Swallow some or all of the bytes from the payload of an HTTP/2 frame.
+     *
+     * @param streamId      Stream being swallowed
+     * @param frameTypeId   Type of HTTP/2 frame for which the bytes will be
+     *                      swallowed
+     * @param len           Number of bytes to swallow
+     * @param isPadding     Are the bytes to be swallowed padding bytes?
+     *
+     * @throws IOException If an I/O error occurs reading additional bytes into
+     *                     the input buffer.
+     * @throws ConnectionException If the swallowed bytes are expected to have a
+     *                             value of zero but do not
+     */
+    protected void swallowPayload(int streamId, int frameTypeId, int len, boolean isPadding)
             throws IOException, ConnectionException {
         if (log.isDebugEnabled()) {
             log.debug(sm.getString("http2Parser.swallow.debug", connectionId,
@@ -484,7 +499,7 @@ class Http2Parser {
         while (read < len) {
             int thisTime = Math.min(buffer.length, len - read);
             input.fill(true, buffer, 0, thisTime);
-            if (mustBeZero) {
+            if (isPadding) {
                 // Validate the padding is zero since receiving non-zero padding
                 // is a strong indication of either a faulty client or a server
                 // side bug.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 08/08: Fix Javadoc errors

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 14adbe2e8e69bf6b10718145d35ff87dc8f3084d
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Mar 15 19:46:10 2021 +0000

    Fix Javadoc errors
---
 java/org/apache/coyote/http2/Http2Parser.java | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/java/org/apache/coyote/http2/Http2Parser.java b/java/org/apache/coyote/http2/Http2Parser.java
index 3f0e1b8..2da5049 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -682,8 +682,9 @@ class Http2Parser {
          * @param swallowedDataBytesCount   The number of bytes that the parser
          *                                  swallowed.
          *
-         * @throws ConnectionException
-         * @throws IOException
+         * @throws ConnectionException If an error fatal to the HTTP/2
+ *                 connection occurs while swallowing the payload
+         * @throws IOException If an I/O occurred while swallowing the payload
          */
         void onSwallowedDataFramePayload(int streamId, int swallowedDataBytesCount) throws ConnectionException, IOException;
 
@@ -724,7 +725,8 @@ class Http2Parser {
          *                      frame
          * @param size          The payload size of the swallowed frame
          *
-         * @throws IOException
+         * @throws IOException If an I/O occurred while swallowing the unknown
+         *         frame
          */
         void onSwallowedUnknownFrame(int streamId, int frameTypeId, int flags, int size) throws IOException;
     }


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 01/08: Rename Output.swallowedPadding to onSwallowedDataFramePayload

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 26a5bfc64c9e67f71180104a41e922a82cb187ad
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Mar 12 09:55:38 2021 +0000

    Rename Output.swallowedPadding to onSwallowedDataFramePayload
    
    This change is in preparation for fixes to BZ 65179
---
 java/org/apache/coyote/http2/Http2Parser.java         | 17 +++++++++++++++--
 java/org/apache/coyote/http2/Http2UpgradeHandler.java |  4 ++--
 test/org/apache/coyote/http2/Http2TestBase.java       |  6 +++---
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/java/org/apache/coyote/http2/Http2Parser.java b/java/org/apache/coyote/http2/Http2Parser.java
index a20ee33..6552d23 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -196,7 +196,7 @@ class Http2Parser {
             }
         }
         if (Flags.hasPadding(flags)) {
-            output.swallowedPadding(streamId, padLength);
+            output.onSwallowedDataFramePayload(streamId, padLength);
         }
     }
 
@@ -641,7 +641,20 @@ class Http2Parser {
         ByteBuffer startRequestBodyFrame(int streamId, int payloadSize, boolean endOfStream) throws Http2Exception;
         void endRequestBodyFrame(int streamId) throws Http2Exception;
         void receivedEndOfStream(int streamId) throws ConnectionException;
-        void swallowedPadding(int streamId, int paddingLength) throws ConnectionException, IOException;
+        /**
+         * Notification triggered when the parser swallows some or all of a DATA
+         * frame payload without writing it to the ByteBuffer returned by
+         * {@link #startRequestBodyFrame(int, int, boolean)}.
+         *
+         * @param streamId  The stream on which the payload that has been
+         *                  swallowed was received
+         * @param swallowedDataBytesCount   The number of bytes that the parser
+         *                                  swallowed.
+         *
+         * @throws ConnectionException
+         * @throws IOException
+         */
+        void onSwallowedDataFramePayload(int streamId, int swallowedDataBytesCount) throws ConnectionException, IOException;
 
         // Header frames
         HeaderEmitter headersStart(int streamId, boolean headersEndStream)
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 149ec79..02e6b9d 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -1610,11 +1610,11 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
 
 
     @Override
-    public void swallowedPadding(int streamId, int paddingLength) throws
+    public void onSwallowedDataFramePayload(int streamId, int swallowedDataBytesCount) throws
             ConnectionException, IOException {
         AbstractNonZeroStream abstractNonZeroStream = getAbstractNonZeroStream(streamId, true);
         // +1 is for the payload byte used to define the padding length
-        writeWindowUpdate(abstractNonZeroStream, paddingLength + 1, false);
+        writeWindowUpdate(abstractNonZeroStream, swallowedDataBytesCount + 1, false);
     }
 
 
diff --git a/test/org/apache/coyote/http2/Http2TestBase.java b/test/org/apache/coyote/http2/Http2TestBase.java
index 332a2f7..bb3f145 100644
--- a/test/org/apache/coyote/http2/Http2TestBase.java
+++ b/test/org/apache/coyote/http2/Http2TestBase.java
@@ -1197,10 +1197,10 @@ public abstract class Http2TestBase extends TomcatBaseTest {
 
 
         @Override
-        public void swallowedPadding(int streamId, int paddingLength) {
+        public void onSwallowedDataFramePayload(int streamId, int swallowedDataBytesCount) {
             trace.append(streamId);
-            trace.append("-SwallowedPadding-[");
-            trace.append(paddingLength);
+            trace.append("-SwallowedDataFramePayload-[");
+            trace.append(swallowedDataBytesCount);
             trace.append("]\n");
         }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org