diff --git a/.github/ISSUE_TEMPLATE/issue-template.md b/.github/ISSUE_TEMPLATE/issue-template.md new file mode 100644 index 000000000000..dd4f14550329 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/issue-template.md @@ -0,0 +1,18 @@ +--- +name: Issue +about: Reporting bugs and problems in Eclipse Jetty +title: '' +assignees: '' + +--- + +**Jetty version** + +**Java version** + +**OS type/version** + +**Description** + + + diff --git a/.github/ISSUE_TEMPLATE/question-template.md b/.github/ISSUE_TEMPLATE/question-template.md new file mode 100644 index 000000000000..d9cb028ca8bc --- /dev/null +++ b/.github/ISSUE_TEMPLATE/question-template.md @@ -0,0 +1,16 @@ +--- +name: Question +about: Asking questions about Eclipse Jetty +title: '' +labels: Question +assignees: '' + +--- + +**Jetty version** + +**Java version** + +**Question** + + diff --git a/.github/stale.yml b/.github/stale.yml new file mode 100644 index 000000000000..c31367f99a58 --- /dev/null +++ b/.github/stale.yml @@ -0,0 +1,20 @@ +# Number of days of inactivity before an issue becomes stale +daysUntilStale: 365 +# Number of days of inactivity before a stale issue is closed +daysUntilClose: 30 +# Issues with these labels will never be considered stale +exemptLabels: + - Pinned + - Security + - Specification + - TCK +# Label to use when marking an issue as stale +staleLabel: Stale +# Comment to post when marking an issue as stale. Set to `false` to disable +markComment: > + This issue has been automatically marked as stale because it has been a + full year without activit. It will be closed if no further activity occurs. + Thank you for your contributions. +# Comment to post when closing a stale issue. Set to `false` to disable +closeComment: > + This issue has been closed due to it having no activity. diff --git a/jetty-alpn/jetty-alpn-java-client/src/main/java/org/eclipse/jetty/alpn/java/client/JDK9ClientALPNProcessor.java b/jetty-alpn/jetty-alpn-java-client/src/main/java/org/eclipse/jetty/alpn/java/client/JDK9ClientALPNProcessor.java index f8e444b051e4..96d5e53c10d7 100644 --- a/jetty-alpn/jetty-alpn-java-client/src/main/java/org/eclipse/jetty/alpn/java/client/JDK9ClientALPNProcessor.java +++ b/jetty-alpn/jetty-alpn-java-client/src/main/java/org/eclipse/jetty/alpn/java/client/JDK9ClientALPNProcessor.java @@ -55,13 +55,13 @@ public void configure(SSLEngine sslEngine, Connection connection) ALPNClientConnection alpn = (ALPNClientConnection)connection; SSLParameters sslParameters = sslEngine.getSSLParameters(); List protocols = alpn.getProtocols(); - sslParameters.setApplicationProtocols(protocols.toArray(new String[protocols.size()])); + sslParameters.setApplicationProtocols(protocols.toArray(new String[0])); sslEngine.setSSLParameters(sslParameters); ((DecryptedEndPoint)connection.getEndPoint()).getSslConnection() .addHandshakeListener(new ALPNListener(alpn)); } - private final class ALPNListener implements SslHandshakeListener + private static final class ALPNListener implements SslHandshakeListener { private final ALPNClientConnection alpnConnection; diff --git a/jetty-alpn/jetty-alpn-java-server/src/main/java/org/eclipse/jetty/alpn/java/server/JDK9ServerALPNProcessor.java b/jetty-alpn/jetty-alpn-java-server/src/main/java/org/eclipse/jetty/alpn/java/server/JDK9ServerALPNProcessor.java index ca091a96e151..7ddf07278714 100644 --- a/jetty-alpn/jetty-alpn-java-server/src/main/java/org/eclipse/jetty/alpn/java/server/JDK9ServerALPNProcessor.java +++ b/jetty-alpn/jetty-alpn-java-server/src/main/java/org/eclipse/jetty/alpn/java/server/JDK9ServerALPNProcessor.java @@ -55,7 +55,7 @@ public void configure(SSLEngine sslEngine, Connection connection) sslEngine.setHandshakeApplicationProtocolSelector(new ALPNCallback((ALPNServerConnection)connection)); } - private final class ALPNCallback implements BiFunction, String>, SslHandshakeListener + private static final class ALPNCallback implements BiFunction, String>, SslHandshakeListener { private final ALPNServerConnection alpnConnection; @@ -68,10 +68,19 @@ private ALPNCallback(ALPNServerConnection connection) @Override public String apply(SSLEngine engine, List protocols) { - if (LOG.isDebugEnabled()) - LOG.debug("apply {} {}", alpnConnection, protocols); - alpnConnection.select(protocols); - return alpnConnection.getProtocol(); + try + { + if (LOG.isDebugEnabled()) + LOG.debug("apply {} {}", alpnConnection, protocols); + alpnConnection.select(protocols); + return alpnConnection.getProtocol(); + } + catch (Throwable x) + { + // Cannot negotiate the protocol, return null to have + // JSSE send Alert.NO_APPLICATION_PROTOCOL to the client. + return null; + } } @Override diff --git a/jetty-alpn/jetty-alpn-java-server/src/test/java/org/eclipse/jetty/alpn/java/server/JDK9ALPNTest.java b/jetty-alpn/jetty-alpn-java-server/src/test/java/org/eclipse/jetty/alpn/java/server/JDK9ALPNTest.java index 4163a3facec1..be9a3e52912d 100644 --- a/jetty-alpn/jetty-alpn-java-server/src/test/java/org/eclipse/jetty/alpn/java/server/JDK9ALPNTest.java +++ b/jetty-alpn/jetty-alpn-java-server/src/test/java/org/eclipse/jetty/alpn/java/server/JDK9ALPNTest.java @@ -19,15 +19,18 @@ package org.eclipse.jetty.alpn.java.server; import java.io.BufferedReader; -import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.io.OutputStream; +import java.net.InetSocketAddress; +import java.nio.ByteBuffer; +import java.nio.channels.SocketChannel; import java.nio.charset.StandardCharsets; import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLEngineResult; import javax.net.ssl.SSLParameters; import javax.net.ssl.SSLSocket; -import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -40,11 +43,16 @@ import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.handler.AbstractHandler; +import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.greaterThan; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; public class JDK9ALPNTest { @@ -65,6 +73,13 @@ public void startServer(Handler handler) throws Exception server.start(); } + @AfterEach + public void dispose() throws Exception + { + if (server != null) + server.stop(); + } + private SslContextFactory newSslContextFactory() { SslContextFactory sslContextFactory = new SslContextFactory.Server(); @@ -83,7 +98,7 @@ public void testClientNotSupportingALPNServerSpeaksDefaultProtocol() throws Exce startServer(new AbstractHandler() { @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) { baseRequest.setHandled(true); } @@ -125,7 +140,7 @@ public void testClientSupportingALPNServerSpeaksNegotiatedProtocol() throws Exce startServer(new AbstractHandler() { @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) { baseRequest.setHandled(true); } @@ -163,4 +178,57 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques } } } + + @Test + public void testClientSupportingALPNCannotNegotiateProtocol() throws Exception + { + startServer(new AbstractHandler() { + @Override + public void handle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) + { + jettyRequest.setHandled(true); + } + }); + + SslContextFactory sslContextFactory = new SslContextFactory.Client(true); + sslContextFactory.start(); + String host = "localhost"; + int port = connector.getLocalPort(); + try (SocketChannel client = SocketChannel.open(new InetSocketAddress(host, port))) + { + client.socket().setSoTimeout(5000); + + SSLEngine sslEngine = sslContextFactory.newSSLEngine(host, port); + sslEngine.setUseClientMode(true); + SSLParameters sslParameters = sslEngine.getSSLParameters(); + sslParameters.setApplicationProtocols(new String[]{"unknown/1.0"}); + sslEngine.setSSLParameters(sslParameters); + sslEngine.beginHandshake(); + assertSame(SSLEngineResult.HandshakeStatus.NEED_WRAP, sslEngine.getHandshakeStatus()); + + ByteBuffer sslBuffer = ByteBuffer.allocate(sslEngine.getSession().getPacketBufferSize()); + + SSLEngineResult result = sslEngine.wrap(BufferUtil.EMPTY_BUFFER, sslBuffer); + assertSame(SSLEngineResult.Status.OK, result.getStatus()); + + sslBuffer.flip(); + client.write(sslBuffer); + + assertSame(SSLEngineResult.HandshakeStatus.NEED_UNWRAP, sslEngine.getHandshakeStatus()); + + sslBuffer.clear(); + int read = client.read(sslBuffer); + assertThat(read, greaterThan(0)); + + sslBuffer.flip(); + // TLS frame layout: record_type, major_version, minor_version, hi_length, lo_length + int recordTypeAlert = 21; + assertEquals(recordTypeAlert, sslBuffer.get(0) & 0xFF); + // Alert record layout: alert_level, alert_code + int alertLevelFatal = 2; + assertEquals(alertLevelFatal, sslBuffer.get(5) & 0xFF); + int alertCodeNoApplicationProtocol = 120; + assertEquals(alertCodeNoApplicationProtocol, sslBuffer.get(6) & 0xFF); + } + } } diff --git a/jetty-alpn/jetty-alpn-server/src/main/config/modules/alpn-impl/alpn-12.mod b/jetty-alpn/jetty-alpn-server/src/main/config/modules/alpn-impl/alpn-12.mod index 689601a41976..eebcdb72615d 100644 --- a/jetty-alpn/jetty-alpn-server/src/main/config/modules/alpn-impl/alpn-12.mod +++ b/jetty-alpn/jetty-alpn-server/src/main/config/modules/alpn-impl/alpn-12.mod @@ -1,4 +1,4 @@ DO NOT EDIT - See: https://www.eclipse.org/jetty/documentation/current/startup-modules.html [depend] -alpn-impl/alpn-9 +alpn-impl/alpn-11 diff --git a/jetty-alpn/jetty-alpn-server/src/main/config/modules/alpn-impl/alpn-13.mod b/jetty-alpn/jetty-alpn-server/src/main/config/modules/alpn-impl/alpn-13.mod index 689601a41976..eebcdb72615d 100644 --- a/jetty-alpn/jetty-alpn-server/src/main/config/modules/alpn-impl/alpn-13.mod +++ b/jetty-alpn/jetty-alpn-server/src/main/config/modules/alpn-impl/alpn-13.mod @@ -1,4 +1,4 @@ DO NOT EDIT - See: https://www.eclipse.org/jetty/documentation/current/startup-modules.html [depend] -alpn-impl/alpn-9 +alpn-impl/alpn-11 diff --git a/jetty-alpn/jetty-alpn-server/src/main/config/modules/alpn-impl/alpn-14.mod b/jetty-alpn/jetty-alpn-server/src/main/config/modules/alpn-impl/alpn-14.mod index 689601a41976..eebcdb72615d 100644 --- a/jetty-alpn/jetty-alpn-server/src/main/config/modules/alpn-impl/alpn-14.mod +++ b/jetty-alpn/jetty-alpn-server/src/main/config/modules/alpn-impl/alpn-14.mod @@ -1,4 +1,4 @@ DO NOT EDIT - See: https://www.eclipse.org/jetty/documentation/current/startup-modules.html [depend] -alpn-impl/alpn-9 +alpn-impl/alpn-11 diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java index 0920daaa76c4..623589d07dfd 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java @@ -119,7 +119,7 @@ void demand(long n) } } - private long demand() + protected long demand() { return demand(LongUnaryOperator.identity()); } 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 216da9f16749..77027ce83060 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 @@ -545,6 +545,12 @@ public Request onResponseContentDemanded(Response.DemandedContentListener listen { this.responseListeners.add(new Response.DemandedContentListener() { + @Override + public void onBeforeContent(Response response, LongConsumer demand) + { + listener.onBeforeContent(response, demand); + } + @Override public void onContent(Response response, LongConsumer demand, ByteBuffer content, Callback callback) { diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java b/jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java index 7ab3ea31760e..83774e328205 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java @@ -551,45 +551,46 @@ interface FailureListener extends RequestListener */ interface Listener extends QueuedListener, BeginListener, HeadersListener, CommitListener, ContentListener, SuccessListener, FailureListener { + @Override + public default void onQueued(Request request) + { + } + + @Override + public default void onBegin(Request request) + { + } + + @Override + public default void onHeaders(Request request) + { + } + + @Override + public default void onCommit(Request request) + { + } + + @Override + public default void onContent(Request request, ByteBuffer content) + { + } + + @Override + public default void onSuccess(Request request) + { + } + + @Override + public default void onFailure(Request request, Throwable failure) + { + } + /** * An empty implementation of {@link Listener} */ class Adapter implements Listener { - @Override - public void onQueued(Request request) - { - } - - @Override - public void onBegin(Request request) - { - } - - @Override - public void onHeaders(Request request) - { - } - - @Override - public void onCommit(Request request) - { - } - - @Override - public void onContent(Request request, ByteBuffer content) - { - } - - @Override - public void onSuccess(Request request) - { - } - - @Override - public void onFailure(Request request, Throwable failure) - { - } } } } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/api/Response.java b/jetty-client/src/main/java/org/eclipse/jetty/client/api/Response.java index df6a8d2fe3ec..b16cd7786aff 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/api/Response.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/api/Response.java @@ -259,70 +259,77 @@ interface CompleteListener extends ResponseListener */ interface Listener extends BeginListener, HeaderListener, HeadersListener, ContentListener, AsyncContentListener, DemandedContentListener, SuccessListener, FailureListener, CompleteListener { - /** - * An empty implementation of {@link Listener} - */ - class Adapter implements Listener + @Override + public default void onBegin(Response response) { - @Override - public void onBegin(Response response) - { - } + } - @Override - public boolean onHeader(Response response, HttpField field) - { - return true; - } + @Override + public default boolean onHeader(Response response, HttpField field) + { + return true; + } - @Override - public void onHeaders(Response response) - { - } + @Override + public default void onHeaders(Response response) + { + } - @Override - public void onContent(Response response, ByteBuffer content) - { - } + @Override + default void onBeforeContent(Response response, LongConsumer demand) + { + demand.accept(1); + } - @Override - public void onContent(Response response, ByteBuffer content, Callback callback) - { - try - { - onContent(response, content); - callback.succeeded(); - } - catch (Throwable x) - { - callback.failed(x); - } - } + @Override + public default void onContent(Response response, ByteBuffer content) + { + } - @Override - public void onContent(Response response, LongConsumer demand, ByteBuffer content, Callback callback) + @Override + public default void onContent(Response response, ByteBuffer content, Callback callback) + { + try { - onContent(response, content, Callback.from(() -> - { - callback.succeeded(); - demand.accept(1); - }, callback::failed)); + onContent(response, content); + callback.succeeded(); } - - @Override - public void onSuccess(Response response) + catch (Throwable x) { + callback.failed(x); } + } - @Override - public void onFailure(Response response, Throwable failure) + @Override + public default void onContent(Response response, LongConsumer demand, ByteBuffer content, Callback callback) + { + onContent(response, content, Callback.from(() -> { - } + callback.succeeded(); + demand.accept(1); + }, callback::failed)); + } - @Override - public void onComplete(Result result) - { - } + @Override + public default void onSuccess(Response response) + { + } + + @Override + public default void onFailure(Response response, Throwable failure) + { + } + + @Override + public default void onComplete(Result result) + { + } + + /** + * An empty implementation of {@link Listener} + */ + class Adapter implements Listener + { } } } diff --git a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java index 0428dc2d9b2a..1fe39176ed52 100644 --- a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java +++ b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java @@ -420,13 +420,13 @@ public void onHeader(int request, HttpField field) } @Override - public void onHeaders(int request) + public boolean onHeaders(int request) { HttpChannelOverFCGI channel = activeChannels.get(request); if (channel != null) - channel.responseHeaders(); - else - noChannel(request); + return !channel.responseHeaders(); + noChannel(request); + return false; } @Override diff --git a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/parser/ClientParser.java b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/parser/ClientParser.java index 3e070a4bd107..0c3c5a6c5345 100644 --- a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/parser/ClientParser.java +++ b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/parser/ClientParser.java @@ -80,9 +80,9 @@ public void onHeader(int request, HttpField field) } @Override - public void onHeaders(int request) + public boolean onHeaders(int request) { - listener.onHeaders(request); + return listener.onHeaders(request); } @Override diff --git a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/parser/Parser.java b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/parser/Parser.java index 22aae4f6e3aa..9ebcae0ed642 100644 --- a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/parser/Parser.java +++ b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/parser/Parser.java @@ -135,7 +135,11 @@ public interface Listener { void onHeader(int request, HttpField field); - void onHeaders(int request); + /** + * @param request the request id + * @return true to signal to the parser to stop parsing, false to continue parsing + */ + boolean onHeaders(int request); /** * @param request the request id @@ -158,8 +162,9 @@ public void onHeader(int request, HttpField field) } @Override - public void onHeaders(int request) + public boolean onHeaders(int request) { + return false; } @Override diff --git a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/parser/ResponseContentParser.java b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/parser/ResponseContentParser.java index a35e9dbed54e..f7ed9724efa9 100644 --- a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/parser/ResponseContentParser.java +++ b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/parser/ResponseContentParser.java @@ -81,7 +81,7 @@ protected void end(int request) parsers.remove(request); } - private class ResponseParser implements HttpParser.ResponseHandler + private static class ResponseParser implements HttpParser.ResponseHandler { private final HttpFields fields = new HttpFields(); private ClientParser.Listener listener; @@ -89,6 +89,7 @@ private class ResponseParser implements HttpParser.ResponseHandler private final FCGIHttpParser httpParser; private State state = State.HEADERS; private boolean seenResponseCode; + private boolean stalled; private ResponseParser(ClientParser.Listener listener, int request) { @@ -110,7 +111,11 @@ public boolean parse(ByteBuffer buffer) case HEADERS: { if (httpParser.parseNext(buffer)) + { state = State.CONTENT_MODE; + if (stalled) + return true; + } remaining = buffer.remaining(); break; } @@ -239,16 +244,17 @@ private void notifyHeaders(HttpFields fields) } } - private void notifyHeaders() + private boolean notifyHeaders() { try { - listener.onHeaders(request); + return listener.onHeaders(request); } catch (Throwable x) { if (LOG.isDebugEnabled()) LOG.debug("Exception while invoking listener " + listener, x); + return false; } } @@ -261,8 +267,10 @@ public boolean headerComplete() notifyBegin(200, "OK"); notifyHeaders(fields); } - notifyHeaders(); - // Return from HTTP parsing so that we can parse the content. + // Remember whether we have demand. + stalled = notifyHeaders(); + // Always return from HTTP parsing so that we + // can parse the content with the FCGI parser. return true; } diff --git a/jetty-fcgi/fcgi-client/src/test/java/org/eclipse/jetty/fcgi/generator/ClientGeneratorTest.java b/jetty-fcgi/fcgi-client/src/test/java/org/eclipse/jetty/fcgi/generator/ClientGeneratorTest.java index 67e6fe9761b0..10050c37f7d4 100644 --- a/jetty-fcgi/fcgi-client/src/test/java/org/eclipse/jetty/fcgi/generator/ClientGeneratorTest.java +++ b/jetty-fcgi/fcgi-client/src/test/java/org/eclipse/jetty/fcgi/generator/ClientGeneratorTest.java @@ -109,10 +109,11 @@ public void onHeader(int request, HttpField field) } @Override - public void onHeaders(int request) + public boolean onHeaders(int request) { assertEquals(id, request); params.set(params.get() * primes[4]); + return false; } }); diff --git a/jetty-fcgi/fcgi-client/src/test/java/org/eclipse/jetty/fcgi/parser/ClientParserTest.java b/jetty-fcgi/fcgi-client/src/test/java/org/eclipse/jetty/fcgi/parser/ClientParserTest.java index 9d9087672389..ddc34fc9f21c 100644 --- a/jetty-fcgi/fcgi-client/src/test/java/org/eclipse/jetty/fcgi/parser/ClientParserTest.java +++ b/jetty-fcgi/fcgi-client/src/test/java/org/eclipse/jetty/fcgi/parser/ClientParserTest.java @@ -90,10 +90,11 @@ public void onHeader(int request, HttpField field) } @Override - public void onHeaders(int request) + public boolean onHeaders(int request) { assertEquals(id, request); params.set(params.get() * primes[2]); + return false; } }); diff --git a/jetty-fcgi/fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/ServerFCGIConnection.java b/jetty-fcgi/fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/ServerFCGIConnection.java index dc9547d7119e..73b45bda8c8f 100644 --- a/jetty-fcgi/fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/ServerFCGIConnection.java +++ b/jetty-fcgi/fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/ServerFCGIConnection.java @@ -144,7 +144,7 @@ public void onHeader(int request, HttpField field) } @Override - public void onHeaders(int request) + public boolean onHeaders(int request) { HttpChannelOverFCGI channel = channels.get(request); if (LOG.isDebugEnabled()) @@ -154,6 +154,7 @@ public void onHeaders(int request) channel.onRequest(); channel.dispatch(); } + return false; } @Override diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java index 736a18473d1e..7a2e2eb1697f 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java @@ -24,7 +24,7 @@ import org.eclipse.jetty.util.QuotedStringTokenizer; import org.eclipse.jetty.util.StringUtil; -// TODO consider replacing this with java.net.HttpCookie +// TODO consider replacing this with java.net.HttpCookie (once it supports RFC6265) public class HttpCookie { private static final String __COOKIE_DELIM = "\",;\\ \t"; @@ -33,14 +33,14 @@ public class HttpCookie /** *If this string is found within the comment parsed with {@link #isHttpOnlyInComment(String)} the check will return true **/ - private static final String HTTP_ONLY_COMMENT = "__HTTP_ONLY__"; + public static final String HTTP_ONLY_COMMENT = "__HTTP_ONLY__"; /** *These strings are used by {@link #getSameSiteFromComment(String)} to check for a SameSite specifier in the comment **/ private static final String SAME_SITE_COMMENT = "__SAME_SITE_"; - private static final String SAME_SITE_NONE_COMMENT = SAME_SITE_COMMENT + "NONE__"; - private static final String SAME_SITE_LAX_COMMENT = SAME_SITE_COMMENT + "LAX__"; - private static final String SAME_SITE_STRICT_COMMENT = SAME_SITE_COMMENT + "STRICT__"; + public static final String SAME_SITE_NONE_COMMENT = SAME_SITE_COMMENT + "NONE__"; + public static final String SAME_SITE_LAX_COMMENT = SAME_SITE_COMMENT + "LAX__"; + public static final String SAME_SITE_STRICT_COMMENT = SAME_SITE_COMMENT + "STRICT__"; public enum SameSite { @@ -431,17 +431,17 @@ public static SameSite getSameSiteFromComment(String comment) { if (comment != null) { - if (comment.contains(SAME_SITE_NONE_COMMENT)) + if (comment.contains(SAME_SITE_STRICT_COMMENT)) { - return SameSite.NONE; + return SameSite.STRICT; } if (comment.contains(SAME_SITE_LAX_COMMENT)) { return SameSite.LAX; } - if (comment.contains(SAME_SITE_STRICT_COMMENT)) + if (comment.contains(SAME_SITE_NONE_COMMENT)) { - return SameSite.STRICT; + return SameSite.NONE; } } @@ -465,6 +465,44 @@ public static String getCommentWithoutAttributes(String comment) return strippedComment.length() == 0 ? null : strippedComment; } + public static String getCommentWithAttributes(String comment, boolean httpOnly, SameSite sameSite) + { + if (comment == null && sameSite == null) + return null; + + StringBuilder builder = new StringBuilder(); + if (StringUtil.isNotBlank(comment)) + { + comment = getCommentWithoutAttributes(comment); + if (StringUtil.isNotBlank(comment)) + builder.append(comment); + } + if (httpOnly) + builder.append(HTTP_ONLY_COMMENT); + + if (sameSite != null) + { + switch (sameSite) + { + case NONE: + builder.append(SAME_SITE_NONE_COMMENT); + break; + case STRICT: + builder.append(SAME_SITE_STRICT_COMMENT); + break; + case LAX: + builder.append(SAME_SITE_LAX_COMMENT); + break; + default: + throw new IllegalArgumentException(sameSite.toString()); + } + } + + if (builder.length() == 0) + return null; + return builder.toString(); + } + public static class SetCookieHttpField extends HttpField { final HttpCookie _cookie; diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpCookieTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpCookieTest.java index 6fc0ef8c9646..243cac89750e 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpCookieTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpCookieTest.java @@ -18,17 +18,25 @@ package org.eclipse.jetty.http; +import java.util.stream.Stream; + import org.hamcrest.Matchers; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; public class HttpCookieTest { @@ -93,7 +101,6 @@ public void testSetRFC6265Cookie() throws Exception httpCookie = new HttpCookie("everything", "value", "domain", "path", 0, true, true, null, -1); assertEquals("everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly", httpCookie.getRFC6265SetCookie()); - httpCookie = new HttpCookie("everything", "value", "domain", "path", 0, true, true, null, -1, HttpCookie.SameSite.NONE); assertEquals("everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly; SameSite=None", httpCookie.getRFC6265SetCookie()); @@ -102,8 +109,11 @@ public void testSetRFC6265Cookie() throws Exception httpCookie = new HttpCookie("everything", "value", "domain", "path", 0, true, true, null, -1, HttpCookie.SameSite.STRICT); assertEquals("everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly; SameSite=Strict", httpCookie.getRFC6265SetCookie()); + } - String[] badNameExamples = { + public static Stream rfc6265BadNameSource() + { + return Stream.of( "\"name\"", "name\t", "na me", @@ -113,25 +123,32 @@ public void testSetRFC6265Cookie() throws Exception "{name}", "[name]", "\"" - }; + ); + } - for (String badNameExample : badNameExamples) - { - try + @ParameterizedTest + @MethodSource("rfc6265BadNameSource") + public void testSetRFC6265CookieBadName(String badNameExample) + { + IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, + () -> { - httpCookie = new HttpCookie(badNameExample, "value", null, "/", 1, true, true, null, -1); + HttpCookie httpCookie = new HttpCookie(badNameExample, "value", null, "/", 1, true, true, null, -1); httpCookie.getRFC6265SetCookie(); - fail(badNameExample); - } - catch (IllegalArgumentException ex) - { - // System.err.printf("%s: %s%n", ex.getClass().getSimpleName(), ex.getMessage()); - assertThat("Testing bad name: [" + badNameExample + "]", ex.getMessage(), - allOf(containsString("RFC6265"), containsString("RFC2616"))); - } - } + }); + // make sure that exception mentions just how mad of a name it truly is + assertThat("message", ex.getMessage(), + allOf( + // violation of Cookie spec + containsString("RFC6265"), + // violation of HTTP spec + containsString("RFC2616") + )); + } - String[] badValueExamples = { + public static Stream rfc6265BadValueSource() + { + return Stream.of( "va\tlue", "\t", "value\u0000", @@ -143,39 +160,44 @@ public void testSetRFC6265Cookie() throws Exception "val\\ue", "val\"ue", "\"" - }; + ); + } - for (String badValueExample : badValueExamples) - { - try + @ParameterizedTest + @MethodSource("rfc6265BadValueSource") + public void testSetRFC6265CookieBadValue(String badValueExample) + { + IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, + () -> { - httpCookie = new HttpCookie("name", badValueExample, null, "/", 1, true, true, null, -1); + HttpCookie httpCookie = new HttpCookie("name", badValueExample, null, "/", 1, true, true, null, -1); httpCookie.getRFC6265SetCookie(); - fail(); - } - catch (IllegalArgumentException ex) - { - // System.err.printf("%s: %s%n", ex.getClass().getSimpleName(), ex.getMessage()); - assertThat("Testing bad value [" + badValueExample + "]", ex.getMessage(), Matchers.containsString("RFC6265")); - } - } + }); + assertThat("message", ex.getMessage(), containsString("RFC6265")); + } - String[] goodNameExamples = { + public static Stream rfc6265GoodNameSource() + { + return Stream.of( "name", "n.a.m.e", "na-me", "+name", "na*me", "na$me", - "#name" - }; + "#name"); + } - for (String goodNameExample : goodNameExamples) - { - httpCookie = new HttpCookie(goodNameExample, "value", null, "/", 1, true, true, null, -1); - // should not throw an exception - } + @ParameterizedTest + @MethodSource("rfc6265GoodNameSource") + public void testSetRFC6265CookieGoodName(String goodNameExample) + { + new HttpCookie(goodNameExample, "value", null, "/", 1, true, true, null, -1); + // should not throw an exception + } + public static Stream rfc6265GoodValueSource() + { String[] goodValueExamples = { "value", "", @@ -185,37 +207,150 @@ public void testSetRFC6265Cookie() throws Exception "val/ue", "v.a.l.u.e" }; + return Stream.of(goodValueExamples); + } - for (String goodValueExample : goodValueExamples) - { - httpCookie = new HttpCookie("name", goodValueExample, null, "/", 1, true, true, null, -1); - // should not throw an exception - } + @ParameterizedTest + @MethodSource("rfc6265GoodValueSource") + public void testSetRFC6265CookieGoodValue(String goodValueExample) + { + new HttpCookie("name", goodValueExample, null, "/", 1, true, true, null, -1); + // should not throw an exception } - @Test - public void testGetHttpOnlyFromComment() + @ParameterizedTest + @ValueSource(strings = { + "__HTTP_ONLY__", + "__HTTP_ONLY__comment", + "comment__HTTP_ONLY__" + }) + public void testIsHttpOnlyInCommentTrue(String comment) { - assertTrue(HttpCookie.isHttpOnlyInComment("__HTTP_ONLY__")); - assertTrue(HttpCookie.isHttpOnlyInComment("__HTTP_ONLY__comment")); - assertFalse(HttpCookie.isHttpOnlyInComment("comment")); + assertTrue(HttpCookie.isHttpOnlyInComment(comment), "Comment \"" + comment + "\""); } - @Test - public void testGetSameSiteFromComment() + @ParameterizedTest + @ValueSource(strings = { + "comment", + "", + "__", + "__HTTP__ONLY__", + "__http_only__", + "HTTP_ONLY", + "__HTTP__comment__ONLY__" + }) + public void testIsHttpOnlyInCommentFalse(String comment) + { + assertFalse(HttpCookie.isHttpOnlyInComment(comment), "Comment \"" + comment + "\""); + } + + @ParameterizedTest + @ValueSource(strings = { + "__SAME_SITE_NONE__", + "__SAME_SITE_NONE____SAME_SITE_NONE__" + }) + public void testGetSameSiteFromCommentNONE(String comment) + { + assertEquals(HttpCookie.getSameSiteFromComment(comment), HttpCookie.SameSite.NONE, "Comment \"" + comment + "\""); + } + + @ParameterizedTest + @ValueSource(strings = { + "__SAME_SITE_LAX__", + "__SAME_SITE_LAX____SAME_SITE_NONE__", + "__SAME_SITE_NONE____SAME_SITE_LAX__", + "__SAME_SITE_LAX____SAME_SITE_NONE__" + }) + public void testGetSameSiteFromCommentLAX(String comment) + { + assertEquals(HttpCookie.getSameSiteFromComment(comment), HttpCookie.SameSite.LAX, "Comment \"" + comment + "\""); + } + + @ParameterizedTest + @ValueSource(strings = { + "__SAME_SITE_STRICT__", + "__SAME_SITE_NONE____SAME_SITE_STRICT____SAME_SITE_LAX__", + "__SAME_SITE_STRICT____SAME_SITE_LAX____SAME_SITE_NONE__", + "__SAME_SITE_STRICT____SAME_SITE_STRICT__" + }) + public void testGetSameSiteFromCommentSTRICT(String comment) { - assertEquals(HttpCookie.getSameSiteFromComment("__SAME_SITE_NONE__"), HttpCookie.SameSite.NONE); - assertEquals(HttpCookie.getSameSiteFromComment("__SAME_SITE_LAX__"), HttpCookie.SameSite.LAX); - assertEquals(HttpCookie.getSameSiteFromComment("__SAME_SITE_STRICT__"), HttpCookie.SameSite.STRICT); - assertEquals(HttpCookie.getSameSiteFromComment("__SAME_SITE_NONE____SAME_SITE_STRICT__"), HttpCookie.SameSite.NONE); - assertNull(HttpCookie.getSameSiteFromComment("comment")); + assertEquals(HttpCookie.getSameSiteFromComment(comment), HttpCookie.SameSite.STRICT, "Comment \"" + comment + "\""); + } + + /** + * A comment that does not have a declared SamesSite attribute defined + */ + @ParameterizedTest + @ValueSource(strings = { + "__HTTP_ONLY__", + "comment", + // not jetty attributes + "SameSite=None", + "SameSite=Lax", + "SameSite=Strict", + // incomplete jetty attributes + "SAME_SITE_NONE", + "SAME_SITE_LAX", + "SAME_SITE_STRICT", + }) + public void testGetSameSiteFromCommentUndefined(String comment) + { + assertNull(HttpCookie.getSameSiteFromComment(comment), "Comment \"" + comment + "\""); + } + + public static Stream getCommentWithoutAttributesSource() + { + return Stream.of( + // normal - only attribute comment + Arguments.of("__SAME_SITE_LAX__", null), + // normal - no attribute comment + Arguments.of("comment", "comment"), + // mixed - attributes at end + Arguments.of("comment__SAME_SITE_NONE__", "comment"), + Arguments.of("comment__HTTP_ONLY____SAME_SITE_NONE__", "comment"), + // mixed - attributes at start + Arguments.of("__SAME_SITE_NONE__comment", "comment"), + Arguments.of("__HTTP_ONLY____SAME_SITE_NONE__comment", "comment"), + // mixed - attributes at start and end + Arguments.of("__SAME_SITE_NONE__comment__HTTP_ONLY__", "comment"), + Arguments.of("__HTTP_ONLY__comment__SAME_SITE_NONE__", "comment") + ); + } + + @ParameterizedTest + @MethodSource("getCommentWithoutAttributesSource") + public void testGetCommentWithoutAttributes(String rawComment, String expectedComment) + { + String actualComment = HttpCookie.getCommentWithoutAttributes(rawComment); + if (expectedComment == null) + { + assertNull(actualComment); + } + else + { + assertEquals(actualComment, expectedComment); + } } @Test - public void getCommentWithoutAttributes() + public void testGetCommentWithAttributes() { - assertEquals(HttpCookie.getCommentWithoutAttributes("comment__SAME_SITE_NONE__"), "comment"); - assertEquals(HttpCookie.getCommentWithoutAttributes("comment__HTTP_ONLY____SAME_SITE_NONE__"), "comment"); - assertNull(HttpCookie.getCommentWithoutAttributes("__SAME_SITE_LAX__")); + assertThat(HttpCookie.getCommentWithAttributes(null, false, null), nullValue()); + assertThat(HttpCookie.getCommentWithAttributes("", false, null), nullValue()); + assertThat(HttpCookie.getCommentWithAttributes("hello", false, null), is("hello")); + + assertThat(HttpCookie.getCommentWithAttributes(null, true, HttpCookie.SameSite.STRICT), + is("__HTTP_ONLY____SAME_SITE_STRICT__")); + assertThat(HttpCookie.getCommentWithAttributes("", true, HttpCookie.SameSite.NONE), + is("__HTTP_ONLY____SAME_SITE_NONE__")); + assertThat(HttpCookie.getCommentWithAttributes("hello", true, HttpCookie.SameSite.LAX), + is("hello__HTTP_ONLY____SAME_SITE_LAX__")); + + assertThat(HttpCookie.getCommentWithAttributes("__HTTP_ONLY____SAME_SITE_LAX__", false, null), nullValue()); + assertThat(HttpCookie.getCommentWithAttributes("__HTTP_ONLY____SAME_SITE_LAX__", true, HttpCookie.SameSite.NONE), + is("__HTTP_ONLY____SAME_SITE_NONE__")); + assertThat(HttpCookie.getCommentWithAttributes("__HTTP_ONLY____SAME_SITE_LAX__hello", true, HttpCookie.SameSite.LAX), + is("hello__HTTP_ONLY____SAME_SITE_LAX__")); } } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/RateControl.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/RateControl.java index be8f82a7cf22..78970f45f440 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/RateControl.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/RateControl.java @@ -18,6 +18,8 @@ package org.eclipse.jetty.http2.parser; +import org.eclipse.jetty.io.EndPoint; + /** * Controls rate of events via {@link #onEvent(Object)}. */ @@ -36,4 +38,19 @@ public interface RateControl * @return true IFF the rate is within limits */ public boolean onEvent(Object event); + + /** + * Factory to create RateControl instances. + */ + public interface Factory + { + /** + * @return a new RateControl instance for the given EndPoint + * @param endPoint the EndPoint for which the RateControl is created + */ + public default RateControl newRateControl(EndPoint endPoint) + { + return NO_RATE_CONTROL; + } + } } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/WindowRateControl.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/WindowRateControl.java index 4c00b01218b9..951051bb3a3c 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/WindowRateControl.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/WindowRateControl.java @@ -23,6 +23,8 @@ import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicInteger; +import org.eclipse.jetty.io.EndPoint; + /** *

