[tomcat] branch master updated (a8852a6 -> e1a1a32)

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

[tomcat] branch master updated (a8852a6 -> e1a1a32)

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

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


    from a8852a6  Fix new shade plugin warnings
     new 9083cea  Improve handling of HTTP/2 connection preface
     new e1a1a32  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 master
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 9083cea2ef8b1bc4a8fdbe5dcd544f4edc204dea
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 c76effc..ed565fd 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 005a046..85b7eee 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -115,6 +115,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 master
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit e1a1a324e93b568804153e02281bc4ec625c3ba7
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]