[tomcat] branch 9.0.x updated (199f501 -> cb137e0)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[tomcat] branch 9.0.x updated (199f501 -> cb137e0)

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

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


    from 199f501  Update to CXF 3.4.0
     new c688f8d  Improve handling of HTTP/2 connection preface
     new cb137e0  Improve test for invalid client preface

The 2 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:
 java/org/apache/coyote/http2/Http2AsyncParser.java |  9 +++---
 .../apache/coyote/http2/TestHttp2Section_3_5.java  | 32 ++++++++++++++++++----
 webapps/docs/changelog.xml                         |  5 ++++
 3 files changed, 36 insertions(+), 10 deletions(-)


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[tomcat] 01/02: Improve handling of HTTP/2 connection preface

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

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

commit c688f8d0b728ca37ffad44d7f50f1aa5778c200f
Author: Mark Thomas <[hidden email]>
AuthorDate: Thu Sep 24 15:50:35 2020 +0100

    Improve handling of HTTP/2 connection preface
   
    Only process the connection if the preface is processed without error.
    If an error is detected, trigger the connection close.
---
 java/org/apache/coyote/http2/Http2AsyncParser.java | 9 +++++----
 webapps/docs/changelog.xml                         | 5 +++++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/java/org/apache/coyote/http2/Http2AsyncParser.java b/java/org/apache/coyote/http2/Http2AsyncParser.java
index 2452572..12fa3bb 100644
--- a/java/org/apache/coyote/http2/Http2AsyncParser.java
+++ b/java/org/apache/coyote/http2/Http2AsyncParser.java
@@ -35,7 +35,7 @@ class Http2AsyncParser extends Http2Parser {
 
     private final SocketWrapperBase<?> socketWrapper;
     private final Http2AsyncUpgradeHandler upgradeHandler;
-    private Throwable error = null;
+    private volatile Throwable error = null;
 
     Http2AsyncParser(String connectionId, Input input, Output output, SocketWrapperBase<?> socketWrapper, Http2AsyncUpgradeHandler upgradeHandler) {
         super(connectionId, input, output);
@@ -117,13 +117,14 @@ class Http2AsyncParser extends Http2Parser {
                 if (payload.hasRemaining()) {
                     socketWrapper.unRead(payload);
                 }
+                // Finish processing the connection
+                upgradeHandler.processConnectionCallback(webConnection, stream);
+            } else {
+                upgradeHandler.closeConnection(new ConnectionException(error.getMessage(), Http2Error.PROTOCOL_ERROR, error));
             }
-            // Finish processing the connection
-            upgradeHandler.processConnectionCallback(webConnection, stream);
             // Continue reading frames
             upgradeHandler.upgradeDispatch(SocketEvent.OPEN_READ);
         }
-
     }
 
     @Override
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 32c2ba7..06ec832 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -114,6 +114,11 @@
         in-flight asynchronous requests. The tracking enables Tomcat to shutdown
         gracefully when asynchronous processing is in use. (markt)
       </fix>
+      <fix>
+        Improve the error handling for the HTTP/2 connection preface when the
+        Connector is configured with <code>useAsycIO=&quot;true&quot;</code>.
+        (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[tomcat] 02/02: Improve test for invalid client preface

markt
In reply to this post by markt
This is an automated email from the ASF dual-hosted git repository.

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

commit cb137e09d3445e0edaf69ccc75fea5eb0c60ce3d
Author: Mark Thomas <[hidden email]>
AuthorDate: Thu Sep 24 15:57:14 2020 +0100

    Improve test for invalid client preface
---
 .../apache/coyote/http2/TestHttp2Section_3_5.java  | 32 ++++++++++++++++++----
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/test/org/apache/coyote/http2/TestHttp2Section_3_5.java b/test/org/apache/coyote/http2/TestHttp2Section_3_5.java
index dc09ca4..e9a9e0e 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_3_5.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_3_5.java
@@ -18,22 +18,42 @@ package org.apache.coyote.http2;
 
 import java.io.IOException;
 
+import org.junit.Assert;
 import org.junit.Test;
 
 public class TestHttp2Section_3_5 extends Http2TestBase {
 
-    @Test(expected=IOException.class)
+    @Test
     public void testNoConnectionPreface() throws Exception {
         enableHttp2();
         configureAndStartWebApplication();
         openClientConnection();
         doHttpUpgrade();
-        // Should send client preface here
-        sendPing();
-        // Send several pings else server will block waiting for the client
-        // preface which is longer than a single ping.
+
+        // Server settings
+        parser.readFrame(true);
+        Assert.assertEquals("0-Settings-[3]-[200]\n" +
+                "0-Settings-End\n"
+                , output.getTrace());
+        output.clearTrace();
+
+        // Should send client preface here. This will trigger an error.
+        // Send two pings (2*(9+8)=34 bytes) as server looks for entire preface
+        // of 24 bytes.
         sendPing();
         sendPing();
-        validateHttp2InitialResponse();
+
+        // If the client preface had been valid, this would be an
+        // acknowledgement. Of the settings. As the preface was invalid, it
+        // should be a GOAWAY frame.
+        try {
+            parser.readFrame(true);
+            Assert.assertTrue(output.getTrace(), output.getTrace().startsWith("0-Goaway-[1]-[1]-["));
+        } catch (IOException ioe) {
+            // Ignore
+            // This is expected on some platforms and depends on timing. Once
+            // the server closes the connection the client may see an exception
+            // when trying to read the GOAWAY frame.
+        }
     }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]