An implementation of {@link RateControl} that limits the number of * events within a time period.

@@ -48,6 +50,21 @@ public WindowRateControl(int maxEvents, Duration window) { this.maxEvents = maxEvents; this.window = window.toNanos(); + if (this.window == 0) + throw new IllegalArgumentException("Invalid duration " + window); + } + + public int getEventsPerSecond() + { + try + { + long rate = maxEvents * 1_000_000_000L / window; + return Math.toIntExact(rate); + } + catch (ArithmeticException x) + { + return Integer.MAX_VALUE; + } } @Override @@ -67,4 +84,20 @@ public boolean onEvent(Object event) events.add(now + window); return size.incrementAndGet() <= maxEvents; } + + public static class Factory implements RateControl.Factory + { + private final int maxEventRate; + + public Factory(int maxEventRate) + { + this.maxEventRate = maxEventRate; + } + + @Override + public RateControl newRateControl(EndPoint endPoint) + { + return WindowRateControl.fromEventsPerSecond(maxEventRate); + } + } } diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java index c15bcd25e262..c608b59b975e 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java @@ -105,12 +105,24 @@ public void onHeaders(Stream stream, HeadersFrame frame) if (frame.isEndStream() || informational) responseSuccess(exchange); } + else + { + if (frame.isEndStream()) + { + // There is no demand to trigger response success, so add + // a poison pill to trigger it when there will be demand. + notifyContent(exchange, new DataFrame(stream.getId(), BufferUtil.EMPTY_BUFFER, true), Callback.NOOP); + } + } } } else // Response trailers. { HttpFields trailers = metaData.getFields(); trailers.forEach(httpResponse::trailer); + // Previous DataFrames had endStream=false, so + // add a poison pill to trigger response success + // after all normal DataFrames have been consumed. notifyContent(exchange, new DataFrame(stream.getId(), BufferUtil.EMPTY_BUFFER, true), Callback.NOOP); } } @@ -200,7 +212,7 @@ private void notifyContent(HttpExchange exchange, DataFrame frame, Callback call contentNotifier.offer(exchange, frame, callback); } - private static class ContentNotifier + private class ContentNotifier { private final Queue queue = new ArrayDeque<>(); private final HttpReceiverOverHTTP2 receiver; @@ -234,9 +246,25 @@ private void enqueue(DataInfo dataInfo) private void process(boolean resume) { // Allow only one thread at a time. - if (active(resume)) + boolean busy = active(resume); + if (LOG.isDebugEnabled()) + LOG.debug("Resuming({}) processing({}) of content", resume, !busy); + if (busy) return; + // Process only if there is demand. + synchronized (this) + { + if (!resume && demand() <= 0) + { + if (LOG.isDebugEnabled()) + LOG.debug("Stalling processing, content available but no demand"); + active = false; + stalled = true; + return; + } + } + while (true) { if (dataInfo != null) @@ -253,7 +281,7 @@ private void process(boolean resume) { dataInfo = queue.poll(); if (LOG.isDebugEnabled()) - LOG.debug("Dequeued content {}", dataInfo); + LOG.debug("Processing content {}", dataInfo); if (dataInfo == null) { active = false; @@ -269,8 +297,12 @@ private void process(boolean resume) boolean proceed = receiver.responseContent(dataInfo.exchange, buffer, Callback.from(callback::succeeded, x -> fail(callback, x))); if (!proceed) { - // Should stall, unless just resumed. - if (stall()) + // The call to responseContent() said we should + // stall, but another thread may have just resumed. + boolean stall = stall(); + if (LOG.isDebugEnabled()) + LOG.debug("Stalling({}) processing", stall); + if (stall) return; } } @@ -287,27 +319,46 @@ private boolean active(boolean resume) { if (active) { + // There is a thread in process(), + // but it may be about to exit, so + // remember "resume" to signal the + // processing thread to continue. if (resume) this.resume = true; return true; } + + // If there is no demand (i.e. stalled + // and not resuming) then don't process. if (stalled && !resume) return true; + + // Start processing. active = true; stalled = false; return false; } } + /** + * Called when there is no demand, this method checks whether + * the processing should really stop or it should continue. + * + * @return true to stop processing, false to continue processing + */ private boolean stall() { synchronized (this) { if (resume) { + // There was no demand, but another thread + // just demanded, continue processing. resume = false; return false; } + + // There is no demand, stop processing. active = false; stalled = true; return true; @@ -332,7 +383,7 @@ private void fail(Callback callback, Throwable failure) receiver.responseFailure(failure); } - private static class DataInfo + private class DataInfo { private final HttpExchange exchange; private final DataFrame frame; diff --git a/jetty-http2/http2-server/src/main/config/etc/jetty-http2.xml b/jetty-http2/http2-server/src/main/config/etc/jetty-http2.xml index 43a59e88c514..398ac69caadd 100644 --- a/jetty-http2/http2-server/src/main/config/etc/jetty-http2.xml +++ b/jetty-http2/http2-server/src/main/config/etc/jetty-http2.xml @@ -1,6 +1,6 @@ - + + - @@ -10,10 +10,10 @@ - - + + - + diff --git a/jetty-http2/http2-server/src/main/config/etc/jetty-http2c.xml b/jetty-http2/http2-server/src/main/config/etc/jetty-http2c.xml index a9e4380c0aa2..be655550cc8e 100644 --- a/jetty-http2/http2-server/src/main/config/etc/jetty-http2c.xml +++ b/jetty-http2/http2-server/src/main/config/etc/jetty-http2c.xml @@ -1,6 +1,6 @@ - + + - @@ -9,10 +9,10 @@ - - + + - + diff --git a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java index 7b5319efbc59..e12c3e93c072 100644 --- a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java +++ b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java @@ -19,7 +19,6 @@ package org.eclipse.jetty.http2.server; import java.io.IOException; -import java.time.Duration; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -61,7 +60,7 @@ public abstract class AbstractHTTP2ServerConnectionFactory extends AbstractConne private int maxHeaderBlockFragment = 0; private int maxFrameLength = Frame.DEFAULT_MAX_LENGTH; private int maxSettingsKeys = SettingsFrame.DEFAULT_MAX_KEYS; - private RateControl rateControl = new WindowRateControl(20, Duration.ofSeconds(1)); + private RateControl.Factory rateControlFactory = new WindowRateControl.Factory(20); private FlowControlStrategy.Factory flowControlStrategyFactory = () -> new BufferingFlowControlStrategy(0.5F); private long streamIdleTimeout; @@ -182,14 +181,47 @@ public void setMaxSettingsKeys(int maxSettingsKeys) this.maxSettingsKeys = maxSettingsKeys; } + /** + * @return null + * @deprecated use {@link #getRateControlFactory()} instead + */ + @Deprecated public RateControl getRateControl() { - return rateControl; + return null; } + /** + * @param rateControl ignored, unless {@code rateControl} it is precisely a WindowRateControl + * (not a subclass) object in which case it is used as a prototype in a WindowRateControl.Factory. + * @throws UnsupportedOperationException when invoked, unless precisely a WindowRateControl object + * @deprecated use {@link #setRateControlFactory(RateControl.Factory)} instead + */ + @Deprecated public void setRateControl(RateControl rateControl) { - this.rateControl = rateControl; + if (rateControl.getClass() == WindowRateControl.class) + setRateControlFactory(new WindowRateControl.Factory(((WindowRateControl)rateControl).getEventsPerSecond())); + else + throw new UnsupportedOperationException(); + } + + /** + * @return the factory that creates RateControl objects + */ + public RateControl.Factory getRateControlFactory() + { + return rateControlFactory; + } + + /** + *

Sets the factory that creates a per-connection RateControl object.

+ * + * @param rateControlFactory the factory that creates RateControl objects + */ + public void setRateControlFactory(RateControl.Factory rateControlFactory) + { + this.rateControlFactory = Objects.requireNonNull(rateControlFactory); } /** @@ -251,7 +283,7 @@ public Connection newConnection(Connector connector, EndPoint endPoint) session.setInitialSessionRecvWindow(getInitialSessionRecvWindow()); session.setWriteThreshold(getHttpConfiguration().getOutputBufferSize()); - ServerParser parser = newServerParser(connector, session, getRateControl()); + ServerParser parser = newServerParser(connector, session, getRateControlFactory().newRateControl(endPoint)); parser.setMaxFrameLength(getMaxFrameLength()); parser.setMaxSettingsKeys(getMaxSettingsKeys()); diff --git a/jetty-memcached/jetty-memcached-sessions/src/test/java/org/eclipse/jetty/memcached/session/TestMemcachedSessions.java b/jetty-memcached/jetty-memcached-sessions/src/test/java/org/eclipse/jetty/memcached/session/TestMemcachedSessions.java index ffc3d8d68f8a..01988effeb16 100644 --- a/jetty-memcached/jetty-memcached-sessions/src/test/java/org/eclipse/jetty/memcached/session/TestMemcachedSessions.java +++ b/jetty-memcached/jetty-memcached-sessions/src/test/java/org/eclipse/jetty/memcached/session/TestMemcachedSessions.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.io.PrintWriter; + import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; @@ -31,12 +32,9 @@ import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.server.NetworkConnector; import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.session.AbstractSessionCache; import org.eclipse.jetty.server.session.CachingSessionDataStore; +import org.eclipse.jetty.server.session.NullSessionCache; import org.eclipse.jetty.server.session.NullSessionDataStore; -import org.eclipse.jetty.server.session.Session; -import org.eclipse.jetty.server.session.SessionData; -import org.eclipse.jetty.server.session.SessionHandler; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; import org.junit.jupiter.api.Test; @@ -88,56 +86,6 @@ else if ("del".equals(arg)) } } - public static class NullSessionCache extends AbstractSessionCache - { - - public NullSessionCache(SessionHandler handler) - { - super(handler); - } - - @Override - public void shutdown() - { - } - - @Override - public Session newSession(SessionData data) - { - return new Session(_handler, data); - } - - @Override - public Session newSession(HttpServletRequest request, SessionData data) - { - return new Session(_handler, request, data); - } - - @Override - public Session doGet(String id) - { - return null; - } - - @Override - public Session doPutIfAbsent(String id, Session session) - { - return null; - } - - @Override - public boolean doReplace(String id, Session oldValue, Session newValue) - { - return true; - } - - @Override - public Session doDelete(String id) - { - return null; - } - } - @Test public void testMemcached() throws Exception { diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java index 0642b422a24b..231d967360a5 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java @@ -251,8 +251,7 @@ public Authentication validateRequest(ServletRequest req, ServletResponse res, b if (LOG.isDebugEnabled()) LOG.debug("Session ID should be cookie for OpenID authentication to work"); - int redirectCode = (baseRequest.getHttpVersion().getVersion() < HttpVersion.HTTP_1_1.getVersion() ? HttpServletResponse.SC_MOVED_TEMPORARILY : HttpServletResponse.SC_SEE_OTHER); - baseResponse.sendRedirect(redirectCode, URIUtil.addPaths(request.getContextPath(), _errorPage)); + baseResponse.sendRedirect(getRedirectCode(baseRequest.getHttpVersion()), URIUtil.addPaths(request.getContextPath(), _errorPage)); return Authentication.SEND_FAILURE; } @@ -297,8 +296,7 @@ public Authentication validateRequest(ServletRequest req, ServletResponse res, b LOG.debug("authenticated {}->{}", openIdAuth, nuri); response.setContentLength(0); - int redirectCode = (baseRequest.getHttpVersion().getVersion() < HttpVersion.HTTP_1_1.getVersion() ? HttpServletResponse.SC_MOVED_TEMPORARILY : HttpServletResponse.SC_SEE_OTHER); - baseResponse.sendRedirect(redirectCode, nuri); + baseResponse.sendRedirect(getRedirectCode(baseRequest.getHttpVersion()), nuri); return openIdAuth; } } @@ -317,8 +315,7 @@ public Authentication validateRequest(ServletRequest req, ServletResponse res, b { if (LOG.isDebugEnabled()) LOG.debug("auth failed {}", _errorPage); - int redirectCode = (baseRequest.getHttpVersion().getVersion() < HttpVersion.HTTP_1_1.getVersion() ? HttpServletResponse.SC_MOVED_TEMPORARILY : HttpServletResponse.SC_SEE_OTHER); - baseResponse.sendRedirect(redirectCode, URIUtil.addPaths(request.getContextPath(), _errorPage)); + baseResponse.sendRedirect(getRedirectCode(baseRequest.getHttpVersion()), URIUtil.addPaths(request.getContextPath(), _errorPage)); } return Authentication.SEND_FAILURE; @@ -408,8 +405,7 @@ public Authentication validateRequest(ServletRequest req, ServletResponse res, b String challengeUri = getChallengeUri(request); if (LOG.isDebugEnabled()) LOG.debug("challenge {}->{}", session.getId(), challengeUri); - int redirectCode = (baseRequest.getHttpVersion().getVersion() < HttpVersion.HTTP_1_1.getVersion() ? HttpServletResponse.SC_MOVED_TEMPORARILY : HttpServletResponse.SC_SEE_OTHER); - baseResponse.sendRedirect(redirectCode, challengeUri); + baseResponse.sendRedirect(getRedirectCode(baseRequest.getHttpVersion()), challengeUri); return Authentication.SEND_CONTINUE; } @@ -437,6 +433,12 @@ public boolean isErrorPage(String pathInContext) return pathInContext != null && (pathInContext.equals(_errorPath)); } + private static int getRedirectCode(HttpVersion httpVersion) + { + return (httpVersion.getVersion() < HttpVersion.HTTP_1_1.getVersion() + ? HttpServletResponse.SC_MOVED_TEMPORARILY : HttpServletResponse.SC_SEE_OTHER); + } + private String getRedirectUri(HttpServletRequest request) { final StringBuffer redirectUri = new StringBuffer(128); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index c2ae7433f178..710e68a62040 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -124,13 +124,14 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu }; public static final int DEFAULT_LISTENER_TYPE_INDEX = 1; + public static final int EXTENDED_LISTENER_TYPE_INDEX = 0; private static final String UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER = "Unimplemented {} - use org.eclipse.jetty.servlet.ServletContextHandler"; private static final Logger LOG = Log.getLogger(ContextHandler.class); - private static final ThreadLocal __context = new ThreadLocal(); + private static final ThreadLocal __context = new ThreadLocal<>(); private static String __serverInfo = "jetty/" + Server.getVersion(); @@ -212,7 +213,7 @@ public static void setServerInfo(String serverInfo) private final List _contextListeners = new CopyOnWriteArrayList<>(); private final List _durableListeners = new CopyOnWriteArrayList<>(); private String[] _protectedTargets; - private final CopyOnWriteArrayList _aliasChecks = new CopyOnWriteArrayList(); + private final CopyOnWriteArrayList _aliasChecks = new CopyOnWriteArrayList<>(); public enum Availability { @@ -245,7 +246,7 @@ private ContextHandler(Context context, HandlerContainer parent, String contextP { _scontext = context == null ? new Context() : context; _attributes = new AttributesMap(); - _initParams = new HashMap(); + _initParams = new HashMap<>(); addAliasCheck(new ApproveNonExistentDirectoryAliases()); if (File.separatorChar == '/') addAliasCheck(new AllowSymLinkAliasChecker()); @@ -355,7 +356,7 @@ public void setVirtualHosts(String[] vhosts) if (connectorIndex == 0) { if (connectorOnlyIndexes == null) - connectorOnlyIndexes = new ArrayList(); + connectorOnlyIndexes = new ArrayList<>(); connectorOnlyIndexes.add(i); } } @@ -414,7 +415,7 @@ public void addVirtualHosts(String[] virtualHosts) } else { - Set currentVirtualHosts = new HashSet(Arrays.asList(getVirtualHosts())); + Set currentVirtualHosts = new HashSet<>(Arrays.asList(getVirtualHosts())); for (String vh : virtualHosts) { currentVirtualHosts.add(normalizeHostname(vh)); @@ -439,7 +440,7 @@ public void removeVirtualHosts(String[] virtualHosts) if (virtualHosts == null || virtualHosts.length == 0 || _vhosts == null || _vhosts.length == 0) return; // do nothing - Set existingVirtualHosts = new HashSet(Arrays.asList(getVirtualHosts())); + Set existingVirtualHosts = new HashSet<>(Arrays.asList(getVirtualHosts())); for (String vh : virtualHosts) { existingVirtualHosts.remove(normalizeHostname(vh)); @@ -612,7 +613,7 @@ public String getDisplayName() public EventListener[] getEventListeners() { - return _eventListeners.toArray(new EventListener[_eventListeners.size()]); + return _eventListeners.toArray(new EventListener[0]); } /** @@ -979,7 +980,7 @@ protected void doStop() throws Exception stopContext(); // retain only durable listeners - setEventListeners(_durableListeners.toArray(new EventListener[_durableListeners.size()])); + setEventListeners(_durableListeners.toArray(new EventListener[0])); _durableListeners.clear(); if (_errorHandler != null) @@ -1133,7 +1134,7 @@ public void doScope(String target, Request baseRequest, HttpServletRequest reque if (LOG.isDebugEnabled()) LOG.debug("scope {}|{}|{} @ {}", baseRequest.getContextPath(), baseRequest.getServletPath(), baseRequest.getPathInfo(), this); - Context oldContext = null; + Context oldContext; String oldContextPath = null; String oldServletPath = null; String oldPathInfo = null; @@ -1151,7 +1152,7 @@ public void doScope(String target, Request baseRequest, HttpServletRequest reque // check the target. if (DispatcherType.REQUEST.equals(dispatch) || DispatcherType.ASYNC.equals(dispatch)) { - if (_compactPath) + if (isCompactPath()) target = URIUtil.compactPath(target); if (!checkContext(target, baseRequest, response)) return; @@ -1197,7 +1198,7 @@ else if (_contextPath.length() == 1) if (_contextPath.length() == 1) baseRequest.setContextPath(""); else - baseRequest.setContextPath(_contextPathEncoded); + baseRequest.setContextPath(getContextPathEncoded()); baseRequest.setServletPath(null); baseRequest.setPathInfo(pathInfo); } @@ -1756,7 +1757,7 @@ public synchronized Class loadClass(String className) throws ClassNotFoundExc public void addLocaleEncoding(String locale, String encoding) { if (_localeEncodingMap == null) - _localeEncodingMap = new HashMap(); + _localeEncodingMap = new HashMap<>(); _localeEncodingMap.put(locale, encoding); } @@ -1836,7 +1837,7 @@ public boolean checkAlias(String path, Resource resource) LOG.debug("Aliased resource: " + resource + "~=" + resource.getAlias()); // alias checks - for (Iterator i = _aliasChecks.iterator(); i.hasNext(); ) + for (Iterator i = getAliasChecks().iterator(); i.hasNext(); ) { AliasCheck check = i.next(); if (check.check(path, resource)) @@ -1902,7 +1903,7 @@ public Set getResourcePaths(String path) String[] l = resource.list(); if (l != null) { - HashSet set = new HashSet(); + HashSet set = new HashSet<>(); for (int i = 0; i < l.length; i++) { set.add(path + l[i]); @@ -1945,7 +1946,7 @@ private String normalizeHostname(String host) */ public void addAliasCheck(AliasCheck check) { - _aliasChecks.add(check); + getAliasChecks().add(check); } /** @@ -1961,8 +1962,8 @@ public List getAliasChecks() */ public void setAliasChecks(List checks) { - _aliasChecks.clear(); - _aliasChecks.addAll(checks); + getAliasChecks().clear(); + getAliasChecks().addAll(checks); } /** @@ -1970,13 +1971,14 @@ public void setAliasChecks(List checks) */ public void clearAliasChecks() { - _aliasChecks.clear(); + getAliasChecks().clear(); } /** * Context. *

- * A partial implementation of {@link javax.servlet.ServletContext}. A complete implementation is provided by the derived {@link ContextHandler}. + * A partial implementation of {@link javax.servlet.ServletContext}. A complete implementation is provided by the + * derived {@link ContextHandler} implementations. *

*/ public class Context extends StaticContext @@ -1999,7 +2001,7 @@ public ContextHandler getContextHandler() @Override public ServletContext getContext(String uripath) { - List contexts = new ArrayList(); + List contexts = new ArrayList<>(); Handler[] handlers = getServer().getChildHandlersByClass(ContextHandler.class); String matchedPath = null; @@ -2074,7 +2076,7 @@ public ServletContext getContext(String uripath) matchedPath = contextPath; } - if (matchedPath != null && matchedPath.equals(contextPath)) + if (matchedPath.equals(contextPath)) contexts.add(ch); } } @@ -2266,7 +2268,7 @@ public synchronized Object getAttribute(String name) @Override public synchronized Enumeration getAttributeNames() { - HashSet set = new HashSet(); + HashSet set = new HashSet<>(); Enumeration e = super.getAttributeNames(); while (e.hasMoreElements()) { @@ -2413,19 +2415,6 @@ public void addListener(Class listenerClass) } } - @Override - public T createListener(Class clazz) throws ServletException - { - try - { - return createInstance(clazz); - } - catch (Exception e) - { - throw new ServletException(e); - } - } - public void checkListener(Class listener) throws IllegalStateException { boolean ok = false; @@ -2459,7 +2448,7 @@ public ClassLoader getClassLoader() throw new UnsupportedOperationException(); // no security manager just return the classloader - if (!_usingSecurityManager) + if (!isUsingSecurityManager()) { return _classLoader; } @@ -2515,12 +2504,6 @@ public boolean isEnabled() return _enabled; } - public T createInstance(Class clazz) throws Exception - { - T o = clazz.getDeclaredConstructor().newInstance(); - return o; - } - @Override public String getVirtualServerName() { @@ -2531,15 +2514,16 @@ public String getVirtualServerName() } } + /** + * A simple implementation of ServletContext that is used when there is no + * ContextHandler. This is also used as the base for all other ServletContext + * implementations. + */ public static class StaticContext extends AttributesMap implements ServletContext { private int _effectiveMajorVersion = SERVLET_MAJOR_VERSION; private int _effectiveMinorVersion = SERVLET_MINOR_VERSION; - public StaticContext() - { - } - @Override public ServletContext getContext(String uripath) { @@ -2603,7 +2587,7 @@ public Set getResourcePaths(String path) @Override public String getServerInfo() { - return __serverInfo; + return ContextHandler.getServerInfo(); } @Override @@ -2720,20 +2704,6 @@ public javax.servlet.ServletRegistration.Dynamic addServlet(String servletName, return null; } - @Override - public T createFilter(Class c) throws ServletException - { - LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "createFilter(Class)"); - return null; - } - - @Override - public T createServlet(Class c) throws ServletException - { - LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "createServlet(Class)"); - return null; - } - @Override public Set getDefaultSessionTrackingModes() { @@ -2807,8 +2777,7 @@ public void addListener(Class listenerClass) LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "addListener(Class)"); } - @Override - public T createListener(Class clazz) throws ServletException + protected T createInstance(Class clazz) throws ServletException { try { @@ -2820,6 +2789,24 @@ public T createListener(Class clazz) throws Servlet } } + @Override + public T createListener(Class clazz) throws ServletException + { + return createInstance(clazz); + } + + @Override + public T createServlet(Class clazz) throws ServletException + { + return createInstance(clazz); + } + + @Override + public T createFilter(Class clazz) throws ServletException + { + return createInstance(clazz); + } + @Override public ClassLoader getClassLoader() { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ManagedAttributeListener.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ManagedAttributeListener.java index 9e4fe6e44c53..d57225d3e535 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ManagedAttributeListener.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ManagedAttributeListener.java @@ -100,7 +100,6 @@ public void contextDestroyed(ServletContextEvent event) protected void updateBean(String name, Object oldBean, Object newBean) { - LOG.info("update {} {}->{} on {}", name, oldBean, newBean, _context); if (LOG.isDebugEnabled()) LOG.debug("update {} {}->{} on {}", name, oldBean, newBean, _context); _context.updateBean(oldBean, newBean, false); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java index 48ccac954815..821485b98f88 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java @@ -21,6 +21,8 @@ import java.util.Collections; import java.util.HashSet; import java.util.Set; +import java.util.function.Function; + import javax.servlet.http.HttpServletRequest; import org.eclipse.jetty.util.StringUtil; @@ -133,6 +135,18 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements * @return null if the session wasn't already in the map, or the existing entry otherwise */ protected abstract Session doPutIfAbsent(String id, Session session); + + /** + * Compute the mappingFunction to create a Session object iff the session + * with the given id isn't already in the map, otherwise return the existing Session. + * This method is expected to have precisely the same behaviour as + * {@link java.util.concurrent.ConcurrentHashMap#computeIfAbsent} + * + * @param id the session id + * @param mappingFunction the function to load the data for the session + * @return an existing Session from the cache + */ + protected abstract Session doComputeIfAbsent(String id, Function mappingFunction); /** * Replace the mapping from id to oldValue with newValue @@ -152,22 +166,6 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements */ public abstract Session doDelete(String id); - /** - * PlaceHolder - */ - protected class PlaceHolderSession extends Session - { - - /** - * @param handler SessionHandler to which this session belongs - * @param data the session data - */ - public PlaceHolderSession(SessionHandler handler, SessionData data) - { - super(handler, data); - } - } - /** * @param handler the {@link SessionHandler} to use */ @@ -343,113 +341,53 @@ public Session get(String id) throws Exception protected Session getAndEnter(String id, boolean enter) throws Exception { Session session = null; - Exception ex = null; - while (true) + session = doComputeIfAbsent(id, k -> { - session = doGet(id); - - if (_sessionDataStore == null) - break; //can't load any session data so just return null or the session object + if (LOG.isDebugEnabled()) + LOG.debug("Session {} not found locally in {}, attempting to load", id, this); - if (session == null) + try { - if (LOG.isDebugEnabled()) - LOG.debug("Session {} not found locally in {}, attempting to load", id, this); - - //didn't get a session, try and create one and put in a placeholder for it - PlaceHolderSession phs = new PlaceHolderSession(_handler, new SessionData(id, null, null, 0, 0, 0, 0)); - Lock phsLock = phs.lock(); - Session s = doPutIfAbsent(id, phs); - if (s == null) + Session s = loadSession(k); + if (s != null) { - //My placeholder won, go ahead and load the full session data - try - { - session = loadSession(id); - if (session == null) - { - //session does not exist, remove the placeholder - doDelete(id); - phsLock.close(); - break; - } - - try (Lock lock = session.lock()) - { - //swap it in instead of the placeholder - boolean success = doReplace(id, phs, session); - if (!success) - { - //something has gone wrong, it should have been our placeholder - doDelete(id); - session = null; - LOG.warn("Replacement of placeholder for session {} failed", id); - phsLock.close(); - break; - } - else - { - //successfully swapped in the session - session.setResident(true); - if (enter) - session.use(); - phsLock.close(); - break; - } - } - } - catch (Exception e) + try (Lock lock = s.lock()) { - ex = e; //remember a problem happened loading the session - doDelete(id); //remove the placeholder - phsLock.close(); - session = null; - break; + s.setResident(true); //ensure freshly loaded session is resident } } else { - //my placeholder didn't win, check the session returned - phsLock.close(); - try (Lock lock = s.lock()) - { - //is it a placeholder? or is a non-resident session? In both cases, chuck it away and start again - if (!s.isResident() || s instanceof PlaceHolderSession) - { - session = null; - continue; - } - //I will use this session too - session = s; - if (enter) - session.use(); - break; - } + if (LOG.isDebugEnabled()) + LOG.debug("Session {} not loaded by store", id); } + return s; } - else + catch (Exception e) { - //check the session returned - try (Lock lock = session.lock()) - { - //is it a placeholder? or is it passivated? In both cases, chuck it away and start again - if (!session.isResident() || session instanceof PlaceHolderSession) - { - session = null; - continue; - } + LOG.warn("Error loading session {}", id, e); + return null; + } + }); - //got the session - if (enter) - session.use(); - break; + if (session != null) + { + try (Lock lock = session.lock()) + { + if (!session.isResident()) //session isn't marked as resident in cache + { + if (LOG.isDebugEnabled()) + LOG.debug("Non-resident session {} in cache", id); + return null; + } + else if (enter) + { + session.use(); } } } - if (ex != null) - throw ex; return session; } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionCache.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionCache.java index ef00dd46edf8..1cc782bcb4d8 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionCache.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionCache.java @@ -19,6 +19,8 @@ package org.eclipse.jetty.server.session; import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Function; + import javax.servlet.http.HttpServletRequest; import org.eclipse.jetty.util.annotation.ManagedAttribute; @@ -80,18 +82,12 @@ public long getSessionsTotal() return _stats.getTotal(); } - /** - * - */ @ManagedOperation(value = "reset statistics", impact = "ACTION") public void resetStats() { _stats.reset(); } - /** - * @see org.eclipse.jetty.server.session.AbstractSessionCache#doGet(java.lang.String) - */ @Override public Session doGet(String id) { @@ -103,26 +99,32 @@ public Session doGet(String id) return session; } - /** - * @see org.eclipse.jetty.server.session.AbstractSessionCache#doPutIfAbsent(java.lang.String, org.eclipse.jetty.server.session.Session) - */ @Override public Session doPutIfAbsent(String id, Session session) { Session s = _sessions.putIfAbsent(id, session); - if (s == null && !(session instanceof PlaceHolderSession)) + if (s == null) _stats.increment(); return s; } - /** - * @see org.eclipse.jetty.server.session.AbstractSessionCache#doDelete(java.lang.String) - */ + @Override + protected Session doComputeIfAbsent(String id, Function mappingFunction) + { + return _sessions.computeIfAbsent(id, k -> + { + Session s = mappingFunction.apply(k); + if (s != null) + _stats.increment(); + return s; + }); + } + @Override public Session doDelete(String id) { Session s = _sessions.remove(id); - if (s != null && !(s instanceof PlaceHolderSession)) + if (s != null) _stats.decrement(); return s; } @@ -168,9 +170,6 @@ public void shutdown() } } - /** - * @see org.eclipse.jetty.server.session.AbstractSessionCache#newSession(javax.servlet.http.HttpServletRequest, org.eclipse.jetty.server.session.SessionData) - */ @Override public Session newSession(HttpServletRequest request, SessionData data) { @@ -178,9 +177,6 @@ public Session newSession(HttpServletRequest request, SessionData data) return s; } - /** - * @see org.eclipse.jetty.server.session.AbstractSessionCache#newSession(org.eclipse.jetty.server.session.SessionData) - */ @Override public Session newSession(SessionData data) { @@ -188,15 +184,10 @@ public Session newSession(SessionData data) return s; } - /** - * @see org.eclipse.jetty.server.session.AbstractSessionCache#doReplace(java.lang.String, org.eclipse.jetty.server.session.Session, org.eclipse.jetty.server.session.Session) - */ @Override public boolean doReplace(String id, Session oldValue, Session newValue) { boolean result = _sessions.replace(id, oldValue, newValue); - if (result && (oldValue instanceof PlaceHolderSession)) - _stats.increment(); return result; } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionCache.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionCache.java index c589aab4bdb4..ad9d05d33439 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionCache.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionCache.java @@ -18,6 +18,8 @@ package org.eclipse.jetty.server.session; +import java.util.function.Function; + import javax.servlet.http.HttpServletRequest; /** @@ -39,35 +41,23 @@ public NullSessionCache(SessionHandler handler) super.setEvictionPolicy(EVICT_ON_SESSION_EXIT); } - /** - * @see org.eclipse.jetty.server.session.SessionCache#shutdown() - */ @Override public void shutdown() { } - /** - * @see org.eclipse.jetty.server.session.AbstractSessionCache#newSession(org.eclipse.jetty.server.session.SessionData) - */ @Override public Session newSession(SessionData data) { return new Session(getSessionHandler(), data); } - /** - * @see org.eclipse.jetty.server.session.AbstractSessionCache#newSession(javax.servlet.http.HttpServletRequest, org.eclipse.jetty.server.session.SessionData) - */ @Override public Session newSession(HttpServletRequest request, SessionData data) { return new Session(getSessionHandler(), request, data); } - /** - * @see org.eclipse.jetty.server.session.AbstractSessionCache#doGet(java.lang.String) - */ @Override public Session doGet(String id) { @@ -75,9 +65,6 @@ public Session doGet(String id) return null; } - /** - * @see org.eclipse.jetty.server.session.AbstractSessionCache#doPutIfAbsent(java.lang.String, org.eclipse.jetty.server.session.Session) - */ @Override public Session doPutIfAbsent(String id, Session session) { @@ -85,9 +72,6 @@ public Session doPutIfAbsent(String id, Session session) return null; } - /** - * @see org.eclipse.jetty.server.session.AbstractSessionCache#doReplace(java.lang.String, org.eclipse.jetty.server.session.Session, org.eclipse.jetty.server.session.Session) - */ @Override public boolean doReplace(String id, Session oldValue, Session newValue) { @@ -95,21 +79,21 @@ public boolean doReplace(String id, Session oldValue, Session newValue) return true; } - /** - * @see org.eclipse.jetty.server.session.AbstractSessionCache#doDelete(java.lang.String) - */ @Override public Session doDelete(String id) { return null; } - /** - * @see org.eclipse.jetty.server.session.AbstractSessionCache#setEvictionPolicy(int) - */ @Override public void setEvictionPolicy(int evictionTimeout) { LOG.warn("Ignoring eviction setting:" + evictionTimeout); } + + @Override + protected Session doComputeIfAbsent(String id, Function mappingFunction) + { + return mappingFunction.apply(id); + } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java index 65c170a0be27..7f3c455cb897 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java @@ -530,6 +530,16 @@ public boolean getHttpOnly() return _httpOnly; } + /** + * @return The sameSite setting for session cookies or null for no setting + * @see HttpCookie#getSameSite() + */ + @ManagedAttribute("SameSite setting for session cookies") + public HttpCookie.SameSite getSameSite() + { + return HttpCookie.getSameSiteFromComment(_sessionComment); + } + /** * Returns the HttpSession with the given session id * @@ -650,30 +660,18 @@ public HttpCookie getSessionCookie(HttpSession session, String contextPath, bool sessionPath = (StringUtil.isEmpty(sessionPath)) ? "/" : sessionPath; String id = getExtendedId(session); HttpCookie cookie = null; - if (_sessionComment == null) - { - cookie = new HttpCookie( - _cookieConfig.getName(), - id, - _cookieConfig.getDomain(), - sessionPath, - _cookieConfig.getMaxAge(), - _cookieConfig.isHttpOnly(), - _cookieConfig.isSecure() || (isSecureRequestOnly() && requestIsSecure)); - } - else - { - cookie = new HttpCookie( - _cookieConfig.getName(), - id, - _cookieConfig.getDomain(), - sessionPath, - _cookieConfig.getMaxAge(), - _cookieConfig.isHttpOnly(), - _cookieConfig.isSecure() || (isSecureRequestOnly() && requestIsSecure), - _sessionComment, - 1); - } + + cookie = new HttpCookie( + _cookieConfig.getName(), + id, + _cookieConfig.getDomain(), + sessionPath, + _cookieConfig.getMaxAge(), + _cookieConfig.isHttpOnly(), + _cookieConfig.isSecure() || (isSecureRequestOnly() && requestIsSecure), + HttpCookie.getCommentWithoutAttributes(_cookieConfig.getComment()), + 0, + HttpCookie.getSameSiteFromComment(_cookieConfig.getComment())); return cookie; } @@ -814,13 +812,28 @@ public void statsReset() } /** - * @param httpOnly The httpOnly to set. + * Set if Session cookies should use HTTP Only + * @param httpOnly True if cookies should be HttpOnly. + * @see HttpCookie */ public void setHttpOnly(boolean httpOnly) { _httpOnly = httpOnly; } + /** + * Set Session cookie sameSite mode. + * Currently this is encoded in the session comment until sameSite is supported by {@link SessionCookieConfig} + * @param sameSite The sameSite setting for Session cookies (or null for no sameSite setting) + */ + public void setSameSite(HttpCookie.SameSite sameSite) + { + // Encode in comment whilst not supported by SessionConfig, so that it can be set/saved in + // web.xml and quickstart. + // Always pass false for httpOnly as it has it's own setter. + _sessionComment = HttpCookie.getCommentWithAttributes(_sessionComment, false, sameSite); + } + /** * @param metaManager The metaManager used for cross context session management. */ @@ -1364,6 +1377,8 @@ public interface SessionIf extends HttpSession * CookieConfig * * Implementation of the javax.servlet.SessionCookieConfig. + * SameSite configuration can be achieved by using setComment + * @see HttpCookie */ public final class CookieConfig implements SessionCookieConfig { diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/session/SessionCookieTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/session/SessionCookieTest.java index 36dd95d13a5a..b5546f59e495 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/session/SessionCookieTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/session/SessionCookieTest.java @@ -19,6 +19,8 @@ package org.eclipse.jetty.server.session; import java.util.concurrent.TimeUnit; +import java.util.function.Function; + import javax.servlet.SessionCookieConfig; import javax.servlet.http.HttpServletRequest; @@ -35,10 +37,10 @@ public class SessionCookieTest { - public class MockSessionStore extends AbstractSessionCache + public class MockSessionCache extends AbstractSessionCache { - public MockSessionStore(SessionHandler manager) + public MockSessionCache(SessionHandler manager) { super(manager); } @@ -83,6 +85,12 @@ public Session newSession(HttpServletRequest request, SessionData data) { return null; } + + @Override + protected Session doComputeIfAbsent(String id, Function mappingFunction) + { + return mappingFunction.apply(id); + } } public class MockSessionIdManager extends DefaultSessionIdManager @@ -118,9 +126,9 @@ public void testSecureSessionCookie() throws Exception MockSessionIdManager idMgr = new MockSessionIdManager(server); idMgr.setWorkerName("node1"); SessionHandler mgr = new SessionHandler(); - MockSessionStore store = new MockSessionStore(mgr); - store.setSessionDataStore(new NullSessionDataStore()); - mgr.setSessionCache(store); + MockSessionCache cache = new MockSessionCache(mgr); + cache.setSessionDataStore(new NullSessionDataStore()); + mgr.setSessionCache(cache); mgr.setSessionIdManager(idMgr); long now = TimeUnit.NANOSECONDS.toMillis(System.nanoTime()); diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/FilterHolder.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/FilterHolder.java index 9b0f0b6261a5..e81125631273 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/FilterHolder.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/FilterHolder.java @@ -114,7 +114,7 @@ public void initialize() throws Exception try { ServletContext context = getServletHandler().getServletContext(); - _filter = (context instanceof ServletContextHandler.Context) + _filter = (context != null) ? context.createFilter(getHeldClass()) : getHeldClass().getDeclaredConstructor().newInstance(); } @@ -234,7 +234,7 @@ public void addMappingForUrlPatterns(EnumSet dispatcherTypes, bo public Collection getServletNameMappings() { FilterMapping[] mappings = getServletHandler().getFilterMappings(); - List names = new ArrayList(); + List names = new ArrayList<>(); for (FilterMapping mapping : mappings) { if (mapping.getFilterHolder() != FilterHolder.this) @@ -250,7 +250,7 @@ public Collection getServletNameMappings() public Collection getUrlPatternMappings() { FilterMapping[] mappings = getServletHandler().getFilterMappings(); - List patterns = new ArrayList(); + List patterns = new ArrayList<>(); for (FilterMapping mapping : mappings) { if (mapping.getFilterHolder() != FilterHolder.this) diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java index 9313237ea4e7..10f98d1f3c18 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java @@ -87,9 +87,9 @@ public void doStart() throws Exception //create an instance of the listener and decorate it try { - ServletContext scontext = contextHandler.getServletContext(); - _listener = (scontext instanceof ServletContextHandler.Context) - ? scontext.createListener(getHeldClass()) + ServletContext context = contextHandler.getServletContext(); + _listener = (context != null) + ? context.createListener(getHeldClass()) : getHeldClass().getDeclaredConstructor().newInstance(); } catch (ServletException ex) diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletContextHandler.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletContextHandler.java index a2b790839d00..a6ae8d98eb6f 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletContextHandler.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletContextHandler.java @@ -265,7 +265,6 @@ private void relinkHandlers() if (handler.getHandler() != _servletHandler) doSetHandler(handler, _servletHandler); - handler = _servletHandler; } } @@ -316,7 +315,7 @@ protected SecurityHandler newSecurityHandler() { try { - return _defaultSecurityHandlerClass.getDeclaredConstructor().newInstance(); + return getDefaultSecurityHandlerClass().getDeclaredConstructor().newInstance(); } catch (Exception e) { @@ -507,7 +506,7 @@ protected void addRoles(String... roleNames) //Get a reference to the SecurityHandler, which must be ConstraintAware if (_securityHandler != null && _securityHandler instanceof ConstraintAware) { - HashSet union = new HashSet(); + HashSet union = new HashSet<>(); Set existing = ((ConstraintAware)_securityHandler).getRoles(); if (existing != null) union.addAll(existing); @@ -695,7 +694,7 @@ public DecoratedObjectFactory getObjectFactory() @Deprecated public List getDecorators() { - List ret = new ArrayList(); + List ret = new ArrayList<>(); for (org.eclipse.jetty.util.Decorator decorator : _objFactory) { ret.add(new LegacyDecorator(decorator)); @@ -750,13 +749,13 @@ public static ServletContextHandler getServletContextHandler(ServletContext cont public static class JspPropertyGroup implements JspPropertyGroupDescriptor { - private List _urlPatterns = new ArrayList(); + private List _urlPatterns = new ArrayList<>(); private String _elIgnored; private String _pageEncoding; private String _scriptingInvalid; private String _isXml; - private List _includePreludes = new ArrayList(); - private List _includeCodas = new ArrayList(); + private List _includePreludes = new ArrayList<>(); + private List _includeCodas = new ArrayList<>(); private String _deferredSyntaxAllowedAsLiteral; private String _trimDirectiveWhitespaces; private String _defaultContentType; @@ -769,7 +768,7 @@ public static class JspPropertyGroup implements JspPropertyGroupDescriptor @Override public Collection getUrlPatterns() { - return new ArrayList(_urlPatterns); // spec says must be a copy + return new ArrayList<>(_urlPatterns); // spec says must be a copy } public void addUrlPattern(String s) @@ -865,7 +864,7 @@ public String getIsXml() @Override public Collection getIncludePreludes() { - return new ArrayList(_includePreludes); //must be a copy + return new ArrayList<>(_includePreludes); //must be a copy } public void addIncludePrelude(String prelude) @@ -880,7 +879,7 @@ public void addIncludePrelude(String prelude) @Override public Collection getIncludeCodas() { - return new ArrayList(_includeCodas); //must be a copy + return new ArrayList<>(_includeCodas); //must be a copy } public void addIncludeCoda(String coda) @@ -937,24 +936,24 @@ public String getErrorOnUndeclaredNamespace() @Override public String toString() { - StringBuffer sb = new StringBuffer(); + StringBuilder sb = new StringBuilder(); sb.append("JspPropertyGroupDescriptor:"); - sb.append(" el-ignored=" + _elIgnored); - sb.append(" is-xml=" + _isXml); - sb.append(" page-encoding=" + _pageEncoding); - sb.append(" scripting-invalid=" + _scriptingInvalid); - sb.append(" deferred-syntax-allowed-as-literal=" + _deferredSyntaxAllowedAsLiteral); - sb.append(" trim-directive-whitespaces" + _trimDirectiveWhitespaces); - sb.append(" default-content-type=" + _defaultContentType); - sb.append(" buffer=" + _buffer); - sb.append(" error-on-undeclared-namespace=" + _errorOnUndeclaredNamespace); + sb.append(" el-ignored=").append(_elIgnored); + sb.append(" is-xml=").append(_isXml); + sb.append(" page-encoding=").append(_pageEncoding); + sb.append(" scripting-invalid=").append(_scriptingInvalid); + sb.append(" deferred-syntax-allowed-as-literal=").append(_deferredSyntaxAllowedAsLiteral); + sb.append(" trim-directive-whitespaces").append(_trimDirectiveWhitespaces); + sb.append(" default-content-type=").append(_defaultContentType); + sb.append(" buffer=").append(_buffer); + sb.append(" error-on-undeclared-namespace=").append(_errorOnUndeclaredNamespace); for (String prelude : _includePreludes) { - sb.append(" include-prelude=" + prelude); + sb.append(" include-prelude=").append(prelude); } for (String coda : _includeCodas) { - sb.append(" include-coda=" + coda); + sb.append(" include-coda=").append(coda); } return sb.toString(); } @@ -1002,8 +1001,8 @@ public String toString() public static class JspConfig implements JspConfigDescriptor { - private List _taglibs = new ArrayList(); - private List _jspPropertyGroups = new ArrayList(); + private List _taglibs = new ArrayList<>(); + private List _jspPropertyGroups = new ArrayList<>(); public JspConfig() { @@ -1015,7 +1014,7 @@ public JspConfig() @Override public Collection getTaglibs() { - return new ArrayList(_taglibs); + return new ArrayList<>(_taglibs); } public void addTaglibDescriptor(TaglibDescriptor d) @@ -1029,7 +1028,7 @@ public void addTaglibDescriptor(TaglibDescriptor d) @Override public Collection getJspPropertyGroups() { - return new ArrayList(_jspPropertyGroups); + return new ArrayList<>(_jspPropertyGroups); } public void addJspPropertyGroup(JspPropertyGroupDescriptor g) @@ -1040,15 +1039,15 @@ public void addJspPropertyGroup(JspPropertyGroupDescriptor g) @Override public String toString() { - StringBuffer sb = new StringBuffer(); + StringBuilder sb = new StringBuilder(); sb.append("JspConfigDescriptor: \n"); for (TaglibDescriptor taglib : _taglibs) { - sb.append(taglib + "\n"); + sb.append(taglib).append("\n"); } for (JspPropertyGroupDescriptor jpg : _jspPropertyGroups) { - sb.append(jpg + "\n"); + sb.append(jpg).append("\n"); } return sb.toString(); } @@ -1056,7 +1055,6 @@ public String toString() public class Context extends ContextHandler.Context { - /* * @see javax.servlet.ServletContext#getNamedDispatcher(java.lang.String) */ @@ -1277,18 +1275,9 @@ public boolean setInitParameter(String name, String value) } @Override - public T createFilter(Class c) throws ServletException + protected T createInstance(Class clazz) throws ServletException { - try - { - T f = createInstance(c); - f = _objFactory.decorate(f); - return f; - } - catch (Exception e) - { - throw new ServletException(e); - } + return _objFactory.decorate(super.createInstance(clazz)); } public void destroyFilter(T f) @@ -1296,21 +1285,6 @@ public void destroyFilter(T f) _objFactory.destroy(f); } - @Override - public T createServlet(Class c) throws ServletException - { - try - { - T s = createInstance(c); - s = _objFactory.decorate(s); - return s; - } - catch (Exception e) - { - throw new ServletException(e); - } - } - public void destroyServlet(T s) { _objFactory.destroy(s); @@ -1348,7 +1322,7 @@ public FilterRegistration getFilterRegistration(String filterName) if (!_enabled) throw new UnsupportedOperationException(); - HashMap registrations = new HashMap(); + HashMap registrations = new HashMap<>(); ServletHandler handler = ServletContextHandler.this.getServletHandler(); FilterHolder[] holders = handler.getFilters(); if (holders != null) @@ -1377,7 +1351,7 @@ public ServletRegistration getServletRegistration(String servletName) if (!_enabled) throw new UnsupportedOperationException(); - HashMap registrations = new HashMap(); + HashMap registrations = new HashMap<>(); ServletHandler handler = ServletContextHandler.this.getServletHandler(); ServletHolder[] holders = handler.getServlets(); if (holders != null) @@ -1460,21 +1434,6 @@ public void addListener(Class listenerClass) super.addListener(listenerClass); } - @Override - public T createListener(Class clazz) throws ServletException - { - try - { - T l = createInstance(clazz); - l = _objFactory.decorate(l); - return l; - } - catch (Exception e) - { - throw new ServletException(e); - } - } - @Override public JspConfigDescriptor getJspConfigDescriptor() { diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java index 5faa43f673a2..db8a0295ab04 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java @@ -258,7 +258,7 @@ public int hashCode() public synchronized void setUserRoleLink(String name, String link) { if (_roleMap == null) - _roleMap = new HashMap(); + _roleMap = new HashMap<>(); _roleMap.put(name, link); } @@ -639,7 +639,7 @@ protected void initJspServlet() throws Exception } /* ensure scratch dir */ - File scratch = null; + File scratch; if (getInitParameter("scratchdir") == null) { File tmp = (File)getServletHandler().getServletContext().getAttribute(ServletContext.TEMPDIR); @@ -855,8 +855,7 @@ public String getPackageOfJspClass(String jsp) { Class jspUtil = Loader.loadClass("org.apache.jasper.compiler.JspUtil"); Method makeJavaPackage = jspUtil.getMethod("makeJavaPackage", String.class); - String p = (String)makeJavaPackage.invoke(null, jsp.substring(0, i)); - return p; + return (String)makeJavaPackage.invoke(null, jsp.substring(0, i)); } catch (Exception e) { @@ -960,7 +959,7 @@ public Set addMapping(String... urlPatterns) if (!mapping.isDefault()) { if (clash == null) - clash = new HashSet(); + clash = new HashSet<>(); clash.add(pattern); } } @@ -983,7 +982,7 @@ public Set addMapping(String... urlPatterns) public Collection getMappings() { ServletMapping[] mappings = getServletHandler().getServletMappings(); - List patterns = new ArrayList(); + List patterns = new ArrayList<>(); if (mappings != null) { for (ServletMapping mapping : mappings) @@ -1049,7 +1048,7 @@ public ServletRegistration.Dynamic getRegistration() private class SingleThreadedWrapper implements Servlet { - Stack _stack = new Stack(); + Stack _stack = new Stack<>(); @Override public void destroy() @@ -1161,7 +1160,7 @@ protected Servlet newInstance() throws ServletException, IllegalAccessException, try { ServletContext ctx = getServletHandler().getServletContext(); - if (ctx instanceof ServletContextHandler.Context) + if (ctx != null) return ctx.createServlet(getHeldClass()); return getHeldClass().getDeclaredConstructor().newInstance(); diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SniX509ExtendedKeyManager.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SniX509ExtendedKeyManager.java index d4afc6409e5d..b1cdfbd29ee1 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SniX509ExtendedKeyManager.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SniX509ExtendedKeyManager.java @@ -50,6 +50,10 @@ public class SniX509ExtendedKeyManager extends X509ExtendedKeyManager private final X509ExtendedKeyManager _delegate; private final SslContextFactory.Server _sslContextFactory; + /** + * @deprecated not supported, you must have a {@link SslContextFactory.Server} for this to work. + */ + @Deprecated public SniX509ExtendedKeyManager(X509ExtendedKeyManager keyManager) { this(keyManager, null); @@ -58,7 +62,7 @@ public SniX509ExtendedKeyManager(X509ExtendedKeyManager keyManager) public SniX509ExtendedKeyManager(X509ExtendedKeyManager keyManager, SslContextFactory.Server sslContextFactory) { _delegate = keyManager; - _sslContextFactory = sslContextFactory; + _sslContextFactory = Objects.requireNonNull(sslContextFactory, "SslContextFactory.Server must be provided"); } @Override diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java index 2cc56b611e48..c81236f9b26f 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java @@ -1264,9 +1264,13 @@ protected KeyManager[] getKeyManagers(KeyStore keyStore) throws Exception return managers; } + /** + * @deprecated use {@link SslContextFactory.Server#newSniX509ExtendedKeyManager(X509ExtendedKeyManager)} instead + */ + @Deprecated protected X509ExtendedKeyManager newSniX509ExtendedKeyManager(X509ExtendedKeyManager keyManager) { - return new SniX509ExtendedKeyManager(keyManager); + throw new UnsupportedOperationException("X509ExtendedKeyManager only supported on Server"); } protected TrustManager[] getTrustManagers(KeyStore trustStore, Collection crls) throws Exception diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/ssl/X509Test.java b/jetty-util/src/test/java/org/eclipse/jetty/util/ssl/X509Test.java index 04f84a5b2640..2445a7618316 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/ssl/X509Test.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/ssl/X509Test.java @@ -19,11 +19,17 @@ package org.eclipse.jetty.util.ssl; import java.security.cert.X509Certificate; +import javax.net.ssl.KeyManager; +import javax.net.ssl.X509ExtendedKeyManager; +import org.eclipse.jetty.util.resource.Resource; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; +import static org.junit.jupiter.api.Assertions.assertThrows; public class X509Test { @@ -123,4 +129,56 @@ public boolean[] getKeyUsage() assertThat("Normal X509", X509.isCertSign(bogusX509), is(false)); } + + private X509ExtendedKeyManager getX509ExtendedKeyManager(SslContextFactory sslContextFactory) throws Exception + { + Resource keystoreResource = Resource.newSystemResource("keystore"); + Resource truststoreResource = Resource.newSystemResource("keystore"); + sslContextFactory.setKeyStoreResource(keystoreResource); + sslContextFactory.setTrustStoreResource(truststoreResource); + sslContextFactory.setKeyStorePassword("storepwd"); + sslContextFactory.setKeyManagerPassword("keypwd"); + sslContextFactory.setTrustStorePassword("storepwd"); + sslContextFactory.start(); + + KeyManager[] keyManagers = sslContextFactory.getKeyManagers(sslContextFactory.getKeyStore()); + X509ExtendedKeyManager x509ExtendedKeyManager = null; + + for (KeyManager keyManager : keyManagers) + { + if (keyManager instanceof X509ExtendedKeyManager) + { + x509ExtendedKeyManager = (X509ExtendedKeyManager)keyManager; + break; + } + } + assertThat("Found X509ExtendedKeyManager", x509ExtendedKeyManager, is(notNullValue())); + return x509ExtendedKeyManager; + } + + @Test + public void testSniX509ExtendedKeyManager_BaseClass() throws Exception + { + SslContextFactory baseSsl = new SslContextFactory(); + X509ExtendedKeyManager x509ExtendedKeyManager = getX509ExtendedKeyManager(baseSsl); + UnsupportedOperationException npe = assertThrows(UnsupportedOperationException.class, () -> baseSsl.newSniX509ExtendedKeyManager(x509ExtendedKeyManager)); + assertThat("UnsupportedOperationException.message", npe.getMessage(), containsString("X509ExtendedKeyManager only supported on Server")); + } + + @Test + public void testSniX509ExtendedKeyManager_ClientClass() throws Exception + { + SslContextFactory clientSsl = new SslContextFactory.Client(); + X509ExtendedKeyManager x509ExtendedKeyManager = getX509ExtendedKeyManager(clientSsl); + UnsupportedOperationException re = assertThrows(UnsupportedOperationException.class, () -> clientSsl.newSniX509ExtendedKeyManager(x509ExtendedKeyManager)); + assertThat("UnsupportedOperationException.message", re.getMessage(), containsString("X509ExtendedKeyManager only supported on Server")); + } + + @Test + public void testSniX509ExtendedKeyManager_ServerClass() throws Exception + { + SslContextFactory serverSsl = new SslContextFactory.Server(); + X509ExtendedKeyManager x509ExtendedKeyManager = getX509ExtendedKeyManager(serverSsl); + serverSsl.newSniX509ExtendedKeyManager(x509ExtendedKeyManager); + } } 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 a0189a019066..4cbdf9fc62da 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 @@ -289,7 +289,7 @@ public void setDisplayName(String servletContextName) { super.setDisplayName(servletContextName); ClassLoader cl = getClassLoader(); - if (cl != null && cl instanceof WebAppClassLoader && servletContextName != null) + if (cl instanceof WebAppClassLoader && servletContextName != null) ((WebAppClassLoader)cl).setName(servletContextName); } @@ -315,7 +315,7 @@ public Throwable getUnavailableException() public void setResourceAlias(String alias, String uri) { if (_resourceAliases == null) - _resourceAliases = new HashMap(5); + _resourceAliases = new HashMap<>(5); _resourceAliases.put(alias, uri); } @@ -369,7 +369,7 @@ public void setClassLoader(ClassLoader classLoader) if (name == null) name = getContextPath(); - if (classLoader != null && classLoader instanceof WebAppClassLoader && getDisplayName() != null) + if (classLoader instanceof WebAppClassLoader && getDisplayName() != null) ((WebAppClassLoader)classLoader).setName(name); } @@ -400,7 +400,7 @@ public Resource getResource(String uriInContext) throws MalformedURLException } } - if (ioe != null && ioe instanceof MalformedURLException) + if (ioe instanceof MalformedURLException) throw (MalformedURLException)ioe; return resource; @@ -519,7 +519,7 @@ protected void doStart() throws Exception { _metadata.setAllowDuplicateFragmentNames(isAllowDuplicateFragmentNames()); Boolean validate = (Boolean)getAttribute(MetaData.VALIDATE_XML); - _metadata.setValidateXml((validate != null && validate.booleanValue())); + _metadata.setValidateXml((validate != null && validate)); preConfigure(); super.doStart(); postConfigure(); @@ -593,7 +593,7 @@ private void dumpUrl() @ManagedAttribute(value = "configuration classes used to configure webapp", readonly = true) public String[] getConfigurationClasses() { - return _configurationClasses.toArray(new String[_configurationClasses.size()]); + return _configurationClasses.toArray(new String[0]); } /** @@ -601,7 +601,7 @@ public String[] getConfigurationClasses() */ public Configuration[] getConfigurations() { - return _configurations.toArray(new Configuration[_configurations.size()]); + return _configurations.toArray(new Configuration[0]); } /** @@ -1014,16 +1014,18 @@ public void dump(Appendable out, String indent) throws IOException { if (_war != null) { - if (_war.indexOf("/webapps/") >= 0) - name = _war.substring(_war.indexOf("/webapps/") + 8); + int webapps = _war.indexOf("/webapps/"); + if (webapps >= 0) + name = _war.substring(webapps + 8); else name = _war; } else if (getResourceBase() != null) { name = getResourceBase(); - if (name.indexOf("/webapps/") >= 0) - name = name.substring(name.indexOf("/webapps/") + 8); + int webapps = name.indexOf("/webapps/"); + if (webapps >= 0) + name = name.substring(webapps + 8); } else { @@ -1061,7 +1063,7 @@ public void setConfigurationClasses(String[] configurations) public void setConfigurationClasses(List configurations) { - setConfigurationClasses(configurations.toArray(new String[configurations.size()])); + setConfigurationClasses(configurations.toArray(new String[0])); } /** @@ -1427,7 +1429,7 @@ protected void stopContext() throws Exception if (_ownClassLoader) { ClassLoader loader = getClassLoader(); - if (loader != null && loader instanceof URLClassLoader) + if (loader instanceof URLClassLoader) ((URLClassLoader)loader).close(); setClassLoader(null); } @@ -1451,7 +1453,7 @@ protected void stopWebapp() throws Exception @Override public Set setServletSecurity(Dynamic registration, ServletSecurityElement servletSecurityElement) { - Set unchangedURLMappings = new HashSet(); + Set unchangedURLMappings = new HashSet<>(); //From javadoc for ServletSecurityElement: /* If a URL pattern of this ServletRegistration is an exact target of a security-constraint that @@ -1523,7 +1525,6 @@ public Set setServletSecurity(Dynamic registration, ServletSecurityEleme public class Context extends ServletContextHandler.Context { - @Override public void checkListener(Class listener) throws IllegalStateException { diff --git a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/HugeResourceTest.java b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/HugeResourceTest.java index 0687f349878b..a3859ffc55d1 100644 --- a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/HugeResourceTest.java +++ b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/HugeResourceTest.java @@ -271,6 +271,7 @@ public void testUpload_Multipart(String filename, long expectedSize) throws Exce multipart.addFilePart(name, filename, new PathContentProvider(inputFile), null); URI destUri = server.getURI().resolve("/multipart"); + client.setIdleTimeout(90_000); Request request = client.newRequest(destUri).method(HttpMethod.POST).content(multipart); ContentResponse response = request.send(); assertThat("HTTP Response Code", response.getStatus(), is(200)); diff --git a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientConnectTest.java b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientConnectTest.java index 343ec19e0955..7831529261dc 100644 --- a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientConnectTest.java +++ b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientConnectTest.java @@ -25,6 +25,8 @@ import java.net.SocketTimeoutException; import java.net.URI; import java.util.EnumSet; +import java.util.List; +import java.util.Map; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; @@ -174,6 +176,27 @@ public void testUpgradeRequest() throws Exception } } + @Test + public void testUpgradeRequest_PercentEncodedQuery() throws Exception + { + CloseTrackingEndpoint cliSock = new CloseTrackingEndpoint(); + client.getPolicy().setIdleTimeout(10000); + + URI wsUri = WSURI.toWebsocket(server.getURI().resolve("/echo?name=%25foo")); + ClientUpgradeRequest request = new ClientUpgradeRequest(); + request.setSubProtocols("echo"); + Future future = client.connect(cliSock, wsUri); + + try (Session sess = future.get(30, TimeUnit.SECONDS)) + { + assertThat("Connect.UpgradeRequest", sess.getUpgradeRequest(), notNullValue()); + Map> paramMap = sess.getUpgradeRequest().getParameterMap(); + List values = paramMap.get("name"); + assertThat("Params[name]", values.get(0), is("%foo")); + assertThat("Connect.UpgradeResponse", sess.getUpgradeResponse(), notNullValue()); + } + } + @Test public void testAltConnect() throws Exception { diff --git a/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/UpgradeResponse.java b/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/UpgradeResponse.java index 7b21617134a6..169ade3f2568 100644 --- a/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/UpgradeResponse.java +++ b/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/UpgradeResponse.java @@ -103,7 +103,9 @@ public interface UpgradeResponse * or was failed (resulting in no upgrade handshake) * * @return true if upgrade response was generated, false if no upgrade response was generated + * @deprecated this has no replacement, will be removed in Jetty 10 */ + @Deprecated boolean isSuccess(); /** @@ -174,6 +176,8 @@ public interface UpgradeResponse * * @param success true to indicate a response to the upgrade handshake was sent, false to indicate no upgrade * response was sent + * @deprecated this has no replacement, will be removed in Jetty 10 */ + @Deprecated void setSuccess(boolean success); } diff --git a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeRequest.java b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeRequest.java index 082093c714d6..0f0611ea0cf6 100644 --- a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeRequest.java +++ b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeRequest.java @@ -152,19 +152,19 @@ public void setRequestURI(URI uri) // parse parameter map Map> pmap = new HashMap<>(); - String query = uri.getQuery(); + String query = uri.getRawQuery(); if (StringUtil.isNotBlank(query)) { - MultiMap params = new MultiMap(); - UrlEncoded.decodeTo(uri.getQuery(), params, StandardCharsets.UTF_8); + MultiMap params = new MultiMap<>(); + UrlEncoded.decodeTo(query, params, StandardCharsets.UTF_8); for (String key : params.keySet()) { List values = params.getValues(key); if (values == null) { - pmap.put(key, new ArrayList()); + pmap.put(key, new ArrayList<>()); } else { diff --git a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeResponse.java b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeResponse.java index 9ead02fa3ac2..d15deb98577f 100644 --- a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeResponse.java +++ b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeResponse.java @@ -55,6 +55,13 @@ public ClientUpgradeResponse(HttpResponse response) setAcceptedSubProtocol(fields.get(HttpHeader.SEC_WEBSOCKET_SUBPROTOCOL)); } + @Override + public boolean isSuccess() + { + // If we reached this point, where we have a UpgradeResponse object, then the upgrade is a success. + return true; + } + @Override public List getExtensions() { diff --git a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketUpgradeRequest.java b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketUpgradeRequest.java index 62867c658c19..841e5de6c2dc 100644 --- a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketUpgradeRequest.java +++ b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketUpgradeRequest.java @@ -596,9 +596,11 @@ public void upgrade(HttpResponse response, HttpConnectionOverHTTP oldConn) URI requestURI = this.getURI(); + ClientUpgradeResponse upgradeResponse = new ClientUpgradeResponse(response); + WebSocketSession session = getSessionFactory().createSession(requestURI, localEndpoint, connection); session.setUpgradeRequest(new ClientUpgradeRequest(this)); - session.setUpgradeResponse(new ClientUpgradeResponse(response)); + session.setUpgradeResponse(upgradeResponse); connection.addListener(session); List extensions = new ArrayList<>(); @@ -637,7 +639,7 @@ public void upgrade(HttpResponse response, HttpConnectionOverHTTP oldConn) if (upgradeListener != null) { - upgradeListener.onHandshakeResponse(new ClientUpgradeResponse(response)); + upgradeListener.onHandshakeResponse(upgradeResponse); } // Now swap out the connection diff --git a/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/CookieTest.java b/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/CookieTest.java index 6a651afcbc46..bbdb41e7fe87 100644 --- a/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/CookieTest.java +++ b/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/CookieTest.java @@ -196,7 +196,8 @@ private String confirmClientUpgradeAndCookies(CookieTrackingSocket clientSocket, serverConn.write(serverCookieFrame); // Confirm client connect on future - clientConnectFuture.get(10, TimeUnit.SECONDS); + Session session = clientConnectFuture.get(10, TimeUnit.SECONDS); + assertTrue(session.getUpgradeResponse().isSuccess(), "UpgradeResponse.isSuccess()"); clientSocket.awaitOpen(2, TimeUnit.SECONDS); // Wait for client receipt of cookie frame via client websocket diff --git a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java index 2b5e116bd080..150db502c719 100644 --- a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java +++ b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java @@ -698,6 +698,8 @@ private boolean upgrade(HttpConnection http, ServletUpgradeRequest request, Serv // Process (version specific) handshake response handshaker.doHandshakeResponse(request, response); + response.setSuccess(true); + if (LOG.isDebugEnabled()) LOG.debug("Websocket upgrade {} {} {} {}", request.getRequestURI(), version, response.getAcceptedSubProtocol(), wsConnection); diff --git a/pom.xml b/pom.xml index d224e0ef0dca..58cfc8a60af4 100644 --- a/pom.xml +++ b/pom.xml @@ -28,7 +28,7 @@ undefined 2.1.0 - 7.1 + 7.2 1.21 benchmarks 1.4.0 @@ -45,7 +45,7 @@ ${bundle-symbolic-name} - 3.0.0-M3 + 3.0.0-M4 3.8.1 3.1.1 3.1.0 @@ -63,6 +63,7 @@ 1.3 ${project.build.directory}/local-repo src/it/settings.xml + 3 @@ -643,6 +644,7 @@ maven-surefire-plugin ${maven.surefire.version} + ${surefire.rerunFailingTestsCount} 3600 @{argLine} -Dfile.encoding=UTF-8 -Duser.language=en -Duser.region=US -showversion -Xmx1g -Xms1g -XX:+PrintGCDetails false diff --git a/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java b/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java index 62140916fa8b..e3f8b3e6df3e 100644 --- a/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java +++ b/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java @@ -33,6 +33,7 @@ import org.eclipse.jetty.unixsocket.client.HttpClientTransportOverUnixSockets; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.ssl.SslContextFactory; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledOnJre; import org.junit.jupiter.api.condition.JRE; @@ -155,6 +156,18 @@ public void testSimpleWebAppWithJSPOnModulePath() throws Exception @Test @DisabledOnJre(JRE.JAVA_8) public void testSimpleWebAppWithJSPOverH2C() throws Exception + { + testSimpleWebAppWithJSPOverHTTP2(false); + } + + @Test + @DisabledOnJre(JRE.JAVA_8) + public void testSimpleWebAppWithJSPOverH2() throws Exception + { + testSimpleWebAppWithJSPOverHTTP2(true); + } + + private void testSimpleWebAppWithJSPOverHTTP2(boolean ssl) throws Exception { String jettyVersion = System.getProperty("jettyVersion"); DistributionTester distribution = DistributionTester.Builder.newInstance() @@ -164,7 +177,7 @@ public void testSimpleWebAppWithJSPOverH2C() throws Exception String[] args1 = { "--create-startd", - "--add-to-start=http2c,jsp,deploy" + "--add-to-start=jsp,deploy," + (ssl ? "http2" : "http2c") }; try (DistributionTester.Run run1 = distribution.start(args1)) { @@ -175,13 +188,14 @@ public void testSimpleWebAppWithJSPOverH2C() throws Exception distribution.installWarFile(war, "test"); int port = distribution.freePort(); - try (DistributionTester.Run run2 = distribution.start("jetty.http.port=" + port)) + String portProp = ssl ? "jetty.ssl.port" : "jetty.http.port"; + try (DistributionTester.Run run2 = distribution.start(portProp + "=" + port)) { assertTrue(run2.awaitConsoleLogsFor("Started @", 10, TimeUnit.SECONDS)); HTTP2Client h2Client = new HTTP2Client(); - startHttpClient(() -> new HttpClient(new HttpClientTransportOverHTTP2(h2Client), null)); - ContentResponse response = client.GET("http://localhost:" + port + "/test/index.jsp"); + startHttpClient(() -> new HttpClient(new HttpClientTransportOverHTTP2(h2Client), new SslContextFactory.Client(true))); + ContentResponse response = client.GET((ssl ? "https" : "http") + "://localhost:" + port + "/test/index.jsp"); assertEquals(HttpStatus.OK_200, response.getStatus()); assertThat(response.getContentAsString(), containsString("Hello")); assertThat(response.getContentAsString(), not(containsString("<%"))); diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientDemandTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientDemandTest.java index 879016d03a60..884853e21d9c 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientDemandTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientDemandTest.java @@ -414,4 +414,119 @@ protected void service(String target, Request jettyRequest, HttpServletRequest r assertTrue(resultLatch.await(5, TimeUnit.SECONDS)); assertArrayEquals(content, bytes); } + + @ParameterizedTest + @ArgumentsSource(TransportProvider.class) + public void testDelayedBeforeContentDemand(Transport transport) throws Exception + { + init(transport); + + byte[] content = new byte[1024]; + new Random().nextBytes(content); + scenario.start(new EmptyServerHandler() + { + @Override + protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + response.setContentLength(content.length); + response.getOutputStream().write(content); + } + }); + + byte[] bytes = new byte[content.length]; + ByteBuffer received = ByteBuffer.wrap(bytes); + AtomicReference beforeContentDemandRef = new AtomicReference<>(); + CountDownLatch beforeContentLatch = new CountDownLatch(1); + CountDownLatch contentLatch = new CountDownLatch(1); + CountDownLatch resultLatch = new CountDownLatch(1); + scenario.client.newRequest(scenario.newURI()) + .onResponseContentDemanded(new Response.DemandedContentListener() + { + @Override + public void onBeforeContent(Response response, LongConsumer demand) + { + // Do not demand now. + beforeContentDemandRef.set(demand); + beforeContentLatch.countDown(); + } + + @Override + public void onContent(Response response, LongConsumer demand, ByteBuffer buffer, Callback callback) + { + contentLatch.countDown(); + received.put(buffer); + callback.succeeded(); + demand.accept(1); + } + }) + .send(result -> + { + assertTrue(result.isSucceeded()); + assertEquals(HttpStatus.OK_200, result.getResponse().getStatus()); + resultLatch.countDown(); + }); + + assertTrue(beforeContentLatch.await(5, TimeUnit.SECONDS)); + LongConsumer demand = beforeContentDemandRef.get(); + + // Content must not be notified until we demand. + assertFalse(contentLatch.await(1, TimeUnit.SECONDS)); + + demand.accept(1); + + assertTrue(resultLatch.await(5, TimeUnit.SECONDS)); + assertArrayEquals(content, bytes); + } + + @ParameterizedTest + @ArgumentsSource(TransportProvider.class) + public void testDelayedBeforeContentDemandWithNoResponseContent(Transport transport) throws Exception + { + init(transport); + + scenario.start(new EmptyServerHandler()); + + AtomicReference beforeContentDemandRef = new AtomicReference<>(); + CountDownLatch beforeContentLatch = new CountDownLatch(1); + CountDownLatch contentLatch = new CountDownLatch(1); + CountDownLatch resultLatch = new CountDownLatch(1); + scenario.client.newRequest(scenario.newURI()) + .onResponseContentDemanded(new Response.DemandedContentListener() + { + @Override + public void onBeforeContent(Response response, LongConsumer demand) + { + // Do not demand now. + beforeContentDemandRef.set(demand); + beforeContentLatch.countDown(); + } + + @Override + public void onContent(Response response, LongConsumer demand, ByteBuffer buffer, Callback callback) + { + contentLatch.countDown(); + callback.succeeded(); + demand.accept(1); + } + }) + .send(result -> + { + assertTrue(result.isSucceeded()); + assertEquals(HttpStatus.OK_200, result.getResponse().getStatus()); + resultLatch.countDown(); + }); + + assertTrue(beforeContentLatch.await(5, TimeUnit.SECONDS)); + LongConsumer demand = beforeContentDemandRef.get(); + + // Content must not be notified until we demand. + assertFalse(contentLatch.await(1, TimeUnit.SECONDS)); + + demand.accept(1); + + // Content must not be notified as there is no content. + assertFalse(contentLatch.await(1, TimeUnit.SECONDS)); + + assertTrue(resultLatch.await(5, TimeUnit.SECONDS)); + } } diff --git a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractClusteredSessionScavengingTest.java b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractClusteredSessionScavengingTest.java index b65cffe0fdc4..253872010c18 100644 --- a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractClusteredSessionScavengingTest.java +++ b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractClusteredSessionScavengingTest.java @@ -69,6 +69,7 @@ public void testClusteredScavenge() throws Exception DefaultSessionCacheFactory cacheFactory1 = new DefaultSessionCacheFactory(); cacheFactory1.setEvictionPolicy(SessionCache.NEVER_EVICT); //don't evict sessions + cacheFactory1.setFlushOnResponseCommit(true); SessionDataStoreFactory storeFactory1 = createSessionDataStoreFactory(); ((AbstractSessionDataStoreFactory)storeFactory1).setGracePeriodSec(scavengePeriod); ((AbstractSessionDataStoreFactory)storeFactory1).setSavePeriodSec(0); //always save when the session exits @@ -77,8 +78,6 @@ public void testClusteredScavenge() throws Exception TestServlet servlet1 = new TestServlet(); ServletHolder holder1 = new ServletHolder(servlet1); ServletContextHandler context = server1.addContext(contextPath); - TestHttpChannelCompleteListener scopeListener = new TestHttpChannelCompleteListener(); - server1.getServerConnector().addBean(scopeListener); TestSessionListener listener1 = new TestSessionListener(); context.getSessionHandler().addEventListener(listener1); context.addServlet(holder1, servletMapping); @@ -91,13 +90,12 @@ public void testClusteredScavenge() throws Exception DefaultSessionCacheFactory cacheFactory2 = new DefaultSessionCacheFactory(); cacheFactory2.setEvictionPolicy(SessionCache.NEVER_EVICT); //don't evict sessions + cacheFactory2.setFlushOnResponseCommit(true); SessionDataStoreFactory storeFactory2 = createSessionDataStoreFactory(); ((AbstractSessionDataStoreFactory)storeFactory2).setGracePeriodSec(scavengePeriod); ((AbstractSessionDataStoreFactory)storeFactory2).setSavePeriodSec(0); //always save when the session exits TestServer server2 = new TestServer(0, maxInactivePeriod, scavengePeriod, cacheFactory2, storeFactory2); ServletContextHandler context2 = server2.addContext(contextPath); - TestHttpChannelCompleteListener scopeListener2 = new TestHttpChannelCompleteListener(); - server2.getServerConnector().addBean(scopeListener2); context2.addServlet(TestServlet.class, servletMapping); SessionHandler m2 = context2.getSessionHandler(); @@ -110,18 +108,12 @@ public void testClusteredScavenge() throws Exception try { // Perform one request to server1 to create a session - CountDownLatch synchronizer = new CountDownLatch(1); - scopeListener.setExitSynchronizer(synchronizer); ContentResponse response1 = client.GET("http://localhost:" + port1 + contextPath + servletMapping.substring(1) + "?action=init"); assertEquals(HttpServletResponse.SC_OK, response1.getStatus()); assertTrue(response1.getContentAsString().startsWith("init")); String sessionCookie = response1.getHeaders().get("Set-Cookie"); assertTrue(sessionCookie != null); String id = TestServer.extractSessionId(sessionCookie); - - //ensure request has finished being handled - synchronizer.await(5, TimeUnit.SECONDS); - assertEquals(1, ((DefaultSessionCache)m1.getSessionCache()).getSessionsCurrent()); assertEquals(1, ((DefaultSessionCache)m1.getSessionCache()).getSessionsMax()); assertEquals(1, ((DefaultSessionCache)m1.getSessionCache()).getSessionsTotal()); @@ -140,14 +132,10 @@ public void testClusteredScavenge() throws Exception long time = start; while (time < end) { - synchronizer = new CountDownLatch(1); - scopeListener2.setExitSynchronizer(synchronizer); Request request = client.newRequest("http://localhost:" + port2 + contextPath + servletMapping.substring(1)); ContentResponse response2 = request.send(); assertEquals(HttpServletResponse.SC_OK, response2.getStatus()); assertTrue(response2.getContentAsString().startsWith("test")); - //ensure request has finished being handled - synchronizer.await(5, TimeUnit.SECONDS); Thread.sleep(requestInterval); assertSessionCounts(1, 1, 1, m2); time = System.currentTimeMillis();