Jetty 9.4.x 2233 ssl flush try again 2 (#2726)

Major refactor of SslConnection to address #2233 and to simplify in preparation for java-11 support.

Made the `needFillInterest` and `onIncompleteFlush` methods the primary stateful methods with state for fill and flush side that does not reproduce state already held by the SslEngine itself.

Signed-off-by: Greg Wilkins <gregw@webtide.com>
Signed-off-by: Simone Bordet <simone.bordet@gmail.com>

jetty-client/src/test/java/org/eclipse/jetty/client/EmptyServerHandler.java

@@ -26,6 +26,7 @@ import javax.servlet.http.HttpServletResponse;
 
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.handler.AbstractHandler;
+import org.eclipse.jetty.util.log.Log;
 
 public class EmptyServerHandler extends AbstractHandler.ErrorDispatchHandler
 {
@@ -38,5 +39,6 @@ public class EmptyServerHandler extends AbstractHandler.ErrorDispatchHandler
 
     protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
     {
+        Log.getRootLogger().info("EMPTY service {}",target);
     }
 }

jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java

@@ -510,7 +510,26 @@ public class HttpClientTest extends AbstractHttpClientServerTest
     @Test
     public void test_ExchangeIsComplete_OnlyWhenBothRequestAndResponseAreComplete() throws Exception
     {
-        start(new RespondThenConsumeHandler());
+        start(new AbstractHandler.ErrorDispatchHandler()
+        {
+            @Override
+            protected void doNonErrorHandle(String target, org.eclipse.jetty.server.Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
+            {
+                baseRequest.setHandled(true);
+                response.setContentLength(0);
+                response.setStatus(200);
+                response.flushBuffer();
+
+                byte[] buffer = new byte[1024];
+                InputStream in = request.getInputStream();
+                while(true)
+                {
+                    int read = in.read(buffer);
+                    if (read < 0)
+                        break;
+                }
+            }
+        });
 
         // Prepare a big file to upload
         Path targetTestsDir = testdir.getEmptyPathDir();

jetty-client/src/test/java/org/eclipse/jetty/client/RespondThenConsumeHandler.java

@@ -1,45 +0,0 @@
-//
-//  ========================================================================
-//  Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd.
-//  ------------------------------------------------------------------------
-//  All rights reserved. This program and the accompanying materials
-//  are made available under the terms of the Eclipse Public License v1.0
-//  and Apache License v2.0 which accompanies this distribution.
-//
-//      The Eclipse Public License is available at
-//      http://www.eclipse.org/legal/epl-v10.html
-//
-//      The Apache License v2.0 is available at
-//      http://www.opensource.org/licenses/apache2.0.php
-//
-//  You may elect to redistribute this code under either of these licenses.
-//  ========================================================================
-//
-
-package org.eclipse.jetty.client;
-
-import java.io.IOException;
-import java.io.InputStream;
-
-import javax.servlet.ServletException;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-
-import org.eclipse.jetty.server.handler.AbstractHandler;
-
-class RespondThenConsumeHandler extends AbstractHandler
-{
-    @Override
-    public void handle(String target, org.eclipse.jetty.server.Request baseRequest, HttpServletRequest request, HttpServletResponse response)
-            throws IOException, ServletException
-    {
-        baseRequest.setHandled(true);
-        response.setContentLength(0);
-        response.setStatus(200);
-        response.flushBuffer();
-        
-        InputStream in = request.getInputStream();
-        while(in.read()>=0);
-    }
-    
-}

jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesServerTest.java

@@ -43,7 +43,6 @@ import java.util.regex.Pattern;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLSocket;
-import javax.servlet.ServletException;
 import javax.servlet.ServletInputStream;
 import javax.servlet.ServletOutputStream;
 import javax.servlet.http.HttpServletRequest;
@@ -75,7 +74,6 @@ import org.junit.After;
 import org.junit.Assert;
 import org.junit.Assume;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 
 public class SslBytesServerTest extends SslBytesTest
@@ -99,6 +97,11 @@ public class SslBytesServerTest extends SslBytesTest
         threadPool = Executors.newCachedThreadPool();
         server = new Server();
 
+        sslFills.set(0);
+        sslFlushes.set(0);
+        httpParses.set(0);
+        serverEndPoint.set(null);
+        
         File keyStore = MavenTestingUtils.getTestResourceFile("keystore.jks");
         sslContextFactory = new SslContextFactory();
         sslContextFactory.setKeyStorePath(keyStore.getAbsolutePath());
@@ -185,7 +188,7 @@ public class SslBytesServerTest extends SslBytesTest
         server.setHandler(new AbstractHandler()
         {
             @Override
-            public void handle(String target, Request request, HttpServletRequest httpRequest, HttpServletResponse httpResponse) throws IOException, ServletException
+            public void handle(String target, Request request, HttpServletRequest httpRequest, HttpServletResponse httpResponse) throws IOException
             {
                 try
                 {
@@ -1320,9 +1323,7 @@ public class SslBytesServerTest extends SslBytesTest
         closeClient(client);
     }
     
-    // TODO work out why this test frequently fails
+    @Test(timeout=60000)
-    @Ignore
-    @Test(timeout=10000)
     public void testRequestWithContentWithRenegotiationInMiddleOfContentWhenRenegotiationIsForbidden() throws Exception
     {
         assumeJavaVersionSupportsTLSRenegotiations();
@@ -1367,37 +1368,28 @@ public class SslBytesServerTest extends SslBytesTest
         Assert.assertEquals(TLSRecord.Type.HANDSHAKE, record.getType());
         proxy.flushToServer(record);
 
-        // Renegotiation now allowed, server has closed
+        // Renegotiation not allowed, server has closed
+        loop: while(true)
+        {
             record = proxy.readFromServer();
+            if (record==null)
+                break;
+            switch(record.getType())
+            {
+                case APPLICATION:
+                    Assert.fail("application data not allows after renegotiate");
+                case ALERT:
+                    break loop;
+                default:
+                    continue;
+            }
+        }
         Assert.assertEquals(TLSRecord.Type.ALERT, record.getType());
         proxy.flushToClient(record);
 
         record = proxy.readFromServer();
         Assert.assertNull(record);
 
-        // Write the rest of the request
-        threadPool.submit(() ->
-        {
-            clientOutput.write(content2.getBytes(StandardCharsets.UTF_8));
-            clientOutput.flush();
-            return null;
-        });
-
-        // Trying to write more application data results in an exception since the server closed
-        record = proxy.readFromClient();
-        proxy.flushToServer(record);
-        try
-        {
-            record = proxy.readFromClient();
-            Assert.assertNotNull(record);
-            proxy.flushToServer(record);
-            Assert.fail();
-        }
-        catch (IOException expected)
-        {
-            // Expected
-        }
-
         // Check that we did not spin
         TimeUnit.MILLISECONDS.sleep(500);
         Assert.assertThat(sslFills.get(), Matchers.lessThan(50));

jetty-client/src/test/resources/jetty-logging.properties

@@ -2,4 +2,5 @@ class=org.eclipse.jetty.util.log.StdErrLog
 #org.eclipse.jetty.LEVEL=INFO
 #org.eclipse.jetty.client.LEVEL=DEBUG
 #org.eclipse.jetty.io.ChannelEndPoint.LEVEL=DEBUG
+#org.eclipse.jetty.io.ssl.LEVEL=DEBUG
 #org.eclipse.jetty.http.LEVEL=DEBUG

jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java

@@ -263,7 +263,7 @@ public abstract class AbstractConnection implements Connection
     @Override
     public final String toString()
     {
-        return String.format("%s<-%s",toConnectionString(),getEndPoint());
+        return String.format("%s@%h::%s",getClass().getSimpleName(),this,getEndPoint());
     }
 
     public String toConnectionString()

jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java

@@ -55,6 +55,7 @@ import org.junit.Test;
 
 public class SslConnectionTest
 {
+    private final static int TIMEOUT = 1000000;
     private static SslContextFactory __sslCtxFactory=new SslContextFactory();
     private static ByteBufferPool __byteBufferPool = new LeakTrackingByteBufferPool(new MappedByteBufferPool.Tagged());
 
@@ -94,7 +95,7 @@ public class SslConnectionTest
         protected EndPoint newEndPoint(SelectableChannel channel, ManagedSelector selector, SelectionKey selectionKey)
         {
             SocketChannelEndPoint endp = new TestEP(channel, selector, selectionKey, getScheduler());
-            endp.setIdleTimeout(60000);
+            endp.setIdleTimeout(TIMEOUT);
             _lastEndp=endp;
             return endp;
         }
@@ -257,7 +258,7 @@ public class SslConnectionTest
     {
         try (Socket client = newClient())
         {
-            client.setSoTimeout(60000);
+            client.setSoTimeout(TIMEOUT);
             try (SocketChannel server = _connector.accept())
             {
                 server.configureBlocking(false);
@@ -283,7 +284,7 @@ public class SslConnectionTest
     {
         try (SSLSocket client = newClient())
         {
-            client.setSoTimeout(60000);
+            client.setSoTimeout(TIMEOUT);
             try (SocketChannel server = _connector.accept())
             {
                 server.configureBlocking(false);
@@ -312,7 +313,7 @@ public class SslConnectionTest
         
         try (SSLSocket client = newClient())
         {
-            client.setSoTimeout(60000);
+            client.setSoTimeout(TIMEOUT);
             try (SocketChannel server = _connector.accept())
             {
                 server.configureBlocking(false);
@@ -348,7 +349,7 @@ public class SslConnectionTest
         
         try (SSLSocket client = newClient())
         {
-            client.setSoTimeout(60000);
+            client.setSoTimeout(TIMEOUT);
             try (SocketChannel server = _connector.accept())
             {
                 server.configureBlocking(false);
@@ -396,18 +397,23 @@ public class SslConnectionTest
         _testFill=false;
         _writeCallback = new FutureCallback();
 
-        try (Socket client = newClient())
+        try (SSLSocket client = newClient())
         {
-            client.setSoTimeout(10000);
+            client.setSoTimeout(TIMEOUT);
             try (SocketChannel server = _connector.accept())
             {
                 server.configureBlocking(false);
                 _manager.accept(server);
 
+                // The server side will write something, and in order
+                // to proceed with the initial TLS handshake we need
+                // to start reading before waiting for the callback.
+
                 byte[] buffer = new byte[1024];
                 int len = client.getInputStream().read(buffer);
                 Assert.assertEquals("Hello Client", new String(buffer, 0, len, StandardCharsets.UTF_8));
-                Assert.assertNull(_writeCallback.get(100, TimeUnit.MILLISECONDS));
+
+                Assert.assertNull(_writeCallback.get(1, TimeUnit.SECONDS));
             }
         }
     }

jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestBase.java

@@ -1170,7 +1170,7 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
             InputStream is = client.getInputStream();
 
             os.write((
-                    "POST /echo?charset=utf-8 HTTP/1.1\r\n" +
+                    "POST /echo/0?charset=utf-8 HTTP/1.1\r\n" +
                             "host: " + _serverURI.getHost() + ":" + _serverURI.getPort() + "\r\n" +
                             "content-type: text/plain; charset=utf-8\r\n" +
                             "content-length: 10\r\n" +
@@ -1181,7 +1181,7 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
             ).getBytes("utf-8"));
 
             os.write((
-                    "POST /echo?charset=utf-8 HTTP/1.1\r\n" +
+                    "POST /echo/1?charset=utf-8 HTTP/1.1\r\n" +
                             "host: " + _serverURI.getHost() + ":" + _serverURI.getPort() + "\r\n" +
                             "content-type: text/plain; charset=utf-8\r\n" +
                             "content-length: 10\r\n" +
@@ -1195,7 +1195,7 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
             String content = "Wibble";
             byte[] contentB = content.getBytes(StandardCharsets.UTF_16);
             os.write((
-                    "POST /echo?charset=utf-8 HTTP/1.1\r\n" +
+                    "POST /echo/2?charset=utf-8 HTTP/1.1\r\n" +
                             "host: " + _serverURI.getHost() + ":" + _serverURI.getPort() + "\r\n" +
                             "content-type: text/plain; charset=utf-16\r\n" +
                             "content-length: " + contentB.length + "\r\n" +

jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestFixture.java

@@ -35,6 +35,7 @@ import org.eclipse.jetty.server.handler.AbstractHandler;
 import org.eclipse.jetty.server.handler.HotSwapHandler;
 import org.eclipse.jetty.toolchain.test.PropertyFlag;
 import org.eclipse.jetty.util.IO;
+import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.thread.QueuedThreadPool;
 import org.junit.After;
 import org.junit.Before;
@@ -114,6 +115,7 @@ public class HttpServerTestFixture
         @Override
         public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
         {
+            Log.getRootLogger().info("handle "+target);
             baseRequest.setHandled(true);
 
             if (request.getContentType()!=null)
@@ -154,6 +156,8 @@ public class HttpServerTestFixture
 
             if (reader.read()>=0)
                 throw new IllegalStateException("Not closed");
+
+            Log.getRootLogger().info("handled "+target);
         }
     }
 

jetty-server/src/test/resources/jetty-logging.properties

@@ -1,5 +1,5 @@
 org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog
 #org.eclipse.jetty.LEVEL=DEBUG
-#org.eclipse.jetty.server.LEVEL=DEBUG
+#org.eclipse.jetty.io.ssl.LEVEL=DEBUG
 #org.eclipse.jetty.server.ConnectionLimit.LEVEL=DEBUG
 #org.eclipse.jetty.server.AcceptRateLimit.LEVEL=DEBUG