From 2ca897c1ea6bf5b42949ebdc64ffd9094548d17e Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 20 Dec 2011 13:01:23 -0700 Subject: [PATCH 1/4] 367219 - WebSocketClient.open() fails when URI uses default ports. + Fixing testcase to not fail if http://localhost/ exists. Reworking code to not rely on existence of server to validate the correct behavior of URI port parsing. --- .../jetty/websocket/WebSocketClient.java | 34 ++++++----- .../jetty/websocket/WebSocketClientTest.java | 56 ++++++------------- 2 files changed, 37 insertions(+), 53 deletions(-) diff --git a/jetty-websocket/src/main/java/org/eclipse/jetty/websocket/WebSocketClient.java b/jetty-websocket/src/main/java/org/eclipse/jetty/websocket/WebSocketClient.java index cff7de04304..cc5db438838 100644 --- a/jetty-websocket/src/main/java/org/eclipse/jetty/websocket/WebSocketClient.java +++ b/jetty-websocket/src/main/java/org/eclipse/jetty/websocket/WebSocketClient.java @@ -332,6 +332,25 @@ public class WebSocketClient { if (!_factory.isStarted()) throw new IllegalStateException("Factory !started"); + + InetSocketAddress address = toSocketAddress(uri); + + SocketChannel channel = SocketChannel.open(); + if (_bindAddress != null) + channel.socket().bind(_bindAddress); + channel.socket().setTcpNoDelay(true); + + WebSocketFuture holder = new WebSocketFuture(websocket, uri, this, channel); + + channel.configureBlocking(false); + channel.connect(address); + _factory.getSelectorManager().register(channel, holder); + + return holder; + } + + public static final InetSocketAddress toSocketAddress(URI uri) + { String scheme = uri.getScheme(); if (!("ws".equalsIgnoreCase(scheme) || "wss".equalsIgnoreCase(scheme))) throw new IllegalArgumentException("Bad WebSocket scheme: " + scheme); @@ -341,20 +360,8 @@ public class WebSocketClient if (port < 0) port = "ws".equals(scheme) ? 80 : 443; - SocketChannel channel = SocketChannel.open(); - if (_bindAddress != null) - channel.socket().bind(_bindAddress); - channel.socket().setTcpNoDelay(true); - InetSocketAddress address = new InetSocketAddress(uri.getHost(), port); - - WebSocketFuture holder = new WebSocketFuture(websocket, uri, this, channel); - - channel.configureBlocking(false); - channel.connect(address); - _factory.getSelectorManager().register(channel, holder); - - return holder; + return address; } /* ------------------------------------------------------------ */ @@ -486,6 +493,7 @@ public class WebSocketClient return _maskGen; } + @Override public String toString() { return "[" + _uri + ","+_websocket+"]@"+hashCode(); diff --git a/jetty-websocket/src/test/java/org/eclipse/jetty/websocket/WebSocketClientTest.java b/jetty-websocket/src/test/java/org/eclipse/jetty/websocket/WebSocketClientTest.java index 77639bf0c65..2ab060ac239 100644 --- a/jetty-websocket/src/test/java/org/eclipse/jetty/websocket/WebSocketClientTest.java +++ b/jetty-websocket/src/test/java/org/eclipse/jetty/websocket/WebSocketClientTest.java @@ -15,12 +15,15 @@ *******************************************************************************/ package org.eclipse.jetty.websocket; +import static org.hamcrest.Matchers.*; + import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.io.OutputStream; import java.net.ConnectException; +import java.net.InetSocketAddress; import java.net.ServerSocket; import java.net.Socket; import java.net.URI; @@ -43,8 +46,6 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; -import static org.hamcrest.Matchers.greaterThan; - public class WebSocketClientTest { private WebSocketClientFactory _factory = new WebSocketClientFactory(); @@ -711,44 +712,19 @@ public class WebSocketClientTest @Test public void testURIWithDefaultPort() throws Exception { - WebSocketClient client = new WebSocketClient(_factory); - - try - { - client.open(new URI("ws://localhost"), new WebSocket() - { - public void onOpen(Connection connection) - { - } - - public void onClose(int closeCode, String message) - { - System.out.println("closeCode = " + closeCode); - } - }).get(5, TimeUnit.SECONDS); - } - catch (ExecutionException x) - { - Assert.assertTrue(x.getCause() instanceof ConnectException); - } - - try - { - client.open(new URI("wss://localhost"), new WebSocket() - { - public void onOpen(Connection connection) - { - } - - public void onClose(int closeCode, String message) - { - } - }).get(5, TimeUnit.SECONDS); - } - catch (ExecutionException x) - { - Assert.assertTrue(x.getCause() instanceof ConnectException); - } + URI uri = new URI("ws://localhost"); + InetSocketAddress addr = WebSocketClient.toSocketAddress(uri); + Assert.assertThat("URI (" + uri + ").host", addr.getHostName(), is("localhost")); + Assert.assertThat("URI (" + uri + ").port", addr.getPort(), is(80)); + } + + @Test + public void testURIWithDefaultWSSPort() throws Exception + { + URI uri = new URI("wss://localhost"); + InetSocketAddress addr = WebSocketClient.toSocketAddress(uri); + Assert.assertThat("URI (" + uri + ").host", addr.getHostName(), is("localhost")); + Assert.assertThat("URI (" + uri + ").port", addr.getPort(), is(443)); } private void respondToClient(Socket connection, String serverResponse) throws IOException From a606529710031eda5d971c2eb1ee40d31ba00a77 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 21 Dec 2011 10:06:55 +1100 Subject: [PATCH 2/4] 364921 SslConnection does real close on idle if already oshut --- .../jetty/client/SslBytesServerTest.java | 63 +++++++++++++++---- .../eclipse/jetty/io/nio/SslConnection.java | 5 +- 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/SslBytesServerTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/SslBytesServerTest.java index 0976b5459c0..f2f04f738fc 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/SslBytesServerTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/SslBytesServerTest.java @@ -5,6 +5,7 @@ import java.io.File; import java.io.IOException; import java.io.InputStreamReader; import java.io.OutputStream; +import java.net.SocketException; import java.net.SocketTimeoutException; import java.nio.channels.SocketChannel; import java.util.Arrays; @@ -14,6 +15,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.net.ssl.SSLContext; @@ -51,6 +53,8 @@ import static org.hamcrest.Matchers.lessThan; public class SslBytesServerTest extends SslBytesTest { + private final static int MAX_IDLE_TIME=5000; + private final AtomicInteger sslHandles = new AtomicInteger(); private final AtomicInteger sslFlushes = new AtomicInteger(); private final AtomicInteger httpParses = new AtomicInteger(); @@ -58,7 +62,8 @@ public class SslBytesServerTest extends SslBytesTest private Server server; private SSLContext sslContext; private SimpleProxy proxy; - + private final AtomicReference sslConnection = new AtomicReference(); + @Before public void init() throws Exception { @@ -70,7 +75,7 @@ public class SslBytesServerTest extends SslBytesTest @Override protected SslConnection newSslConnection(AsyncEndPoint endPoint, SSLEngine engine) { - return new SslConnection(engine, endPoint) + SslConnection connection = new SslConnection(engine, endPoint) { @Override public Connection handle() throws IOException @@ -93,6 +98,8 @@ public class SslBytesServerTest extends SslBytesTest }; } }; + sslConnection.set(connection); + return connection; } @Override @@ -116,7 +123,7 @@ public class SslBytesServerTest extends SslBytesTest }; } }; - connector.setMaxIdleTime(5000); + connector.setMaxIdleTime(MAX_IDLE_TIME); // connector.setPort(5870); connector.setPort(0); @@ -1237,7 +1244,6 @@ public class SslBytesServerTest extends SslBytesTest closeClient(client); } - @Ignore @Test public void testServerCloseClientDoesNotClose() throws Exception { @@ -1273,16 +1279,51 @@ public class SslBytesServerTest extends SslBytesTest break; } + // Check client is at EOF + Assert.assertEquals(-1,client.getInputStream().read()); + + // Client should close the socket, but let's hold it open. + // Check that we did not spin + TimeUnit.MILLISECONDS.sleep(100); Assert.assertThat(sslHandles.get(), lessThan(20)); Assert.assertThat(httpParses.get(), lessThan(50)); - - // TODO: instead of sleeping, we should expect the connection being closed by the idle timeout - // TODO: mechanism; unfortunately this now is not working, and this test fails because the idle - // TODO: timeout will not trigger. - TimeUnit.SECONDS.sleep(100); - - closeClient(client); + + // Check it is still half closed on the server. + Assert.assertTrue(((AsyncEndPoint)sslConnection.get().getEndPoint()).isOpen()); + Assert.assertTrue(((AsyncEndPoint)sslConnection.get().getEndPoint()).isOutputShutdown()); + Assert.assertFalse(((AsyncEndPoint)sslConnection.get().getEndPoint()).isInputShutdown()); + + // wait for a bit more than MAX_IDLE_TIME + TimeUnit.MILLISECONDS.sleep(MAX_IDLE_TIME+1000); + + // Server should have closed the endpoint + Assert.assertFalse(((AsyncEndPoint)sslConnection.get().getEndPoint()).isOpen()); + Assert.assertTrue(((AsyncEndPoint)sslConnection.get().getEndPoint()).isOutputShutdown()); + Assert.assertTrue(((AsyncEndPoint)sslConnection.get().getEndPoint()).isInputShutdown()); + + + // writing to client will eventually get broken pipe exception or similar + try + { + for (int i=0;i<100;i++) + { + clientOutput.write(("" + + "POST / HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Content-Type: text/plain\r\n" + + "Content-Length: " + content.length() + "\r\n" + + "Connection: close\r\n" + + "\r\n" + + content).getBytes("UTF-8")); + clientOutput.flush(); + } + Assert.fail("Client should have seen server close"); + } + catch(SocketException e) + { + // this was expected. + } } private void assumeJavaVersionSupportsTLSRenegotiations() diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/nio/SslConnection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/nio/SslConnection.java index d859d126e10..d9af5953ca4 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/nio/SslConnection.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/nio/SslConnection.java @@ -249,7 +249,10 @@ public class SslConnection extends AbstractConnection implements AsyncConnection try { LOG.debug("onIdleExpired {}ms on {}",idleForMs,this); - _sslEndPoint.shutdownOutput(); + if (_endp.isOutputShutdown()) + _sslEndPoint.close(); + else + _sslEndPoint.shutdownOutput(); } catch (IOException e) { From 93a89026ce51bb8dc38cd44093365b2b62bdfc2b Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 21 Dec 2011 12:44:50 +1100 Subject: [PATCH 3/4] 364921 Improved timeout tests --- .../jetty/server/ConnectorTimeoutTest.java | 101 +++++++++++++++++- 1 file changed, 99 insertions(+), 2 deletions(-) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ConnectorTimeoutTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ConnectorTimeoutTest.java index 7b2e2b5650e..0b66eda2f93 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ConnectorTimeoutTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ConnectorTimeoutTest.java @@ -15,21 +15,23 @@ package org.eclipse.jetty.server; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.matchers.JUnitMatchers.containsString; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.net.Socket; +import java.net.SocketException; +import java.util.concurrent.TimeUnit; import javax.net.ssl.SSLException; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import junit.framework.Assert; - import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.util.IO; +import org.junit.Assert; import org.junit.Test; public abstract class ConnectorTimeoutTest extends HttpServerTestFixture @@ -103,6 +105,101 @@ public abstract class ConnectorTimeoutTest extends HttpServerTestFixture Assert.assertTrue(System.currentTimeMillis()-start>200); Assert.assertTrue(System.currentTimeMillis()-start<5000); } + + @Test + public void testMaxIdleWithRequest10NoClientClose() throws Exception + { + configureServer(new HelloWorldHandler()); + Socket client=newSocket(HOST,_connector.getLocalPort()); + client.setSoTimeout(10000); + + assertFalse(client.isClosed()); + + OutputStream os=client.getOutputStream(); + InputStream is=client.getInputStream(); + + os.write(( + "GET / HTTP/1.0\r\n"+ + "host: "+HOST+":"+_connector.getLocalPort()+"\r\n"+ + "connection: close\r\n"+ + "\r\n").getBytes("utf-8")); + os.flush(); + + String result=IO.toString(is); + Assert.assertThat("OK",result,containsString("200 OK")); + assertEquals(-1, is.read()); + + TimeUnit.MILLISECONDS.sleep(MAX_IDLE_TIME); + + // further writes will get broken pipe or similar + try + { + for (int i=0;i<100;i++) + { + os.write(( + "GET / HTTP/1.0\r\n"+ + "host: "+HOST+":"+_connector.getLocalPort()+"\r\n"+ + "connection: keep-alive\r\n"+ + "\r\n").getBytes("utf-8")); + os.flush(); + } + Assert.fail("half close should have timed out"); + } + catch(SocketException e) + { + // expected + } + } + + @Test + public void testMaxIdleWithRequest11NoClientClose() throws Exception + { + configureServer(new EchoHandler()); + Socket client=newSocket(HOST,_connector.getLocalPort()); + client.setSoTimeout(10000); + + assertFalse(client.isClosed()); + + OutputStream os=client.getOutputStream(); + InputStream is=client.getInputStream(); + + String content="Wibble"; + byte[] contentB=content.getBytes("utf-8"); + os.write(( + "POST /echo HTTP/1.1\r\n"+ + "host: "+HOST+":"+_connector.getLocalPort()+"\r\n"+ + "content-type: text/plain; charset=utf-8\r\n"+ + "content-length: "+contentB.length+"\r\n"+ + "connection: close\r\n"+ + "\r\n").getBytes("utf-8")); + os.write(contentB); + os.flush(); + + IO.toString(is); + + assertEquals(-1, is.read()); + + TimeUnit.MILLISECONDS.sleep(MAX_IDLE_TIME); + + // further writes will get broken pipe or similar + try + { + for (int i=0;i<100;i++) + { + os.write(( + "GET / HTTP/1.0\r\n"+ + "host: "+HOST+":"+_connector.getLocalPort()+"\r\n"+ + "connection: keep-alive\r\n"+ + "\r\n").getBytes("utf-8")); + os.flush(); + } + Assert.fail("half close should have timed out"); + } + catch(SocketException e) + { + // expected + } + } @Test From 2028bbb444196d7e1b9d5f1813e44aa47e099b2f Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Wed, 21 Dec 2011 12:57:33 +1100 Subject: [PATCH 4/4] JETTY-1465 NPE in ContextHandler.toString --- .../jetty/server/handler/ContextHandler.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index 8af31771943..1ed28f49f64 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -1381,14 +1381,17 @@ public class ContextHandler extends ScopedHandler implements Attributes, Server. StringBuilder b = new StringBuilder(); - String p = getClass().getPackage().getName(); - if (p != null && p.length() > 0) + Package pkg = getClass().getPackage(); + if (pkg != null) { - String[] ss = p.split("\\."); - for (String s : ss) - b.append(s.charAt(0)).append('.'); + String p = pkg.getName(); + if (p != null && p.length() > 0) + { + String[] ss = p.split("\\."); + for (String s : ss) + b.append(s.charAt(0)).append('.'); + } } - b.append(getClass().getSimpleName()); b.append('{').append(getContextPath()).append(',').append(getBaseResource());