From f7b382064f05de2c2176e313db25ddf52dde66e9 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 8 Jan 2015 12:06:53 -0700 Subject: [PATCH 1/5] 457017 - Reflective call to websocket methods that fail have ambiguous exceptions + Making JSR onOpen close and use onError properly, as well we unwrapping the InvocationTargetException cause as to WHY the call to onOpen failed. --- .../jsr356/annotations/JsrEvents.java | 2 +- .../annotations/OnMessagePongCallable.java | 4 +- .../endpoints/JsrAnnotatedEventDriver.java | 54 ++++---- .../endpoints/JsrEndpointEventDriver.java | 22 +-- .../misbehaving/AnnotatedRuntimeOnOpen.java | 65 +++++++++ .../misbehaving/EndpointRuntimeOnOpen.java | 63 +++++++++ .../misbehaving/MisbehavingClassTest.java | 127 ++++++++++++++++++ .../jsr356/server/MemoryUsageTest.java | 2 + .../jetty/websocket/api/CloseStatus.java | 5 + .../websocket/common/WebSocketSession.java | 2 +- .../websocket/common/events/EventDriver.java | 4 +- .../events/JettyAnnotatedEventDriver.java | 20 ++- .../AbstractMethodAnnotationScanner.java | 9 +- .../events/annotated/CallableMethod.java | 75 +++++++---- .../annotated/InvalidSignatureException.java | 5 +- 15 files changed, 377 insertions(+), 82 deletions(-) create mode 100644 jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/misbehaving/AnnotatedRuntimeOnOpen.java create mode 100644 jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/misbehaving/EndpointRuntimeOnOpen.java create mode 100644 jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/misbehaving/MisbehavingClassTest.java diff --git a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/annotations/JsrEvents.java b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/annotations/JsrEvents.java index e1fc4eba733..745daabe0f3 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/annotations/JsrEvents.java +++ b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/annotations/JsrEvents.java @@ -162,7 +162,7 @@ public class JsrEvents onOpen.call(websocket,config); } - public void callPong(RemoteEndpoint.Async endpoint, Object websocket, ByteBuffer pong) throws DecodeException, IOException + public void callPong(RemoteEndpoint.Async endpoint, Object websocket, ByteBuffer pong) { if (onPong == null) { diff --git a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/annotations/OnMessagePongCallable.java b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/annotations/OnMessagePongCallable.java index c7ff794712c..8da55978555 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/annotations/OnMessagePongCallable.java +++ b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/annotations/OnMessagePongCallable.java @@ -21,8 +21,6 @@ package org.eclipse.jetty.websocket.jsr356.annotations; import java.lang.reflect.Method; import java.nio.ByteBuffer; -import javax.websocket.DecodeException; - import org.eclipse.jetty.websocket.jsr356.JsrPongMessage; import org.eclipse.jetty.websocket.jsr356.JsrSession; import org.eclipse.jetty.websocket.jsr356.annotations.Param.Role; @@ -45,7 +43,7 @@ public class OnMessagePongCallable extends OnMessageCallable super(copy); } - public Object call(Object endpoint, ByteBuffer buf) throws DecodeException + public Object call(Object endpoint, ByteBuffer buf) { super.args[idxMessageObject] = new JsrPongMessage(buf); return super.call(endpoint,super.args); diff --git a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/endpoints/JsrAnnotatedEventDriver.java b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/endpoints/JsrAnnotatedEventDriver.java index 3fe2f11da17..8ec9efb3dcd 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/endpoints/JsrAnnotatedEventDriver.java +++ b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/endpoints/JsrAnnotatedEventDriver.java @@ -23,6 +23,7 @@ import java.io.InputStream; import java.io.Reader; import java.nio.ByteBuffer; import java.util.Map; + import javax.websocket.CloseReason; import javax.websocket.DecodeException; @@ -123,7 +124,7 @@ public class JsrAnnotatedEventDriver extends AbstractJsrEventDriver { events.callBinaryStream(jsrsession.getAsyncRemote(),websocket,stream); } - catch (DecodeException | IOException e) + catch (Throwable e) { onFatalError(e); } @@ -167,7 +168,7 @@ public class JsrAnnotatedEventDriver extends AbstractJsrEventDriver // FIN is always true here events.callBinary(jsrsession.getAsyncRemote(),websocket,buf,true); } - catch (DecodeException e) + catch (Throwable e) { onFatalError(e); } @@ -188,7 +189,15 @@ public class JsrAnnotatedEventDriver extends AbstractJsrEventDriver @Override public void onError(Throwable cause) { - events.callError(websocket,cause); + try + { + events.callError(websocket,cause); + } + catch (Throwable e) + { + LOG.warn("Unable to call onError with cause", cause); + LOG.warn("Call to onError resulted in exception", e); + } } private void onFatalError(Throwable t) @@ -203,15 +212,15 @@ public class JsrAnnotatedEventDriver extends AbstractJsrEventDriver } @Override - public void onInputStream(InputStream stream) + public void onInputStream(InputStream stream) throws IOException { try { events.callBinaryStream(jsrsession.getAsyncRemote(),websocket,stream); } - catch (DecodeException | IOException e) + catch (DecodeException e) { - onFatalError(e); + throw new RuntimeException("Unable decode input stream", e); } } @@ -223,7 +232,7 @@ public class JsrAnnotatedEventDriver extends AbstractJsrEventDriver } catch (DecodeException e) { - onFatalError(e); + throw new RuntimeException("Unable decode partial binary message", e); } } @@ -235,46 +244,33 @@ public class JsrAnnotatedEventDriver extends AbstractJsrEventDriver } catch (DecodeException e) { - onFatalError(e); + throw new RuntimeException("Unable decode partial text message", e); } } @Override public void onPing(ByteBuffer buffer) { - try - { - events.callPong(jsrsession.getAsyncRemote(),websocket,buffer); - } - catch (DecodeException | IOException e) - { - onFatalError(e); - } + // Call pong, as there is no "onPing" method in the JSR + events.callPong(jsrsession.getAsyncRemote(),websocket,buffer); } @Override public void onPong(ByteBuffer buffer) { - try - { - events.callPong(jsrsession.getAsyncRemote(),websocket,buffer); - } - catch (DecodeException | IOException e) - { - onFatalError(e); - } + events.callPong(jsrsession.getAsyncRemote(),websocket,buffer); } @Override - public void onReader(Reader reader) + public void onReader(Reader reader) throws IOException { try { events.callTextStream(jsrsession.getAsyncRemote(),websocket,reader); } - catch (DecodeException | IOException e) + catch (DecodeException e) { - onFatalError(e); + throw new RuntimeException("Unable decode reader", e); } } @@ -343,7 +339,7 @@ public class JsrAnnotatedEventDriver extends AbstractJsrEventDriver { events.callTextStream(jsrsession.getAsyncRemote(),websocket,stream); } - catch (DecodeException | IOException e) + catch (Throwable e) { onFatalError(e); } @@ -380,7 +376,7 @@ public class JsrAnnotatedEventDriver extends AbstractJsrEventDriver // FIN is always true here events.callText(jsrsession.getAsyncRemote(),websocket,message,true); } - catch (DecodeException e) + catch (Throwable e) { onFatalError(e); } diff --git a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/endpoints/JsrEndpointEventDriver.java b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/endpoints/JsrEndpointEventDriver.java index 0cc577e9455..c96ffaafc92 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/endpoints/JsrEndpointEventDriver.java +++ b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/endpoints/JsrEndpointEventDriver.java @@ -23,6 +23,7 @@ import java.io.InputStream; import java.io.Reader; import java.nio.ByteBuffer; import java.util.Map; + import javax.websocket.CloseReason; import javax.websocket.Endpoint; import javax.websocket.MessageHandler; @@ -134,20 +135,23 @@ public class JsrEndpointEventDriver extends AbstractJsrEventDriver { LOG.debug("onConnect({}, {})",jsrsession,config); } - try - { - endpoint.onOpen(jsrsession,config); - } - catch (Throwable t) - { - LOG.warn("Uncaught exception",t); - } + + // Let unhandled exceptions flow out + endpoint.onOpen(jsrsession,config); } @Override public void onError(Throwable cause) { - endpoint.onError(jsrsession,cause); + LOG.warn(cause); + try + { + endpoint.onError(jsrsession,cause); + } + catch (Throwable t) + { + LOG.warn("Unable to report to onError due to exception",t); + } } @Override diff --git a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/misbehaving/AnnotatedRuntimeOnOpen.java b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/misbehaving/AnnotatedRuntimeOnOpen.java new file mode 100644 index 00000000000..e841e7d441b --- /dev/null +++ b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/misbehaving/AnnotatedRuntimeOnOpen.java @@ -0,0 +1,65 @@ +// +// ======================================================================== +// Copyright (c) 1995-2015 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.websocket.jsr356.misbehaving; + +import java.util.LinkedList; +import java.util.concurrent.CountDownLatch; + +import javax.websocket.ClientEndpoint; +import javax.websocket.CloseReason; +import javax.websocket.EndpointConfig; +import javax.websocket.OnClose; +import javax.websocket.OnError; +import javax.websocket.OnOpen; +import javax.websocket.Session; + +/** + * A JSR-356 Annotated that tosses a RuntimeException during its onOpen call + */ +@ClientEndpoint +public class AnnotatedRuntimeOnOpen +{ + public CountDownLatch closeLatch = new CountDownLatch(1); + public CloseReason closeReason; + public LinkedList errors = new LinkedList<>(); + + @OnOpen + public void onOpen(Session session, EndpointConfig config) + { + // Intentional runtime exception. + int[] arr = new int[5]; + for (int i = 0; i < 10; i++) + { + arr[i] = 222; + } + } + + @OnClose + public void onClose(Session session, CloseReason closeReason) + { + this.closeReason = closeReason; + closeLatch.countDown(); + } + + @OnError + public void onError(Session session, Throwable thr) + { + errors.add(thr); + } +} diff --git a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/misbehaving/EndpointRuntimeOnOpen.java b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/misbehaving/EndpointRuntimeOnOpen.java new file mode 100644 index 00000000000..b9a9eeab762 --- /dev/null +++ b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/misbehaving/EndpointRuntimeOnOpen.java @@ -0,0 +1,63 @@ +// +// ======================================================================== +// Copyright (c) 1995-2015 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.websocket.jsr356.misbehaving; + +import java.util.LinkedList; +import java.util.concurrent.CountDownLatch; + +import javax.websocket.CloseReason; +import javax.websocket.Endpoint; +import javax.websocket.EndpointConfig; +import javax.websocket.Session; + +/** + * A JSR-356 Endpoint that tosses a RuntimeException during its onOpen call + */ +public class EndpointRuntimeOnOpen extends Endpoint +{ + public CountDownLatch closeLatch = new CountDownLatch(1); + public CloseReason closeReason; + public LinkedList errors = new LinkedList<>(); + + @Override + public void onOpen(Session session, EndpointConfig config) + { + // Intentional runtime exception. + int[] arr = new int[5]; + for (int i = 0; i < 10; i++) + { + arr[i] = 222; + } + } + + @Override + public void onClose(Session session, CloseReason closeReason) + { + super.onClose(session,closeReason); + this.closeReason = closeReason; + closeLatch.countDown(); + } + + @Override + public void onError(Session session, Throwable thr) + { + super.onError(session,thr); + errors.add(thr); + } +} diff --git a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/misbehaving/MisbehavingClassTest.java b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/misbehaving/MisbehavingClassTest.java new file mode 100644 index 00000000000..ce006483b96 --- /dev/null +++ b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/misbehaving/MisbehavingClassTest.java @@ -0,0 +1,127 @@ +// +// ======================================================================== +// Copyright (c) 1995-2015 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.websocket.jsr356.misbehaving; + +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; + +import java.net.URI; +import java.util.concurrent.TimeUnit; + +import javax.websocket.ContainerProvider; +import javax.websocket.Session; +import javax.websocket.WebSocketContainer; + +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.server.handler.ContextHandler; +import org.eclipse.jetty.util.log.StacklessLogging; +import org.eclipse.jetty.websocket.jsr356.EchoHandler; +import org.eclipse.jetty.websocket.jsr356.endpoints.JsrEndpointEventDriver; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +public class MisbehavingClassTest +{ + private static Server server; + private static EchoHandler handler; + private static URI serverUri; + + @BeforeClass + public static void startServer() throws Exception + { + server = new Server(); + ServerConnector connector = new ServerConnector(server); + server.addConnector(connector); + + handler = new EchoHandler(); + + ContextHandler context = new ContextHandler(); + context.setContextPath("/"); + context.setHandler(handler); + server.setHandler(context); + + // Start Server + server.start(); + + String host = connector.getHost(); + if (host == null) + { + host = "localhost"; + } + int port = connector.getLocalPort(); + serverUri = new URI(String.format("ws://%s:%d/",host,port)); + } + + @AfterClass + public static void stopServer() + { + try + { + server.stop(); + } + catch (Exception e) + { + e.printStackTrace(System.err); + } + } + + @Test + public void testEndpointRuntimeOnOpen() throws Exception + { + WebSocketContainer container = ContainerProvider.getWebSocketContainer(); + EndpointRuntimeOnOpen socket = new EndpointRuntimeOnOpen(); + + try (StacklessLogging logging = new StacklessLogging(EndpointRuntimeOnOpen.class,JsrEndpointEventDriver.class)) + { + // expecting ArrayIndexOutOfBoundsException during onOpen + Session session = container.connectToServer(socket,serverUri); + assertThat("Close should have occurred",socket.closeLatch.await(1,TimeUnit.SECONDS),is(true)); + + // technically, the session object isn't invalid here. + assertThat("Session.isOpen",session.isOpen(),is(false)); + assertThat("Should have only had 1 error",socket.errors.size(),is(1)); + + Throwable cause = socket.errors.pop(); + assertThat("Error",cause,instanceOf(ArrayIndexOutOfBoundsException.class)); + } + } + + @Test + public void testAnnotatedRuntimeOnOpen() throws Exception + { + WebSocketContainer container = ContainerProvider.getWebSocketContainer(); + AnnotatedRuntimeOnOpen socket = new AnnotatedRuntimeOnOpen(); + + try (StacklessLogging logging = new StacklessLogging(AnnotatedRuntimeOnOpen.class)) + { + // expecting ArrayIndexOutOfBoundsException during onOpen + Session session = container.connectToServer(socket,serverUri); + assertThat("Close should have occurred",socket.closeLatch.await(1,TimeUnit.SECONDS),is(true)); + + // technically, the session object isn't invalid here. + assertThat("Session.isOpen",session.isOpen(),is(false)); + assertThat("Should have only had 1 error",socket.errors.size(),is(1)); + + Throwable cause = socket.errors.pop(); + assertThat("Error",cause,instanceOf(ArrayIndexOutOfBoundsException.class)); + } + } +} diff --git a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/MemoryUsageTest.java b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/MemoryUsageTest.java index 13167d63c30..854f28e75da 100644 --- a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/MemoryUsageTest.java +++ b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/MemoryUsageTest.java @@ -26,6 +26,7 @@ import java.lang.management.MemoryUsage; import java.net.URI; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; + import javax.websocket.ContainerProvider; import javax.websocket.Endpoint; import javax.websocket.EndpointConfig; @@ -74,6 +75,7 @@ public class MemoryUsageTest server.stop(); } + @SuppressWarnings("unused") @Test public void testMemoryUsage() throws Exception { diff --git a/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/CloseStatus.java b/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/CloseStatus.java index bb804036736..417221e9261 100644 --- a/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/CloseStatus.java +++ b/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/CloseStatus.java @@ -32,6 +32,11 @@ public class CloseStatus */ public static String trimMaxReasonLength(String reason) { + if (reason == null) + { + return null; + } + if (reason.length() > MAX_REASON_PHRASE) { return reason.substring(0,MAX_REASON_PHRASE); diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketSession.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketSession.java index 23b8bbdde06..cfd5323d52b 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketSession.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketSession.java @@ -105,7 +105,7 @@ public class WebSocketSession extends ContainerLifeCycle implements Session, Inc @Override public void close(int statusCode, String reason) { - connection.close(statusCode,reason); + connection.close(statusCode,CloseStatus.trimMaxReasonLength(reason)); } /** diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/EventDriver.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/EventDriver.java index b0a47876f7c..e4d4d836bf1 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/EventDriver.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/EventDriver.java @@ -49,13 +49,13 @@ public interface EventDriver extends IncomingFrames public void onFrame(Frame frame); - public void onInputStream(InputStream stream); + public void onInputStream(InputStream stream) throws IOException; public void onPing(ByteBuffer buffer); public void onPong(ByteBuffer buffer); - public void onReader(Reader reader); + public void onReader(Reader reader) throws IOException; public void onTextFrame(ByteBuffer buffer, boolean fin) throws IOException; diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/JettyAnnotatedEventDriver.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/JettyAnnotatedEventDriver.java index e6044102ba2..45dcaf2b70e 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/JettyAnnotatedEventDriver.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/JettyAnnotatedEventDriver.java @@ -86,7 +86,15 @@ public class JettyAnnotatedEventDriver extends AbstractEventDriver @Override public void run() { - events.onBinary.call(websocket,session,msg); + try + { + events.onBinary.call(websocket,session,msg); + } + catch (Throwable t) + { + // dispatched calls need to be reported + onError(t); + } } }); } @@ -188,7 +196,15 @@ public class JettyAnnotatedEventDriver extends AbstractEventDriver @Override public void run() { - events.onText.call(websocket,session,msg); + try + { + events.onText.call(websocket,session,msg); + } + catch (Throwable t) + { + // dispatched calls need to be reported + onError(t); + } } }); } diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/annotated/AbstractMethodAnnotationScanner.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/annotated/AbstractMethodAnnotationScanner.java index 741d6a30eff..480e10b3539 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/annotated/AbstractMethodAnnotationScanner.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/annotated/AbstractMethodAnnotationScanner.java @@ -22,7 +22,6 @@ import java.lang.annotation.Annotation; import java.lang.reflect.Method; import java.lang.reflect.Modifier; -import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.websocket.api.InvalidWebSocketException; import org.eclipse.jetty.websocket.common.events.ParamList; @@ -39,7 +38,7 @@ public abstract class AbstractMethodAnnotationScanner StringBuilder err = new StringBuilder(); err.append("Invalid declaration of "); err.append(method); - err.append(StringUtil.__LINE_SEPARATOR); + err.append(System.lineSeparator()); err.append("Method modifier must be public"); @@ -51,7 +50,7 @@ public abstract class AbstractMethodAnnotationScanner StringBuilder err = new StringBuilder(); err.append("Invalid declaration of "); err.append(method); - err.append(StringUtil.__LINE_SEPARATOR); + err.append(System.lineSeparator()); err.append("Method modifier may not be static"); @@ -66,7 +65,7 @@ public abstract class AbstractMethodAnnotationScanner StringBuilder err = new StringBuilder(); err.append("Invalid declaration of "); err.append(method); - err.append(StringUtil.__LINE_SEPARATOR); + err.append(System.lineSeparator()); err.append("Return type must be ").append(type); @@ -87,7 +86,7 @@ public abstract class AbstractMethodAnnotationScanner StringBuilder err = new StringBuilder(); err.append("Duplicate @").append(annoClass.getSimpleName()).append(" declaration on "); err.append(method); - err.append(StringUtil.__LINE_SEPARATOR); + err.append(System.lineSeparator()); err.append("@").append(annoClass.getSimpleName()).append(" previously declared at "); err.append(callable.getMethod()); diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/annotated/CallableMethod.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/annotated/CallableMethod.java index eaca8e0a4d6..69b97c6e3fa 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/annotated/CallableMethod.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/annotated/CallableMethod.java @@ -24,7 +24,6 @@ import java.util.Objects; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; -import org.eclipse.jetty.websocket.api.WebSocketException; import org.eclipse.jetty.websocket.common.util.ReflectUtils; /** @@ -70,36 +69,58 @@ public class CallableMethod { return this.method.invoke(obj,args); } - catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException e) + catch (Throwable t) { - StringBuilder err = new StringBuilder(); - err.append("Cannot call method "); - err.append(ReflectUtils.toString(pojo,method)); - err.append(" with args: ["); - - boolean delim = false; - for (Object arg : args) - { - if (delim) - { - err.append(", "); - } - if (arg == null) - { - err.append(""); - } - else - { - err.append(arg.getClass().getName()); - } - delim = true; - } - err.append("]"); - - throw new WebSocketException(err.toString(),e); + String err = formatMethodCallError(args); + throw unwrapRuntimeException(err,t); } } + private RuntimeException unwrapRuntimeException(String err, final Throwable t) + { + Throwable ret = t; + + while (ret instanceof InvocationTargetException) + { + ret = ((InvocationTargetException)ret).getCause(); + } + + if (ret instanceof RuntimeException) + { + return (RuntimeException)ret; + } + + return new RuntimeException(err,ret); + } + + public String formatMethodCallError(Object... args) + { + StringBuilder err = new StringBuilder(); + err.append("Cannot call method "); + err.append(ReflectUtils.toString(pojo,method)); + err.append(" with args: ["); + + boolean delim = false; + for (Object arg : args) + { + if (delim) + { + err.append(", "); + } + if (arg == null) + { + err.append(""); + } + else + { + err.append(arg.getClass().getName()); + } + delim = true; + } + err.append("]"); + return err.toString(); + } + public Method getMethod() { return method; diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/annotated/InvalidSignatureException.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/annotated/InvalidSignatureException.java index 2c2297e6d81..44459645bf5 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/annotated/InvalidSignatureException.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/annotated/InvalidSignatureException.java @@ -21,7 +21,6 @@ package org.eclipse.jetty.websocket.common.events.annotated; import java.lang.annotation.Annotation; import java.lang.reflect.Method; -import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.websocket.api.InvalidWebSocketException; import org.eclipse.jetty.websocket.common.events.ParamList; @@ -34,7 +33,7 @@ public class InvalidSignatureException extends InvalidWebSocketException StringBuilder err = new StringBuilder(); err.append("Invalid declaration of "); err.append(method); - err.append(StringUtil.__LINE_SEPARATOR); + err.append(System.lineSeparator()); err.append("Acceptable method declarations for @"); err.append(annoClass.getSimpleName()); @@ -43,7 +42,7 @@ public class InvalidSignatureException extends InvalidWebSocketException { for (Class[] params : validParams) { - err.append(StringUtil.__LINE_SEPARATOR); + err.append(System.lineSeparator()); err.append("public void ").append(method.getName()); err.append('('); boolean delim = false; From 0984796282ff91fe1ac0c9e0ff44d23de010a57d Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 8 Jan 2015 12:13:45 -0700 Subject: [PATCH 2/5] Cleanup of .gitignore --- .gitignore | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/.gitignore b/.gitignore index ff5ed7e2d15..52260df3621 100644 --- a/.gitignore +++ b/.gitignore @@ -2,21 +2,30 @@ .classpath .project .settings -.gitignore # maven target/ */src/main/java/META-INF/ *.versionsBackup +*.releaseBackup bin/ # common junk *.log -*.swp *.diff *.patch +*.sw[a-p] +*.bak +*.backup +*.debug +*.dump -# intellij +# vim +.*.sw[a-p] +*~ +~* + +# intellij / android studio *.iml *.ipr *.iws @@ -32,12 +41,5 @@ bin/ # netbeans /nbproject -# vim -.*.sw[a-p] - # merge tooling *.orig - -#maven -*.versionsBackup -*.releaseBackup From 3456c78d54e984b4934efd7eb9758a3b4a51a2d0 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 8 Jan 2015 14:38:12 -0700 Subject: [PATCH 3/5] 457017 - Reflective call to websocket methods that fail have ambiguous exceptions + Ensuring that the Jetty WebSocket API behaves in the same way --- .../AnnotatedRuntimeOnConnectSocket.java | 70 +++++++++ .../server/misbehaving/BadSocketsServlet.java | 56 ++++++++ .../ListenerRuntimeOnConnectSocket.java | 74 ++++++++++ .../misbehaving/MisbehavingClassTest.java | 133 ++++++++++++++++++ 4 files changed, 333 insertions(+) create mode 100644 jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/misbehaving/AnnotatedRuntimeOnConnectSocket.java create mode 100644 jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/misbehaving/BadSocketsServlet.java create mode 100644 jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/misbehaving/ListenerRuntimeOnConnectSocket.java create mode 100644 jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/misbehaving/MisbehavingClassTest.java diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/misbehaving/AnnotatedRuntimeOnConnectSocket.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/misbehaving/AnnotatedRuntimeOnConnectSocket.java new file mode 100644 index 00000000000..c8d5afd498e --- /dev/null +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/misbehaving/AnnotatedRuntimeOnConnectSocket.java @@ -0,0 +1,70 @@ +// +// ======================================================================== +// Copyright (c) 1995-2015 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.websocket.server.misbehaving; + +import java.util.LinkedList; +import java.util.concurrent.CountDownLatch; + +import org.eclipse.jetty.websocket.api.Session; +import org.eclipse.jetty.websocket.api.annotations.OnWebSocketClose; +import org.eclipse.jetty.websocket.api.annotations.OnWebSocketConnect; +import org.eclipse.jetty.websocket.api.annotations.OnWebSocketError; +import org.eclipse.jetty.websocket.api.annotations.WebSocket; + +@WebSocket +public class AnnotatedRuntimeOnConnectSocket +{ + public LinkedList errors = new LinkedList<>(); + public CountDownLatch closeLatch = new CountDownLatch(1); + public int closeStatusCode; + public String closeReason; + + @OnWebSocketConnect + public void onWebSocketConnect(Session sess) + { + // Intentional runtime exception. + int[] arr = new int[5]; + for (int i = 0; i < 10; i++) + { + arr[i] = 222; + } + } + + @OnWebSocketClose + public void onWebSocketClose(int statusCode, String reason) + { + closeLatch.countDown(); + closeStatusCode = statusCode; + closeReason = reason; + } + + @OnWebSocketError + public void onWebSocketError(Throwable cause) + { + this.errors.add(cause); + } + + public void reset() + { + this.closeLatch = new CountDownLatch(1); + this.closeStatusCode = -1; + this.closeReason = null; + this.errors.clear(); + } +} diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/misbehaving/BadSocketsServlet.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/misbehaving/BadSocketsServlet.java new file mode 100644 index 00000000000..fad8d82e4d6 --- /dev/null +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/misbehaving/BadSocketsServlet.java @@ -0,0 +1,56 @@ +// +// ======================================================================== +// Copyright (c) 1995-2015 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.websocket.server.misbehaving; + +import org.eclipse.jetty.websocket.servlet.ServletUpgradeRequest; +import org.eclipse.jetty.websocket.servlet.ServletUpgradeResponse; +import org.eclipse.jetty.websocket.servlet.WebSocketCreator; +import org.eclipse.jetty.websocket.servlet.WebSocketServlet; +import org.eclipse.jetty.websocket.servlet.WebSocketServletFactory; + +@SuppressWarnings("serial") +public class BadSocketsServlet extends WebSocketServlet implements WebSocketCreator +{ + public ListenerRuntimeOnConnectSocket listenerRuntimeConnect; + public AnnotatedRuntimeOnConnectSocket annotatedRuntimeConnect; + + @Override + public void configure(WebSocketServletFactory factory) + { + factory.setCreator(this); + + this.listenerRuntimeConnect = new ListenerRuntimeOnConnectSocket(); + this.annotatedRuntimeConnect = new AnnotatedRuntimeOnConnectSocket(); + } + + @Override + public Object createWebSocket(ServletUpgradeRequest req, ServletUpgradeResponse resp) + { + if (req.hasSubProtocol("listener-runtime-connect")) + { + return this.listenerRuntimeConnect; + } + else if (req.hasSubProtocol("annotated-runtime-connect")) + { + return this.annotatedRuntimeConnect; + } + + return null; + } +} diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/misbehaving/ListenerRuntimeOnConnectSocket.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/misbehaving/ListenerRuntimeOnConnectSocket.java new file mode 100644 index 00000000000..7ff032716a6 --- /dev/null +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/misbehaving/ListenerRuntimeOnConnectSocket.java @@ -0,0 +1,74 @@ +// +// ======================================================================== +// Copyright (c) 1995-2015 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.websocket.server.misbehaving; + +import java.util.LinkedList; +import java.util.concurrent.CountDownLatch; + +import org.eclipse.jetty.websocket.api.Session; +import org.eclipse.jetty.websocket.api.WebSocketAdapter; + +public class ListenerRuntimeOnConnectSocket extends WebSocketAdapter +{ + public LinkedList errors = new LinkedList<>(); + public CountDownLatch closeLatch = new CountDownLatch(1); + public int closeStatusCode; + public String closeReason; + + @Override + public void onWebSocketConnect(Session sess) + { + super.onWebSocketConnect(sess); + + // Intentional runtime exception. + int[] arr = new int[5]; + for (int i = 0; i < 10; i++) + { + arr[i] = 222; + } + } + + @Override + public void onWebSocketClose(int statusCode, String reason) + { + closeLatch.countDown(); + closeStatusCode = statusCode; + closeReason = reason; + } + + @Override + public void onWebSocketError(Throwable cause) + { + this.errors.add(cause); + } + + @Override + public void onWebSocketText(String message) + { + getRemote().sendStringByFuture(message); + } + + public void reset() + { + this.closeLatch = new CountDownLatch(1); + this.closeStatusCode = -1; + this.closeReason = null; + this.errors.clear(); + } +} diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/misbehaving/MisbehavingClassTest.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/misbehaving/MisbehavingClassTest.java new file mode 100644 index 00000000000..ddeeda9b394 --- /dev/null +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/misbehaving/MisbehavingClassTest.java @@ -0,0 +1,133 @@ +// +// ======================================================================== +// Copyright (c) 1995-2015 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.websocket.server.misbehaving; + +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; + +import java.util.concurrent.TimeUnit; + +import org.eclipse.jetty.toolchain.test.EventQueue; +import org.eclipse.jetty.util.log.StacklessLogging; +import org.eclipse.jetty.websocket.api.StatusCode; +import org.eclipse.jetty.websocket.common.CloseInfo; +import org.eclipse.jetty.websocket.common.OpCode; +import org.eclipse.jetty.websocket.common.WebSocketFrame; +import org.eclipse.jetty.websocket.common.events.AbstractEventDriver; +import org.eclipse.jetty.websocket.common.test.BlockheadClient; +import org.eclipse.jetty.websocket.server.SimpleServletServer; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +/** + * Testing badly behaving Socket class implementations to get the best + * error messages and state out of the websocket implementation. + */ +public class MisbehavingClassTest +{ + private static SimpleServletServer server; + private static BadSocketsServlet badSocketsServlet; + + @BeforeClass + public static void startServer() throws Exception + { + badSocketsServlet = new BadSocketsServlet(); + server = new SimpleServletServer(badSocketsServlet); + server.start(); + } + + @AfterClass + public static void stopServer() + { + server.stop(); + } + + @Test + public void testListenerRuntimeOnConnect() throws Exception + { + try (BlockheadClient client = new BlockheadClient(server.getServerUri())) + { + client.setProtocols("listener-runtime-connect"); + client.setTimeout(1,TimeUnit.SECONDS); + try (StacklessLogging scope = new StacklessLogging(AbstractEventDriver.class)) + { + ListenerRuntimeOnConnectSocket socket = badSocketsServlet.listenerRuntimeConnect; + socket.reset(); + + client.connect(); + client.sendStandardRequest(); + client.expectUpgradeResponse(); + + EventQueue frames = client.readFrames(1,1,TimeUnit.SECONDS); + WebSocketFrame frame = frames.poll(); + assertThat("frames[0].opcode",frame.getOpCode(),is(OpCode.CLOSE)); + CloseInfo close = new CloseInfo(frame); + assertThat("Close Status Code",close.getStatusCode(),is(StatusCode.SERVER_ERROR)); + + client.write(close.asFrame()); // respond with close + + // ensure server socket got close event + assertThat("Close Latch",socket.closeLatch.await(1,TimeUnit.SECONDS),is(true)); + assertThat("closeStatusCode",socket.closeStatusCode,is(StatusCode.SERVER_ERROR)); + + // Validate errors + assertThat("socket.onErrors",socket.errors.size(),is(1)); + Throwable cause = socket.errors.pop(); + assertThat("Error type",cause,instanceOf(ArrayIndexOutOfBoundsException.class)); + } + } + } + + @Test + public void testAnnotatedRuntimeOnConnect() throws Exception + { + try (BlockheadClient client = new BlockheadClient(server.getServerUri())) + { + client.setProtocols("annotated-runtime-connect"); + client.setTimeout(1,TimeUnit.SECONDS); + try (StacklessLogging scope = new StacklessLogging(AbstractEventDriver.class)) + { + AnnotatedRuntimeOnConnectSocket socket = badSocketsServlet.annotatedRuntimeConnect; + socket.reset(); + + client.connect(); + client.sendStandardRequest(); + client.expectUpgradeResponse(); + + EventQueue frames = client.readFrames(1,1,TimeUnit.SECONDS); + WebSocketFrame frame = frames.poll(); + assertThat("frames[0].opcode",frame.getOpCode(),is(OpCode.CLOSE)); + CloseInfo close = new CloseInfo(frame); + assertThat("Close Status Code",close.getStatusCode(),is(StatusCode.SERVER_ERROR)); + + client.write(close.asFrame()); // respond with close + + // ensure server socket got close event + assertThat("Close Latch",socket.closeLatch.await(1,TimeUnit.SECONDS),is(true)); + assertThat("closeStatusCode",socket.closeStatusCode,is(StatusCode.SERVER_ERROR)); + + // Validate errors + assertThat("socket.onErrors",socket.errors.size(),is(1)); + Throwable cause = socket.errors.pop(); + assertThat("Error type",cause,instanceOf(ArrayIndexOutOfBoundsException.class)); + } + } + } +} From adae3193d8adf641f9e819838595de375e136b0d Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 9 Jan 2015 11:40:47 +0100 Subject: [PATCH 4/5] 457130 - HTTPS request with IP host and HTTP proxy throws IllegalArgumentException. Fixed by handling the case of non-URI request target. --- .../eclipse/jetty/client/HttpConnection.java | 12 ++-- .../org/eclipse/jetty/client/HttpRequest.java | 63 +++++++++++++------ .../jetty/proxy/ProxyTunnellingTest.java | 9 +-- 3 files changed, 58 insertions(+), 26 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java index 28e5ce19a2c..54ace6faf68 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java @@ -134,10 +134,14 @@ public abstract class HttpConnection implements Connection CookieStore cookieStore = getHttpClient().getCookieStore(); if (cookieStore != null) { - StringBuilder cookies = convertCookies(cookieStore.get(request.getURI()), null); - cookies = convertCookies(request.getCookies(), cookies); - if (cookies != null) - request.header(HttpHeader.COOKIE.asString(), cookies.toString()); + URI uri = request.getURI(); + if (uri != null) + { + StringBuilder cookies = convertCookies(cookieStore.get(uri), null); + cookies = convertCookies(request.getCookies(), cookies); + if (cookies != null) + request.header(HttpHeader.COOKIE.asString(), cookies.toString()); + } } // Authorization diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java index 57c79facc4e..2dd02292c5c 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.io.UnsupportedEncodingException; import java.net.HttpCookie; import java.net.URI; +import java.net.URISyntaxException; import java.net.URLDecoder; import java.net.URLEncoder; import java.nio.ByteBuffer; @@ -57,6 +58,8 @@ import org.eclipse.jetty.util.Fields; public class HttpRequest implements Request { + private static final URI NULL_URI = URI.create("null:0"); + private final HttpFields headers = new HttpFields(); private final Fields params = new Fields(true); private final List responseListeners = new ArrayList<>(); @@ -158,22 +161,30 @@ public class HttpRequest implements Request @Override public Request path(String path) { - URI uri = URI.create(path); - String rawPath = uri.getRawPath(); - if (uri.isOpaque()) - rawPath = path; - if (rawPath == null) - rawPath = ""; - this.path = rawPath; - String query = uri.getRawQuery(); - if (query != null) + URI uri = newURI(path); + if (uri == null) { - this.query = query; - params.clear(); - extractParams(query); + this.path = path; + this.query = null; + } + else + { + String rawPath = uri.getRawPath(); + if (uri.isOpaque()) + rawPath = path; + if (rawPath == null) + rawPath = ""; + this.path = rawPath; + String query = uri.getRawQuery(); + if (query != null) + { + this.query = query; + params.clear(); + extractParams(query); + } + if (uri.isAbsolute()) + this.path = buildURI(false).toString(); } - if (uri.isAbsolute()) - this.path = buildURI(false).toString(); this.uri = null; return this; } @@ -187,9 +198,9 @@ public class HttpRequest implements Request @Override public URI getURI() { - if (uri != null) - return uri; - return uri = buildURI(true); + if (uri == null) + uri = buildURI(true); + return uri == NULL_URI ? null : uri; } @Override @@ -762,12 +773,28 @@ public class HttpRequest implements Request String query = getQuery(); if (query != null && withQuery) path += "?" + query; - URI result = URI.create(path); + URI result = newURI(path); + if (result == null) + return NULL_URI; if (!result.isAbsolute() && !result.isOpaque()) result = URI.create(new Origin(getScheme(), getHost(), getPort()).asString() + path); return result; } + private URI newURI(String uri) + { + try + { + return new URI(uri); + } + catch (URISyntaxException x) + { + // The "path" of a HTTP request may not be a URI, + // for example for CONNECT 127.0.0.1:8080 or OPTIONS *. + return null; + } + } + @Override public String toString() { diff --git a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyTunnellingTest.java b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyTunnellingTest.java index eab6ca76719..dd89a0715b1 100644 --- a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyTunnellingTest.java +++ b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyTunnellingTest.java @@ -18,8 +18,6 @@ package org.eclipse.jetty.proxy; -import static org.junit.Assert.assertEquals; - import java.io.IOException; import java.net.ConnectException; import java.net.Socket; @@ -28,7 +26,6 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; - import javax.servlet.ServletException; import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServletRequest; @@ -64,6 +61,8 @@ import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; +import static org.junit.Assert.assertEquals; + public class ProxyTunnellingTest { @Rule @@ -148,8 +147,10 @@ public class ProxyTunnellingTest try { + // Use a numeric host to test the URI of the CONNECT request. + String host = "127.0.0.1"; String body = "BODY"; - ContentResponse response = httpClient.newRequest("localhost", serverConnector.getLocalPort()) + ContentResponse response = httpClient.newRequest(host, serverConnector.getLocalPort()) .scheme(HttpScheme.HTTPS.asString()) .method(HttpMethod.GET) .path("/echo?body=" + URLEncoder.encode(body, "UTF-8")) From bb2872b7895bcb459973b25f328d62f4eec9c5a0 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 9 Jan 2015 12:46:56 +0100 Subject: [PATCH 5/5] Fixed test assumption. --- .../java/org/eclipse/jetty/client/HttpClientTimeoutTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTimeoutTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTimeoutTest.java index 1a67edfdd8d..df347c4bc5f 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTimeoutTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTimeoutTest.java @@ -445,6 +445,11 @@ public class HttpClientTimeoutTest extends AbstractHttpClientServerTest // Expected timeout during connect, continue the test. Assume.assumeTrue(true); } + catch (Throwable x) + { + // Abort if any other exception happens. + Assume.assumeTrue(false); + } } private class TimeoutHandler extends AbstractHandler