From 28566c72c8bb091b272fd7b25fb7b4389363b024 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 24 Oct 2013 17:22:08 +1100 Subject: [PATCH 01/27] 420142 reimplemented graceful shutdown --- .../java/org/eclipse/jetty/server/Server.java | 7 +- .../server/handler/StatisticsHandler.java | 54 ++++++++- .../jetty/server/GracefulStopTest.java | 105 ++++++++++++++++++ .../util/statistic/CounterStatistic.java | 24 ++-- 4 files changed, 167 insertions(+), 23 deletions(-) create mode 100644 jetty-server/src/test/java/org/eclipse/jetty/server/GracefulStopTest.java diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java index 31a67de487d..0fbc89f6f56 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java @@ -359,9 +359,10 @@ public class Server extends HandlerWrapper implements Attributes futures.add(connector.shutdown()); // Then tell the contexts that we are shutting down - Handler[] contexts = getChildHandlersByClass(Graceful.class); - for (Handler context : contexts) - futures.add(((Graceful)context).shutdown()); + + Handler[] gracefuls = getChildHandlersByClass(Graceful.class); + for (Handler graceful : gracefuls) + futures.add(((Graceful)graceful).shutdown()); // Shall we gracefully wait for zero connections? long stopTimeout = getStopTimeout(); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/StatisticsHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/StatisticsHandler.java index 78e8eb71ef3..bc93a6501cb 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/StatisticsHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/StatisticsHandler.java @@ -19,8 +19,11 @@ package org.eclipse.jetty.server.handler; import java.io.IOException; +import java.util.concurrent.Future; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; import javax.servlet.AsyncEvent; import javax.servlet.AsyncListener; @@ -32,14 +35,16 @@ import org.eclipse.jetty.server.AsyncContextEvent; import org.eclipse.jetty.server.HttpChannelState; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; +import org.eclipse.jetty.util.FutureCallback; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.annotation.ManagedOperation; +import org.eclipse.jetty.util.component.Graceful; import org.eclipse.jetty.util.statistic.CounterStatistic; import org.eclipse.jetty.util.statistic.SampleStatistic; @ManagedObject("Request Statistics Gathering") -public class StatisticsHandler extends HandlerWrapper +public class StatisticsHandler extends HandlerWrapper implements Graceful { private final AtomicLong _statsStartedAt = new AtomicLong(); @@ -59,6 +64,8 @@ public class StatisticsHandler extends HandlerWrapper private final AtomicInteger _responses5xx = new AtomicInteger(); private final AtomicLong _responsesTotalBytes = new AtomicLong(); + private final AtomicReference _shutdown=new AtomicReference<>(); + private final AsyncListener _onCompletion = new AsyncListener() { @Override @@ -81,21 +88,27 @@ public class StatisticsHandler extends HandlerWrapper @Override public void onComplete(AsyncEvent event) throws IOException { - HttpChannelState state = ((AsyncContextEvent)event).getHttpChannelState(); Request request = state.getBaseRequest(); final long elapsed = System.currentTimeMillis()-request.getTimeStamp(); - _requestStats.decrement(); + long d=_requestStats.decrement(); _requestTimeStats.set(elapsed); updateResponse(request); if (!state.isDispatched()) _asyncWaitStats.decrement(); + + // If we have no more dispatches, should we signal shutdown? + if (d==0) + { + FutureCallback shutdown = _shutdown.get(); + if (shutdown!=null) + shutdown.succeeded(); + } } - }; /** @@ -164,9 +177,18 @@ public class StatisticsHandler extends HandlerWrapper } else if (state.isInitial()) { - _requestStats.decrement(); + long d=_requestStats.decrement(); _requestTimeStats.set(dispatched); updateResponse(request); + + // If we have no more dispatches, should we signal shutdown? + FutureCallback shutdown = _shutdown.get(); + if (shutdown!=null) + { + httpResponse.flushBuffer(); + if (d==0) + shutdown.succeeded(); + } } // else onCompletion will handle it. } @@ -207,9 +229,20 @@ public class StatisticsHandler extends HandlerWrapper @Override protected void doStart() throws Exception { + _shutdown.set(null); super.doStart(); statsReset(); } + + + @Override + protected void doStop() throws Exception + { + super.doStop(); + FutureCallback shutdown = _shutdown.get(); + if (shutdown!=null && !shutdown.isDone()) + shutdown.failed(new TimeoutException()); + } /** * @return the number of requests handled by this handler @@ -525,4 +558,15 @@ public class StatisticsHandler extends HandlerWrapper return sb.toString(); } + + @Override + public Future shutdown() + { + FutureCallback shutdown=new FutureCallback(false); + _shutdown.compareAndSet(null,shutdown); + shutdown=_shutdown.get(); + if (_dispatchedStats.getCurrent()==0) + shutdown.succeeded(); + return shutdown; + } } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/GracefulStopTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/GracefulStopTest.java new file mode 100644 index 00000000000..0c11502ff68 --- /dev/null +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/GracefulStopTest.java @@ -0,0 +1,105 @@ +// +// ======================================================================== +// Copyright (c) 1995-2013 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.server; + +import java.io.IOException; +import java.net.Socket; +import java.util.concurrent.TimeUnit; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.handler.AbstractHandler; +import org.eclipse.jetty.server.handler.StatisticsHandler; +import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.StringUtil; +import org.hamcrest.Matchers; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +public class GracefulStopTest +{ + private Server server; + + @Before + public void setup() throws Exception + { + server = new Server(0); + StatisticsHandler stats = new StatisticsHandler(); + TestHandler test=new TestHandler(); + server.setHandler(stats); + stats.setHandler(test); + server.setStopTimeout(10 * 1000); + + server.start(); + } + + @Test + public void testGraceful() throws Exception + { + new Thread() + { + @Override + public void run() + { + try + { + TimeUnit.SECONDS.sleep(1); + server.stop(); + } + catch (Exception e) + { + e.printStackTrace(); + } + } + }.start(); + + try(Socket socket = new Socket("localhost",server.getBean(NetworkConnector.class).getLocalPort());) + { + socket.getOutputStream().write("GET / HTTP/1.0\r\n\r\n".getBytes(StringUtil.__ISO_8859_1_CHARSET)); + String out = IO.toString(socket.getInputStream()); + Assert.assertThat(out,Matchers.containsString("200 OK")); + } + } + + private static class TestHandler extends AbstractHandler + { + @Override + public void handle(final String s, final Request request, final HttpServletRequest httpServletRequest, final HttpServletResponse httpServletResponse) + throws IOException, ServletException + { + try + { + TimeUnit.SECONDS.sleep(2); + } + catch (InterruptedException e) + { + } + + httpServletResponse.getWriter().write("OK"); + httpServletResponse.setStatus(200); + request.setHandled(true); + } + } + +} \ No newline at end of file diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/statistic/CounterStatistic.java b/jetty-util/src/main/java/org/eclipse/jetty/util/statistic/CounterStatistic.java index 67699911c9b..51a82ea3bb4 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/statistic/CounterStatistic.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/statistic/CounterStatistic.java @@ -55,37 +55,31 @@ public class CounterStatistic /** * @param delta the amount to add to the count */ - public void add(final long delta) + public long add(final long delta) { long value=_curr.addAndGet(delta); if (delta > 0) + { _total.addAndGet(delta); - Atomics.updateMax(_max,value); - } - - /* ------------------------------------------------------------ */ - /** - * @param delta the amount to subtract the count by. - */ - public void subtract(final long delta) - { - add(-delta); + Atomics.updateMax(_max,value); + } + return value; } /* ------------------------------------------------------------ */ /** */ - public void increment() + public long increment() { - add(1); + return add(1); } /* ------------------------------------------------------------ */ /** */ - public void decrement() + public long decrement() { - add(-1); + return add(-1); } /* ------------------------------------------------------------ */ From 2e434ef1e8720847b743b245c574b1b6ed5a3d20 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 25 Oct 2013 13:02:18 +0200 Subject: [PATCH 02/27] Refactored HttpDestinationOverHTTP into a PoolingHttpDestination base class for reuse from other transports. --- .../jetty/client/PoolingHttpDestination.java | 189 ++++++++++++++++++ .../client/http/HttpDestinationOverHTTP.java | 160 +-------------- .../http/HttpDestinationOverHTTPTest.java | 2 +- 3 files changed, 194 insertions(+), 157 deletions(-) create mode 100644 jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java b/jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java new file mode 100644 index 00000000000..4354b7b5de6 --- /dev/null +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java @@ -0,0 +1,189 @@ +// +// ======================================================================== +// Copyright (c) 1995-2013 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.util.Arrays; + +import org.eclipse.jetty.client.api.Connection; +import org.eclipse.jetty.client.api.Request; +import org.eclipse.jetty.util.Promise; +import org.eclipse.jetty.util.component.ContainerLifeCycle; + +public abstract class PoolingHttpDestination extends HttpDestination implements Promise +{ + private final ConnectionPool connectionPool; + + public PoolingHttpDestination(HttpClient client, Origin origin) + { + super(client, origin); + this.connectionPool = newConnectionPool(client); + } + + protected ConnectionPool newConnectionPool(HttpClient client) + { + return new ConnectionPool(this, client.getMaxConnectionsPerDestination(), this); + } + + public ConnectionPool getConnectionPool() + { + return connectionPool; + } + + @Override + @SuppressWarnings("unchecked") + public void succeeded(Connection connection) + { + process((C)connection, true); + } + + @Override + public void failed(final Throwable x) + { + getHttpClient().getExecutor().execute(new Runnable() + { + @Override + public void run() + { + abort(x); + } + }); + } + + protected void send() + { + C connection = acquire(); + if (connection != null) + process(connection, false); + } + + @SuppressWarnings("unchecked") + public C acquire() + { + return (C)connectionPool.acquire(); + } + + /** + *

Processes a new connection making it idle or active depending on whether requests are waiting to be sent.

+ *

A new connection is created when a request needs to be executed; it is possible that the request that + * triggered the request creation is executed by another connection that was just released, so the new connection + * may become idle.

+ *

If a request is waiting to be executed, it will be dequeued and executed by the new connection.

