diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2021-10-04 13:54:51 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-10-04 13:54:51 +0200 |
commit | 8e9a8a54493e7d4b211f4453c2d92cd5c43038f8 (patch) | |
tree | e8342ccaeeede5e29376dcb1f3dc5b7424678f2d | |
parent | fed386f182a0b600a72b333bb308c15870c3f04e (diff) | |
parent | 93b789c66b883d5ce2dc9ad9edcd164bd4539ff3 (diff) |
Merge pull request #19407 from vespa-engine/balder/ensure-exceptions-does-not-escape
Balder/ensure exceptions does not escape
16 files changed, 48 insertions, 26 deletions
diff --git a/application/src/test/java/com/yahoo/application/container/docprocs/MockDispatchDocproc.java b/application/src/test/java/com/yahoo/application/container/docprocs/MockDispatchDocproc.java index d91522ee5ed..9e8e1a87c0a 100644 --- a/application/src/test/java/com/yahoo/application/container/docprocs/MockDispatchDocproc.java +++ b/application/src/test/java/com/yahoo/application/container/docprocs/MockDispatchDocproc.java @@ -54,7 +54,7 @@ public class MockDispatchDocproc extends DocumentProcessor { return new RequestDispatch() { @Override protected Request newRequest() { - return new MbusRequest(currentContainer, uri, message.setRoute(route)).setServerRequest(false); + return new MbusRequest(currentContainer, uri, message.setRoute(route), false); } }; } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/HttpRequest.java b/container-core/src/main/java/com/yahoo/jdisc/http/HttpRequest.java index 0b5e9ddde58..8fb711c34ed 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/HttpRequest.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/HttpRequest.java @@ -52,7 +52,7 @@ public class HttpRequest extends Request implements ServletOrJdiscHttpRequest { private final String str; - private Version(String str) { + Version(String str) { this.str = str; } @@ -96,7 +96,7 @@ public class HttpRequest extends Request implements ServletOrJdiscHttpRequest { } else { this.connectedAt = creationTime(TimeUnit.MILLISECONDS); } - } catch (RuntimeException e) { + } catch (Throwable e) { release(); throw e; } @@ -110,7 +110,7 @@ public class HttpRequest extends Request implements ServletOrJdiscHttpRequest { this.remoteAddress = null; this.parameters.putAll(getUriQueryParameters(uri)); this.connectedAt = creationTime(TimeUnit.MILLISECONDS); - } catch (RuntimeException e) { + } catch (Throwable e) { release(); throw e; } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java b/container-core/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java index 72068bd2dd5..6ed82319544 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java @@ -65,6 +65,7 @@ public abstract class DiscFilterRequest { return parent.getUri(); } + @Deprecated public abstract void setUri(URI uri); public HttpRequest getParentRequest() { @@ -313,6 +314,7 @@ public abstract class DiscFilterRequest { return getUri().getScheme(); } + @Deprecated public void setScheme(String scheme, boolean isSecure) { String uri = getUri().toString(); String arr [] = uri.split("://"); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/filter/JdiscFilterRequest.java b/container-core/src/main/java/com/yahoo/jdisc/http/filter/JdiscFilterRequest.java index f8d9e6b2642..d61644b65e7 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/filter/JdiscFilterRequest.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/filter/JdiscFilterRequest.java @@ -33,6 +33,7 @@ public class JdiscFilterRequest extends DiscFilterRequest { return parent; } + @Deprecated public void setUri(URI uri) { parent.setUri(uri); } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterRequest.java b/container-core/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterRequest.java index f06f9e256ff..4cf3cdde650 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterRequest.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/filter/ServletFilterRequest.java @@ -32,6 +32,7 @@ class ServletFilterRequest extends DiscFilterRequest { return parent; } + @Deprecated public void setUri(URI uri) { parent.setUri(uri); } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java index 6b206116de8..0b7e27b9598 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java @@ -197,6 +197,7 @@ class HttpRequestDispatch { requestContentChannel = requestHandler.handleRequest(jdiscRequest, servletResponseController.responseHandler); } + //TODO If the below method throws requestContentChannel will not be close and there is a reference leak ServletInputStream servletInputStream = jettyRequest.getInputStream(); ServletRequestReader servletRequestReader = new ServletRequestReader(servletInputStream, @@ -204,6 +205,9 @@ class HttpRequestDispatch { jDiscContext.janitor, metricReporter); + //TODO If the below method throws servletRequestReader will not complete and + // requestContentChannel will not be closed and there is a reference leak + // Ditto for the servletInputStream servletInputStream.setReadListener(servletRequestReader); return servletRequestReader; } diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/HttpRequestTestCase.java b/container-core/src/test/java/com/yahoo/jdisc/http/HttpRequestTestCase.java index a3cb31d5ecb..43fc67cfabe 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/HttpRequestTestCase.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/HttpRequestTestCase.java @@ -73,6 +73,7 @@ public class HttpRequestTestCase { } @Test + @SuppressWarnings("deprecation") public void requireThatAccessorsWork() { URI uri = URI.create("http://localhost/path?foo=bar&foo=baz&cox=69"); InetSocketAddress address = new InetSocketAddress("remotehost", 69); diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/filter/DiscFilterRequestTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/filter/DiscFilterRequestTest.java index 1c05a3f3db2..606060bc6b4 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/filter/DiscFilterRequestTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/filter/DiscFilterRequestTest.java @@ -263,6 +263,7 @@ public class DiscFilterRequestTest { } @Test + @SuppressWarnings("deprecation") public void testSetScheme() throws Exception { URI uri = URI.create("https://example.yahoo.com:8080/test"); HttpRequest httpReq = newRequest(uri, HttpRequest.Method.GET, HttpRequest.Version.HTTP_1_1); @@ -274,6 +275,7 @@ public class DiscFilterRequestTest { } @Test + @SuppressWarnings("deprecation") public void testGetServerPort() throws Exception { URI uri = URI.create("http://example.yahoo.com/test"); HttpRequest httpReq = newRequest(uri, HttpRequest.Method.GET, HttpRequest.Version.HTTP_1_1); @@ -286,6 +288,7 @@ public class DiscFilterRequestTest { } @Test + @SuppressWarnings("deprecation") public void testIsSecure() throws Exception { URI uri = URI.create("http://example.yahoo.com/test"); HttpRequest httpReq = newRequest(uri, HttpRequest.Method.GET, HttpRequest.Version.HTTP_1_1); diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/filter/ServletFilterRequestTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/filter/ServletFilterRequestTest.java index ed4c9b66068..896f441aa6f 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/filter/ServletFilterRequestTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/filter/ServletFilterRequestTest.java @@ -86,6 +86,7 @@ public class ServletFilterRequestTest { } @Test + @SuppressWarnings("deprecation") public void uri_can_be_set() throws Exception { URI newUri = new URI("http", null, host, port + 1, path, paramName + "=" + paramValue, null); filterRequest.setUri(newUri); diff --git a/container-messagebus/src/main/java/com/yahoo/messagebus/jdisc/MbusRequest.java b/container-messagebus/src/main/java/com/yahoo/messagebus/jdisc/MbusRequest.java index a0bedd678eb..1adad51a052 100644 --- a/container-messagebus/src/main/java/com/yahoo/messagebus/jdisc/MbusRequest.java +++ b/container-messagebus/src/main/java/com/yahoo/messagebus/jdisc/MbusRequest.java @@ -15,7 +15,10 @@ public class MbusRequest extends Request { private final Message message; public MbusRequest(CurrentContainer current, URI uri, Message msg) { - super(current, uri); + this(current, uri, msg, true); + } + public MbusRequest(CurrentContainer current, URI uri, Message msg, boolean isServerRequest) { + super(current, uri, isServerRequest); this.message = validateMessage(msg); } diff --git a/container-messagebus/src/main/java/com/yahoo/messagebus/jdisc/test/ClientTestDriver.java b/container-messagebus/src/main/java/com/yahoo/messagebus/jdisc/test/ClientTestDriver.java index ea0ed7eadc8..34b4d47b0b4 100644 --- a/container-messagebus/src/main/java/com/yahoo/messagebus/jdisc/test/ClientTestDriver.java +++ b/container-messagebus/src/main/java/com/yahoo/messagebus/jdisc/test/ClientTestDriver.java @@ -7,7 +7,12 @@ import com.yahoo.jdisc.ResourceReference; import com.yahoo.jdisc.application.ContainerBuilder; import com.yahoo.jdisc.handler.ResponseHandler; import com.yahoo.jdisc.test.TestDriver; -import com.yahoo.messagebus.*; +import com.yahoo.messagebus.Message; +import com.yahoo.messagebus.MessageBusParams; +import com.yahoo.messagebus.Protocol; +import com.yahoo.messagebus.Reply; +import com.yahoo.messagebus.SourceSession; +import com.yahoo.messagebus.SourceSessionParams; import com.yahoo.messagebus.jdisc.MbusClient; import com.yahoo.messagebus.jdisc.MbusRequest; import com.yahoo.messagebus.network.rpc.RPCNetworkParams; diff --git a/container-messagebus/src/test/java/com/yahoo/messagebus/jdisc/ClientThreadingTestCase.java b/container-messagebus/src/test/java/com/yahoo/messagebus/jdisc/ClientThreadingTestCase.java index 62a9a864781..1ccf0b43bf3 100644 --- a/container-messagebus/src/test/java/com/yahoo/messagebus/jdisc/ClientThreadingTestCase.java +++ b/container-messagebus/src/test/java/com/yahoo/messagebus/jdisc/ClientThreadingTestCase.java @@ -98,10 +98,8 @@ public class ClientThreadingTestCase { driver.activateContainer(builder); delegate.start(); } - void send(final Message msg, final ResponseHandler handler) { - final MbusRequest request = new MbusRequest(driver, URI.create("mbus://remote/"), msg); - request.setServerRequest(false); + final MbusRequest request = new MbusRequest(driver, URI.create("mbus://remote/"), msg, false); request.connect(handler).close(null); request.release(); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ApplicationRequestToDiscFilterRequestWrapper.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ApplicationRequestToDiscFilterRequestWrapper.java index 7cc00f1ad52..df1590bd2ee 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ApplicationRequestToDiscFilterRequestWrapper.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ApplicationRequestToDiscFilterRequestWrapper.java @@ -114,6 +114,7 @@ public class ApplicationRequestToDiscFilterRequestWrapper extends DiscFilterRequ } @Override + @Deprecated public void setUri(URI uri) { throw new UnsupportedOperationException(); } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/Request.java b/jdisc_core/src/main/java/com/yahoo/jdisc/Request.java index 895ddb30a6d..e2cde8e806c 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/Request.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/Request.java @@ -45,9 +45,9 @@ public class Request extends AbstractResource { private volatile boolean cancel = false; private BindingMatch<RequestHandler> bindingMatch; private TimeoutManager timeoutManager; - private boolean serverRequest; + private boolean serverRequest; // TODO could be final, only used in tests private Long timeout; - private URI uri; + private URI uri; // TODO Could be made final, public enum RequestType { READ, WRITE, MONITORING @@ -79,12 +79,16 @@ public class Request extends AbstractResource { * @param uri The identifier of this request. */ public Request(CurrentContainer current, URI uri) { - container = current.newReference(uri); + this(current, uri, true); + } + + public Request(CurrentContainer current, URI uri, boolean isServerRequest) { parent = null; parentReference = null; - creationTime = container.currentTimeMillis(); - serverRequest = true; + serverRequest = isServerRequest; setUri(uri); + container = current.newReference(uri); + creationTime = container.currentTimeMillis(); } /** @@ -113,11 +117,11 @@ public class Request extends AbstractResource { */ public Request(Request parent, URI uri) { this.parent = parent; - this.parentReference = this.parent.refer(); container = null; creationTime = parent.container().currentTimeMillis(); serverRequest = false; setUri(uri); + parentReference = this.parent.refer(); } /** Returns the {@link Container} for which this Request was created */ @@ -142,6 +146,7 @@ public class Request extends AbstractResource { * @return this, to allow chaining * @see #getUri() */ + @Deprecated public Request setUri(URI uri) { this.uri = uri.normalize(); return this; @@ -166,6 +171,7 @@ public class Request extends AbstractResource { * @return this, to allow chaining * @see #isServerRequest() */ + @Deprecated public Request setServerRequest(boolean serverRequest) { this.serverRequest = serverRequest; return this; diff --git a/jdisc_core/src/test/java/com/yahoo/jdisc/RequestTestCase.java b/jdisc_core/src/test/java/com/yahoo/jdisc/RequestTestCase.java index ab92f5fdcca..e85d34b9982 100644 --- a/jdisc_core/src/test/java/com/yahoo/jdisc/RequestTestCase.java +++ b/jdisc_core/src/test/java/com/yahoo/jdisc/RequestTestCase.java @@ -34,6 +34,7 @@ import static org.junit.Assert.fail; public class RequestTestCase { @Test + @SuppressWarnings("deprecation") public void requireThatAccessorsWork() throws BindingSetNotFoundException { MyTimer timer = new MyTimer(); timer.currentTime = 69; @@ -249,6 +250,7 @@ public class RequestTestCase { assertTrue(driver.close()); } + @SuppressWarnings("deprecation") private static void assertUri(CurrentContainer container, String requestUri, String expectedUri) { Request serverReq = new Request(container, URI.create(requestUri)); assertEquals(expectedUri, serverReq.getUri().toString()); diff --git a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerSnapshotTestCase.java b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerSnapshotTestCase.java index 0ea1040306c..ac2efafbba5 100644 --- a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerSnapshotTestCase.java +++ b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerSnapshotTestCase.java @@ -42,8 +42,7 @@ public class ContainerSnapshotTestCase { assertNotNull(request.getBindingMatch()); request.release(); - request = new Request(driver, URI.create("http://foo/")); - request.setServerRequest(false); + request = new Request(driver, URI.create("http://foo/"), false); assertNull(request.container().resolveHandler(request)); assertNull(request.getBindingMatch()); request.release(); @@ -53,8 +52,7 @@ public class ContainerSnapshotTestCase { assertNull(request.getBindingMatch()); request.release(); - request = new Request(driver, URI.create("http://bar/")); - request.setServerRequest(false); + request = new Request(driver, URI.create("http://bar/"), false); assertNull(request.container().resolveHandler(request)); assertNull(request.getBindingMatch()); request.release(); @@ -74,8 +72,7 @@ public class ContainerSnapshotTestCase { assertNull(request.getBindingMatch()); request.release(); - request = new Request(driver, URI.create("http://foo/")); - request.setServerRequest(false); + request = new Request(driver, URI.create("http://foo/"), false); assertNotNull(request.container().resolveHandler(request)); assertNotNull(request.getBindingMatch()); request.release(); @@ -85,8 +82,7 @@ public class ContainerSnapshotTestCase { assertNull(request.getBindingMatch()); request.release(); - request = new Request(driver, URI.create("http://bar/")); - request.setServerRequest(false); + request = new Request(driver, URI.create("http://bar/"), false); assertNull(request.container().resolveHandler(request)); assertNull(request.getBindingMatch()); request.release(); @@ -100,9 +96,7 @@ public class ContainerSnapshotTestCase { ContainerBuilder builder = driver.newContainerBuilder(); builder.clientBindings().bind("http://host/path", MyRequestHandler.newInstance()); driver.activateContainer(builder); - Request request = new Request(driver, URI.create("http://host/path")); - assertNull(request.container().resolveHandler(request)); - request.setServerRequest(false); + Request request = new Request(driver, URI.create("http://host/path"), false); assertNotNull(request.container().resolveHandler(request)); request.release(); assertTrue(driver.close()); |