+ * + * @param connection the new connection + * @param dispatch whether to dispatch the processing to another thread + */ + public void process(final C connection, boolean dispatch) + { + HttpClient client = getHttpClient(); + final HttpExchange exchange = getHttpExchanges().poll(); + LOG.debug("Processing exchange {} on connection {}", exchange, connection); + if (exchange == null) + { + // TODO: review this part... may not be 100% correct + // TODO: e.g. is client is not running, there should be no need to close the connection + + if (!connectionPool.release(connection)) + connection.close(); + + if (!client.isRunning()) + { + LOG.debug("{} is stopping", client); + connection.close(); + } + } + else + { + final Request request = exchange.getRequest(); + Throwable cause = request.getAbortCause(); + if (cause != null) + { + abort(exchange, cause); + LOG.debug("Aborted before processing {}: {}", exchange, cause); + } + else + { + if (dispatch) + { + client.getExecutor().execute(new Runnable() + { + @Override + public void run() + { + send(connection, exchange); + } + }); + } + else + { + send(connection, exchange); + } + } + } + } + + protected abstract void send(C connection, HttpExchange exchange); + + public void release(C connection) + { + LOG.debug("{} released", connection); + HttpClient client = getHttpClient(); + if (client.isRunning()) + { + if (connectionPool.isActive(connection)) + process(connection, false); + else + LOG.debug("{} explicit", connection); + } + else + { + LOG.debug("{} is stopped", client); + close(connection); + connection.close(); + } + } + + @Override + public void close(Connection oldConnection) + { + super.close(oldConnection); + connectionPool.remove(oldConnection); + + // We need to execute queued requests even if this connection failed. + // We may create a connection that is not needed, but it will eventually + // idle timeout, so no worries + if (!getHttpExchanges().isEmpty()) + { + C newConnection = acquire(); + if (newConnection != null) + process(newConnection, false); + } + } + + public void close() + { + connectionPool.close(); + } + + @Override + public void dump(Appendable out, String indent) throws IOException + { + ContainerLifeCycle.dump(out, indent, Arrays.asList(connectionPool)); + } +} diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTP.java b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTP.java index 632365c417e..6e639f5dcd3 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTP.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTP.java @@ -18,173 +18,21 @@ package org.eclipse.jetty.client.http; -import java.io.IOException; -import java.util.Arrays; - -import org.eclipse.jetty.client.ConnectionPool; import org.eclipse.jetty.client.HttpClient; -import org.eclipse.jetty.client.HttpDestination; import org.eclipse.jetty.client.HttpExchange; import org.eclipse.jetty.client.Origin; -import org.eclipse.jetty.client.api.Connection; -import org.eclipse.jetty.client.api.Request; -import org.eclipse.jetty.util.Promise; -import org.eclipse.jetty.util.component.ContainerLifeCycle; +import org.eclipse.jetty.client.PoolingHttpDestination; -public class HttpDestinationOverHTTP extends HttpDestination implements Promise +public class HttpDestinationOverHTTP extends PoolingHttpDestination { - private final ConnectionPool connectionPool; - public HttpDestinationOverHTTP(HttpClient client, Origin origin) { super(client, origin); - this.connectionPool = newConnectionPool(client); - } - - protected ConnectionPool newConnectionPool(HttpClient client) - { - return new ConnectionPool(this, client.getMaxConnectionsPerDestination(), this); - } - - public ConnectionPool getConnectionPool() - { - return connectionPool; } @Override - public void succeeded(Connection connection) + protected void send(HttpConnectionOverHTTP connection, HttpExchange exchange) { - process((HttpConnectionOverHTTP)connection, true); - } - - @Override - public void failed(final Throwable x) - { - getHttpClient().getExecutor().execute(new Runnable() - { - @Override - public void run() - { - abort(x); - } - }); - } - - protected void send() - { - HttpConnectionOverHTTP connection = acquire(); - if (connection != null) - process(connection, false); - } - - protected HttpConnectionOverHTTP acquire() - { - return (HttpConnectionOverHTTP)connectionPool.acquire(); - } - - /** - *

Processes a new connection making it idle or active depending on whether requests are waiting to be sent.

- *

A new connection is created when a request needs to be executed; it is possible that the request that - * triggered the request creation is executed by another connection that was just released, so the new connection - * may become idle.

- *

If a request is waiting to be executed, it will be dequeued and executed by the new connection.

- * - * @param connection the new connection - * @param dispatch whether to dispatch the processing to another thread - */ - protected void process(final HttpConnectionOverHTTP connection, boolean dispatch) - { - HttpClient client = getHttpClient(); - final HttpExchange exchange = getHttpExchanges().poll(); - LOG.debug("Processing exchange {} on connection {}", exchange, connection); - if (exchange == null) - { - // TODO: review this part... may not be 100% correct - // TODO: e.g. is client is not running, there should be no need to close the connection - - if (!connectionPool.release(connection)) - connection.close(); - - if (!client.isRunning()) - { - LOG.debug("{} is stopping", client); - connection.close(); - } - } - else - { - final Request request = exchange.getRequest(); - Throwable cause = request.getAbortCause(); - if (cause != null) - { - abort(exchange, cause); - LOG.debug("Aborted before processing {}: {}", exchange, cause); - } - else - { - if (dispatch) - { - client.getExecutor().execute(new Runnable() - { - @Override - public void run() - { - connection.send(exchange); - } - }); - } - else - { - connection.send(exchange); - } - } - } - } - - protected void release(HttpConnectionOverHTTP connection) - { - LOG.debug("{} released", connection); - HttpClient client = getHttpClient(); - if (client.isRunning()) - { - if (connectionPool.isActive(connection)) - process(connection, false); - else - LOG.debug("{} explicit", connection); - } - else - { - LOG.debug("{} is stopped", client); - close(connection); - connection.close(); - } - } - - @Override - public void close(Connection oldConnection) - { - super.close(oldConnection); - connectionPool.remove(oldConnection); - - // We need to execute queued requests even if this connection failed. - // We may create a connection that is not needed, but it will eventually - // idle timeout, so no worries - if (!getHttpExchanges().isEmpty()) - { - HttpConnectionOverHTTP newConnection = acquire(); - if (newConnection != null) - process(newConnection, false); - } - } - - public void close() - { - connectionPool.close(); - } - - @Override - public void dump(Appendable out, String indent) throws IOException - { - ContainerLifeCycle.dump(out, indent, Arrays.asList(connectionPool)); + connection.send(exchange); } } diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java index f7ed4c965b0..55109b2bfe3 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java @@ -91,7 +91,7 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest HttpDestinationOverHTTP destination = new HttpDestinationOverHTTP(client, new Origin("http", "localhost", connector.getLocalPort())) { @Override - protected void process(HttpConnectionOverHTTP connection, boolean dispatch) + public void process(HttpConnectionOverHTTP connection, boolean dispatch) { try { From a28e4730ad48012b367900ee5a8b40e95462dde2 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 25 Oct 2013 14:08:55 +0200 Subject: [PATCH 03/27] 420362 - Response/request listeners called too many times. Wrapped on[Request|Response]XXX(XXXListener) listeners into their specific interface so that they don't get notified multiple times. --- .../org/eclipse/jetty/client/HttpRequest.java | 145 ++++++++++++++---- .../org/eclipse/jetty/client/HttpSender.java | 2 +- .../eclipse/jetty/client/HttpClientTest.java | 144 +++++++++++++++++ 3 files changed, 264 insertions(+), 27 deletions(-) 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 3ea2e0e6c22..0986a6af13f 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 @@ -23,6 +23,7 @@ import java.io.UnsupportedEncodingException; import java.net.URI; import java.net.URLDecoder; import java.net.URLEncoder; +import java.nio.ByteBuffer; import java.nio.charset.UnsupportedCharsetException; import java.nio.file.Path; import java.util.ArrayList; @@ -43,6 +44,7 @@ import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.client.api.Response; import org.eclipse.jetty.client.util.FutureResponseListener; import org.eclipse.jetty.client.util.PathContentProvider; +import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpMethod; @@ -295,93 +297,184 @@ public class HttpRequest implements Request } @Override - public Request onRequestQueued(QueuedListener listener) + public Request onRequestQueued(final QueuedListener listener) { - this.requestListeners.add(listener); + this.requestListeners.add(new QueuedListener() + { + @Override + public void onQueued(Request request) + { + listener.onQueued(request); + } + }); return this; } @Override - public Request onRequestBegin(BeginListener listener) + public Request onRequestBegin(final BeginListener listener) { - this.requestListeners.add(listener); + this.requestListeners.add(new BeginListener() + { + @Override + public void onBegin(Request request) + { + listener.onBegin(request); + } + }); return this; } @Override - public Request onRequestHeaders(HeadersListener listener) + public Request onRequestHeaders(final HeadersListener listener) { - this.requestListeners.add(listener); + this.requestListeners.add(new HeadersListener() + { + @Override + public void onHeaders(Request request) + { + listener.onHeaders(request); + } + }); return this; } @Override - public Request onRequestCommit(CommitListener listener) + public Request onRequestCommit(final CommitListener listener) { - this.requestListeners.add(listener); + this.requestListeners.add(new CommitListener() + { + @Override + public void onCommit(Request request) + { + listener.onCommit(request); + } + }); return this; } @Override - public Request onRequestContent(ContentListener listener) + public Request onRequestContent(final ContentListener listener) { - this.requestListeners.add(listener); + this.requestListeners.add(new ContentListener() + { + @Override + public void onContent(Request request, ByteBuffer content) + { + listener.onContent(request, content); + } + }); return this; } @Override - public Request onRequestSuccess(SuccessListener listener) + public Request onRequestSuccess(final SuccessListener listener) { - this.requestListeners.add(listener); + this.requestListeners.add(new SuccessListener() + { + @Override + public void onSuccess(Request request) + { + listener.onSuccess(request); + } + }); return this; } @Override - public Request onRequestFailure(FailureListener listener) + public Request onRequestFailure(final FailureListener listener) { - this.requestListeners.add(listener); + this.requestListeners.add(new FailureListener() + { + @Override + public void onFailure(Request request, Throwable failure) + { + listener.onFailure(request, failure); + } + }); return this; } @Override - public Request onResponseBegin(Response.BeginListener listener) + public Request onResponseBegin(final Response.BeginListener listener) { - this.responseListeners.add(listener); + this.responseListeners.add(new Response.BeginListener() + { + @Override + public void onBegin(Response response) + { + listener.onBegin(response); + } + }); return this; } @Override - public Request onResponseHeader(Response.HeaderListener listener) + public Request onResponseHeader(final Response.HeaderListener listener) { - this.responseListeners.add(listener); + this.responseListeners.add(new Response.HeaderListener() + { + @Override + public boolean onHeader(Response response, HttpField field) + { + return listener.onHeader(response, field); + } + }); return this; } @Override - public Request onResponseHeaders(Response.HeadersListener listener) + public Request onResponseHeaders(final Response.HeadersListener listener) { - this.responseListeners.add(listener); + this.responseListeners.add(new Response.HeadersListener() + { + @Override + public void onHeaders(Response response) + { + listener.onHeaders(response); + } + }); return this; } @Override - public Request onResponseContent(Response.ContentListener listener) + public Request onResponseContent(final Response.ContentListener listener) { - this.responseListeners.add(listener); + this.responseListeners.add(new Response.ContentListener() + { + @Override + public void onContent(Response response, ByteBuffer content) + { + listener.onContent(response, content); + } + }); return this; } @Override - public Request onResponseSuccess(Response.SuccessListener listener) + public Request onResponseSuccess(final Response.SuccessListener listener) { - this.responseListeners.add(listener); + this.responseListeners.add(new Response.SuccessListener() + { + @Override + public void onSuccess(Response response) + { + listener.onSuccess(response); + } + }); return this; } @Override - public Request onResponseFailure(Response.FailureListener listener) + public Request onResponseFailure(final Response.FailureListener listener) { - this.responseListeners.add(listener); + this.responseListeners.add(new Response.FailureListener() + { + @Override + public void onFailure(Response response, Throwable failure) + { + listener.onFailure(response, failure); + } + }); return this; } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java index eff0576761c..4d607b803e4 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java @@ -360,7 +360,7 @@ public class HttpSender implements AsyncContentProvider.Listener if (!commit(request)) return false; - if (content != null) + if (content != null && content.hasRemaining()) { RequestNotifier notifier = connection.getDestination().getRequestNotifier(); notifier.notifyContent(request, content); diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java index 9d80ad3b722..9fc6a81630f 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java @@ -860,4 +860,148 @@ public class HttpClientTest extends AbstractHttpClientServerTest Assert.assertEquals(200, response.getStatus()); } + + @Test + public void testRequestListenerForMultipleEventsIsInvokedOncePerEvent() throws Exception + { + start(new EmptyServerHandler()); + + final AtomicInteger counter = new AtomicInteger(); + Request.Listener listener = new Request.Listener() + { + @Override + public void onQueued(Request request) + { + counter.incrementAndGet(); + } + + @Override + public void onBegin(Request request) + { + counter.incrementAndGet(); + } + + @Override + public void onHeaders(Request request) + { + counter.incrementAndGet(); + } + + @Override + public void onCommit(Request request) + { + counter.incrementAndGet(); + } + + @Override + public void onContent(Request request, ByteBuffer content) + { + // Should not be invoked + counter.incrementAndGet(); + } + + @Override + public void onFailure(Request request, Throwable failure) + { + // Should not be invoked + counter.incrementAndGet(); + } + + @Override + public void onSuccess(Request request) + { + counter.incrementAndGet(); + } + }; + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .scheme(scheme) + .onRequestQueued(listener) + .onRequestBegin(listener) + .onRequestHeaders(listener) + .onRequestCommit(listener) + .onRequestContent(listener) + .onRequestSuccess(listener) + .onRequestFailure(listener) + .listener(listener) + .send(); + + Assert.assertEquals(200, response.getStatus()); + int expectedEventsTriggeredByOnRequestXXXListeners = 5; + int expectedEventsTriggeredByListener = 5; + int expected = expectedEventsTriggeredByOnRequestXXXListeners + expectedEventsTriggeredByListener; + Assert.assertEquals(expected, counter.get()); + } + + @Test + public void testResponseListenerForMultipleEventsIsInvokedOncePerEvent() throws Exception + { + start(new EmptyServerHandler()); + + final AtomicInteger counter = new AtomicInteger(); + final CountDownLatch latch = new CountDownLatch(1); + Response.Listener listener = new Response.Listener() + { + @Override + public void onBegin(Response response) + { + counter.incrementAndGet(); + } + + @Override + public boolean onHeader(Response response, HttpField field) + { + // Number of header may vary, so don't count + return true; + } + + @Override + public void onHeaders(Response response) + { + counter.incrementAndGet(); + } + + @Override + public void onContent(Response response, ByteBuffer content) + { + // Should not be invoked + counter.incrementAndGet(); + } + + @Override + public void onSuccess(Response response) + { + counter.incrementAndGet(); + } + + @Override + public void onFailure(Response response, Throwable failure) + { + // Should not be invoked + counter.incrementAndGet(); + } + + @Override + public void onComplete(Result result) + { + Assert.assertEquals(200, result.getResponse().getStatus()); + counter.incrementAndGet(); + latch.countDown(); + } + }; + client.newRequest("localhost", connector.getLocalPort()) + .scheme(scheme) + .onResponseBegin(listener) + .onResponseHeader(listener) + .onResponseHeaders(listener) + .onResponseContent(listener) + .onResponseSuccess(listener) + .onResponseFailure(listener) + .send(listener); + + Assert.assertTrue(latch.await(5, TimeUnit.SECONDS)); + int expectedEventsTriggeredByOnResponseXXXListeners = 3; + int expectedEventsTriggeredByCompletionListener = 4; + int expected = expectedEventsTriggeredByOnResponseXXXListeners + expectedEventsTriggeredByCompletionListener; + Assert.assertEquals(expected, counter.get()); + } } From b9fcd6695ce04aac2c6777617b7e43db84b013b5 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 25 Oct 2013 14:08:55 +0200 Subject: [PATCH 04/27] Merged via cherry-pick branch 'master' into 'jetty-9.1'. --- .../org/eclipse/jetty/client/HttpRequest.java | 144 ++++++++++++++---- .../eclipse/jetty/client/HttpClientTest.java | 144 ++++++++++++++++++ 2 files changed, 262 insertions(+), 26 deletions(-) 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 0efef0fd24b..c7240a9367c 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 @@ -23,6 +23,7 @@ import java.io.UnsupportedEncodingException; import java.net.URI; import java.net.URLDecoder; import java.net.URLEncoder; +import java.nio.ByteBuffer; import java.nio.charset.UnsupportedCharsetException; import java.nio.file.Path; import java.util.ArrayList; @@ -296,93 +297,184 @@ public class HttpRequest implements Request } @Override - public Request onRequestQueued(QueuedListener listener) + public Request onRequestQueued(final QueuedListener listener) { - this.requestListeners.add(listener); + this.requestListeners.add(new QueuedListener() + { + @Override + public void onQueued(Request request) + { + listener.onQueued(request); + } + }); return this; } @Override - public Request onRequestBegin(BeginListener listener) + public Request onRequestBegin(final BeginListener listener) { - this.requestListeners.add(listener); + this.requestListeners.add(new BeginListener() + { + @Override + public void onBegin(Request request) + { + listener.onBegin(request); + } + }); return this; } @Override - public Request onRequestHeaders(HeadersListener listener) + public Request onRequestHeaders(final HeadersListener listener) { - this.requestListeners.add(listener); + this.requestListeners.add(new HeadersListener() + { + @Override + public void onHeaders(Request request) + { + listener.onHeaders(request); + } + }); return this; } @Override - public Request onRequestCommit(CommitListener listener) + public Request onRequestCommit(final CommitListener listener) { - this.requestListeners.add(listener); + this.requestListeners.add(new CommitListener() + { + @Override + public void onCommit(Request request) + { + listener.onCommit(request); + } + }); return this; } @Override - public Request onRequestContent(ContentListener listener) + public Request onRequestContent(final ContentListener listener) { - this.requestListeners.add(listener); + this.requestListeners.add(new ContentListener() + { + @Override + public void onContent(Request request, ByteBuffer content) + { + listener.onContent(request, content); + } + }); return this; } @Override - public Request onRequestSuccess(SuccessListener listener) + public Request onRequestSuccess(final SuccessListener listener) { - this.requestListeners.add(listener); + this.requestListeners.add(new SuccessListener() + { + @Override + public void onSuccess(Request request) + { + listener.onSuccess(request); + } + }); return this; } @Override - public Request onRequestFailure(FailureListener listener) + public Request onRequestFailure(final FailureListener listener) { - this.requestListeners.add(listener); + this.requestListeners.add(new FailureListener() + { + @Override + public void onFailure(Request request, Throwable failure) + { + listener.onFailure(request, failure); + } + }); return this; } @Override - public Request onResponseBegin(Response.BeginListener listener) + public Request onResponseBegin(final Response.BeginListener listener) { - this.responseListeners.add(listener); + this.responseListeners.add(new Response.BeginListener() + { + @Override + public void onBegin(Response response) + { + listener.onBegin(response); + } + }); return this; } @Override - public Request onResponseHeader(Response.HeaderListener listener) + public Request onResponseHeader(final Response.HeaderListener listener) { - this.responseListeners.add(listener); + this.responseListeners.add(new Response.HeaderListener() + { + @Override + public boolean onHeader(Response response, HttpField field) + { + return listener.onHeader(response, field); + } + }); return this; } @Override - public Request onResponseHeaders(Response.HeadersListener listener) + public Request onResponseHeaders(final Response.HeadersListener listener) { - this.responseListeners.add(listener); + this.responseListeners.add(new Response.HeadersListener() + { + @Override + public void onHeaders(Response response) + { + listener.onHeaders(response); + } + }); return this; } @Override - public Request onResponseContent(Response.ContentListener listener) + public Request onResponseContent(final Response.ContentListener listener) { - this.responseListeners.add(listener); + this.responseListeners.add(new Response.ContentListener() + { + @Override + public void onContent(Response response, ByteBuffer content) + { + listener.onContent(response, content); + } + }); return this; } @Override - public Request onResponseSuccess(Response.SuccessListener listener) + public Request onResponseSuccess(final Response.SuccessListener listener) { - this.responseListeners.add(listener); + this.responseListeners.add(new Response.SuccessListener() + { + @Override + public void onSuccess(Response response) + { + listener.onSuccess(response); + } + }); return this; } @Override - public Request onResponseFailure(Response.FailureListener listener) + public Request onResponseFailure(final Response.FailureListener listener) { - this.responseListeners.add(listener); + this.responseListeners.add(new Response.FailureListener() + { + @Override + public void onFailure(Response response, Throwable failure) + { + listener.onFailure(response, failure); + } + }); return this; } diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java index 9d241dcbcd9..ef6240a206c 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java @@ -863,4 +863,148 @@ public class HttpClientTest extends AbstractHttpClientServerTest Assert.assertEquals(200, response.getStatus()); } + + @Test + public void testRequestListenerForMultipleEventsIsInvokedOncePerEvent() throws Exception + { + start(new EmptyServerHandler()); + + final AtomicInteger counter = new AtomicInteger(); + Request.Listener listener = new Request.Listener() + { + @Override + public void onQueued(Request request) + { + counter.incrementAndGet(); + } + + @Override + public void onBegin(Request request) + { + counter.incrementAndGet(); + } + + @Override + public void onHeaders(Request request) + { + counter.incrementAndGet(); + } + + @Override + public void onCommit(Request request) + { + counter.incrementAndGet(); + } + + @Override + public void onContent(Request request, ByteBuffer content) + { + // Should not be invoked + counter.incrementAndGet(); + } + + @Override + public void onFailure(Request request, Throwable failure) + { + // Should not be invoked + counter.incrementAndGet(); + } + + @Override + public void onSuccess(Request request) + { + counter.incrementAndGet(); + } + }; + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .scheme(scheme) + .onRequestQueued(listener) + .onRequestBegin(listener) + .onRequestHeaders(listener) + .onRequestCommit(listener) + .onRequestContent(listener) + .onRequestSuccess(listener) + .onRequestFailure(listener) + .listener(listener) + .send(); + + Assert.assertEquals(200, response.getStatus()); + int expectedEventsTriggeredByOnRequestXXXListeners = 5; + int expectedEventsTriggeredByListener = 5; + int expected = expectedEventsTriggeredByOnRequestXXXListeners + expectedEventsTriggeredByListener; + Assert.assertEquals(expected, counter.get()); + } + + @Test + public void testResponseListenerForMultipleEventsIsInvokedOncePerEvent() throws Exception + { + start(new EmptyServerHandler()); + + final AtomicInteger counter = new AtomicInteger(); + final CountDownLatch latch = new CountDownLatch(1); + Response.Listener listener = new Response.Listener() + { + @Override + public void onBegin(Response response) + { + counter.incrementAndGet(); + } + + @Override + public boolean onHeader(Response response, HttpField field) + { + // Number of header may vary, so don't count + return true; + } + + @Override + public void onHeaders(Response response) + { + counter.incrementAndGet(); + } + + @Override + public void onContent(Response response, ByteBuffer content) + { + // Should not be invoked + counter.incrementAndGet(); + } + + @Override + public void onSuccess(Response response) + { + counter.incrementAndGet(); + } + + @Override + public void onFailure(Response response, Throwable failure) + { + // Should not be invoked + counter.incrementAndGet(); + } + + @Override + public void onComplete(Result result) + { + Assert.assertEquals(200, result.getResponse().getStatus()); + counter.incrementAndGet(); + latch.countDown(); + } + }; + client.newRequest("localhost", connector.getLocalPort()) + .scheme(scheme) + .onResponseBegin(listener) + .onResponseHeader(listener) + .onResponseHeaders(listener) + .onResponseContent(listener) + .onResponseSuccess(listener) + .onResponseFailure(listener) + .send(listener); + + Assert.assertTrue(latch.await(5, TimeUnit.SECONDS)); + int expectedEventsTriggeredByOnResponseXXXListeners = 3; + int expectedEventsTriggeredByCompletionListener = 4; + int expected = expectedEventsTriggeredByOnResponseXXXListeners + expectedEventsTriggeredByCompletionListener; + Assert.assertEquals(expected, counter.get()); + } } From abac85e1291054f93a73f27644480dbec30bd50f Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 25 Oct 2013 14:55:48 +0200 Subject: [PATCH 05/27] 420364 - Bad synchronization in HttpConversation. --- .../main/java/org/eclipse/jetty/client/HttpConversation.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConversation.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConversation.java index 6ab9a3a50d1..596e239d8f0 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConversation.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConversation.java @@ -131,8 +131,7 @@ public class HttpConversation extends AttributesMap // will notify a listener that may send a new request and trigger // another call to this method which will build different listeners // which may be iterated over when the iteration continues. - listeners = new ArrayList<>(); - + List listeners = new ArrayList<>(); HttpExchange firstExchange = exchanges.peekFirst(); HttpExchange lastExchange = exchanges.peekLast(); if (firstExchange == lastExchange) @@ -151,6 +150,7 @@ public class HttpConversation extends AttributesMap else listeners.addAll(firstExchange.getResponseListeners()); } + this.listeners = listeners; } public void complete() From c101e55c7f933bd86d65640f49e78d50e46751c5 Mon Sep 17 00:00:00 2001 From: Mikhail Mazursky Date: Sat, 26 Oct 2013 12:43:06 +0600 Subject: [PATCH 06/27] [Bug 420374] Call super.close() in a finally block Signed-off-by: Mikhail Mazursky --- .../jetty/util/MultiPartOutputStream.java | 37 +++++++++++++------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartOutputStream.java b/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartOutputStream.java index 024bb6378ab..85cc700254b 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartOutputStream.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartOutputStream.java @@ -35,12 +35,12 @@ public class MultiPartOutputStream extends FilterOutputStream private static final byte[] __CRLF={'\r','\n'}; private static final byte[] __DASHDASH={'-','-'}; - public static String MULTIPART_MIXED="multipart/mixed"; - public static String MULTIPART_X_MIXED_REPLACE="multipart/x-mixed-replace"; + public static final String MULTIPART_MIXED="multipart/mixed"; + public static final String MULTIPART_X_MIXED_REPLACE="multipart/x-mixed-replace"; /* ------------------------------------------------------------ */ - private String boundary; - private byte[] boundaryBytes; + private final String boundary; + private final byte[] boundaryBytes; /* ------------------------------------------------------------ */ private boolean inPart=false; @@ -54,8 +54,15 @@ public class MultiPartOutputStream extends FilterOutputStream boundary = "jetty"+System.identityHashCode(this)+ Long.toString(System.currentTimeMillis(),36); boundaryBytes=boundary.getBytes(StringUtil.__ISO_8859_1); + } - inPart=false; + public MultiPartOutputStream(OutputStream out, String boundary) + throws IOException + { + super(out); + + this.boundary = boundary; + boundaryBytes=boundary.getBytes(StringUtil.__ISO_8859_1); } /* ------------------------------------------------------------ */ @@ -66,14 +73,20 @@ public class MultiPartOutputStream extends FilterOutputStream public void close() throws IOException { - if (inPart) + try + { + if (inPart) + out.write(__CRLF); + out.write(__DASHDASH); + out.write(boundaryBytes); + out.write(__DASHDASH); out.write(__CRLF); - out.write(__DASHDASH); - out.write(boundaryBytes); - out.write(__DASHDASH); - out.write(__CRLF); - inPart=false; - super.close(); + inPart=false; + } + finally + { + super.close(); + } } /* ------------------------------------------------------------ */ From de75b82f991395525d529ef32384855b4647d601 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 28 Oct 2013 09:58:33 +1100 Subject: [PATCH 07/27] 419350 Do not borrow space from passed arrays --- .../eclipse/jetty/server/HttpConnection.java | 3 +- .../org/eclipse/jetty/server/HttpOutput.java | 3 +- .../eclipse/jetty/server/HttpOutputTest.java | 35 +++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java index d56fdd3ddc3..a85f7b937f8 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java @@ -647,7 +647,8 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http { case NEED_HEADER: { - if (_lastContent && _content!=null && BufferUtil.space(_content)>_config.getResponseHeaderSize() && _content.hasArray() ) + // Look for optimisation to avoid allocating a _header buffer + if (_lastContent && _content!=null && !_content.isReadOnly() && _content.hasArray() && BufferUtil.space(_content)>_config.getResponseHeaderSize() ) { // use spare space in content buffer for header buffer int p=_content.position(); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java index bbea718ffa9..968a5a00406 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java @@ -207,7 +207,8 @@ public class HttpOutput extends ServletOutputStream // write any remaining content in the buffer directly if (len>0) - _channel.write(ByteBuffer.wrap(b, off, len), complete); + // pass as readonly to avoid space stealing optimisation in HttpConnection + _channel.write(ByteBuffer.wrap(b, off, len).asReadOnlyBuffer(), complete); else if (complete) _channel.write(BufferUtil.EMPTY_BUFFER,complete); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpOutputTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpOutputTest.java index c64387b6c65..65ac35d8195 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpOutputTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpOutputTest.java @@ -19,6 +19,7 @@ package org.eclipse.jetty.server; import static org.hamcrest.Matchers.containsString; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; import java.io.FilterInputStream; @@ -26,6 +27,7 @@ import java.io.IOException; import java.io.InputStream; import java.nio.ByteBuffer; import java.nio.channels.ReadableByteChannel; +import java.util.Arrays; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; @@ -35,6 +37,7 @@ import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.util.resource.Resource; import org.hamcrest.Matchers; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -78,6 +81,26 @@ public class HttpOutputTest assertThat(response,containsString("HTTP/1.1 200 OK")); } + @Test + public void testSendArray() throws Exception + { + byte[] buffer=new byte[16*1024]; + Arrays.fill(buffer,0,4*1024,(byte)0x99); + Arrays.fill(buffer,4*1024,12*1024,(byte)0x58); + Arrays.fill(buffer,12*1024,16*1024,(byte)0x66); + _handler._content=ByteBuffer.wrap(buffer); + _handler._content.limit(12*1024); + _handler._content.position(4*1024); + String response=_connector.getResponses("GET / HTTP/1.0\nHost: localhost:80\n\n"); + assertThat(response,containsString("HTTP/1.1 200 OK")); + assertThat(response,containsString("\r\nXXXXXXXXXXXXXXXXXXXXXXXXXXX")); + + for (int i=0;i<4*1024;i++) + assertEquals("i="+i,(byte)0x99,buffer[i]); + for (int i=12*1024;i<16*1024;i++) + assertEquals("i="+i,(byte)0x66,buffer[i]); + } + @Test public void testSendInputStreamSimple() throws Exception { @@ -195,6 +218,7 @@ public class HttpOutputTest { InputStream _contentInputStream; ReadableByteChannel _contentChannel; + ByteBuffer _content; @Override public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException @@ -204,6 +228,17 @@ public class HttpOutputTest HttpOutput out = (HttpOutput) response.getOutputStream(); + if (_content!=null) + { + response.setContentLength(_content.remaining()); + if (_content.hasArray()) + out.write(_content.array(),_content.arrayOffset()+_content.position(),_content.remaining()); + else + out.sendContent(_content); + _content=null; + return; + } + if (_contentInputStream!=null) { out.sendContent(_contentInputStream); From 960c03b8acfd52083ba16632e59523e605aa12cf Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 28 Oct 2013 10:14:12 +1100 Subject: [PATCH 08/27] 419350 Do not borrow space from passed arrays --- .../src/main/java/org/eclipse/jetty/server/HttpOutput.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java index 968a5a00406..5f17d0205f3 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java @@ -300,12 +300,14 @@ public class HttpOutput extends ServletOutputStream /* ------------------------------------------------------------ */ /** Blocking send of content. - * @param content The content to send + * @param content The content to send. * @throws IOException */ public void sendContent(ByteBuffer content) throws IOException { final BlockingCallback callback =_channel.getWriteBlockingCallback(); + if (content.hasArray()&&content.limit() Date: Mon, 28 Oct 2013 10:17:55 +1100 Subject: [PATCH 09/27] fixed merge --- .../eclipse/jetty/server/HttpOutputTest.java | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpOutputTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpOutputTest.java index 3c9dc638340..8fcc49bb384 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpOutputTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpOutputTest.java @@ -40,7 +40,6 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.resource.Resource; import org.hamcrest.Matchers; import org.junit.After; -import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -409,7 +408,6 @@ public class HttpOutputTest boolean _async; ByteBuffer _buffer; byte[] _bytes; - ByteBuffer _content; InputStream _contentInputStream; ReadableByteChannel _contentChannel; ByteBuffer _content; @@ -422,16 +420,6 @@ public class HttpOutputTest final HttpOutput out = (HttpOutput) response.getOutputStream(); - if (_content!=null) - { - response.setContentLength(_content.remaining()); - if (_content.hasArray()) - out.write(_content.array(),_content.arrayOffset()+_content.position(),_content.remaining()); - else - out.sendContent(_content); - _content=null; - return; - } if (_contentInputStream!=null) { @@ -545,10 +533,13 @@ public class HttpOutputTest return; } - if (_content!=null) { - out.sendContent(_content); + response.setContentLength(_content.remaining()); + if (_content.hasArray()) + out.write(_content.array(),_content.arrayOffset()+_content.position(),_content.remaining()); + else + out.sendContent(_content); _content=null; return; } From fc6493b5b1153a282f533e2deee316e4db0b40c8 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Mon, 28 Oct 2013 13:09:26 +1100 Subject: [PATCH 10/27] Remove accidental checkin of println --- .../java/org/eclipse/jetty/annotations/AnnotationParser.java | 1 - 1 file changed, 1 deletion(-) diff --git a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java index 002877546f7..7362eaf99e3 100644 --- a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java +++ b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java @@ -692,7 +692,6 @@ public class AnnotationParser parseDir(handlers, res, resolver); else { -System.err.println("TRYING TO SCAN "+res); //we've already verified the directories, so just verify the class file name File file = res.getFile(); if (isValidClassFileName((file==null?null:file.getName()))) From 9d756dc018de2af33dfd3fd4e307427e7b6f441e Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Mon, 28 Oct 2013 15:25:21 +1100 Subject: [PATCH 11/27] 294531 Unpacking webapp twice to the same directory name causes problems with updated jars in WEB-INF/lib --- .../jetty/ant/AntWebInfConfiguration.java | 7 +- .../maven/plugin/JettyRunForkedMojo.java | 23 +- .../plugin/MavenWebInfConfiguration.java | 3 - .../eclipse/jetty/maven/plugin/Starter.java | 5 +- .../eclipse/jetty/webapp/WebAppContext.java | 26 +- .../jetty/webapp/WebInfConfiguration.java | 238 ++++++------------ 6 files changed, 129 insertions(+), 173 deletions(-) diff --git a/jetty-ant/src/main/java/org/eclipse/jetty/ant/AntWebInfConfiguration.java b/jetty-ant/src/main/java/org/eclipse/jetty/ant/AntWebInfConfiguration.java index bb3e3223778..a8535e99c72 100644 --- a/jetty-ant/src/main/java/org/eclipse/jetty/ant/AntWebInfConfiguration.java +++ b/jetty-ant/src/main/java/org/eclipse/jetty/ant/AntWebInfConfiguration.java @@ -40,12 +40,7 @@ public class AntWebInfConfiguration extends WebInfConfiguration @Override public void preConfigure(final WebAppContext context) throws Exception - { - // Look for a work directory - File work = findWorkDirectory(context); - if (work != null) - makeTempDirectory(work, context, false); - + { //Make a temp directory for the webapp if one is not already set resolveTempDirectory(context); diff --git a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunForkedMojo.java b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunForkedMojo.java index d1dcf49320a..14265714171 100644 --- a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunForkedMojo.java +++ b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunForkedMojo.java @@ -132,11 +132,20 @@ public class JettyRunForkedMojo extends AbstractMojo * The temporary directory to use for the webapp. * Defaults to target/tmp * - * @parameter expression="${project.build.directory}/tmp" + * @parameter alias="tmpDirectory" expression="${project.build.directory}/tmp" * @required * @readonly */ - protected File tmpDirectory; + protected File tempDirectory; + + + + /** + * Whether temporary directory contents should survive webapp restarts. + * + * @parameter default-value="false" + */ + private boolean persistTempDirectory; /** @@ -418,12 +427,14 @@ public class JettyRunForkedMojo extends AbstractMojo props.put("context.path", contextPath); //sort out the tmp directory (make it if it doesn't exist) - if (tmpDirectory != null) + if (tempDirectory != null) { - if (!tmpDirectory.exists()) - tmpDirectory.mkdirs(); - props.put("tmp.dir", tmpDirectory.getAbsolutePath()); + if (!tempDirectory.exists()) + tempDirectory.mkdirs(); + props.put("tmp.dir", tempDirectory.getAbsolutePath()); } + + props.put("tmp.dir.persist", Boolean.toString(persistTempDirectory)); //sort out base dir of webapp if (webAppSourceDirectory == null || !webAppSourceDirectory.exists()) diff --git a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/MavenWebInfConfiguration.java b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/MavenWebInfConfiguration.java index 680f8ba93a2..bfa467b1c0d 100644 --- a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/MavenWebInfConfiguration.java +++ b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/MavenWebInfConfiguration.java @@ -284,9 +284,6 @@ public class MavenWebInfConfiguration extends WebInfConfiguration { LOG.debug("Unpacking overlay: " + overlay); - //resolve if not already resolved - resolveTempDirectory(context); - if (overlay.getResource() == null) return null; //nothing to unpack diff --git a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/Starter.java b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/Starter.java index 87f7fe61ba1..15414f6e441 100644 --- a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/Starter.java +++ b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/Starter.java @@ -207,6 +207,9 @@ public class Starter if (str != null) webApp.setTempDirectory(new File(str.trim())); + str = (String)props.getProperty("tmp.dir.persist"); + if (str != null) + webApp.setPersistTempDirectory(Boolean.valueOf(str)); // - the base directory str = (String)props.getProperty("base.dir"); @@ -219,7 +222,7 @@ public class Starter // - put virtual webapp base resource first on resource path or not str = (String)props.getProperty("base.first"); if (str != null && !"".equals(str.trim())) - webApp.setBaseAppFirst(Boolean.getBoolean(str)); + webApp.setBaseAppFirst(Boolean.valueOf(str)); //For overlays diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java index 2fe990edb68..81534cb1a3c 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java @@ -154,6 +154,8 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL private String[] _contextWhiteList = null; private File _tmpDir; + private boolean _persistTmpDir = false; + private String _war; private String _extraClasspath; private Throwable _unavailableException; @@ -1222,7 +1224,7 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL LOG.warn(e); } _tmpDir=dir; - setAttribute(TEMPDIR,_tmpDir); + setAttribute(TEMPDIR,_tmpDir); } /* ------------------------------------------------------------ */ @@ -1232,6 +1234,27 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL return _tmpDir; } + /** + * If true the temp directory for this + * webapp will be kept when the webapp stops. Otherwise, + * it will be deleted. + * + * @param delete + */ + public void setPersistTempDirectory(boolean persist) + { + _persistTmpDir = persist; + } + + /** + * @return + */ + public boolean isPersistTempDirectory() + { + return _persistTmpDir; + } + + /* ------------------------------------------------------------ */ /** * @param war The war to set as a file name or URL @@ -1480,5 +1503,4 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL { return _metadata; } - } diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java index 88db011c2ea..cc3f875da79 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java @@ -27,6 +27,7 @@ import java.net.URLClassLoader; import java.util.ArrayList; import java.util.List; import java.util.Locale; +import java.util.Random; import java.util.Set; import java.util.StringTokenizer; import java.util.concurrent.TimeUnit; @@ -57,17 +58,15 @@ public class WebInfConfiguration extends AbstractConfiguration * resource base as a resource collection. */ public static final String RESOURCE_DIRS = "org.eclipse.jetty.resources"; + protected Resource _preUnpackBaseResource; + + @Override public void preConfigure(final WebAppContext context) throws Exception { - // Look for a work directory - File work = findWorkDirectory(context); - if (work != null) - makeTempDirectory(work, context, false); - //Make a temp directory for the webapp if one is not already set resolveTempDirectory(context); @@ -192,20 +191,16 @@ public class WebInfConfiguration extends AbstractConfiguration @Override public void deconfigure(WebAppContext context) throws Exception { - // delete temp directory if we had to create it or if it isn't called work - Boolean tmpdirConfigured = (Boolean)context.getAttribute(TEMPDIR_CONFIGURED); - - if (context.getTempDirectory()!=null && (tmpdirConfigured == null || !tmpdirConfigured.booleanValue()) && !isTempWorkDirectory(context.getTempDirectory())) + //if we're not persisting the temp dir contents delete it + if (!context.isPersistTempDirectory()) { IO.delete(context.getTempDirectory()); - context.setTempDirectory(null); - - //clear out the context attributes for the tmp dir only if we had to - //create the tmp dir - context.setAttribute(TEMPDIR_CONFIGURED, null); - context.setAttribute(WebAppContext.TEMPDIR, null); } - + + //if it wasn't explicitly configured by the user, then unset it + Boolean tmpdirConfigured = (Boolean)context.getAttribute(TEMPDIR_CONFIGURED); + if (tmpdirConfigured != null && !tmpdirConfigured) + context.setTempDirectory(null); //reset the base resource back to what it was before we did any unpacking of resources context.setBaseResource(_preUnpackBaseResource); @@ -238,51 +233,44 @@ public class WebInfConfiguration extends AbstractConfiguration *

A. Try to use an explicit directory specifically for this webapp:

*
    *
  1. - * Iff an explicit directory is set for this webapp, use it. Do NOT set - * delete on exit. + * Iff an explicit directory is set for this webapp, use it. Set delete on + * exit depends on value of persistTempDirectory. *
    2. *
  2. * Iff javax.servlet.context.tempdir context attribute is set for - * this webapp && exists && writeable, then use it. Do NOT set delete on exit. + * this webapp && exists && writeable, then use it. Set delete on exit depends on + * value of persistTempDirectory. *
    3. *
* *

B. Create a directory based on global settings. The new directory - * will be called "Jetty_"+host+"_"+port+"__"+context+"_"+virtualhost - * Work out where to create this directory: - *

    - *
  1. - * Iff $(jetty.home)/work exists create the directory there. Do NOT - * set delete on exit. Do NOT delete contents if dir already exists. - *
    2. - *
  2. - * Iff WEB-INF/work exists create the directory there. Do NOT set - * delete on exit. Do NOT delete contents if dir already exists. - *
    3. - *
  3. - * Else create dir in $(java.io.tmpdir). Set delete on exit. Delete - * contents if dir already exists. - *
    4. - *
+ * will be called "Jetty-"+host+"-"+port+"__"+context+"-"+virtualhost+"-"+randomdigits+".dir" + *

+ *

+ * If the user has specified the context attribute org.eclipse.jetty.webapp.basetempdir, the + * directory specified by this attribute will be the parent of the temp dir created. Otherwise, + * the parent dir is $(java.io.tmpdir). Set delete on exit depends on value of persistTempDirectory. + *

*/ public void resolveTempDirectory (WebAppContext context) + throws Exception { - //If a tmp directory is already set, we're done + //If a tmp directory is already set we should use it File tmpDir = context.getTempDirectory(); - if (tmpDir != null && tmpDir.isDirectory() && tmpDir.canWrite()) + if (tmpDir != null) { - context.setAttribute(TEMPDIR_CONFIGURED, Boolean.TRUE); - return; // Already have a suitable tmp dir configured + configureTempDirectory(tmpDir, context); + context.setAttribute(TEMPDIR_CONFIGURED, Boolean.TRUE); //the tmp dir was set explicitly + return; } - - // No temp directory configured, try to establish one. - // First we check the context specific, javax.servlet specified, temp directory attribute + // No temp directory configured, try to establish one via the javax.servlet.context.tempdir. File servletTmpDir = asFile(context.getAttribute(WebAppContext.TEMPDIR)); - if (servletTmpDir != null && servletTmpDir.isDirectory() && servletTmpDir.canWrite()) + if (servletTmpDir != null) { // Use as tmpDir tmpDir = servletTmpDir; + configureTempDirectory(tmpDir, context); // Ensure Attribute has File object context.setAttribute(WebAppContext.TEMPDIR,tmpDir); // Set as TempDir in context. @@ -290,60 +278,25 @@ public class WebInfConfiguration extends AbstractConfiguration return; } - try + //We need to make a temp dir. Check if the user has set a directory to use instead + //of java.io.tmpdir as the parent of the dir + File baseTemp = asFile(context.getAttribute(WebAppContext.BASETEMPDIR)); + if (baseTemp != null && baseTemp.isDirectory() && baseTemp.canWrite()) { - // Put the tmp dir in the work directory if we had one - File work = new File(System.getProperty("jetty.base"),"work"); - if (work.exists() && work.canWrite() && work.isDirectory()) - { - makeTempDirectory(work, context, false); //make a tmp dir inside work, don't delete if it exists - } - else - { - File baseTemp = asFile(context.getAttribute(WebAppContext.BASETEMPDIR)); - if (baseTemp != null && baseTemp.isDirectory() && baseTemp.canWrite()) - { - // Use baseTemp directory (allow the funky Jetty_0_0_0_0.. subdirectory logic to kick in - makeTempDirectory(baseTemp,context,false); - } - else - { - makeTempDirectory(new File(System.getProperty("java.io.tmpdir")),context,true); //make a tmpdir, delete if it already exists - } - } + //Make a temp directory as a child of the given base dir + makeTempDirectory(baseTemp,context); } - catch(Exception e) + else { - tmpDir=null; - LOG.ignore(e); - } - - //Third ... Something went wrong trying to make the tmp directory, just make - //a jvm managed tmp directory - if (context.getTempDirectory() == null) - { - try - { - // Last resort - tmpDir=File.createTempFile("JettyContext",""); - if (tmpDir.exists()) - IO.delete(tmpDir); - tmpDir.mkdir(); - tmpDir.deleteOnExit(); - context.setTempDirectory(tmpDir); - } - catch(IOException e) - { - tmpDir = null; - throw new IllegalStateException("Cannot create tmp dir in "+System.getProperty("java.io.tmpdir")+ " for context "+context,e); - } + //Make a temp directory in java.io.tmpdir + makeTempDirectory(new File(System.getProperty("java.io.tmpdir")),context); } } /** * Given an Object, return File reference for object. * Typically used to convert anonymous Object from getAttribute() calls to a File object. - * @param fileattr the file attribute to analyze and return from (supports type File and type String, all others return null) + * @param fileattr the file attribute to analyze and return from (supports type File and type String, all others return null * @return the File object, null if null, or null if not a File or String */ private File asFile(Object fileattr) @@ -365,45 +318,47 @@ public class WebInfConfiguration extends AbstractConfiguration - public void makeTempDirectory (File parent, WebAppContext context, boolean deleteExisting) - throws IOException + public void makeTempDirectory (File parent, WebAppContext context) + throws Exception { - if (parent != null && parent.exists() && parent.canWrite() && parent.isDirectory()) + if (parent == null || !parent.exists() || !parent.canWrite() || !parent.isDirectory()) + throw new IllegalStateException("Parent for temp dir not configured correctly: "+(parent==null?"null":"writeable="+parent.canWrite())); + + String temp = getCanonicalNameForWebAppTmpDir(context); + File tmpDir = File.createTempFile(temp, ".dir", parent); + //delete the file that was created + tmpDir.delete(); + //and make a directory of the same name + tmpDir.mkdirs(); + configureTempDirectory(tmpDir, context); + + if(LOG.isDebugEnabled()) + LOG.debug("Set temp dir "+tmpDir); + context.setTempDirectory(tmpDir); + } + + private void configureTempDirectory (File dir, WebAppContext context) + { + if (dir == null) + throw new IllegalArgumentException("Null temp dir"); + + //if dir exists and we don't want it persisted, delete it + if (dir.exists() && !context.isPersistTempDirectory()) { - String temp = getCanonicalNameForWebAppTmpDir(context); - File tmpDir = new File(parent,temp); - - if (deleteExisting && tmpDir.exists()) - { - if (!IO.delete(tmpDir)) - { - if(LOG.isDebugEnabled())LOG.debug("Failed to delete temp dir "+tmpDir); - } - - //If we can't delete the existing tmp dir, create a new one - if (tmpDir.exists()) - { - String old=tmpDir.toString(); - tmpDir=File.createTempFile(temp+"_",""); - if (tmpDir.exists()) - IO.delete(tmpDir); - LOG.warn("Can't reuse "+old+", using "+tmpDir); - } - } - - if (!tmpDir.exists()) - tmpDir.mkdir(); - - //If the parent is not a work directory - if (!isTempWorkDirectory(tmpDir)) - { - tmpDir.deleteOnExit(); - } - - if(LOG.isDebugEnabled()) - LOG.debug("Set temp dir "+tmpDir); - context.setTempDirectory(tmpDir); + if (!IO.delete(dir)) + throw new IllegalStateException("Failed to delete temp dir "+dir); } + + //if it doesn't exist make it + if (!dir.exists()) + dir.mkdirs(); + + if (!context.isPersistTempDirectory()) + dir.deleteOnExit(); + + //is it useable + if (!dir.canWrite() || !dir.isDirectory()) + throw new IllegalStateException("Temp dir "+dir+" not useable: writeable="+dir.canWrite()+", dir="+dir.isDirectory()); } @@ -566,45 +521,17 @@ public class WebInfConfiguration extends AbstractConfiguration } - public File findWorkDirectory (WebAppContext context) throws IOException - { - if (context.getBaseResource() != null) - { - Resource web_inf = context.getWebInf(); - if (web_inf !=null && web_inf.exists()) - { - return new File(web_inf.getFile(),"work"); - } - } - return null; - } - - - /** - * Check if the tmpDir itself is called "work", or if the tmpDir - * is in a directory called "work". - * @return true if File is a temporary or work directory - */ - public boolean isTempWorkDirectory (File tmpDir) - { - if (tmpDir == null) - return false; - if (tmpDir.getName().equalsIgnoreCase("work")) - return true; - File t = tmpDir.getParentFile(); - if (t == null) - return false; - return (t.getName().equalsIgnoreCase("work")); - } /** * Create a canonical name for a webapp temp directory. * The form of the name is: - * "Jetty_"+host+"_"+port+"__"+resourceBase+"_"+context+"_"+virtualhost+base36_hashcode_of_whole_string + * "jetty-"+host+"-"+port+"-"+resourceBase+"-_"+context+"-"+virtualhost+"-"+randomdigits+".dir" * * host and port uniquely identify the server * context and virtual host uniquely identify the webapp + * randomdigits ensure every tmp directory is unique + * * @return the canonical name for the webapp temp directory */ public static String getCanonicalNameForWebAppTmpDir (WebAppContext context) @@ -697,6 +624,7 @@ public class WebInfConfiguration extends AbstractConfiguration } canonicalName.append("-"); + return canonicalName.toString(); } From f671fbaa85fcc64190fd2cf4c92a79171b97cf6f Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Mon, 28 Oct 2013 18:09:54 +1100 Subject: [PATCH 12/27] 419330 Allow access to setters on jetty-jspc-maven-plugin --- .../src/main/java/org/eclipse/jetty/jspc/plugin/JspcMojo.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/jetty-jspc-maven-plugin/src/main/java/org/eclipse/jetty/jspc/plugin/JspcMojo.java b/jetty-jspc-maven-plugin/src/main/java/org/eclipse/jetty/jspc/plugin/JspcMojo.java index 1ff828a613e..817afef2d0b 100644 --- a/jetty-jspc-maven-plugin/src/main/java/org/eclipse/jetty/jspc/plugin/JspcMojo.java +++ b/jetty-jspc-maven-plugin/src/main/java/org/eclipse/jetty/jspc/plugin/JspcMojo.java @@ -288,6 +288,9 @@ public class JspcMojo extends AbstractMojo Thread.currentThread().setContextClassLoader(webAppClassLoader); + if (jspc == null) + jspc = new JspC(); + jspc.setWebXmlFragment(webXmlFragment); jspc.setUriroot(webAppSourceDirectory); jspc.setOutputDir(generatedClasses); From 3a10aa41648e45bb544bea2cbbeebce7229de0e3 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 28 Oct 2013 11:13:17 +0100 Subject: [PATCH 13/27] 419964 - InputStreamContentProvider does not close provided InputStream. Now closing the provided InputStream when reading -1 or when an exception is thrown. --- .../util/InputStreamContentProvider.java | 39 +++++++++++++++++-- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/util/InputStreamContentProvider.java b/jetty-client/src/main/java/org/eclipse/jetty/client/util/InputStreamContentProvider.java index eacbb09168b..2382ef62787 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/util/InputStreamContentProvider.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/util/InputStreamContentProvider.java @@ -18,6 +18,7 @@ package org.eclipse.jetty.client.util; +import java.io.IOException; import java.io.InputStream; import java.nio.ByteBuffer; import java.util.Iterator; @@ -25,6 +26,8 @@ import java.util.NoSuchElementException; import org.eclipse.jetty.client.api.ContentProvider; import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; /** * A {@link ContentProvider} for an {@link InputStream}. @@ -33,19 +36,26 @@ import org.eclipse.jetty.util.BufferUtil; * Invocations to the {@link #iterator()} method after the first will return an "empty" iterator * because the stream has been consumed on the first invocation. *

- * It is possible to specify, at the constructor, a buffer size used to read content from the - * stream, by default 4096 bytes. - *

* However, it is possible for subclasses to override {@link #onRead(byte[], int, int)} to copy * the content read from the stream to another location (for example a file), and be able to * support multiple invocations of {@link #iterator()}, returning the iterator provided by this * class on the first invocation, and an iterator on the bytes copied to the other location * for subsequent invocations. + *

+ * It is possible to specify, at the constructor, a buffer size used to read content from the + * stream, by default 4096 bytes. + *

+ * The {@link InputStream} passed to the constructor is by default closed when is it fully + * consumed (or when an exception is thrown while reading it), unless otherwise specified + * to the {@link #InputStreamContentProvider(java.io.InputStream, int, boolean) constructor}. */ public class InputStreamContentProvider implements ContentProvider { + private static final Logger LOG = Log.getLogger(InputStreamContentProvider.class); + private final InputStream stream; private final int bufferSize; + private final boolean autoClose; public InputStreamContentProvider(InputStream stream) { @@ -53,9 +63,15 @@ public class InputStreamContentProvider implements ContentProvider } public InputStreamContentProvider(InputStream stream, int bufferSize) + { + this(stream, bufferSize, true); + } + + public InputStreamContentProvider(InputStream stream, int bufferSize, boolean autoClose) { this.stream = stream; this.bufferSize = bufferSize; + this.autoClose = autoClose; } @Override @@ -107,6 +123,7 @@ public class InputStreamContentProvider implements ContentProvider } else if (read < 0) { + close(); return false; } else @@ -122,6 +139,7 @@ public class InputStreamContentProvider implements ContentProvider failure = x; // Signal we have more content to cause a call to // next() which will throw NoSuchElementException. + close(); return true; } return false; @@ -145,6 +163,21 @@ public class InputStreamContentProvider implements ContentProvider { throw new UnsupportedOperationException(); } + + private void close() + { + if (autoClose) + { + try + { + stream.close(); + } + catch (IOException x) + { + LOG.ignore(x); + } + } + } }; } } From c822ee4f82e92a39401c4301bed173d44564f958 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 28 Oct 2013 12:19:16 +0100 Subject: [PATCH 14/27] Make sure we never exit the selector loop unless stopped. --- .../src/main/java/org/eclipse/jetty/io/SelectorManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java index 07bb5a42670..eed227e27f8 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java @@ -520,7 +520,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa } selectedKeys.clear(); } - catch (Exception x) + catch (Throwable x) { if (isRunning()) LOG.warn(x); From b6a306a6b8f443b001f477390c33f07e1ad7e6fc Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Tue, 29 Oct 2013 12:24:03 +1100 Subject: [PATCH 15/27] 420530 AbstractLoginModule never fails a login Conflicts: jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/AbstractLoginModule.java jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/PropertyFileLoginModule.java --- .../jetty/jaas/spi/AbstractLoginModule.java | 30 ++++++++++++++----- .../jaas/spi/PropertyFileLoginModule.java | 7 +++-- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/AbstractLoginModule.java b/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/AbstractLoginModule.java index 20646e2e714..4e6c700b062 100644 --- a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/AbstractLoginModule.java +++ b/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/AbstractLoginModule.java @@ -31,6 +31,7 @@ import javax.security.auth.callback.CallbackHandler; import javax.security.auth.callback.NameCallback; import javax.security.auth.callback.PasswordCallback; import javax.security.auth.callback.UnsupportedCallbackException; +import javax.security.auth.login.FailedLoginException; import javax.security.auth.login.LoginException; import javax.security.auth.spi.LoginModule; @@ -199,9 +200,14 @@ public abstract class AbstractLoginModule implements LoginModule callbacks[2] = new PasswordCallback("Enter password", false); //only used if framework does not support the ObjectCallback return callbacks; } - - - + + + public boolean isIgnored () + { + return false; + } + + public abstract UserInfo getUserInfo (String username) throws Exception; @@ -214,7 +220,10 @@ public abstract class AbstractLoginModule implements LoginModule public boolean login() throws LoginException { try - { + { + if (isIgnored()) + return false; + if (callbackHandler == null) throw new LoginException ("No callback handler"); @@ -231,7 +240,7 @@ public abstract class AbstractLoginModule implements LoginModule if ((webUserName == null) || (webCredential == null)) { setAuthenticated(false); - return isAuthenticated(); + throw new FailedLoginException(); } UserInfo userInfo = getUserInfo(webUserName); @@ -239,12 +248,16 @@ public abstract class AbstractLoginModule implements LoginModule if (userInfo == null) { setAuthenticated(false); - return isAuthenticated(); + throw new FailedLoginException(); } currentUser = new JAASUserInfo(userInfo); setAuthenticated(currentUser.checkCredential(webCredential)); - return isAuthenticated(); + + if (isAuthenticated()) + return true; + else + throw new FailedLoginException(); } catch (IOException e) { @@ -256,7 +269,8 @@ public abstract class AbstractLoginModule implements LoginModule } catch (Exception e) { - e.printStackTrace(); + if (e instanceof LoginException) + throw (LoginException)e; throw new LoginException (e.toString()); } } diff --git a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/PropertyFileLoginModule.java b/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/PropertyFileLoginModule.java index 028e4f269dc..69ea3e9a600 100644 --- a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/PropertyFileLoginModule.java +++ b/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/PropertyFileLoginModule.java @@ -112,9 +112,10 @@ public class PropertyFileLoginModule extends AbstractLoginModule PropertyUserStore propertyUserStore = _propertyUserStores.get(_filename); if (propertyUserStore == null) throw new IllegalStateException("PropertyUserStore should never be null here!"); - + + LOG.debug("Checking PropertyUserStore "+_filename+" for "+userName); UserIdentity userIdentity = propertyUserStore.getUserIdentity(userName); - if(userIdentity==null) + if (userIdentity==null) return null; Set principals = userIdentity.getSubject().getPrincipals(); @@ -127,7 +128,7 @@ public class PropertyFileLoginModule extends AbstractLoginModule } Credential credential = (Credential)userIdentity.getSubject().getPrivateCredentials().iterator().next(); - LOG.debug("Found: " + userName + " in PropertyUserStore"); + LOG.debug("Found: " + userName + " in PropertyUserStore "+_filename); return new UserInfo(userName, credential, roles); } From 00867b094b2d0d272fefcd6c79d4cd07c6c75dd0 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 29 Oct 2013 09:53:18 +0100 Subject: [PATCH 16/27] Making classes implement Closeable, rather than AutoCloseable, since it is more semantically correct. --- jetty-io/src/main/java/org/eclipse/jetty/io/Connection.java | 4 +++- .../main/java/org/eclipse/jetty/server/NetworkConnector.java | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/Connection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/Connection.java index 840e175ab2e..912dc2033f6 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/Connection.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/Connection.java @@ -18,6 +18,8 @@ package org.eclipse.jetty.io; +import java.io.Closeable; + import org.eclipse.jetty.util.Callback; /** @@ -28,7 +30,7 @@ import org.eclipse.jetty.util.Callback; * and when the {@link EndPoint} signals read readyness, this {@link Connection} can * read bytes from the network and interpret them.

*/ -public interface Connection extends AutoCloseable +public interface Connection extends Closeable { public void addListener(Listener listener); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/NetworkConnector.java b/jetty-server/src/main/java/org/eclipse/jetty/server/NetworkConnector.java index df8ce174c62..fbbffb6d032 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/NetworkConnector.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/NetworkConnector.java @@ -18,12 +18,13 @@ package org.eclipse.jetty.server; +import java.io.Closeable; import java.io.IOException; /** *

A {@link Connector} for TCP/IP network connectors

*/ -public interface NetworkConnector extends Connector, AutoCloseable +public interface NetworkConnector extends Connector, Closeable { /** *

Performs the activities needed to open the network communication From edcb39cc8970a5dc7e1ddc20414ff303fd0be98c Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 29 Oct 2013 10:03:08 +0100 Subject: [PATCH 17/27] Catching Throwable and closing channels rigorously. --- .../org/eclipse/jetty/io/SelectorManager.java | 64 ++++++++----------- 1 file changed, 27 insertions(+), 37 deletions(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java index eed227e27f8..b2cc0673f90 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java @@ -25,8 +25,6 @@ import java.net.Socket; import java.net.SocketAddress; import java.net.SocketTimeoutException; import java.nio.channels.CancelledKeyException; -import java.nio.channels.ClosedChannelException; -import java.nio.channels.ClosedSelectorException; import java.nio.channels.SelectionKey; import java.nio.channels.Selector; import java.nio.channels.ServerSocketChannel; @@ -58,14 +56,10 @@ import org.eclipse.jetty.util.thread.Scheduler; */ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpable { - protected static final Logger LOG = Log.getLogger(SelectorManager.class); - public static final String SUBMIT_KEY_UPDATES="org.eclipse.jetty.io.SelectorManager.submitKeyUpdates"; - /** - * The default connect timeout, in milliseconds - */ + public static final String SUBMIT_KEY_UPDATES = "org.eclipse.jetty.io.SelectorManager.submitKeyUpdates"; public static final int DEFAULT_CONNECT_TIMEOUT = 15000; - - private final static boolean __submitKeyUpdates=Boolean.valueOf(System.getProperty(SUBMIT_KEY_UPDATES,"FALSE")); + protected static final Logger LOG = Log.getLogger(SelectorManager.class); + private final static boolean __submitKeyUpdates = Boolean.valueOf(System.getProperty(SUBMIT_KEY_UPDATES, "false")); private final Executor executor; private final Scheduler scheduler; @@ -233,7 +227,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa { connection.onOpen(); } - catch (Exception x) + catch (Throwable x) { if (isRunning()) LOG.warn("Exception while notifying connection " + connection, x); @@ -253,9 +247,9 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa { connection.onClose(); } - catch (Exception x) + catch (Throwable x) { - LOG.info("Exception while notifying connection " + connection, x); + LOG.debug("Exception while notifying connection " + connection, x); } } @@ -368,11 +362,12 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa public void updateKey(Runnable update) { if (__submitKeyUpdates) + { submit(update); + } else { - update.run(); - + runChange(update); if (_state.compareAndSet(State.SELECT, State.WAKEUP)) wakeup(); } @@ -435,8 +430,15 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa protected void runChange(Runnable change) { - LOG.debug("Running change {}", change); - change.run(); + try + { + LOG.debug("Running change {}", change); + change.run(); + } + catch (Throwable x) + { + LOG.debug("Could not run change " + change, x); + } } @Override @@ -553,7 +555,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa if (attachment instanceof EndPoint) ((EndPoint)attachment).close(); } - catch (Exception x) + catch (Throwable x) { LOG.warn("Could not process key for channel " + key.channel(), x); if (attachment instanceof EndPoint) @@ -563,10 +565,10 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa private void processConnect(SelectionKey key, Connect connect) { - key.attach(connect.attachment); SocketChannel channel = (SocketChannel)key.channel(); try { + key.attach(connect.attachment); boolean connected = finishConnect(channel); if (connected) { @@ -580,10 +582,9 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa throw new ConnectException(); } } - catch (Exception x) + catch (Throwable x) { connect.failed(x); - closeNoExceptions(channel); } } @@ -593,7 +594,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa { closeable.close(); } - catch (IOException x) + catch (Throwable x) { LOG.ignore(x); } @@ -740,8 +741,9 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa EndPoint endpoint = createEndPoint(_channel, key); key.attach(endpoint); } - catch (IOException x) + catch (Throwable x) { + closeNoExceptions(_channel); LOG.debug(x); } } @@ -768,7 +770,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa { channel.register(_selector, SelectionKey.OP_CONNECT, this); } - catch (ClosedSelectorException | ClosedChannelException x) + catch (Throwable x) { failed(x); } @@ -779,22 +781,10 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa if (failed.compareAndSet(false, true)) { timeout.cancel(); - close(); + closeNoExceptions(channel); connectionFailed(channel, failure, attachment); } } - - private void close() - { - try - { - channel.close(); - } - catch (IOException x) - { - LOG.ignore(x); - } - } } private class ConnectTimeout implements Runnable @@ -878,7 +868,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa { try { - endPoint.getConnection().close(); + closeNoExceptions(endPoint.getConnection()); } finally { From 44ce72164e9570aedc3f19d242597da63508a1d4 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Wed, 30 Oct 2013 10:51:24 +1100 Subject: [PATCH 18/27] 420687 XML errors in jetty-plus/src/test/resources/web-fragment-*.xml --- jetty-plus/src/test/resources/web-fragment-1.xml | 2 +- jetty-plus/src/test/resources/web-fragment-2.xml | 4 ++-- jetty-plus/src/test/resources/web-fragment-3.xml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/jetty-plus/src/test/resources/web-fragment-1.xml b/jetty-plus/src/test/resources/web-fragment-1.xml index b213172ef01..2c563e86eee 100644 --- a/jetty-plus/src/test/resources/web-fragment-1.xml +++ b/jetty-plus/src/test/resources/web-fragment-1.xml @@ -9,7 +9,7 @@ Fragment1 - others + diff --git a/jetty-plus/src/test/resources/web-fragment-2.xml b/jetty-plus/src/test/resources/web-fragment-2.xml index e2fff6786f1..878ec0f9ba2 100644 --- a/jetty-plus/src/test/resources/web-fragment-2.xml +++ b/jetty-plus/src/test/resources/web-fragment-2.xml @@ -9,13 +9,13 @@ Fragment2 - others + jdbc/mydatasource javax.sql.DataSource - User + Application - 300000 - 2 - false + + + diff --git a/jetty-server/src/main/config/modules/xinetd.mod b/jetty-server/src/main/config/modules/xinetd.mod index 85e6865bcc4..e53618e4890 100644 --- a/jetty-server/src/main/config/modules/xinetd.mod +++ b/jetty-server/src/main/config/modules/xinetd.mod @@ -7,3 +7,11 @@ server [xml] etc/jetty-xinetd.xml + +[ini-template] +## Xinetd Configuration +## See ${jetty.home}/etc/jetty-xinetd.xml for example service entry +jetty.xinetd.idleTimeout=300000 +jetty.xinetd.acceptors=2 +jetty.xinetd.statsOn=false + From ed4d8241caa7431135befebc19c27e0e6a314811 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 30 Oct 2013 09:29:25 -0700 Subject: [PATCH 23/27] 418923 - Missing parameterization of etc/jetty-proxy.xml --- jetty-proxy/src/main/config/etc/jetty-proxy.xml | 12 ++++++------ jetty-proxy/src/main/config/modules/proxy.mod | 9 +++++++++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/jetty-proxy/src/main/config/etc/jetty-proxy.xml b/jetty-proxy/src/main/config/etc/jetty-proxy.xml index 8a09bba66e1..75a7671c3cd 100644 --- a/jetty-proxy/src/main/config/etc/jetty-proxy.xml +++ b/jetty-proxy/src/main/config/etc/jetty-proxy.xml @@ -9,8 +9,8 @@ - 16 - 256 + + @@ -20,7 +20,7 @@ - 300000 + @@ -34,7 +34,7 @@ / maxThreads - 128 + @@ -42,7 +42,7 @@ - true - 1000 + + diff --git a/jetty-proxy/src/main/config/modules/proxy.mod b/jetty-proxy/src/main/config/modules/proxy.mod index 9486a839430..eb1b38884aa 100644 --- a/jetty-proxy/src/main/config/modules/proxy.mod +++ b/jetty-proxy/src/main/config/modules/proxy.mod @@ -11,3 +11,12 @@ lib/jetty-proxy-${jetty.version}.jar [xml] etc/jetty-proxy.xml + +[ini-template] +## Proxy Configuration +jetty.proxy.threadpool.min=16 +jetty.proxy.threadpool.max=256 +jetty.proxy.idleTimeout=300000 +jetty.proxy.threads.max=128 +jetty.proxy.stopAtShutdown=true +jetty.proxy.stopTimeout=1000 From 1705825346d14ba4aa38e6904c668e401663932d Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 30 Oct 2013 13:11:27 -0700 Subject: [PATCH 24/27] 417932 - resources.mod should make ${jetty.base}/resources/ directory --- jetty-server/src/main/config/modules/ext.mod | 9 +++++++++ jetty-server/src/main/config/modules/resources.mod | 6 +++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/jetty-server/src/main/config/modules/ext.mod b/jetty-server/src/main/config/modules/ext.mod index 3af9897c03a..329a833d442 100644 --- a/jetty-server/src/main/config/modules/ext.mod +++ b/jetty-server/src/main/config/modules/ext.mod @@ -1,2 +1,11 @@ +# +# Module to add all lib/ext/*.jar files to classpath +# + [lib] lib/ext/*.jar + +[files] +lib/ +lib/ext/ + diff --git a/jetty-server/src/main/config/modules/resources.mod b/jetty-server/src/main/config/modules/resources.mod index 6ed073a3b4b..b3a4d2d74f6 100644 --- a/jetty-server/src/main/config/modules/resources.mod +++ b/jetty-server/src/main/config/modules/resources.mod @@ -3,4 +3,8 @@ # [lib] -resources \ No newline at end of file +resources + +[files] +resources/ + From 845c08970ea59bad52b25cb8b190e24a0d6ae0b6 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 30 Oct 2013 13:14:22 -0700 Subject: [PATCH 25/27] 417933 - logging.mod ini template should include commented log.class settings --- jetty-util/src/main/config/modules/logging.mod | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/jetty-util/src/main/config/modules/logging.mod b/jetty-util/src/main/config/modules/logging.mod index f72863d1f20..ec7d2ca4ea3 100644 --- a/jetty-util/src/main/config/modules/logging.mod +++ b/jetty-util/src/main/config/modules/logging.mod @@ -6,6 +6,16 @@ etc/jetty-logging.xml [ini-template] -## STDERR / STDOUT Logging +## Logging Configuration +# Configure jetty logging for default internal behavior STDERR output +# -Dorg.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog + +# Configure jetty logging for slf4j +# -Dorg.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.Slf4jLog + +# Configure jetty logging for java.util.logging +# -Dorg.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.JavaUtilLog + +# STDERR / STDOUT Logging # Number of days to retain logs # jetty.log.retain=90 From 466725e3434ea95579224f71f1cafbbf0e3f4987 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 31 Oct 2013 13:59:37 +1100 Subject: [PATCH 26/27] 420776 complete error pages after startAsync Conflicts: jetty-server/src/main/java/org/eclipse/jetty/server/AbstractHttpConnection.java jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncContextTest.java --- .../org/eclipse/jetty/server/HttpChannel.java | 1 + .../eclipse/jetty/servlet/ServletHandler.java | 19 +++++--- .../jetty/servlet/AsyncContextTest.java | 43 ++++++++++++++++++- 3 files changed, 54 insertions(+), 9 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java index 4c2bd54bbfc..9c94256400f 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java @@ -380,6 +380,7 @@ public class HttpChannel implements HttpParser.RequestHandler, Runnable boolean committed = sendResponse(info, null, true); if (!committed) LOG.warn("Could not send response error 500: "+x); + _request.getAsyncContext().complete(); } else if (isCommitted()) { diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java index 1470d34bd93..c9ccb06944e 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java @@ -569,10 +569,10 @@ public class ServletHandler extends ScopedHandler LOG.debug(request.toString()); } + request.setAttribute(RequestDispatcher.ERROR_EXCEPTION_TYPE,th.getClass()); + request.setAttribute(RequestDispatcher.ERROR_EXCEPTION,th); if (!response.isCommitted()) { - request.setAttribute(RequestDispatcher.ERROR_EXCEPTION_TYPE,th.getClass()); - request.setAttribute(RequestDispatcher.ERROR_EXCEPTION,th); if (th instanceof UnavailableException) { UnavailableException ue = (UnavailableException)th; @@ -586,6 +586,10 @@ public class ServletHandler extends ScopedHandler } else LOG.debug("Response already committed for handling "+th); + + // Complete async requests + if (request.isAsyncStarted()) + request.getAsyncContext().complete(); } catch(Error e) { @@ -596,15 +600,16 @@ public class ServletHandler extends ScopedHandler LOG.warn("Error for "+request.getRequestURI(),e); if(LOG.isDebugEnabled())LOG.debug(request.toString()); - // TODO httpResponse.getHttpConnection().forceClose(); + request.setAttribute(RequestDispatcher.ERROR_EXCEPTION_TYPE,e.getClass()); + request.setAttribute(RequestDispatcher.ERROR_EXCEPTION,e); if (!response.isCommitted()) - { - request.setAttribute(RequestDispatcher.ERROR_EXCEPTION_TYPE,e.getClass()); - request.setAttribute(RequestDispatcher.ERROR_EXCEPTION,e); response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); - } else LOG.debug("Response already committed for handling ",e); + + // Complete async requests + if (request.isAsyncStarted()) + request.getAsyncContext().complete(); } finally { diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncContextTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncContextTest.java index 121874ce762..e707884f9f5 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncContextTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncContextTest.java @@ -41,6 +41,7 @@ import javax.servlet.http.HttpServletResponseWrapper; import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.LocalConnector; +import org.eclipse.jetty.server.QuietServletException; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.handler.DefaultHandler; @@ -68,7 +69,7 @@ public class AsyncContextTest _server = new Server(); _contextHandler = new ServletContextHandler(ServletContextHandler.NO_SESSIONS); _connector = new LocalConnector(_server); - _connector.setIdleTimeout(30000); + _connector.setIdleTimeout(5000); _server.setConnectors(new Connector[] { _connector }); @@ -76,6 +77,7 @@ public class AsyncContextTest _contextHandler.addServlet(new ServletHolder(new TestServlet()),"/servletPath"); _contextHandler.addServlet(new ServletHolder(new TestServlet()),"/path with spaces/servletPath"); _contextHandler.addServlet(new ServletHolder(new TestServlet2()),"/servletPath2"); + _contextHandler.addServlet(new ServletHolder(new TestStartThrowServlet()),"/startthrow/*"); _contextHandler.addServlet(new ServletHolder(new ForwardingServlet()),"/forward"); _contextHandler.addServlet(new ServletHolder(new AsyncDispatchingServlet()),"/dispatchingServlet"); _contextHandler.addServlet(new ServletHolder(new ExpireServlet()),"/expire/*"); @@ -84,7 +86,8 @@ public class AsyncContextTest ErrorPageErrorHandler error_handler = new ErrorPageErrorHandler(); _contextHandler.setErrorHandler(error_handler); - error_handler.addErrorPage(500,"/error"); + error_handler.addErrorPage(500,"/error/500"); + error_handler.addErrorPage(IOException.class.getName(),"/error/IOE"); HandlerList handlers = new HandlerList(); handlers.setHandlers(new Handler[] @@ -116,6 +119,25 @@ public class AsyncContextTest } + @Test + public void testStartThrow() throws Exception + { + String request = "GET /ctx/startthrow HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Type: application/x-www-form-urlencoded\r\n" + + "Connection: close\r\n" + "\r\n"; + String responseString = _connector.getResponses(request); + + BufferedReader br = new BufferedReader(new StringReader(responseString)); + + assertEquals("HTTP/1.1 500 Server Error",br.readLine()); + br.readLine();// connection close + br.readLine();// server + br.readLine();// empty + + Assert.assertEquals("error servlet","ERROR: /error",br.readLine()); + Assert.assertEquals("error servlet","PathInfo= /IOE",br.readLine()); + Assert.assertEquals("error servlet","EXCEPTION: java.io.IOException: Test",br.readLine()); + } + @Test public void testDispatchAsyncContext() throws Exception { @@ -327,6 +349,7 @@ public class AsyncContextTest br.readLine();// empty Assert.assertEquals("error servlet","ERROR: /error",br.readLine()); + Assert.assertEquals("error servlet","PathInfo= /500",br.readLine()); Assert.assertEquals("error servlet","EXCEPTION: java.io.IOException: TEST",br.readLine()); } @@ -365,6 +388,7 @@ public class AsyncContextTest protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { response.getOutputStream().print("ERROR: " + request.getServletPath() + "\n"); + response.getOutputStream().print("PathInfo= " + request.getPathInfo() + "\n"); if (request.getAttribute(RequestDispatcher.ERROR_EXCEPTION)!=null) response.getOutputStream().print("EXCEPTION: " + request.getAttribute(RequestDispatcher.ERROR_EXCEPTION) + "\n"); } @@ -462,6 +486,21 @@ public class AsyncContextTest asyncContext.start(new AsyncRunnable(asyncContext)); } } + + private class TestStartThrowServlet extends HttpServlet + { + private static final long serialVersionUID = 1L; + + @Override + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + { + if (request.getDispatcherType()==DispatcherType.REQUEST) + { + request.startAsync(request, response); + throw new QuietServletException(new IOException("Test")); + } + } + } private class AsyncRunnable implements Runnable { From b1e277af21fda1ce3e64379cac9ab4c7941dad1f Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 31 Oct 2013 10:08:52 +0100 Subject: [PATCH 27/27] Refactored addition of proxy headers into two methods, to allow subclasses to override this behavior more easily. --- .../org/eclipse/jetty/proxy/ProxyServlet.java | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ProxyServlet.java b/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ProxyServlet.java index f2465145622..fcd00176d90 100644 --- a/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ProxyServlet.java +++ b/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ProxyServlet.java @@ -137,6 +137,11 @@ public class ProxyServlet extends HttpServlet } } + public String getViaHost() + { + return _viaHost; + } + public long getTimeout() { return _timeout; @@ -419,11 +424,8 @@ public class ProxyServlet extends HttpServlet proxyRequest.header(HttpHeader.HOST, _hostHeader); // Add proxy headers - proxyRequest.header(HttpHeader.VIA, "http/1.1 " + _viaHost); - proxyRequest.header(HttpHeader.X_FORWARDED_FOR, request.getRemoteAddr()); - proxyRequest.header(HttpHeader.X_FORWARDED_PROTO, request.getScheme()); - proxyRequest.header(HttpHeader.X_FORWARDED_HOST, request.getHeader(HttpHeader.HOST.asString())); - proxyRequest.header(HttpHeader.X_FORWARDED_SERVER, request.getLocalName()); + addViaHeader(proxyRequest); + addXForwardedHeaders(proxyRequest, request); if (hasContent) { @@ -488,6 +490,19 @@ public class ProxyServlet extends HttpServlet proxyRequest.send(new ProxyResponseListener(request, response)); } + protected Request addViaHeader(Request proxyRequest) + { + return proxyRequest.header(HttpHeader.VIA, "http/1.1 " + getViaHost()); + } + + protected void addXForwardedHeaders(Request proxyRequest, HttpServletRequest request) + { + proxyRequest.header(HttpHeader.X_FORWARDED_FOR, request.getRemoteAddr()); + proxyRequest.header(HttpHeader.X_FORWARDED_PROTO, request.getScheme()); + proxyRequest.header(HttpHeader.X_FORWARDED_HOST, request.getHeader(HttpHeader.HOST.asString())); + proxyRequest.header(HttpHeader.X_FORWARDED_SERVER, request.getLocalName()); + } + protected void onResponseHeaders(HttpServletRequest request, HttpServletResponse response, Response proxyResponse) { for (HttpField field : proxyResponse.getHeaders())