diff --git a/Jenkinsfile b/Jenkinsfile index 2c342706781..9bde00c2d88 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -50,6 +50,16 @@ pipeline { } } + stage("Build / Test - JDK12") { + agent { node { label 'linux' } } + options { timeout(time: 120, unit: 'MINUTES') } + steps { + mavenBuild("jdk12", "-Pmongodb install", "maven3", false) + warnings consoleParsers: [[parserName: 'Maven'], [parserName: 'Java']] + maven_invoker reportsFilenamePattern: "**/target/invoker-reports/BUILD*.xml", invokerBuildDir: "**/target/it" + } + } + stage("Build Javadoc") { agent { node { label 'linux' } } options { timeout(time: 30, unit: 'MINUTES') } diff --git a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/LikeJettyXml.java b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/LikeJettyXml.java index f2fe8dcffe4..3d5759e8d4e 100644 --- a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/LikeJettyXml.java +++ b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/LikeJettyXml.java @@ -98,7 +98,7 @@ public class LikeJettyXml Server server = new Server(threadPool); // Scheduler - server.addBean(new ScheduledExecutorScheduler()); + server.addBean(new ScheduledExecutorScheduler(null,false)); // HTTP Configuration HttpConfiguration http_config = new HttpConfiguration(); diff --git a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/OneWebApp.java b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/OneWebApp.java index d4495c6cc78..e6457c784c0 100644 --- a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/OneWebApp.java +++ b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/OneWebApp.java @@ -23,7 +23,6 @@ import java.lang.management.ManagementFactory; import org.eclipse.jetty.jmx.MBeanContainer; import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.handler.AllowSymLinkAliasChecker; import org.eclipse.jetty.webapp.WebAppContext; public class OneWebApp @@ -58,11 +57,11 @@ public class OneWebApp // the server so it is aware of where to send the appropriate requests. server.setHandler(webapp); - // Start things up! + // Start things up! server.start(); server.dumpStdErr(); - + // The use of server.join() the will make the current thread join and // wait until the server is done executing. // See http://docs.oracle.com/javase/7/docs/api/java/lang/Thread.html#join() diff --git a/jetty-alpn/jetty-alpn-conscrypt-client/src/main/java/org/eclipse/jetty/alpn/conscrypt/client/ConscryptClientALPNProcessor.java b/jetty-alpn/jetty-alpn-conscrypt-client/src/main/java/org/eclipse/jetty/alpn/conscrypt/client/ConscryptClientALPNProcessor.java index 03d83a5d1c4..b180f7edc6b 100644 --- a/jetty-alpn/jetty-alpn-conscrypt-client/src/main/java/org/eclipse/jetty/alpn/conscrypt/client/ConscryptClientALPNProcessor.java +++ b/jetty-alpn/jetty-alpn-conscrypt-client/src/main/java/org/eclipse/jetty/alpn/conscrypt/client/ConscryptClientALPNProcessor.java @@ -18,12 +18,11 @@ package org.eclipse.jetty.alpn.conscrypt.client; -import java.lang.reflect.Method; -import java.nio.charset.StandardCharsets; import java.security.Security; import javax.net.ssl.SSLEngine; +import org.conscrypt.Conscrypt; import org.conscrypt.OpenSSLProvider; import org.eclipse.jetty.alpn.client.ALPNClientConnection; import org.eclipse.jetty.io.Connection; @@ -40,7 +39,7 @@ public class ConscryptClientALPNProcessor implements ALPNProcessor.Client @Override public void init() { - if (Security.getProvider("Conscrypt")==null) + if (Security.getProvider("Conscrypt") == null) { Security.addProvider(new OpenSSLProvider()); if (LOG.isDebugEnabled()) @@ -59,11 +58,9 @@ public class ConscryptClientALPNProcessor implements ALPNProcessor.Client { try { - Method setAlpnProtocols = sslEngine.getClass().getDeclaredMethod("setApplicationProtocols", String[].class); - setAlpnProtocols.setAccessible(true); ALPNClientConnection alpn = (ALPNClientConnection)connection; String[] protocols = alpn.getProtocols().toArray(new String[0]); - setAlpnProtocols.invoke(sslEngine, (Object)protocols); + Conscrypt.setApplicationProtocols(sslEngine, protocols); ((SslConnection.DecryptedEndPoint)connection.getEndPoint()).getSslConnection() .addHandshakeListener(new ALPNListener(alpn)); } @@ -92,9 +89,9 @@ public class ConscryptClientALPNProcessor implements ALPNProcessor.Client try { SSLEngine sslEngine = alpnConnection.getSSLEngine(); - Method method = sslEngine.getClass().getDeclaredMethod("getApplicationProtocol"); - method.setAccessible(true); - String protocol = (String)method.invoke(sslEngine); + String protocol = Conscrypt.getApplicationProtocol(sslEngine); + if (LOG.isDebugEnabled()) + LOG.debug("Selected {} for {}", protocol, alpnConnection); alpnConnection.selected(protocol); } catch (Throwable e) diff --git a/jetty-alpn/jetty-alpn-conscrypt-server/pom.xml b/jetty-alpn/jetty-alpn-conscrypt-server/pom.xml index a584b4d5b95..4dcef327633 100644 --- a/jetty-alpn/jetty-alpn-conscrypt-server/pom.xml +++ b/jetty-alpn/jetty-alpn-conscrypt-server/pom.xml @@ -38,6 +38,31 @@ ${project.version} test + + org.eclipse.jetty + jetty-alpn-conscrypt-client + ${project.version} + test + + + org.eclipse.jetty + jetty-client + ${project.version} + test + + + org.eclipse.jetty.http2 + http2-client + ${project.version} + test + + + org.eclipse.jetty.http2 + http2-http-client-transport + ${project.version} + test + + diff --git a/jetty-alpn/jetty-alpn-conscrypt-server/src/main/java/org/eclipse/jetty/alpn/conscrypt/server/ConscryptServerALPNProcessor.java b/jetty-alpn/jetty-alpn-conscrypt-server/src/main/java/org/eclipse/jetty/alpn/conscrypt/server/ConscryptServerALPNProcessor.java index 9444683b58b..7eef4c189b1 100644 --- a/jetty-alpn/jetty-alpn-conscrypt-server/src/main/java/org/eclipse/jetty/alpn/conscrypt/server/ConscryptServerALPNProcessor.java +++ b/jetty-alpn/jetty-alpn-conscrypt-server/src/main/java/org/eclipse/jetty/alpn/conscrypt/server/ConscryptServerALPNProcessor.java @@ -18,13 +18,14 @@ package org.eclipse.jetty.alpn.conscrypt.server; -import java.lang.reflect.Method; import java.security.Security; import java.util.List; -import java.util.function.BiFunction; import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLSocket; +import org.conscrypt.ApplicationProtocolSelector; +import org.conscrypt.Conscrypt; import org.conscrypt.OpenSSLProvider; import org.eclipse.jetty.alpn.server.ALPNServerConnection; import org.eclipse.jetty.io.Connection; @@ -41,7 +42,7 @@ public class ConscryptServerALPNProcessor implements ALPNProcessor.Server @Override public void init() { - if (Security.getProvider("Conscrypt")==null) + if (Security.getProvider("Conscrypt") == null) { Security.addProvider(new OpenSSLProvider()); if (LOG.isDebugEnabled()) @@ -56,13 +57,11 @@ public class ConscryptServerALPNProcessor implements ALPNProcessor.Server } @Override - public void configure(SSLEngine sslEngine,Connection connection) + public void configure(SSLEngine sslEngine, Connection connection) { try { - Method method = sslEngine.getClass().getMethod("setHandshakeApplicationProtocolSelector", BiFunction.class); - method.setAccessible(true); - method.invoke(sslEngine,new ALPNCallback((ALPNServerConnection)connection)); + Conscrypt.setApplicationProtocolSelector(sslEngine, new ALPNCallback((ALPNServerConnection)connection)); } catch (RuntimeException x) { @@ -74,23 +73,31 @@ public class ConscryptServerALPNProcessor implements ALPNProcessor.Server } } - private final class ALPNCallback implements BiFunction,String>, SslHandshakeListener + private final class ALPNCallback extends ApplicationProtocolSelector implements SslHandshakeListener { private final ALPNServerConnection alpnConnection; + private ALPNCallback(ALPNServerConnection connection) { - alpnConnection = connection; + alpnConnection = connection; ((DecryptedEndPoint)alpnConnection.getEndPoint()).getSslConnection().addHandshakeListener(this); } @Override - public String apply(SSLEngine engine, List protocols) + public String selectApplicationProtocol(SSLEngine engine, List protocols) { - if (LOG.isDebugEnabled()) - LOG.debug("apply {} {}", alpnConnection, protocols); alpnConnection.select(protocols); - return alpnConnection.getProtocol(); + String protocol = alpnConnection.getProtocol(); + if (LOG.isDebugEnabled()) + LOG.debug("Selected {} among {} for {}", protocol, protocols, alpnConnection); + return protocol; + } + + @Override + public String selectApplicationProtocol(SSLSocket socket, List protocols) + { + throw new UnsupportedOperationException(); } @Override @@ -99,7 +106,7 @@ public class ConscryptServerALPNProcessor implements ALPNProcessor.Server String protocol = alpnConnection.getProtocol(); if (LOG.isDebugEnabled()) LOG.debug("TLS handshake succeeded, protocol={} for {}", protocol, alpnConnection); - if (protocol ==null) + if (protocol == null) alpnConnection.unsupported(); } diff --git a/jetty-alpn/jetty-alpn-conscrypt-server/src/test/java/org/eclipse/jetty/alpn/conscrypt/server/ConscryptHTTP2Server.java b/jetty-alpn/jetty-alpn-conscrypt-server/src/test/java/org/eclipse/jetty/alpn/conscrypt/server/ConscryptHTTP2Server.java deleted file mode 100644 index 26f6ec1e92f..00000000000 --- a/jetty-alpn/jetty-alpn-conscrypt-server/src/test/java/org/eclipse/jetty/alpn/conscrypt/server/ConscryptHTTP2Server.java +++ /dev/null @@ -1,72 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. -// ------------------------------------------------------------------------ -// All rights reserved. This program and the accompanying materials -// are made available under the terms of the Eclipse Public License v1.0 -// and Apache License v2.0 which accompanies this distribution. -// -// The Eclipse Public License is available at -// http://www.eclipse.org/legal/epl-v10.html -// -// The Apache License v2.0 is available at -// http://www.opensource.org/licenses/apache2.0.php -// -// You may elect to redistribute this code under either of these licenses. -// ======================================================================== -// - -package org.eclipse.jetty.alpn.conscrypt.server; - -import java.security.Security; - -import org.conscrypt.OpenSSLProvider; -import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory; -import org.eclipse.jetty.http2.HTTP2Cipher; -import org.eclipse.jetty.http2.server.HTTP2ServerConnectionFactory; -import org.eclipse.jetty.server.HttpConfiguration; -import org.eclipse.jetty.server.HttpConnectionFactory; -import org.eclipse.jetty.server.SecureRequestCustomizer; -import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.ServerConnector; -import org.eclipse.jetty.server.SslConnectionFactory; -import org.eclipse.jetty.util.ssl.SslContextFactory; - -/** - * Test server that verifies that the Conscrypt ALPN mechanism works. - */ -public class ConscryptHTTP2Server -{ - public static void main(String[] args) throws Exception - { - Security.addProvider(new OpenSSLProvider()); - - Server server = new Server(); - - HttpConfiguration httpsConfig = new HttpConfiguration(); - httpsConfig.setSecureScheme("https"); - httpsConfig.setSecurePort(8443); - httpsConfig.setSendXPoweredBy(true); - httpsConfig.setSendServerVersion(true); - httpsConfig.addCustomizer(new SecureRequestCustomizer()); - - SslContextFactory sslContextFactory = new SslContextFactory(); - sslContextFactory.setProvider("Conscrypt"); - sslContextFactory.setKeyStorePath("src/test/resources/keystore.jks"); - sslContextFactory.setKeyStorePassword("OBF:1vny1zlo1x8e1vnw1vn61x8g1zlu1vn4"); - sslContextFactory.setKeyManagerPassword("OBF:1u2u1wml1z7s1z7a1wnl1u2g"); - sslContextFactory.setCipherComparator(HTTP2Cipher.COMPARATOR); - - HttpConnectionFactory http = new HttpConnectionFactory(httpsConfig); - HTTP2ServerConnectionFactory h2 = new HTTP2ServerConnectionFactory(httpsConfig); - ALPNServerConnectionFactory alpn = new ALPNServerConnectionFactory(); - alpn.setDefaultProtocol(http.getProtocol()); - SslConnectionFactory ssl = new SslConnectionFactory(sslContextFactory, alpn.getProtocol()); - - ServerConnector http2Connector = new ServerConnector(server, ssl, alpn, h2, http); - http2Connector.setPort(8443); - server.addConnector(http2Connector); - - server.start(); - } -} diff --git a/jetty-alpn/jetty-alpn-conscrypt-server/src/test/java/org/eclipse/jetty/alpn/conscrypt/server/ConscryptHTTP2ServerTest.java b/jetty-alpn/jetty-alpn-conscrypt-server/src/test/java/org/eclipse/jetty/alpn/conscrypt/server/ConscryptHTTP2ServerTest.java new file mode 100644 index 00000000000..cb621a020c3 --- /dev/null +++ b/jetty-alpn/jetty-alpn-conscrypt-server/src/test/java/org/eclipse/jetty/alpn/conscrypt/server/ConscryptHTTP2ServerTest.java @@ -0,0 +1,141 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.alpn.conscrypt.server; + +import java.io.File; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.security.Security; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.conscrypt.OpenSSLProvider; +import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory; +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.client.api.ContentResponse; +import org.eclipse.jetty.http2.client.HTTP2Client; +import org.eclipse.jetty.http2.client.http.HttpClientTransportOverHTTP2; +import org.eclipse.jetty.http2.server.HTTP2ServerConnectionFactory; +import org.eclipse.jetty.server.HttpConfiguration; +import org.eclipse.jetty.server.HttpConnectionFactory; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.SecureRequestCustomizer; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.server.SslConnectionFactory; +import org.eclipse.jetty.server.handler.AbstractHandler; +import org.eclipse.jetty.util.JavaVersion; +import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Test server that verifies that the Conscrypt ALPN mechanism works for both server and client side + */ +public class ConscryptHTTP2ServerTest +{ + static + { + Security.addProvider(new OpenSSLProvider()); + } + + private Server server = new Server(); + + private SslContextFactory newSslContextFactory() + { + Path path = Paths.get("src", "test", "resources"); + File keys = path.resolve("keystore").toFile(); + + SslContextFactory sslContextFactory = new SslContextFactory(); + sslContextFactory.setKeyManagerPassword("OBF:1vny1zlo1x8e1vnw1vn61x8g1zlu1vn4"); + sslContextFactory.setTrustStorePath(keys.getAbsolutePath()); + sslContextFactory.setKeyStorePath(keys.getAbsolutePath()); + sslContextFactory.setTrustStorePassword("OBF:1vny1zlo1x8e1vnw1vn61x8g1zlu1vn4"); + sslContextFactory.setProvider("Conscrypt"); + sslContextFactory.setEndpointIdentificationAlgorithm(null); + if (JavaVersion.VERSION.getPlatform() < 9) + { + // Conscrypt enables TLSv1.3 by default but it's not supported in Java 8. + sslContextFactory.addExcludeProtocols("TLSv1.3"); + } + return sslContextFactory; + } + + @BeforeEach + public void startServer() throws Exception + { + HttpConfiguration httpsConfig = new HttpConfiguration(); + httpsConfig.setSecureScheme("https"); + + httpsConfig.setSendXPoweredBy(true); + httpsConfig.setSendServerVersion(true); + httpsConfig.addCustomizer(new SecureRequestCustomizer()); + + HttpConnectionFactory http = new HttpConnectionFactory(httpsConfig); + HTTP2ServerConnectionFactory h2 = new HTTP2ServerConnectionFactory(httpsConfig); + ALPNServerConnectionFactory alpn = new ALPNServerConnectionFactory(); + alpn.setDefaultProtocol(http.getProtocol()); + SslConnectionFactory ssl = new SslConnectionFactory(newSslContextFactory(), alpn.getProtocol()); + + ServerConnector http2Connector = new ServerConnector(server, ssl, alpn, h2, http); + http2Connector.setPort(0); + server.addConnector(http2Connector); + + server.setHandler(new AbstractHandler() + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) + { + response.setStatus(200); + baseRequest.setHandled(true); + } + }); + + server.start(); + } + + @AfterEach + public void stopServer() throws Exception + { + if (server != null) + server.stop(); + } + + @Test + public void testSimpleRequest() throws Exception + { + HTTP2Client h2Client = new HTTP2Client(); + HttpClient client = new HttpClient(new HttpClientTransportOverHTTP2(h2Client), newSslContextFactory()); + client.start(); + try + { + int port = ((ServerConnector)server.getConnectors()[0]).getLocalPort(); + ContentResponse contentResponse = client.GET("https://localhost:" + port); + assertEquals(200, contentResponse.getStatus()); + } + finally + { + client.stop(); + } + } +} diff --git a/jetty-alpn/jetty-alpn-conscrypt-server/src/test/resources/keystore b/jetty-alpn/jetty-alpn-conscrypt-server/src/test/resources/keystore new file mode 100644 index 00000000000..428ba54776e Binary files /dev/null and b/jetty-alpn/jetty-alpn-conscrypt-server/src/test/resources/keystore differ diff --git a/jetty-alpn/jetty-alpn-conscrypt-server/src/test/resources/keystore.jks b/jetty-alpn/jetty-alpn-conscrypt-server/src/test/resources/keystore.jks deleted file mode 100644 index d6592f95ee9..00000000000 Binary files a/jetty-alpn/jetty-alpn-conscrypt-server/src/test/resources/keystore.jks and /dev/null differ diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/ContentDecoder.java b/jetty-client/src/main/java/org/eclipse/jetty/client/ContentDecoder.java index 471823f3d7f..c4449b703f8 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/ContentDecoder.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/ContentDecoder.java @@ -35,6 +35,15 @@ public interface ContentDecoder */ public abstract ByteBuffer decode(ByteBuffer buffer); + /** + *

Releases the ByteBuffer returned by {@link #decode(ByteBuffer)}.

+ * + * @param decoded the ByteBuffer returned by {@link #decode(ByteBuffer)} + */ + public default void release(ByteBuffer decoded) + { + } + /** * Factory for {@link ContentDecoder}s; subclasses must implement {@link #newContentDecoder()}. *

diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java b/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java index 857d634f044..7a02c1b4f04 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java @@ -18,6 +18,8 @@ package org.eclipse.jetty.client; +import java.nio.ByteBuffer; + import org.eclipse.jetty.io.ByteBufferPool; /** @@ -25,7 +27,7 @@ import org.eclipse.jetty.io.ByteBufferPool; */ public class GZIPContentDecoder extends org.eclipse.jetty.http.GZIPContentDecoder implements ContentDecoder { - private static final int DEFAULT_BUFFER_SIZE = 2048; + public static final int DEFAULT_BUFFER_SIZE = 8192; public GZIPContentDecoder() { @@ -42,6 +44,13 @@ public class GZIPContentDecoder extends org.eclipse.jetty.http.GZIPContentDecode super(byteBufferPool, bufferSize); } + @Override + protected boolean decodedChunk(ByteBuffer chunk) + { + super.decodedChunk(chunk); + return true; + } + /** * Specialized {@link ContentDecoder.Factory} for the "gzip" encoding. */ 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 7690791347b..63fe07ef5d2 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 @@ -21,7 +21,6 @@ package org.eclipse.jetty.client; import java.io.IOException; import java.net.URI; import java.nio.ByteBuffer; -import java.util.ArrayList; import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; @@ -37,7 +36,7 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; -import org.eclipse.jetty.util.CountingCallback; +import org.eclipse.jetty.util.IteratingNestedCallback; import org.eclipse.jetty.util.component.Destroyable; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -339,35 +338,7 @@ public abstract class HttpReceiver } else { - try - { - List decodeds = new ArrayList<>(2); - while (buffer.hasRemaining()) - { - ByteBuffer decoded = decoder.decode(buffer); - if (!decoded.hasRemaining()) - continue; - decodeds.add(decoded); - if (LOG.isDebugEnabled()) - LOG.debug("Response content decoded ({}) {}{}{}", decoder, response, System.lineSeparator(), BufferUtil.toDetailString(decoded)); - } - - if (decodeds.isEmpty()) - { - callback.succeeded(); - } - else - { - int size = decodeds.size(); - CountingCallback counter = new CountingCallback(callback, size); - for (ByteBuffer decoded : decodeds) - notifier.notifyContent(response, decoded, counter, contentListeners); - } - } - catch (Throwable x) - { - callback.failed(x); - } + new Decoder(notifier, response, decoder, buffer, callback).iterate(); } if (updateResponseState(ResponseState.TRANSIENT, ResponseState.CONTENT)) @@ -615,4 +586,47 @@ public abstract class HttpReceiver */ FAILURE } + + private class Decoder extends IteratingNestedCallback + { + private final ResponseNotifier notifier; + private final HttpResponse response; + private final ContentDecoder decoder; + private final ByteBuffer buffer; + private ByteBuffer decoded; + + public Decoder(ResponseNotifier notifier, HttpResponse response, ContentDecoder decoder, ByteBuffer buffer, Callback callback) + { + super(callback); + this.notifier = notifier; + this.response = response; + this.decoder = decoder; + this.buffer = buffer; + } + + @Override + protected Action process() throws Throwable + { + while (true) + { + decoded = decoder.decode(buffer); + if (decoded.hasRemaining()) + break; + if (!buffer.hasRemaining()) + return Action.SUCCEEDED; + } + if (LOG.isDebugEnabled()) + LOG.debug("Response content decoded ({}) {}{}{}", decoder, response, System.lineSeparator(), BufferUtil.toDetailString(decoded)); + + notifier.notifyContent(response, decoded, this, contentListeners); + return Action.SCHEDULED; + } + + @Override + public void succeeded() + { + decoder.release(decoded); + super.succeeded(); + } + } } diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/GZIPContentDecoderTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/GZIPContentDecoderTest.java deleted file mode 100644 index c627ba84547..00000000000 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/GZIPContentDecoderTest.java +++ /dev/null @@ -1,288 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. -// ------------------------------------------------------------------------ -// All rights reserved. This program and the accompanying materials -// are made available under the terms of the Eclipse Public License v1.0 -// and Apache License v2.0 which accompanies this distribution. -// -// The Eclipse Public License is available at -// http://www.eclipse.org/legal/epl-v10.html -// -// The Apache License v2.0 is available at -// http://www.opensource.org/licenses/apache2.0.php -// -// You may elect to redistribute this code under either of these licenses. -// ======================================================================== -// - -package org.eclipse.jetty.client; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.nio.ByteBuffer; -import java.nio.charset.StandardCharsets; -import java.util.zip.GZIPInputStream; -import java.util.zip.GZIPOutputStream; - -import org.junit.jupiter.api.Test; - -@Deprecated -public class GZIPContentDecoderTest -{ - @Test - public void testStreamNoBlocks() throws Exception - { - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - GZIPOutputStream output = new GZIPOutputStream(baos); - output.close(); - byte[] bytes = baos.toByteArray(); - - GZIPInputStream input = new GZIPInputStream(new ByteArrayInputStream(bytes), 1); - int read = input.read(); - assertEquals(-1, read); - } - - @Test - public void testStreamBigBlockOneByteAtATime() throws Exception - { - String data = "0123456789ABCDEF"; - for (int i = 0; i < 10; ++i) - data += data; - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - GZIPOutputStream output = new GZIPOutputStream(baos); - output.write(data.getBytes(StandardCharsets.UTF_8)); - output.close(); - byte[] bytes = baos.toByteArray(); - - baos = new ByteArrayOutputStream(); - GZIPInputStream input = new GZIPInputStream(new ByteArrayInputStream(bytes), 1); - int read; - while ((read = input.read()) >= 0) - baos.write(read); - assertEquals(data, new String(baos.toByteArray(), StandardCharsets.UTF_8)); - } - - @Test - public void testNoBlocks() throws Exception - { - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - GZIPOutputStream output = new GZIPOutputStream(baos); - output.close(); - byte[] bytes = baos.toByteArray(); - - GZIPContentDecoder decoder = new GZIPContentDecoder(); - ByteBuffer decoded = decoder.decode(ByteBuffer.wrap(bytes)); - assertEquals(0, decoded.remaining()); - } - - @Test - public void testSmallBlock() throws Exception - { - String data = "0"; - - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - GZIPOutputStream output = new GZIPOutputStream(baos); - output.write(data.getBytes(StandardCharsets.UTF_8)); - output.close(); - byte[] bytes = baos.toByteArray(); - - GZIPContentDecoder decoder = new GZIPContentDecoder(); - ByteBuffer decoded = decoder.decode(ByteBuffer.wrap(bytes)); - assertEquals(data, StandardCharsets.UTF_8.decode(decoded).toString()); - } - - @Test - public void testSmallBlockWithGZIPChunkedAtBegin() throws Exception - { - String data = "0"; - - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - GZIPOutputStream output = new GZIPOutputStream(baos); - output.write(data.getBytes(StandardCharsets.UTF_8)); - output.close(); - byte[] bytes = baos.toByteArray(); - - // The header is 10 bytes, chunk at 11 bytes - byte[] bytes1 = new byte[11]; - System.arraycopy(bytes, 0, bytes1, 0, bytes1.length); - byte[] bytes2 = new byte[bytes.length - bytes1.length]; - System.arraycopy(bytes, bytes1.length, bytes2, 0, bytes2.length); - - GZIPContentDecoder decoder = new GZIPContentDecoder(); - ByteBuffer decoded = decoder.decode(ByteBuffer.wrap(bytes1)); - assertEquals(0, decoded.capacity()); - decoded = decoder.decode(ByteBuffer.wrap(bytes2)); - assertEquals(data, StandardCharsets.UTF_8.decode(decoded).toString()); - } - - @Test - public void testSmallBlockWithGZIPChunkedAtEnd() throws Exception - { - String data = "0"; - - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - GZIPOutputStream output = new GZIPOutputStream(baos); - output.write(data.getBytes(StandardCharsets.UTF_8)); - output.close(); - byte[] bytes = baos.toByteArray(); - - // The trailer is 8 bytes, chunk the last 9 bytes - byte[] bytes1 = new byte[bytes.length - 9]; - System.arraycopy(bytes, 0, bytes1, 0, bytes1.length); - byte[] bytes2 = new byte[bytes.length - bytes1.length]; - System.arraycopy(bytes, bytes1.length, bytes2, 0, bytes2.length); - - GZIPContentDecoder decoder = new GZIPContentDecoder(); - ByteBuffer decoded = decoder.decode(ByteBuffer.wrap(bytes1)); - assertEquals(data, StandardCharsets.UTF_8.decode(decoded).toString()); - assertFalse(decoder.isFinished()); - decoded = decoder.decode(ByteBuffer.wrap(bytes2)); - assertEquals(0, decoded.remaining()); - assertTrue(decoder.isFinished()); - } - - @Test - public void testSmallBlockWithGZIPTrailerChunked() throws Exception - { - String data = "0"; - - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - GZIPOutputStream output = new GZIPOutputStream(baos); - output.write(data.getBytes(StandardCharsets.UTF_8)); - output.close(); - byte[] bytes = baos.toByteArray(); - - // The trailer is 4+4 bytes, chunk the last 3 bytes - byte[] bytes1 = new byte[bytes.length - 3]; - System.arraycopy(bytes, 0, bytes1, 0, bytes1.length); - byte[] bytes2 = new byte[bytes.length - bytes1.length]; - System.arraycopy(bytes, bytes1.length, bytes2, 0, bytes2.length); - - GZIPContentDecoder decoder = new GZIPContentDecoder(); - ByteBuffer decoded = decoder.decode(ByteBuffer.wrap(bytes1)); - assertEquals(0, decoded.capacity()); - decoded = decoder.decode(ByteBuffer.wrap(bytes2)); - assertEquals(data, StandardCharsets.UTF_8.decode(decoded).toString()); - } - - @Test - public void testTwoSmallBlocks() throws Exception - { - String data1 = "0"; - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - GZIPOutputStream output = new GZIPOutputStream(baos); - output.write(data1.getBytes(StandardCharsets.UTF_8)); - output.close(); - byte[] bytes1 = baos.toByteArray(); - - String data2 = "1"; - baos = new ByteArrayOutputStream(); - output = new GZIPOutputStream(baos); - output.write(data2.getBytes(StandardCharsets.UTF_8)); - output.close(); - byte[] bytes2 = baos.toByteArray(); - - byte[] bytes = new byte[bytes1.length + bytes2.length]; - System.arraycopy(bytes1, 0, bytes, 0, bytes1.length); - System.arraycopy(bytes2, 0, bytes, bytes1.length, bytes2.length); - - GZIPContentDecoder decoder = new GZIPContentDecoder(); - ByteBuffer buffer = ByteBuffer.wrap(bytes); - ByteBuffer decoded = decoder.decode(buffer); - assertEquals(data1, StandardCharsets.UTF_8.decode(decoded).toString()); - assertTrue(decoder.isFinished()); - assertTrue(buffer.hasRemaining()); - decoded = decoder.decode(buffer); - assertEquals(data2, StandardCharsets.UTF_8.decode(decoded).toString()); - assertTrue(decoder.isFinished()); - assertFalse(buffer.hasRemaining()); - } - - @Test - public void testBigBlock() throws Exception - { - String data = "0123456789ABCDEF"; - for (int i = 0; i < 10; ++i) - data += data; - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - GZIPOutputStream output = new GZIPOutputStream(baos); - output.write(data.getBytes(StandardCharsets.UTF_8)); - output.close(); - byte[] bytes = baos.toByteArray(); - - String result = ""; - GZIPContentDecoder decoder = new GZIPContentDecoder(); - ByteBuffer buffer = ByteBuffer.wrap(bytes); - while (buffer.hasRemaining()) - { - ByteBuffer decoded = decoder.decode(buffer); - result += StandardCharsets.UTF_8.decode(decoded).toString(); - } - assertEquals(data, result); - } - - @Test - public void testBigBlockOneByteAtATime() throws Exception - { - String data = "0123456789ABCDEF"; - for (int i = 0; i < 10; ++i) - data += data; - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - GZIPOutputStream output = new GZIPOutputStream(baos); - output.write(data.getBytes(StandardCharsets.UTF_8)); - output.close(); - byte[] bytes = baos.toByteArray(); - - String result = ""; - GZIPContentDecoder decoder = new GZIPContentDecoder(64); - ByteBuffer buffer = ByteBuffer.wrap(bytes); - while (buffer.hasRemaining()) - { - ByteBuffer decoded = decoder.decode(ByteBuffer.wrap(new byte[]{buffer.get()})); - if (decoded.hasRemaining()) - result += StandardCharsets.UTF_8.decode(decoded).toString(); - } - assertEquals(data, result); - assertTrue(decoder.isFinished()); - } - - @Test - public void testBigBlockWithExtraBytes() throws Exception - { - String data1 = "0123456789ABCDEF"; - for (int i = 0; i < 10; ++i) - data1 += data1; - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - GZIPOutputStream output = new GZIPOutputStream(baos); - output.write(data1.getBytes(StandardCharsets.UTF_8)); - output.close(); - byte[] bytes1 = baos.toByteArray(); - - String data2 = "HELLO"; - byte[] bytes2 = data2.getBytes(StandardCharsets.UTF_8); - - byte[] bytes = new byte[bytes1.length + bytes2.length]; - System.arraycopy(bytes1, 0, bytes, 0, bytes1.length); - System.arraycopy(bytes2, 0, bytes, bytes1.length, bytes2.length); - - String result = ""; - GZIPContentDecoder decoder = new GZIPContentDecoder(64); - ByteBuffer buffer = ByteBuffer.wrap(bytes); - while (buffer.hasRemaining()) - { - ByteBuffer decoded = decoder.decode(buffer); - if (decoded.hasRemaining()) - result += StandardCharsets.UTF_8.decode(decoded).toString(); - if (decoder.isFinished()) - break; - } - assertEquals(data1, result); - assertTrue(buffer.hasRemaining()); - assertEquals(data2, StandardCharsets.UTF_8.decode(buffer).toString()); - } -} diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java index 0e47e96617b..acd6232cec0 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java @@ -18,29 +18,36 @@ package org.eclipse.jetty.client; -import static org.junit.jupiter.api.Assertions.assertArrayEquals; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InterruptedIOException; +import java.nio.charset.StandardCharsets; import java.util.Arrays; +import java.util.Random; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.zip.GZIPOutputStream; -import javax.servlet.ServletException; import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.client.api.ContentResponse; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.io.ByteBufferPool; +import org.eclipse.jetty.io.MappedByteBufferPool; import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.handler.AbstractHandler; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ArgumentsSource; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.lessThan; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + public class HttpClientGZIPTest extends AbstractHttpClientServerTest { @ParameterizedTest @@ -48,12 +55,11 @@ public class HttpClientGZIPTest extends AbstractHttpClientServerTest public void testGZIPContentEncoding(Scenario scenario) throws Exception { final byte[] data = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; - start(scenario, new AbstractHandler() + start(scenario, new EmptyServerHandler() { @Override - public void handle(String target, org.eclipse.jetty.server.Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException { - baseRequest.setHandled(true); response.setHeader("Content-Encoding", "gzip"); GZIPOutputStream gzipOutput = new GZIPOutputStream(response.getOutputStream()); gzipOutput.write(data); @@ -75,12 +81,11 @@ public class HttpClientGZIPTest extends AbstractHttpClientServerTest public void testGZIPContentOneByteAtATime(Scenario scenario) throws Exception { final byte[] data = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; - start(scenario, new AbstractHandler() + start(scenario, new EmptyServerHandler() { @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException { - baseRequest.setHandled(true); response.setHeader("Content-Encoding", "gzip"); ByteArrayOutputStream gzipData = new ByteArrayOutputStream(); @@ -112,12 +117,11 @@ public class HttpClientGZIPTest extends AbstractHttpClientServerTest public void testGZIPContentSentTwiceInOneWrite(Scenario scenario) throws Exception { final byte[] data = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; - start(scenario, new AbstractHandler() + start(scenario, new EmptyServerHandler() { @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException { - baseRequest.setHandled(true); response.setHeader("Content-Encoding", "gzip"); ByteArrayOutputStream gzipData = new ByteArrayOutputStream(); @@ -164,12 +168,11 @@ public class HttpClientGZIPTest extends AbstractHttpClientServerTest private void testGZIPContentFragmented(Scenario scenario, final int fragment) throws Exception { final byte[] data = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; - start(scenario, new AbstractHandler() + start(scenario, new EmptyServerHandler() { @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException { - baseRequest.setHandled(true); response.setHeader("Content-Encoding", "gzip"); ByteArrayOutputStream gzipData = new ByteArrayOutputStream(); @@ -204,12 +207,11 @@ public class HttpClientGZIPTest extends AbstractHttpClientServerTest @ArgumentsSource(ScenarioProvider.class) public void testGZIPContentCorrupted(Scenario scenario) throws Exception { - start(scenario, new AbstractHandler() + start(scenario, new EmptyServerHandler() { @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException { - baseRequest.setHandled(true); response.setHeader("Content-Encoding", "gzip"); // Not gzipped, will cause the client to blow up. response.getOutputStream().print("0123456789"); @@ -228,6 +230,46 @@ public class HttpClientGZIPTest extends AbstractHttpClientServerTest assertTrue(latch.await(5, TimeUnit.SECONDS)); } + @ParameterizedTest + @ArgumentsSource(ScenarioProvider.class) + public void testLargeGZIPContentDoesNotPolluteByteBufferPool(Scenario scenario) throws Exception + { + String digits = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; + Random random = new Random(); + byte[] content = new byte[1024 * 1024]; + for (int i = 0; i < content.length; ++i) + content[i] = (byte)digits.charAt(random.nextInt(digits.length())); + start(scenario, new EmptyServerHandler() + { + @Override + protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException + { + response.setContentType("text/plain;charset=" + StandardCharsets.US_ASCII.name()); + response.setHeader(HttpHeader.CONTENT_ENCODING.asString(), "gzip"); + GZIPOutputStream gzip = new GZIPOutputStream(response.getOutputStream()); + gzip.write(content); + gzip.finish(); + } + }); + + ByteBufferPool pool = client.getByteBufferPool(); + assumeTrue(pool instanceof MappedByteBufferPool); + MappedByteBufferPool bufferPool = (MappedByteBufferPool)pool; + + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .scheme(scenario.getScheme()) + .timeout(5, TimeUnit.SECONDS) + .send(); + + assertEquals(HttpStatus.OK_200, response.getStatus()); + assertArrayEquals(content, response.getContent()); + + long directMemory = bufferPool.getMemory(true); + assertThat(directMemory, lessThan((long)content.length)); + long heapMemory = bufferPool.getMemory(false); + assertThat(heapMemory, lessThan((long)content.length)); + } + private static void sleep(long ms) throws IOException { try diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java index f683ec3268c..12edd273bfd 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java @@ -50,7 +50,7 @@ import org.eclipse.jetty.util.thread.ExecutorThreadPool; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.condition.DisabledOnJre; +import org.junit.jupiter.api.condition.EnabledOnJre; import org.junit.jupiter.api.condition.JRE; import static org.hamcrest.MatcherAssert.assertThat; @@ -139,12 +139,10 @@ public class HttpClientTLSTest }); assertThrows(ExecutionException.class, () -> - { - client.newRequest("localhost", connector.getLocalPort()) - .scheme(HttpScheme.HTTPS.asString()) - .timeout(5, TimeUnit.SECONDS) - .send(); - }); + client.newRequest("localhost", connector.getLocalPort()) + .scheme(HttpScheme.HTTPS.asString()) + .timeout(5, TimeUnit.SECONDS) + .send()); assertTrue(serverLatch.await(1, TimeUnit.SECONDS)); assertTrue(clientLatch.await(1, TimeUnit.SECONDS)); @@ -182,12 +180,10 @@ public class HttpClientTLSTest }); assertThrows(ExecutionException.class, () -> - { - client.newRequest("localhost", connector.getLocalPort()) - .scheme(HttpScheme.HTTPS.asString()) - .timeout(5, TimeUnit.SECONDS) - .send(); - }); + client.newRequest("localhost", connector.getLocalPort()) + .scheme(HttpScheme.HTTPS.asString()) + .timeout(5, TimeUnit.SECONDS) + .send()); assertTrue(serverLatch.await(1, TimeUnit.SECONDS)); assertTrue(clientLatch.await(1, TimeUnit.SECONDS)); @@ -226,24 +222,21 @@ public class HttpClientTLSTest }); assertThrows(ExecutionException.class, () -> - { - client.newRequest("localhost", connector.getLocalPort()) - .scheme(HttpScheme.HTTPS.asString()) - .timeout(5, TimeUnit.SECONDS) - .send(); - }); + client.newRequest("localhost", connector.getLocalPort()) + .scheme(HttpScheme.HTTPS.asString()) + .timeout(5, TimeUnit.SECONDS) + .send()); assertTrue(serverLatch.await(1, TimeUnit.SECONDS)); assertTrue(clientLatch.await(1, TimeUnit.SECONDS)); } - // In JDK 11, a mismatch on the client does not generate any bytes towards - // the server, while in TLS 1.2 the client sends to the server the close_notify. - @DisabledOnJre(JRE.JAVA_11) + // In JDK 11+, a mismatch on the client does not generate any bytes towards + // the server, while in previous JDKs the client sends to the server the close_notify. + @EnabledOnJre({JRE.JAVA_8, JRE.JAVA_9, JRE.JAVA_10}) @Test public void testMismatchBetweenTLSProtocolAndTLSCiphersOnClient() throws Exception { - SslContextFactory serverTLSFactory = createSslContextFactory(); startServer(serverTLSFactory, new EmptyServerHandler()); @@ -274,12 +267,10 @@ public class HttpClientTLSTest }); assertThrows(ExecutionException.class, () -> - { - client.newRequest("localhost", connector.getLocalPort()) - .scheme(HttpScheme.HTTPS.asString()) - .timeout(5, TimeUnit.SECONDS) - .send(); - }); + client.newRequest("localhost", connector.getLocalPort()) + .scheme(HttpScheme.HTTPS.asString()) + .timeout(5, TimeUnit.SECONDS) + .send()); assertTrue(serverLatch.await(1, TimeUnit.SECONDS)); assertTrue(clientLatch.await(1, TimeUnit.SECONDS)); @@ -321,8 +312,9 @@ public class HttpClientTLSTest assertTrue(clientLatch.await(1, TimeUnit.SECONDS)); } - // Excluded because of a bug in JDK 11+27 where session resumption does not work. - @DisabledOnJre(JRE.JAVA_11) + // Excluded in JDK 11+ because resumed sessions cannot be compared + // using their session IDs even though they are resumed correctly. + @EnabledOnJre({JRE.JAVA_8, JRE.JAVA_9, JRE.JAVA_10}) @Test public void testHandshakeSucceededWithSessionResumption() throws Exception { @@ -400,8 +392,9 @@ public class HttpClientTLSTest assertTrue(clientLatch.await(1, TimeUnit.SECONDS)); } - // Excluded because of a bug in JDK 11+27 where session resumption does not work. - @DisabledOnJre(JRE.JAVA_11) + // Excluded in JDK 11+ because resumed sessions cannot be compared + // using their session IDs even though they are resumed correctly. + @EnabledOnJre({JRE.JAVA_8, JRE.JAVA_9, JRE.JAVA_10}) @Test public void testClientRawCloseDoesNotInvalidateSession() throws Exception { diff --git a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/App.java b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/App.java index ab66563d054..365f5e8ecf5 100644 --- a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/App.java +++ b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/App.java @@ -69,7 +69,6 @@ public class App _context = context; } - /* ------------------------------------------------------------ */ /** * @return The deployment manager */ @@ -78,7 +77,6 @@ public class App return _manager; } - /* ------------------------------------------------------------ */ /** * @return The AppProvider */ @@ -87,7 +85,6 @@ public class App return _provider; } - /* ------------------------------------------------------------ */ /** * Get ContextHandler for the App. * @@ -149,7 +146,6 @@ public class App return this._context.getContextPath(); } - /** * The origin of this {@link App} as specified by the {@link AppProvider} * diff --git a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/AppProvider.java b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/AppProvider.java index 8d7fd9adf08..1daa58c5a2c 100644 --- a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/AppProvider.java +++ b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/AppProvider.java @@ -36,8 +36,7 @@ public interface AppProvider extends LifeCycle * if the provider {@link #isRunning()}. */ void setDeploymentManager(DeploymentManager deploymentManager); - - /* ------------------------------------------------------------ */ + /** Create a ContextHandler for an App * @param app The App * @return A ContextHandler diff --git a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/DeploymentManager.java b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/DeploymentManager.java index 10021292749..5fcfbbc1cae 100644 --- a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/DeploymentManager.java +++ b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/DeploymentManager.java @@ -152,7 +152,6 @@ public class DeploymentManager extends ContainerLifeCycle } } - /* ------------------------------------------------------------ */ /** Set the AppProviders. * The providers passed are added via {@link #addBean(Object)} so that * their lifecycles may be managed as a {@link ContainerLifeCycle}. @@ -170,7 +169,6 @@ public class DeploymentManager extends ContainerLifeCycle addBean(provider); } - @ManagedAttribute("Application Providers") public Collection getAppProviders() { return Collections.unmodifiableList(_providers); @@ -181,7 +179,7 @@ public class DeploymentManager extends ContainerLifeCycle if (isRunning()) throw new IllegalStateException(); _providers.add(provider); - addBean(provider); + addBean(provider); } public void setLifeCycleBindings(Collection bindings) @@ -292,7 +290,6 @@ public class DeploymentManager extends ContainerLifeCycle return Collections.unmodifiableCollection(_apps); } - @ManagedAttribute("Deployed Apps") public Collection getApps() { List ret = new ArrayList< >(); diff --git a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/bindings/DebugListenerBinding.java b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/bindings/DebugListenerBinding.java index 93065927492..2ffc02aa9e7 100644 --- a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/bindings/DebugListenerBinding.java +++ b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/bindings/DebugListenerBinding.java @@ -22,7 +22,6 @@ import org.eclipse.jetty.deploy.App; import org.eclipse.jetty.deploy.graph.Node; import org.eclipse.jetty.server.DebugListener; - /** A Deployment binding that installs a DebugListener in all deployed contexts */ public class DebugListenerBinding extends DebugBinding diff --git a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/bindings/GlobalWebappConfigBinding.java b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/bindings/GlobalWebappConfigBinding.java index dc805dccf96..d93d8c04ad4 100644 --- a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/bindings/GlobalWebappConfigBinding.java +++ b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/bindings/GlobalWebappConfigBinding.java @@ -18,8 +18,6 @@ package org.eclipse.jetty.deploy.bindings; -import java.io.File; - import org.eclipse.jetty.deploy.App; import org.eclipse.jetty.deploy.AppLifeCycle; import org.eclipse.jetty.deploy.graph.Node; @@ -48,7 +46,6 @@ public class GlobalWebappConfigBinding implements AppLifeCycle.Binding { private static final Logger LOG = Log.getLogger(GlobalWebappConfigBinding.class); - private String _jettyXml; public String getJettyXml() diff --git a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/graph/Graph.java b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/graph/Graph.java index ec2bb0a0c51..bfa30f68ef1 100644 --- a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/graph/Graph.java +++ b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/graph/Graph.java @@ -203,7 +203,6 @@ public class Graph Path path = breadthFirst(from,to,new CopyOnWriteArrayList(),new HashSet()); return path; } - private Path breadthFirst(Node from, Node destination, CopyOnWriteArrayList paths, Set seen) { @@ -246,7 +245,6 @@ public class Graph return null; } - public Set getEdges() { return _edges; diff --git a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/jmx/DeploymentManagerMBean.java b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/jmx/DeploymentManagerMBean.java index af8fe6db6fa..fa532548737 100644 --- a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/jmx/DeploymentManagerMBean.java +++ b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/jmx/DeploymentManagerMBean.java @@ -24,11 +24,11 @@ import java.util.List; import java.util.stream.Collectors; import org.eclipse.jetty.deploy.App; -import org.eclipse.jetty.deploy.AppProvider; import org.eclipse.jetty.deploy.DeploymentManager; import org.eclipse.jetty.deploy.graph.Node; import org.eclipse.jetty.jmx.ObjectMBean; import org.eclipse.jetty.server.handler.ContextHandler; +import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.annotation.ManagedOperation; import org.eclipse.jetty.util.annotation.Name; @@ -45,7 +45,7 @@ public class DeploymentManagerMBean extends ObjectMBean _manager = (DeploymentManager) managedObject; } - @ManagedOperation(value = "list apps being tracked", impact = "INFO") + @ManagedAttribute(value = "list apps being tracked") public Collection getApps() { List ret = new ArrayList<>(); @@ -95,9 +95,10 @@ public class DeploymentManagerMBean extends ObjectMBean return apps; } - public Collection getAppProviders() + @ManagedAttribute("Registered AppProviders") + public List getAppProviders() { - return _manager.getAppProviders(); + return _manager.getAppProviders().stream().map(String::valueOf).collect(Collectors.toList()); } public void requestAppGoal(String appId, String nodeName) diff --git a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java index 3ce17135cc7..85e39a3d9f3 100644 --- a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java +++ b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/ScanningAppProvider.java @@ -27,6 +27,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.stream.Collectors; import org.eclipse.jetty.deploy.App; import org.eclipse.jetty.deploy.AppProvider; @@ -34,6 +35,7 @@ import org.eclipse.jetty.deploy.DeploymentManager; import org.eclipse.jetty.util.Scanner; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; +import org.eclipse.jetty.util.annotation.ManagedOperation; import org.eclipse.jetty.util.component.AbstractLifeCycle; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -55,7 +57,6 @@ public abstract class ScanningAppProvider extends AbstractLifeCycle implements A private int _scanInterval = 10; private Scanner _scanner; - /* ------------------------------------------------------------ */ private final Scanner.DiscreteListener _scannerListener = new Scanner.DiscreteListener() { @Override @@ -77,26 +78,22 @@ public abstract class ScanningAppProvider extends AbstractLifeCycle implements A } }; - /* ------------------------------------------------------------ */ protected ScanningAppProvider() { } - - /* ------------------------------------------------------------ */ + protected ScanningAppProvider(FilenameFilter filter) { _filenameFilter = filter; } - /* ------------------------------------------------------------ */ protected void setFilenameFilter(FilenameFilter filter) { if (isRunning()) throw new IllegalStateException(); _filenameFilter = filter; } - - /* ------------------------------------------------------------ */ + /** * @return The index of currently deployed applications. */ @@ -105,7 +102,6 @@ public abstract class ScanningAppProvider extends AbstractLifeCycle implements A return _appMap; } - /* ------------------------------------------------------------ */ /** * Called by the Scanner.DiscreteListener to create a new App object. * Isolated in a method so that it is possible to override the default App @@ -121,7 +117,6 @@ public abstract class ScanningAppProvider extends AbstractLifeCycle implements A return new App(_deploymentManager,this,filename); } - /* ------------------------------------------------------------ */ @Override protected void doStart() throws Exception { @@ -150,7 +145,6 @@ public abstract class ScanningAppProvider extends AbstractLifeCycle implements A _scanner.start(); } - /* ------------------------------------------------------------ */ @Override protected void doStop() throws Exception { @@ -161,14 +155,12 @@ public abstract class ScanningAppProvider extends AbstractLifeCycle implements A _scanner = null; } } - - /* ------------------------------------------------------------ */ + protected boolean exists(String path) { return _scanner.exists(path); } - /* ------------------------------------------------------------ */ protected void fileAdded(String filename) throws Exception { if (LOG.isDebugEnabled()) @@ -181,7 +173,6 @@ public abstract class ScanningAppProvider extends AbstractLifeCycle implements A } } - /* ------------------------------------------------------------ */ protected void fileChanged(String filename) throws Exception { if (LOG.isDebugEnabled()) @@ -198,8 +189,7 @@ public abstract class ScanningAppProvider extends AbstractLifeCycle implements A _deploymentManager.addApp(app); } } - - /* ------------------------------------------------------------ */ + protected void fileRemoved(String filename) throws Exception { if (LOG.isDebugEnabled()) @@ -208,8 +198,7 @@ public abstract class ScanningAppProvider extends AbstractLifeCycle implements A if (app != null) _deploymentManager.removeApp(app); } - - /* ------------------------------------------------------------ */ + /** * Get the deploymentManager. * @@ -220,8 +209,6 @@ public abstract class ScanningAppProvider extends AbstractLifeCycle implements A return _deploymentManager; } - - /* ------------------------------------------------------------ */ public Resource getMonitoredDirResource() { if (_monitored.size()==0) @@ -231,60 +218,51 @@ public abstract class ScanningAppProvider extends AbstractLifeCycle implements A return _monitored.get(0); } - /* ------------------------------------------------------------ */ public String getMonitoredDirName() { Resource resource=getMonitoredDirResource(); return resource==null?null:resource.toString(); } - /* ------------------------------------------------------------ */ @ManagedAttribute("scanning interval to detect changes which need reloaded") public int getScanInterval() { return _scanInterval; } - /* ------------------------------------------------------------ */ @ManagedAttribute("recursive scanning supported") public boolean isRecursive() { return _recursive; } - /* ------------------------------------------------------------ */ @Override public void setDeploymentManager(DeploymentManager deploymentManager) { _deploymentManager = deploymentManager; } - - /* ------------------------------------------------------------ */ + public void setMonitoredResources(List resources) { _monitored.clear(); _monitored.addAll(resources); } - - /* ------------------------------------------------------------ */ + public List getMonitoredResources() { return Collections.unmodifiableList(_monitored); } - - /* ------------------------------------------------------------ */ + public void setMonitoredDirResource(Resource resource) { setMonitoredResources(Collections.singletonList(resource)); } - /* ------------------------------------------------------------ */ public void addScannerListener(Scanner.Listener listener) { _scanner.addListener(listener); } - - /* ------------------------------------------------------------ */ + /** * @param dir * Directory to scan for context descriptors or war files @@ -294,7 +272,6 @@ public abstract class ScanningAppProvider extends AbstractLifeCycle implements A setMonitoredDirectories(Collections.singletonList(dir)); } - /* ------------------------------------------------------------ */ public void setMonitoredDirectories(Collection directories) { try @@ -309,16 +286,24 @@ public abstract class ScanningAppProvider extends AbstractLifeCycle implements A throw new IllegalArgumentException(e); } } - - /* ------------------------------------------------------------ */ + protected void setRecursive(boolean recursive) { _recursive = recursive; } - /* ------------------------------------------------------------ */ public void setScanInterval(int scanInterval) { _scanInterval = scanInterval; } + + @ManagedOperation(value = "Scan the monitored directories", impact = "ACTION") + public void scan() + { + LOG.info("Performing scan of monitored directories: {}", + getMonitoredResources().stream().map((r) -> r.getURI().toASCIIString()) + .collect(Collectors.joining(", ", "[", "]")) + ); + _scanner.scan(); + } } diff --git a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/WebAppProvider.java b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/WebAppProvider.java index 3586a8243ab..96812414e0e 100644 --- a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/WebAppProvider.java +++ b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/WebAppProvider.java @@ -124,7 +124,6 @@ public class WebAppProvider extends ScanningAppProvider } } - /* ------------------------------------------------------------ */ public WebAppProvider() { super(); @@ -132,7 +131,6 @@ public class WebAppProvider extends ScanningAppProvider setScanInterval(0); } - /* ------------------------------------------------------------ */ /** Get the extractWars. * @return the extractWars */ @@ -142,7 +140,6 @@ public class WebAppProvider extends ScanningAppProvider return _extractWars; } - /* ------------------------------------------------------------ */ /** Set the extractWars. * @param extractWars the extractWars to set */ @@ -151,7 +148,6 @@ public class WebAppProvider extends ScanningAppProvider _extractWars = extractWars; } - /* ------------------------------------------------------------ */ /** Get the parentLoaderPriority. * @return the parentLoaderPriority */ @@ -161,7 +157,6 @@ public class WebAppProvider extends ScanningAppProvider return _parentLoaderPriority; } - /* ------------------------------------------------------------ */ /** Set the parentLoaderPriority. * @param parentLoaderPriority the parentLoaderPriority to set */ @@ -169,8 +164,7 @@ public class WebAppProvider extends ScanningAppProvider { _parentLoaderPriority = parentLoaderPriority; } - - /* ------------------------------------------------------------ */ + /** Get the defaultsDescriptor. * @return the defaultsDescriptor */ @@ -180,7 +174,6 @@ public class WebAppProvider extends ScanningAppProvider return _defaultsDescriptor; } - /* ------------------------------------------------------------ */ /** Set the defaultsDescriptor. * @param defaultsDescriptor the defaultsDescriptor to set */ @@ -189,13 +182,11 @@ public class WebAppProvider extends ScanningAppProvider _defaultsDescriptor = defaultsDescriptor; } - /* ------------------------------------------------------------ */ public ConfigurationManager getConfigurationManager() { return _configurationManager; } - - /* ------------------------------------------------------------ */ + /** Set the configurationManager. * @param configurationManager the configurationManager to set */ @@ -203,8 +194,7 @@ public class WebAppProvider extends ScanningAppProvider { _configurationManager = configurationManager; } - - /* ------------------------------------------------------------ */ + /** * @param configurations The configuration class names. */ @@ -212,8 +202,7 @@ public class WebAppProvider extends ScanningAppProvider { _configurationClasses = configurations==null?null:(String[])configurations.clone(); } - - /* ------------------------------------------------------------ */ + @ManagedAttribute("configuration classes for webapps to be processed through") public String[] getConfigurationClasses() { @@ -231,8 +220,7 @@ public class WebAppProvider extends ScanningAppProvider { _tempDirectory = directory; } - - /* ------------------------------------------------------------ */ + /** * Get the user supplied Work Directory. * @@ -244,7 +232,6 @@ public class WebAppProvider extends ScanningAppProvider return _tempDirectory; } - /* ------------------------------------------------------------ */ protected void initializeWebAppContextDefaults(WebAppContext webapp) { if (_defaultsDescriptor != null) @@ -265,8 +252,7 @@ public class WebAppProvider extends ScanningAppProvider webapp.setAttribute(WebAppContext.BASETEMPDIR, _tempDirectory); } } - - /* ------------------------------------------------------------ */ + @Override public ContextHandler createContextHandler(final App app) throws Exception { @@ -349,8 +335,7 @@ public class WebAppProvider extends ScanningAppProvider return webAppContext; } - - /* ------------------------------------------------------------ */ + @Override protected void fileChanged(String filename) throws Exception { @@ -410,7 +395,6 @@ public class WebAppProvider extends ScanningAppProvider super.fileChanged(filename); } - /* ------------------------------------------------------------ */ @Override protected void fileAdded(String filename) throws Exception { @@ -433,7 +417,6 @@ public class WebAppProvider extends ScanningAppProvider return; } - //is the file that was added a .war file? String lowname = file.getName().toLowerCase(Locale.ENGLISH); if (lowname.endsWith(".war")) @@ -453,8 +436,6 @@ public class WebAppProvider extends ScanningAppProvider super.fileAdded(filename); } - - /* ------------------------------------------------------------ */ @Override protected void fileRemoved(String filename) throws Exception { diff --git a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/jmx/WebAppProviderMBean.java b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/jmx/WebAppProviderMBean.java new file mode 100644 index 00000000000..67156322260 --- /dev/null +++ b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/providers/jmx/WebAppProviderMBean.java @@ -0,0 +1,44 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.deploy.providers.jmx; + +import java.util.List; +import java.util.stream.Collectors; + +import org.eclipse.jetty.deploy.providers.WebAppProvider; +import org.eclipse.jetty.server.handler.jmx.AbstractHandlerMBean; +import org.eclipse.jetty.util.annotation.ManagedAttribute; +import org.eclipse.jetty.util.annotation.ManagedObject; + +@ManagedObject("WebAppProvider mbean wrapper") +public class WebAppProviderMBean extends AbstractHandlerMBean +{ + public WebAppProviderMBean(Object managedObject) + { + super(managedObject); + } + + @ManagedAttribute("List of monitored resources") + public List getMonitoredResources() + { + return ((WebAppProvider) _managed).getMonitoredResources().stream() + .map((r) -> r.getURI().toASCIIString()) + .collect(Collectors.toList()); + } +} diff --git a/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/JmxServiceConnection.java b/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/JmxServiceConnection.java index 98daaa738c4..d112ed0f834 100644 --- a/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/JmxServiceConnection.java +++ b/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/JmxServiceConnection.java @@ -20,7 +20,6 @@ package org.eclipse.jetty.deploy; import java.io.IOException; import java.lang.management.ManagementFactory; - import javax.management.MBeanServer; import javax.management.MBeanServerConnection; import javax.management.remote.JMXConnector; @@ -74,7 +73,6 @@ public class JmxServiceConnection return serviceUrl; } - /* ------------------------------------------------------------ */ /** * Retrieve a connection to MBean server * diff --git a/jetty-documentation/src/main/asciidoc/administration/startup/startup-base-vs-home.adoc b/jetty-documentation/src/main/asciidoc/administration/startup/startup-base-vs-home.adoc index 41ecada6ffc..6e3f8b6a959 100644 --- a/jetty-documentation/src/main/asciidoc/administration/startup/startup-base-vs-home.adoc +++ b/jetty-documentation/src/main/asciidoc/administration/startup/startup-base-vs-home.adoc @@ -20,6 +20,7 @@ === Managing Jetty Base and Jetty Home Instead of managing multiple Jetty implementations out of several different distribution locations, it is possible to maintain a separation between the binary installation of the standalone Jetty (known as `${jetty.home}`), and the customizations for your specific environment(s) (known as `${jetty.base}`). +In addition to easy management of multiple server instances, is allows for quick, drop-in upgrades of Jetty. There should always only be *one* Jetty Home (per version of Jetty), but there can be multiple Jetty Base directories that reference it. Jetty Base:: diff --git a/jetty-documentation/src/main/asciidoc/administration/startup/startup-windows-service.adoc b/jetty-documentation/src/main/asciidoc/administration/startup/startup-windows-service.adoc index 086b216a267..2bc40090b31 100644 --- a/jetty-documentation/src/main/asciidoc/administration/startup/startup-windows-service.adoc +++ b/jetty-documentation/src/main/asciidoc/administration/startup/startup-windows-service.adoc @@ -147,7 +147,7 @@ We'll start by specifying which modules we want to use (this will create a start [source, screen, subs="{sub-order}"] .... -C:\opt\myappbase>java -jar ..\jetty\start.jar --add-to-start=deploy,http,logging +C:\opt\myappbase>java -jar ..\jetty\start.jar --add-to-start=deploy,http,console-capture WARNING: deploy initialised in ${jetty.base}\start.ini (appended) WARNING: deploy enabled in ${jetty.base}\start.ini @@ -260,7 +260,7 @@ set PR_STOPPARAMS=--stop;STOP.KEY="%STOPKEY%";STOP.PORT=%STOPPORT%;STOP.WAIT=10 --JvmMs="%PR_JVMMS%" ^ --JvmMx="%PR_JVMMX%" ^ --JvmSs="%PR_JVMSS%" ^ - --JvmOptions="%PR_JVMOPTIONS%" ^ + --JvmOptions=%PR_JVMOPTIONS% ^ --Classpath="%PR_CLASSPATH%" ^ --StartMode="%PR_STARTMODE%" ^ --StartClass="%JETTY_START_CLASS%" ^ diff --git a/jetty-documentation/src/main/asciidoc/configuring/connectors/configuring-ssl.adoc b/jetty-documentation/src/main/asciidoc/configuring/connectors/configuring-ssl.adoc index eff950eb2ef..6abde51fe5f 100644 --- a/jetty-documentation/src/main/asciidoc/configuring/connectors/configuring-ssl.adoc +++ b/jetty-documentation/src/main/asciidoc/configuring/connectors/configuring-ssl.adoc @@ -739,6 +739,9 @@ sslContextFactory.setProvider("Conscrypt"); If you are using the Jetty Distribution, please see the section on enabling the link:#jetty-conscrypt-distribution[Conscrypt SSL module.] +If you are using Conscrypt with Java 8, you must exclude `TLSv1.3` protocol as it is now enabled per default with Conscrypt 2.0.0 but not supported by Java 8. + + ==== Configuring SNI From Java 8, the JVM contains support for the http://en.wikipedia.org/wiki/Server_Name_Indication[Server Name Indicator (SNI)] extension, which allows a SSL connection handshake to indicate one or more DNS names that it applies to. diff --git a/jetty-documentation/src/main/asciidoc/configuring/contexts/setting-form-size.adoc b/jetty-documentation/src/main/asciidoc/configuring/contexts/setting-form-size.adoc index 7a0ee0c1910..9ffc5f8ce3f 100644 --- a/jetty-documentation/src/main/asciidoc/configuring/contexts/setting-form-size.adoc +++ b/jetty-documentation/src/main/asciidoc/configuring/contexts/setting-form-size.adoc @@ -44,7 +44,7 @@ In either case the syntax of the XML file is the same: ==== For All Apps on a Server -Set an attribute on the Server instance for which you want to modify the maximum form content size: +Set an attribute in `jetty.xml` on the Server instance for which you want to modify the maximum form content size: [source, xml, subs="{sub-order}"] ---- @@ -56,7 +56,32 @@ Set an attribute on the Server instance for which you want to modify the maximum ---- +____ +[IMPORTANT] +It is important to remember that you should *not* modify the XML files in your `$JETTY_HOME`. +If you do for some reason feel you want to change the way an XML file operates, it is best to make a copy of it in your `$JETTY_BASE` in an `/etc` directory. +Jetty will always look first to the `$JETTY_BASE` for configuration. +____ + ==== For All Apps in the JVM Use the system property `org.eclipse.jetty.server.Request.maxFormContentSize`. -This can be set on the command line or in the `start.ini` or `start.d\server.ini` file. +This can be set on the command line or in the `$JETTY_BASE\start.ini` or any `$JETTY_BASE\start.d\*.ini` link:#startup-modules[module ini file.] +Using `$JETTY_BASE\start.d\server.ini` as an example: + +[source, console, subs="{sub-order}"] +---- +# --------------------------------------- +# Module: server +# Enables the core Jetty server on the classpath. +# --------------------------------------- +--module=server + +### Common HTTP configuration +## Scheme to use to build URIs for secure redirects +# jetty.httpConfig.secureScheme=https + +... + +-Dorg.eclipse.jetty.server.Request.maxFormContentSize=200000 +---- diff --git a/jetty-documentation/src/main/asciidoc/configuring/jsp/configuring-jsp.adoc b/jetty-documentation/src/main/asciidoc/configuring/jsp/configuring-jsp.adoc index 9689e565a8d..705a03ec1e8 100644 --- a/jetty-documentation/src/main/asciidoc/configuring/jsp/configuring-jsp.adoc +++ b/jetty-documentation/src/main/asciidoc/configuring/jsp/configuring-jsp.adoc @@ -171,7 +171,7 @@ The JSP engine has many configuration parameters. Some parameters affect only precompilation, and some affect runtime recompilation checking. Parameters also differ among the various versions of the JSP engine. This page lists the configuration parameters, their meanings, and their default settings. -Set all parameters on the `org.apache.jasper.JspServlet` instance defined in the link:#webdefault-xml[`webdefault.xml`] file. +Set all parameters on the `org.apache.jasper.servlet.JspServlet` instance defined in the link:#webdefault-xml[`webdefault.xml`] file. ____ [NOTE] diff --git a/jetty-documentation/src/main/asciidoc/configuring/security/jaas-support.adoc b/jetty-documentation/src/main/asciidoc/configuring/security/jaas-support.adoc index ce69083655b..37f8f7f95ca 100644 --- a/jetty-documentation/src/main/asciidoc/configuring/security/jaas-support.adoc +++ b/jetty-documentation/src/main/asciidoc/configuring/security/jaas-support.adoc @@ -52,13 +52,13 @@ Let's look at an example. ===== Step 1 -Configure a Jetty `org.eclipse.jetty.jaas.JAASLoginService` to match the `` in your `web.xml` file. For example, if the `web.xml` contains a realm called "xyz" like so: +Configure a Jetty `org.eclipse.jetty.jaas.JAASLoginService` to match the `` in your `web.xml` file. For example, if the `web.xml` contains a realm called "Test JAAS Realm" like so: [source, xml, subs="{sub-order}"] ---- FORM - xyz + Test JAAS Realm /login/login /login/error @@ -66,7 +66,7 @@ Configure a Jetty `org.eclipse.jetty.jaas.JAASLoginService` to match the ` ---- -Then you need to create a `JAASLoginService` with the matching name of "xyz": +then you need to create a `JAASLoginService` with the matching realm name of "Test JAAS Realm": [source, xml, subs="{sub-order}"] ---- @@ -76,9 +76,10 @@ Then you need to create a `JAASLoginService` with the matching name of "xyz": ---- +The `LoginModuleName` must match the name of your LoginModule as declared in your login module configuration file (see <>). ____ [CAUTION] -The name of the realm-name that you declare in `web.xml` must match exactly the name of your `JAASLoginService`. +The name of the realm-name that you declare in `web.xml` must match *exactly* the `Name` field of your `JAASLoginService`. ____ You can declare your `JAASLoginService` in a couple of different ways: @@ -135,7 +136,7 @@ xyz { ____ [CAUTION] -It is imperative that the application name on the first line is exactly the same as the `LoginModuleName` of your `JAASLoginService`. +It is imperative that the application name on the first line is *exactly* the same as the `LoginModuleName` of your `JAASLoginService`. ____ You may find it convenient to name this configuration file as `etc/login.conf` because, as we will see below, some of the wiring up for JAAS has been done for you. @@ -179,7 +180,6 @@ To allow the greatest degree of flexibility in using JAAS with web applications, Note that you don't ordinarily need to set these explicitly, as Jetty has defaults which will work in 99% of cases. However, should you need to, you can configure: -* a policy for role-based authorization (Default: `org.eclipse.jetty.jaas.StrictRoleCheckPolicy`) * a CallbackHandler (Default: `org.eclipse.jetty.jaas.callback.DefaultCallbackHandler`) * a list of classnames for the Principal implementation that equate to a user role (Default: `org.eclipse.jetty.jaas.JAASRole`) @@ -190,9 +190,6 @@ Here's an example of setting each of these (to their default values): Test JAAS Realm xyz - - - org.eclipse.jetty.jaas.callback.DefaultCallbackHandler @@ -204,16 +201,6 @@ Here's an example of setting each of these (to their default values): ---- -===== RoleCheckPolicy - -The `RoleCheckPolicy` must be an implementation of the `org.eclipse.jetty.jaas.RoleCheckPolicy` interface and its purpose is to help answer the question "is User X in Role Y" for role-based authorization requests. -The default implementation distributed with Jetty is the `org.eclipse.jetty.jaas.StrictRoleCheckPolicy`, which will assess a user as having a particular role if that role is at the top of the stack of roles that have been temporarily pushed onto the user. -If the user has no temporarily assigned roles, the role is amongst those configured for the user. - -Roles can be temporarily assigned to a user programmatically by using the `pushRole(String rolename)` method of the `org.eclipse.jetty.jaas.JAASUserPrincipal` class. - -For the majority of webapps, the default `StrictRoleCheckPolicy` will be quite adequate, however you may provide your own implementation and set it on your `JAASLoginService` instance. - ===== CallbackHandler A CallbackHandler is responsible for interfacing with the user to obtain usernames and credentials to be authenticated. diff --git a/jetty-home/pom.xml b/jetty-home/pom.xml index 6b80f2a91af..70e3d3deea9 100644 --- a/jetty-home/pom.xml +++ b/jetty-home/pom.xml @@ -651,7 +651,6 @@ org.jboss.logging jboss-logging - 3.3.0.Final org.eclipse.jetty.cdi diff --git a/jetty-home/src/main/resources/modules/conscrypt.mod b/jetty-home/src/main/resources/modules/conscrypt.mod index 66bcf8d06fc..9a886357be8 100644 --- a/jetty-home/src/main/resources/modules/conscrypt.mod +++ b/jetty-home/src/main/resources/modules/conscrypt.mod @@ -29,6 +29,6 @@ Conscrypt is distributed under the Apache Licence 2.0 https://github.com/google/conscrypt/blob/master/LICENSE [ini] -conscrypt.version?=1.1.4 +conscrypt.version?=2.0.0 jetty.sslContext.provider?=Conscrypt diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java b/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java index e56bab22639..6ca334aba29 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java @@ -18,7 +18,10 @@ package org.eclipse.jetty.http; +import java.nio.Buffer; import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.List; import java.util.zip.DataFormatException; import java.util.zip.Inflater; import java.util.zip.ZipException; @@ -28,13 +31,13 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.component.Destroyable; /** - * Decoder for the "gzip" encoding. - *

- * A decoder that inflates gzip compressed data that has been - * optimized for async usage with minimal data copies. + *

Decoder for the "gzip" content encoding.

+ *

This decoder inflates gzip compressed data, and has + * been optimized for async usage with minimal data copies.

*/ public class GZIPContentDecoder implements Destroyable { + private final List _inflateds = new ArrayList<>(); private final Inflater _inflater = new Inflater(true); private final ByteBufferPool _pool; private final int _bufferSize; @@ -46,14 +49,14 @@ public class GZIPContentDecoder implements Destroyable public GZIPContentDecoder() { - this(null,2048); + this(null, 2048); } public GZIPContentDecoder(int bufferSize) { - this(null,bufferSize); + this(null, bufferSize); } - + public GZIPContentDecoder(ByteBufferPool pool, int bufferSize) { _bufferSize = bufferSize; @@ -61,68 +64,95 @@ public class GZIPContentDecoder implements Destroyable reset(); } - /** Inflate compressed data from a buffer. - * - * @param compressed Buffer containing compressed data. - * @return Buffer containing inflated data. + /** + *

Inflates compressed data from a buffer.

+ *

The buffers returned by this method should be released + * via {@link #release(ByteBuffer)}.

+ *

This method may fully consume the input buffer, but return + * only a chunk of the inflated bytes, to allow applications to + * consume the inflated chunk before performing further inflation, + * applying backpressure. In this case, this method should be + * invoked again with the same input buffer (even if + * it's already fully consumed) and that will produce another + * chunk of inflated bytes. Termination happens when the input + * buffer is fully consumed, and the returned buffer is empty.

+ *

See {@link #decodedChunk(ByteBuffer)} to perform inflating + * in a non-blocking way that allows to apply backpressure.

+ * + * @param compressed the buffer containing compressed data. + * @return a buffer containing inflated data. */ public ByteBuffer decode(ByteBuffer compressed) { decodeChunks(compressed); - if (BufferUtil.isEmpty(_inflated) || _state==State.CRC || _state==State.ISIZE ) - return BufferUtil.EMPTY_BUFFER; - - ByteBuffer result = _inflated; - _inflated = null; - return result; - } - /** Called when a chunk of data is inflated. - *

The default implementation aggregates all the chunks - * into a single buffer returned from {@link #decode(ByteBuffer)}. - * Derived implementations may choose to consume chunks individually - * and return false to prevent further inflation until a subsequent - * call to {@link #decode(ByteBuffer)} or {@link #decodeChunks(ByteBuffer)}. - * - * @param chunk The inflated chunk of data - * @return False if inflating should continue, or True if the call - * to {@link #decodeChunks(ByteBuffer)} or {@link #decode(ByteBuffer)} - * should return, allowing back pressure of compressed data. - */ - protected boolean decodedChunk(ByteBuffer chunk) - { - if (_inflated==null) + if (_inflateds.isEmpty()) { - _inflated=chunk; + if (BufferUtil.isEmpty(_inflated) || _state == State.CRC || _state == State.ISIZE) + return BufferUtil.EMPTY_BUFFER; + ByteBuffer result = _inflated; + _inflated = null; + return result; } else { - int size = _inflated.remaining() + chunk.remaining(); - if (size<=_inflated.capacity()) + _inflateds.add(_inflated); + _inflated = null; + int length = _inflateds.stream().mapToInt(Buffer::remaining).sum(); + ByteBuffer result = acquire(length); + for (ByteBuffer buffer : _inflateds) { - BufferUtil.append(_inflated,chunk); + BufferUtil.append(result, buffer); + release(buffer); + } + _inflateds.clear(); + return result; + } + } + + /** + *

Called when a chunk of data is inflated.

+ *

The default implementation aggregates all the chunks + * into a single buffer returned from {@link #decode(ByteBuffer)}.

+ *

Derived implementations may choose to consume inflated chunks + * individually and return {@code true} from this method to prevent + * further inflation until a subsequent call to {@link #decode(ByteBuffer)} + * or {@link #decodeChunks(ByteBuffer)} is made. + * + * @param chunk the inflated chunk of data + * @return false if inflating should continue, or true if the call + * to {@link #decodeChunks(ByteBuffer)} or {@link #decode(ByteBuffer)} + * should return, allowing to consume the inflated chunk and apply + * backpressure + */ + protected boolean decodedChunk(ByteBuffer chunk) + { + if (_inflated == null) + { + _inflated = chunk; + } + else + { + if (BufferUtil.space(_inflated) >= chunk.remaining()) + { + BufferUtil.append(_inflated, chunk); release(chunk); } else { - ByteBuffer bigger=acquire(size); - int pos=BufferUtil.flipToFill(bigger); - BufferUtil.put(_inflated,bigger); - BufferUtil.put(chunk,bigger); - BufferUtil.flipToFlush(bigger,pos); - release(_inflated); - release(chunk); - _inflated = bigger; + _inflateds.add(_inflated); + _inflated = chunk; } } return false; } /** - * Inflate compressed data. - *

Inflation continues until the compressed block end is reached, there is no - * more compressed data or a call to {@link #decodedChunk(ByteBuffer)} returns true. - * @param compressed Buffer of compressed data to inflate + *

Inflates compressed data.

+ *

Inflation continues until the compressed block end is reached, there is no + * more compressed data or a call to {@link #decodedChunk(ByteBuffer)} returns true.

+ * + * @param compressed the buffer of compressed data to inflate */ protected void decodeChunks(ByteBuffer compressed) { @@ -164,24 +194,24 @@ public class GZIPContentDecoder implements Destroyable } break; } - + case DATA: { while (true) { - if (buffer==null) + if (buffer == null) buffer = acquire(_bufferSize); - + try { - int length = _inflater.inflate(buffer.array(),buffer.arrayOffset(),buffer.capacity()); + int length = _inflater.inflate(buffer.array(), buffer.arrayOffset(), buffer.capacity()); buffer.limit(length); } catch (DataFormatException x) { throw new ZipException(x.getMessage()); } - + if (buffer.hasRemaining()) { ByteBuffer chunk = buffer; @@ -195,7 +225,7 @@ public class GZIPContentDecoder implements Destroyable return; if (compressed.hasArray()) { - _inflater.setInput(compressed.array(),compressed.arrayOffset()+compressed.position(),compressed.remaining()); + _inflater.setInput(compressed.array(), compressed.arrayOffset() + compressed.position(), compressed.remaining()); compressed.position(compressed.limit()); } else @@ -204,7 +234,7 @@ public class GZIPContentDecoder implements Destroyable byte[] input = new byte[compressed.remaining()]; compressed.get(input); _inflater.setInput(input); - } + } } else if (_inflater.finished()) { @@ -218,14 +248,14 @@ public class GZIPContentDecoder implements Destroyable } continue; } - + default: break; } - + if (!compressed.hasRemaining()) break; - + byte currByte = compressed.get(); switch (_state) { @@ -354,7 +384,7 @@ public class GZIPContentDecoder implements Destroyable // TODO ByteBuffer result = output == null ? BufferUtil.EMPTY_BUFFER : ByteBuffer.wrap(output); reset(); - return ; + return; } break; } @@ -369,7 +399,7 @@ public class GZIPContentDecoder implements Destroyable } finally { - if (buffer!=null) + if (buffer != null) release(buffer); } } @@ -398,28 +428,28 @@ public class GZIPContentDecoder implements Destroyable { INITIAL, ID, CM, FLG, MTIME, XFL, OS, FLAGS, EXTRA_LENGTH, EXTRA, NAME, COMMENT, HCRC, DATA, CRC, ISIZE } - + /** - * @param capacity capacity capacity of the allocated ByteBuffer - * @return An indirect buffer of the configured buffersize either from the pool or freshly allocated. + * @param capacity capacity of the ByteBuffer to acquire + * @return a heap buffer of the configured capacity either from the pool or freshly allocated. */ public ByteBuffer acquire(int capacity) { - return _pool==null?BufferUtil.allocate(capacity):_pool.acquire(capacity,false); + return _pool == null ? BufferUtil.allocate(capacity) : _pool.acquire(capacity, false); } - + /** - * Release an allocated buffer. - *

This method will called {@link ByteBufferPool#release(ByteBuffer)} if a buffer pool has - * been configured. This method should be called once for all buffers returned from {@link #decode(ByteBuffer)} - * or passed to {@link #decodedChunk(ByteBuffer)}. - * @param buffer The buffer to release. + *

Releases an allocated buffer.

+ *

This method calls {@link ByteBufferPool#release(ByteBuffer)} if a buffer pool has + * been configured.

+ *

This method should be called once for all buffers returned from {@link #decode(ByteBuffer)} + * or passed to {@link #decodedChunk(ByteBuffer)}.

+ * + * @param buffer the buffer to release. */ public void release(ByteBuffer buffer) { - @SuppressWarnings("ReferenceEquality") - boolean isTheEmptyBuffer = (buffer==BufferUtil.EMPTY_BUFFER); - if (_pool!=null && !isTheEmptyBuffer) + if (_pool != null && !BufferUtil.isTheEmptyBuffer(buffer)) _pool.release(buffer); } } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/Http1FieldPreEncoder.java b/jetty-http/src/main/java/org/eclipse/jetty/http/Http1FieldPreEncoder.java index c94ebffdbf5..17da781202e 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/Http1FieldPreEncoder.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/Http1FieldPreEncoder.java @@ -19,10 +19,10 @@ package org.eclipse.jetty.http; -import static java.nio.charset.StandardCharsets.ISO_8859_1; - import java.util.Arrays; +import static java.nio.charset.StandardCharsets.ISO_8859_1; + /* ------------------------------------------------------------ */ /** @@ -60,7 +60,8 @@ public class Http1FieldPreEncoder implements HttpFieldPreEncoder byte[] v=value.getBytes(ISO_8859_1); byte[] bytes=Arrays.copyOf(n,n.length+2+v.length+2); bytes[n.length]=(byte)':'; - bytes[n.length]=(byte)' '; + bytes[n.length+1]=(byte)' '; + System.arraycopy(v, 0, bytes, n.length+2, v.length); bytes[bytes.length-2]=(byte)'\r'; bytes[bytes.length-1]=(byte)'\n'; diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java index 14c4335eb5a..395b87bde35 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java @@ -31,6 +31,7 @@ import java.util.Map; import java.util.NoSuchElementException; import java.util.Set; import java.util.StringTokenizer; +import java.util.function.ToIntFunction; import java.util.stream.Stream; import java.util.stream.StreamSupport; @@ -437,6 +438,19 @@ public class HttpFields implements Iterable * @param header The header */ public List getQualityCSV(HttpHeader header) + { + return getQualityCSV(header,null); + } + + /** + * Get multiple field values of the same name, split and + * sorted as a {@link QuotedQualityCSV} + * + * @param header The header + * @param secondaryOrdering Function to apply an ordering other than specified by quality + * @return List the values in quality order with the q param and OWS stripped + */ + public List getQualityCSV(HttpHeader header, ToIntFunction secondaryOrdering) { QuotedQualityCSV values = null; for (HttpField f : this) @@ -444,7 +458,7 @@ public class HttpFields implements Iterable if (f.getHeader()==header) { if (values==null) - values = new QuotedQualityCSV(); + values = new QuotedQualityCSV(secondaryOrdering); values.addValue(f.getValue()); } } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/PreEncodedHttpField.java b/jetty-http/src/main/java/org/eclipse/jetty/http/PreEncodedHttpField.java index 11ad88821c0..7b5c0c47d88 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/PreEncodedHttpField.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/PreEncodedHttpField.java @@ -21,7 +21,6 @@ package org.eclipse.jetty.http; import java.nio.ByteBuffer; import java.util.ArrayList; -import java.util.Arrays; import java.util.Iterator; import java.util.List; import java.util.ServiceLoader; @@ -73,7 +72,7 @@ public class PreEncodedHttpField extends HttpField else LOG.warn("multiple PreEncoders for "+e.getHttpVersion()); } - + // Always support HTTP1 if (__encoders[0]==null) __encoders[0] = new Http1FieldPreEncoder(); diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/QuotedQualityCSV.java b/jetty-http/src/main/java/org/eclipse/jetty/http/QuotedQualityCSV.java index 61cee88bfd5..d148d9e65e1 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/QuotedQualityCSV.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/QuotedQualityCSV.java @@ -18,66 +18,73 @@ package org.eclipse.jetty.http; -import static java.lang.Integer.MIN_VALUE; - import java.util.ArrayList; import java.util.Iterator; import java.util.List; -import java.util.function.Function; +import java.util.function.ToIntFunction; + +import org.eclipse.jetty.util.log.Log; + +import static java.lang.Integer.MIN_VALUE; /* ------------------------------------------------------------ */ + /** * Implements a quoted comma separated list of quality values * in accordance with RFC7230 and RFC7231. - * Values are returned sorted in quality order, with OWS and the + * Values are returned sorted in quality order, with OWS and the * quality parameters removed. + * * @see "https://tools.ietf.org/html/rfc7230#section-3.2.6" * @see "https://tools.ietf.org/html/rfc7230#section-7" * @see "https://tools.ietf.org/html/rfc7231#section-5.3.1" */ public class QuotedQualityCSV extends QuotedCSV implements Iterable -{ - private final static Double ZERO=new Double(0.0); - private final static Double ONE=new Double(1.0); - - +{ /** - * Function to apply a most specific MIME encoding secondary ordering + * Lambda to apply a most specific MIME encoding secondary ordering. + * + * @see "https://tools.ietf.org/html/rfc7231#section-5.3.2" */ - public static Function MOST_SPECIFIC = new Function() + public static ToIntFunction MOST_SPECIFIC_MIME_ORDERING = s -> { - @Override - public Integer apply(String s) - { - String[] elements = s.split("/"); - return 1000000*elements.length+1000*elements[0].length()+elements[elements.length-1].length(); - } + if ("*/*".equals(s)) + return 0; + if (s.endsWith("/*")) + return 1; + if (s.indexOf(';') < 0) + return 2; + return 3; }; - + private final List _quality = new ArrayList<>(); private boolean _sorted = false; - private final Function _secondaryOrdering; - + private final ToIntFunction _secondaryOrdering; + /* ------------------------------------------------------------ */ + /** * Sorts values with equal quality according to the length of the value String. */ public QuotedQualityCSV() { - this((s) -> 0); + this((ToIntFunction)null); } /* ------------------------------------------------------------ */ + /** * Sorts values with equal quality according to given order. + * * @param preferredOrder Array indicating the preferred order of known values */ public QuotedQualityCSV(String[] preferredOrder) { - this((s) -> { - for (int i=0;i + { + for (int i = 0; i < preferredOrder.length; ++i) if (preferredOrder[i].equals(s)) - return preferredOrder.length-i; + return preferredOrder.length - i; if ("*".equals(s)) return preferredOrder.length; @@ -87,51 +94,57 @@ public class QuotedQualityCSV extends QuotedCSV implements Iterable } /* ------------------------------------------------------------ */ + /** * Orders values with equal quality with the given function. + * * @param secondaryOrdering Function to apply an ordering other than specified by quality */ - public QuotedQualityCSV(Function secondaryOrdering) + public QuotedQualityCSV(ToIntFunction secondaryOrdering) { - this._secondaryOrdering = secondaryOrdering; + this._secondaryOrdering = secondaryOrdering == null ? s -> 0 : secondaryOrdering; } - + /* ------------------------------------------------------------ */ @Override protected void parsedValue(StringBuffer buffer) { super.parsedValue(buffer); - _quality.add(ONE); + + // Assume a quality of ONE + _quality.add(1.0D); } /* ------------------------------------------------------------ */ @Override protected void parsedParam(StringBuffer buffer, int valueLength, int paramName, int paramValue) { - if (paramName<0) + if (paramName < 0) { - if (buffer.charAt(buffer.length()-1)==';') - buffer.setLength(buffer.length()-1); + if (buffer.charAt(buffer.length() - 1) == ';') + buffer.setLength(buffer.length() - 1); } - else if (paramValue>=0 && - buffer.charAt(paramName)=='q' && paramValue>paramName && - buffer.length()>=paramName && buffer.charAt(paramName+1)=='=') + else if (paramValue >= 0 && + buffer.charAt(paramName) == 'q' && paramValue > paramName && + buffer.length() >= paramName && buffer.charAt(paramName + 1) == '=') { Double q; try { - q=(_keepQuotes && buffer.charAt(paramValue)=='"') - ?new Double(buffer.substring(paramValue+1,buffer.length()-1)) - :new Double(buffer.substring(paramValue)); + q = (_keepQuotes && buffer.charAt(paramValue) == '"') + ? Double.valueOf(buffer.substring(paramValue + 1, buffer.length() - 1)) + : Double.valueOf(buffer.substring(paramValue)); } - catch(Exception e) + catch (Exception e) { - q=ZERO; - } - buffer.setLength(Math.max(0,paramName-1)); - - if (!ONE.equals(q)) - _quality.set(_quality.size()-1,q); + Log.getLogger(QuotedQualityCSV.class).ignore(e); + q = 0.0D; + } + buffer.setLength(Math.max(0, paramName - 1)); + + if (q != 1.0D) + // replace assumed quality + _quality.set(_quality.size() - 1, q); } } @@ -142,7 +155,7 @@ public class QuotedQualityCSV extends QuotedCSV implements Iterable sort(); return _values; } - + @Override public Iterator iterator() { @@ -153,35 +166,35 @@ public class QuotedQualityCSV extends QuotedCSV implements Iterable protected void sort() { - _sorted=true; + _sorted = true; - Double last = ZERO; + Double last = 0.0D; int lastSecondaryOrder = Integer.MIN_VALUE; - for (int i = _values.size(); i-- > 0;) + for (int i = _values.size(); i-- > 0; ) { String v = _values.get(i); Double q = _quality.get(i); - int compare=last.compareTo(q); - if (compare>0 || (compare==0 && _secondaryOrdering.apply(v) 0 || (compare == 0 && _secondaryOrdering.applyAsInt(v) < lastSecondaryOrder)) { _values.set(i, _values.get(i + 1)); _values.set(i + 1, v); _quality.set(i, _quality.get(i + 1)); _quality.set(i + 1, q); - last = ZERO; - lastSecondaryOrder=0; + last = 0.0D; + lastSecondaryOrder = 0; i = _values.size(); continue; } - last=q; - lastSecondaryOrder=_secondaryOrdering.apply(v); + last = q; + lastSecondaryOrder = _secondaryOrdering.applyAsInt(v); } - - int last_element=_quality.size(); - while(last_element>0 && _quality.get(--last_element).equals(ZERO)) + + int last_element = _quality.size(); + while (last_element > 0 && _quality.get(--last_element).equals(0.0D)) { _quality.remove(last_element); _values.remove(last_element); diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java index b5dea752126..46b22aa36b3 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java @@ -18,10 +18,6 @@ package org.eclipse.jetty.http; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.nio.ByteBuffer; @@ -35,52 +31,55 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; public class GZIPContentDecoderTest { - ArrayByteBufferPool pool; - AtomicInteger buffers = new AtomicInteger(0); - + private ArrayByteBufferPool pool; + private AtomicInteger buffers = new AtomicInteger(0); + @BeforeEach - public void beforeClass() throws Exception + public void before() { buffers.set(0); pool = new ArrayByteBufferPool() + { + + @Override + public ByteBuffer acquire(int size, boolean direct) { + buffers.incrementAndGet(); + return super.acquire(size, direct); + } - @Override - public ByteBuffer acquire(int size, boolean direct) - { - buffers.incrementAndGet(); - return super.acquire(size,direct); - } + @Override + public void release(ByteBuffer buffer) + { + buffers.decrementAndGet(); + super.release(buffer); + } - @Override - public void release(ByteBuffer buffer) - { - buffers.decrementAndGet(); - super.release(buffer); - } - - }; + }; } - + @AfterEach - public void afterClass() throws Exception + public void after() { - assertEquals(0,buffers.get()); + assertEquals(0, buffers.get()); } - + @Test - public void testCompresedContentFormat() throws Exception + public void testCompressedContentFormat() { - assertTrue(CompressedContentFormat.tagEquals("tag","tag")); - assertTrue(CompressedContentFormat.tagEquals("\"tag\"","\"tag\"")); - assertTrue(CompressedContentFormat.tagEquals("\"tag\"","\"tag--gzip\"")); - assertFalse(CompressedContentFormat.tagEquals("Zag","Xag--gzip")); - assertFalse(CompressedContentFormat.tagEquals("xtag","tag")); + assertTrue(CompressedContentFormat.tagEquals("tag", "tag")); + assertTrue(CompressedContentFormat.tagEquals("\"tag\"", "\"tag\"")); + assertTrue(CompressedContentFormat.tagEquals("\"tag\"", "\"tag--gzip\"")); + assertFalse(CompressedContentFormat.tagEquals("Zag", "Xag--gzip")); + assertFalse(CompressedContentFormat.tagEquals("xtag", "tag")); } - + @Test public void testStreamNoBlocks() throws Exception { @@ -122,7 +121,7 @@ public class GZIPContentDecoderTest output.close(); byte[] bytes = baos.toByteArray(); - GZIPContentDecoder decoder = new GZIPContentDecoder(pool,2048); + GZIPContentDecoder decoder = new GZIPContentDecoder(pool, 2048); ByteBuffer decoded = decoder.decode(ByteBuffer.wrap(bytes)); assertEquals(0, decoded.remaining()); } @@ -138,7 +137,7 @@ public class GZIPContentDecoderTest output.close(); byte[] bytes = baos.toByteArray(); - GZIPContentDecoder decoder = new GZIPContentDecoder(pool,2048); + GZIPContentDecoder decoder = new GZIPContentDecoder(pool, 2048); ByteBuffer decoded = decoder.decode(ByteBuffer.wrap(bytes)); assertEquals(data, StandardCharsets.UTF_8.decode(decoded).toString()); decoder.release(decoded); @@ -161,7 +160,7 @@ public class GZIPContentDecoderTest byte[] bytes2 = new byte[bytes.length - bytes1.length]; System.arraycopy(bytes, bytes1.length, bytes2, 0, bytes2.length); - GZIPContentDecoder decoder = new GZIPContentDecoder(pool,2048); + GZIPContentDecoder decoder = new GZIPContentDecoder(pool, 2048); ByteBuffer decoded = decoder.decode(ByteBuffer.wrap(bytes1)); assertEquals(0, decoded.capacity()); decoded = decoder.decode(ByteBuffer.wrap(bytes2)); @@ -186,7 +185,7 @@ public class GZIPContentDecoderTest byte[] bytes2 = new byte[bytes.length - bytes1.length]; System.arraycopy(bytes, bytes1.length, bytes2, 0, bytes2.length); - GZIPContentDecoder decoder = new GZIPContentDecoder(pool,2048); + GZIPContentDecoder decoder = new GZIPContentDecoder(pool, 2048); ByteBuffer decoded = decoder.decode(ByteBuffer.wrap(bytes1)); assertEquals(data, StandardCharsets.UTF_8.decode(decoded).toString()); assertFalse(decoder.isFinished()); @@ -214,7 +213,7 @@ public class GZIPContentDecoderTest byte[] bytes2 = new byte[bytes.length - bytes1.length]; System.arraycopy(bytes, bytes1.length, bytes2, 0, bytes2.length); - GZIPContentDecoder decoder = new GZIPContentDecoder(pool,2048); + GZIPContentDecoder decoder = new GZIPContentDecoder(pool, 2048); ByteBuffer decoded = decoder.decode(ByteBuffer.wrap(bytes1)); assertEquals(0, decoded.capacity()); decoder.release(decoded); @@ -244,7 +243,7 @@ public class GZIPContentDecoderTest System.arraycopy(bytes1, 0, bytes, 0, bytes1.length); System.arraycopy(bytes2, 0, bytes, bytes1.length, bytes2.length); - GZIPContentDecoder decoder = new GZIPContentDecoder(pool,2048); + GZIPContentDecoder decoder = new GZIPContentDecoder(pool, 2048); ByteBuffer buffer = ByteBuffer.wrap(bytes); ByteBuffer decoded = decoder.decode(buffer); assertEquals(data1, StandardCharsets.UTF_8.decode(decoded).toString()); @@ -271,7 +270,7 @@ public class GZIPContentDecoderTest byte[] bytes = baos.toByteArray(); String result = ""; - GZIPContentDecoder decoder = new GZIPContentDecoder(pool,2048); + GZIPContentDecoder decoder = new GZIPContentDecoder(pool, 2048); ByteBuffer buffer = ByteBuffer.wrap(bytes); while (buffer.hasRemaining()) { diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java index 3ec1b84c454..258eb766df8 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java @@ -18,15 +18,6 @@ package org.eclipse.jetty.http; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.is; -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 java.nio.ByteBuffer; import java.util.Collections; import java.util.Enumeration; @@ -40,6 +31,15 @@ import org.eclipse.jetty.util.BufferUtil; import org.hamcrest.Matchers; 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.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; + public class HttpFieldsTest { @Test @@ -299,6 +299,46 @@ public class HttpFieldsTest assertEquals(false, e.hasMoreElements()); } + @Test + public void testPreEncodedField() + { + ByteBuffer buffer = BufferUtil.allocate(1024); + + PreEncodedHttpField known = new PreEncodedHttpField(HttpHeader.CONNECTION, HttpHeaderValue.CLOSE.asString()); + BufferUtil.clearToFill(buffer); + known.putTo(buffer,HttpVersion.HTTP_1_1); + BufferUtil.flipToFlush(buffer,0); + assertThat(BufferUtil.toString(buffer),is("Connection: close\r\n")); + + PreEncodedHttpField unknown = new PreEncodedHttpField(null, "Header", "Value"); + BufferUtil.clearToFill(buffer); + unknown.putTo(buffer,HttpVersion.HTTP_1_1); + BufferUtil.flipToFlush(buffer,0); + assertThat(BufferUtil.toString(buffer),is("Header: Value\r\n")); + } + + @Test + public void testAddPreEncodedField() + { + final PreEncodedHttpField X_XSS_PROTECTION_FIELD = new PreEncodedHttpField("X-XSS-Protection", "1; mode=block"); + + HttpFields fields = new HttpFields(); + fields.add(X_XSS_PROTECTION_FIELD); + + assertThat("Fields output", fields.toString(), containsString("X-XSS-Protection: 1; mode=block")); + } + + @Test + public void testAddFinalHttpField() + { + final HttpField X_XSS_PROTECTION_FIELD = new HttpField("X-XSS-Protection", "1; mode=block"); + + HttpFields fields = new HttpFields(); + fields.add(X_XSS_PROTECTION_FIELD); + + assertThat("Fields output", fields.toString(), containsString("X-XSS-Protection: 1; mode=block")); + } + @Test public void testGetValues() throws Exception { diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/QuotedQualityCSVTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/QuotedQualityCSVTest.java index ba0db4a8972..f03657ba3e5 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/QuotedQualityCSVTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/QuotedQualityCSVTest.java @@ -61,7 +61,7 @@ public class QuotedQualityCSVTest @Test public void test7231_5_3_2_example3_most_specific() { - QuotedQualityCSV values = new QuotedQualityCSV(QuotedQualityCSV.MOST_SPECIFIC); + QuotedQualityCSV values = new QuotedQualityCSV(QuotedQualityCSV.MOST_SPECIFIC_MIME_ORDERING); values.addValue("text/*, text/plain, text/plain;format=flowed, */*"); assertThat(values,Matchers.contains("text/plain;format=flowed","text/plain","text/*","*/*")); diff --git a/jetty-infinispan/pom.xml b/jetty-infinispan/pom.xml index 34b54138414..eddf3d5ac8a 100644 --- a/jetty-infinispan/pom.xml +++ b/jetty-infinispan/pom.xml @@ -10,7 +10,6 @@ http://www.eclipse.org/jetty ${project.groupId}.infinispan - 9.1.0.Final install @@ -36,30 +35,34 @@ - org.infinispan - infinispan-core - ${infinispan.version} + org.infinispan + infinispan-core + ${infinispan.version} - org.infinispan.protostream - protostream - 4.1.0.Final + org.infinispan.protostream + protostream + 4.2.2.Final - org.eclipse.jetty - jetty-server - ${project.version} + org.eclipse.jetty + jetty-server + ${project.version} + + + org.jboss.logging + jboss-logging org.infinispan infinispan-client-hotrod - 9.1.0.Final + ${infinispan.version} provided org.infinispan infinispan-remote-query-client - 9.1.0.Final + ${infinispan.version} provided diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractByteBufferPool.java b/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractByteBufferPool.java new file mode 100644 index 00000000000..0b599e47596 --- /dev/null +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractByteBufferPool.java @@ -0,0 +1,108 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.io; + +import java.nio.ByteBuffer; +import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Consumer; + +import org.eclipse.jetty.util.annotation.ManagedAttribute; +import org.eclipse.jetty.util.annotation.ManagedObject; +import org.eclipse.jetty.util.annotation.ManagedOperation; + +@ManagedObject +abstract class AbstractByteBufferPool implements ByteBufferPool +{ + private final int _factor; + private final int _maxQueueLength; + private final long _maxHeapMemory; + private final AtomicLong _heapMemory = new AtomicLong(); + private final long _maxDirectMemory; + private final AtomicLong _directMemory = new AtomicLong(); + + protected AbstractByteBufferPool(int factor, int maxQueueLength, long maxHeapMemory, long maxDirectMemory) + { + _factor = factor <= 0 ? 1024 : factor; + _maxQueueLength = maxQueueLength; + _maxHeapMemory = maxHeapMemory; + _maxDirectMemory = maxDirectMemory; + } + + protected int getCapacityFactor() + { + return _factor; + } + + protected int getMaxQueueLength() + { + return _maxQueueLength; + } + + protected void decrementMemory(ByteBuffer buffer) + { + updateMemory(buffer, false); + } + + protected void incrementMemory(ByteBuffer buffer) + { + updateMemory(buffer, true); + } + + private void updateMemory(ByteBuffer buffer, boolean addOrSub) + { + AtomicLong memory = buffer.isDirect() ? _directMemory : _heapMemory; + int capacity = buffer.capacity(); + memory.addAndGet(addOrSub ? capacity : -capacity); + } + + protected void releaseExcessMemory(boolean direct, Consumer clearFn) + { + long maxMemory = direct ? _maxDirectMemory : _maxHeapMemory; + if (maxMemory > 0) + { + while (getMemory(direct) > maxMemory) + clearFn.accept(direct); + } + } + + @ManagedAttribute("The bytes retained by direct ByteBuffers") + public long getDirectMemory() + { + return getMemory(true); + } + + @ManagedAttribute("The bytes retained by heap ByteBuffers") + public long getHeapMemory() + { + return getMemory(false); + } + + public long getMemory(boolean direct) + { + AtomicLong memory = direct ? _directMemory : _heapMemory; + return memory.get(); + } + + @ManagedOperation(value = "Clears this ByteBufferPool", impact = "ACTION") + public void clear() + { + _heapMemory.set(0); + _directMemory.set(0); + } +} diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java index f2fc9cf6b7e..aec162a9ff3 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java @@ -19,96 +19,205 @@ package org.eclipse.jetty.io; import java.nio.ByteBuffer; +import java.util.Arrays; +import java.util.Objects; +import java.util.function.IntFunction; -public class ArrayByteBufferPool implements ByteBufferPool +import org.eclipse.jetty.util.annotation.ManagedAttribute; +import org.eclipse.jetty.util.annotation.ManagedObject; + +/** + *

A ByteBuffer pool where ByteBuffers are held in queues that are held in array elements.

+ *

Given a capacity {@code factor} of 1024, the first array element holds a queue of ByteBuffers + * each of capacity 1024, the second array element holds a queue of ByteBuffers each of capacity + * 2048, and so on.

+ */ +@ManagedObject +public class ArrayByteBufferPool extends AbstractByteBufferPool { - private final int _min; - private final int _maxQueue; + private final int _minCapacity; private final ByteBufferPool.Bucket[] _direct; private final ByteBufferPool.Bucket[] _indirect; - private final int _inc; + /** + * Creates a new ArrayByteBufferPool with a default configuration. + */ public ArrayByteBufferPool() { - this(-1,-1,-1,-1); + this(-1, -1, -1); } - public ArrayByteBufferPool(int minSize, int increment, int maxSize) + /** + * Creates a new ArrayByteBufferPool with the given configuration. + * + * @param minCapacity the minimum ByteBuffer capacity + * @param factor the capacity factor + * @param maxCapacity the maximum ByteBuffer capacity + */ + public ArrayByteBufferPool(int minCapacity, int factor, int maxCapacity) { - this(minSize,increment,maxSize,-1); + this(minCapacity, factor, maxCapacity, -1, -1, -1); } - - public ArrayByteBufferPool(int minSize, int increment, int maxSize, int maxQueue) + + /** + * Creates a new ArrayByteBufferPool with the given configuration. + * + * @param minCapacity the minimum ByteBuffer capacity + * @param factor the capacity factor + * @param maxCapacity the maximum ByteBuffer capacity + * @param maxQueueLength the maximum ByteBuffer queue length + */ + public ArrayByteBufferPool(int minCapacity, int factor, int maxCapacity, int maxQueueLength) { - if (minSize<=0) - minSize=0; - if (increment<=0) - increment=1024; - if (maxSize<=0) - maxSize=64*1024; - if (minSize>=increment) - throw new IllegalArgumentException("minSize >= increment"); - if ((maxSize%increment)!=0 || increment>=maxSize) - throw new IllegalArgumentException("increment must be a divisor of maxSize"); - _min=minSize; - _inc=increment; + this(minCapacity, factor, maxCapacity, maxQueueLength, -1, -1); + } - _direct=new ByteBufferPool.Bucket[maxSize/increment]; - _indirect=new ByteBufferPool.Bucket[maxSize/increment]; - _maxQueue=maxQueue; + /** + * Creates a new ArrayByteBufferPool with the given configuration. + * + * @param minCapacity the minimum ByteBuffer capacity + * @param factor the capacity factor + * @param maxCapacity the maximum ByteBuffer capacity + * @param maxQueueLength the maximum ByteBuffer queue length + * @param maxHeapMemory the max heap memory in bytes + * @param maxDirectMemory the max direct memory in bytes + */ + public ArrayByteBufferPool(int minCapacity, int factor, int maxCapacity, int maxQueueLength, long maxHeapMemory, long maxDirectMemory) + { + super(factor, maxQueueLength, maxHeapMemory, maxDirectMemory); - int size=0; - for (int i=0;i<_direct.length;i++) - { - size+=_inc; - _direct[i]=new ByteBufferPool.Bucket(this,size,_maxQueue); - _indirect[i]=new ByteBufferPool.Bucket(this,size,_maxQueue); - } + factor = getCapacityFactor(); + if (minCapacity <= 0) + minCapacity = 0; + if (maxCapacity <= 0) + maxCapacity = 64 * 1024; + if ((maxCapacity % factor) != 0 || factor >= maxCapacity) + throw new IllegalArgumentException("The capacity factor must be a divisor of maxCapacity"); + _minCapacity = minCapacity; + + int length = maxCapacity / factor; + _direct = new ByteBufferPool.Bucket[length]; + _indirect = new ByteBufferPool.Bucket[length]; } @Override public ByteBuffer acquire(int size, boolean direct) { - ByteBufferPool.Bucket bucket = bucketFor(size,direct); - if (bucket==null) - return newByteBuffer(size,direct); - - return bucket.acquire(direct); - + int capacity = size < _minCapacity ? size : (bucketFor(size) + 1) * getCapacityFactor(); + ByteBufferPool.Bucket bucket = bucketFor(size, direct, null); + if (bucket == null) + return newByteBuffer(capacity, direct); + ByteBuffer buffer = bucket.acquire(); + if (buffer == null) + return newByteBuffer(capacity, direct); + decrementMemory(buffer); + return buffer; } @Override public void release(ByteBuffer buffer) { - if (buffer!=null) - { - ByteBufferPool.Bucket bucket = bucketFor(buffer.capacity(),buffer.isDirect()); - if (bucket!=null) - bucket.release(buffer); + if (buffer == null) + return; + boolean direct = buffer.isDirect(); + ByteBufferPool.Bucket bucket = bucketFor(buffer.capacity(), direct, this::newBucket); + if (bucket != null) + { + bucket.release(buffer); + incrementMemory(buffer); + releaseExcessMemory(direct, this::clearOldestBucket); } } + private Bucket newBucket(int key) + { + return new Bucket(this, key * getCapacityFactor(), getMaxQueueLength()); + } + + @Override public void clear() { - for (int i=0;i<_direct.length;i++) + super.clear(); + for (int i = 0; i < _direct.length; ++i) { - _direct[i].clear(); - _indirect[i].clear(); + Bucket bucket = _direct[i]; + if (bucket != null) + bucket.clear(); + _direct[i] = null; + bucket = _indirect[i]; + if (bucket != null) + bucket.clear(); + _indirect[i] = null; } } - private ByteBufferPool.Bucket bucketFor(int size,boolean direct) + private void clearOldestBucket(boolean direct) { - if (size<=_min) + long oldest = Long.MAX_VALUE; + int index = -1; + Bucket[] buckets = bucketsFor(direct); + for (int i = 0; i < buckets.length; ++i) + { + Bucket bucket = buckets[i]; + if (bucket == null) + continue; + long lastUpdate = bucket.getLastUpdate(); + if (lastUpdate < oldest) + { + oldest = lastUpdate; + index = i; + } + } + if (index >= 0) + { + Bucket bucket = buckets[index]; + buckets[index] = null; + // The same bucket may be concurrently + // removed, so we need this null guard. + if (bucket != null) + bucket.clear(this::decrementMemory); + } + } + + private int bucketFor(int capacity) + { + return (capacity - 1) / getCapacityFactor(); + } + + private ByteBufferPool.Bucket bucketFor(int capacity, boolean direct, IntFunction newBucket) + { + if (capacity < _minCapacity) return null; - int b=(size-1)/_inc; - if (b>=_direct.length) + int b = bucketFor(capacity); + if (b >= _direct.length) return null; - ByteBufferPool.Bucket bucket = direct?_direct[b]:_indirect[b]; - + Bucket[] buckets = bucketsFor(direct); + Bucket bucket = buckets[b]; + if (bucket == null && newBucket != null) + buckets[b] = bucket = newBucket.apply(b + 1); return bucket; } + @ManagedAttribute("The number of pooled direct ByteBuffers") + public long getDirectByteBufferCount() + { + return getByteBufferCount(true); + } + + @ManagedAttribute("The number of pooled heap ByteBuffers") + public long getHeapByteBufferCount() + { + return getByteBufferCount(false); + } + + private long getByteBufferCount(boolean direct) + { + return Arrays.stream(bucketsFor(direct)) + .filter(Objects::nonNull) + .mapToLong(Bucket::size) + .sum(); + } + // Package local for testing ByteBufferPool.Bucket[] bucketsFor(boolean direct) { diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java index 541953edb31..604b8188692 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java @@ -24,6 +24,7 @@ import java.util.Deque; import java.util.List; import java.util.concurrent.ConcurrentLinkedDeque; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; import org.eclipse.jetty.util.BufferUtil; @@ -56,6 +57,13 @@ public interface ByteBufferPool */ public void release(ByteBuffer buffer); + /** + *

Creates a new ByteBuffer of the given capacity and the given directness.

+ * + * @param capacity the ByteBuffer capacity + * @param direct the ByteBuffer directness + * @return a newly allocated ByteBuffer + */ default ByteBuffer newByteBuffer(int capacity, boolean direct) { return direct ? BufferUtil.allocateDirect(capacity) : BufferUtil.allocate(capacity); @@ -124,54 +132,80 @@ public interface ByteBufferPool } } - class Bucket + public static class Bucket { private final Deque _queue = new ConcurrentLinkedDeque<>(); private final ByteBufferPool _pool; private final int _capacity; - private final AtomicInteger _space; + private final int _maxSize; + private final AtomicInteger _size; + private long _lastUpdate = System.nanoTime(); - public Bucket(ByteBufferPool pool, int bufferSize, int maxSize) + public Bucket(ByteBufferPool pool, int capacity, int maxSize) { _pool = pool; - _capacity = bufferSize; - _space = maxSize > 0 ? new AtomicInteger(maxSize) : null; + _capacity = capacity; + _maxSize = maxSize; + _size = maxSize > 0 ? new AtomicInteger() : null; } + public ByteBuffer acquire() + { + ByteBuffer buffer = queuePoll(); + if (buffer == null) + return null; + if (_size != null) + _size.decrementAndGet(); + return buffer; + } + + /** + * @param direct whether to create a direct buffer when none is available + * @return a ByteBuffer + * @deprecated use {@link #acquire()} instead + */ + @Deprecated public ByteBuffer acquire(boolean direct) { ByteBuffer buffer = queuePoll(); if (buffer == null) return _pool.newByteBuffer(_capacity, direct); - if (_space != null) - _space.incrementAndGet(); + if (_size != null) + _size.decrementAndGet(); return buffer; } public void release(ByteBuffer buffer) { + _lastUpdate = System.nanoTime(); BufferUtil.clear(buffer); - if (_space == null) + if (_size == null) queueOffer(buffer); - else if (_space.decrementAndGet() >= 0) + else if (_size.incrementAndGet() <= _maxSize) queueOffer(buffer); else - _space.incrementAndGet(); + _size.decrementAndGet(); } public void clear() { - if (_space == null) + clear(null); + } + + void clear(Consumer memoryFn) + { + int size = _size == null ? 0 : _size.get() - 1; + while (size >= 0) { - queueClear(); - } - else - { - int s = _space.getAndSet(0); - while (s-- > 0) + ByteBuffer buffer = queuePoll(); + if (buffer == null) + break; + if (memoryFn != null) + memoryFn.accept(buffer); + if (_size != null) { - if (queuePoll() == null) - _space.incrementAndGet(); + _size.decrementAndGet(); + --size; } } } @@ -186,11 +220,6 @@ public interface ByteBufferPool return _queue.poll(); } - private void queueClear() - { - _queue.clear(); - } - boolean isEmpty() { return _queue.isEmpty(); @@ -201,10 +230,15 @@ public interface ByteBufferPool return _queue.size(); } + long getLastUpdate() + { + return _lastUpdate; + } + @Override public String toString() { - return String.format("Bucket@%x{%d/%d}", hashCode(), size(), _capacity); + return String.format("%s@%x{%d/%d@%d}", getClass().getSimpleName(), hashCode(), size(), _maxSize, _capacity); } } } diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/CyclicTimeout.java b/jetty-io/src/main/java/org/eclipse/jetty/io/CyclicTimeout.java index 4b26c3d184c..c9f05f95591 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/CyclicTimeout.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/CyclicTimeout.java @@ -33,6 +33,20 @@ import static java.lang.Long.MAX_VALUE; *

Subclasses should implement {@link #onTimeoutExpired()}.

*

This implementation is optimised assuming that the timeout * will mostly be cancelled and then reused with a similar value.

+ *

This implementation has a {@link Timeout} holding the time + * at which the scheduled task should fire, and a linked list of + * {@link Wakeup}, each holding the actual scheduled task.

+ *

Calling {@link #schedule(long, TimeUnit)} the first time will + * create a Timeout with an associated Wakeup and submit a task to + * the scheduler. + * Calling {@link #schedule(long, TimeUnit)} again with the same or + * a larger delay will cancel the previous Timeout, but keep the + * previous Wakeup without submitting a new task to the scheduler, + * therefore reducing the pressure on the scheduler and avoid it + * becomes a bottleneck. + * When the Wakeup task fires, it will see that the Timeout is now + * in the future and will attach a new Wakeup with the future time + * to the Timeout, and submit a scheduler task for the new Wakeup.

*/ public abstract class CyclicTimeout implements Destroyable { @@ -59,24 +73,24 @@ public abstract class CyclicTimeout implements Destroyable } /** - * Schedules a timeout, even if already set, cancelled or expired. + *

Schedules a timeout, even if already set, cancelled or expired.

+ *

If a timeout is already set, it will be cancelled and replaced + * by the new one.

* * @param delay The period of time before the timeout expires. * @param units The unit of time of the period. - * @return true if the timer was already set. + * @return true if the timeout was already set. */ public boolean schedule(long delay, TimeUnit units) { long now = System.nanoTime(); long new_timeout_at = now + units.toNanos(delay); + Wakeup new_wakeup = null; boolean result; - Wakeup new_wakeup; while (true) { Timeout timeout = _timeout.get(); - - new_wakeup = null; result = timeout._at != MAX_VALUE; // Is the current wakeup good to use? ie before our timeout time? @@ -114,14 +128,12 @@ public abstract class CyclicTimeout implements Destroyable public boolean cancel() { boolean result; - Timeout timeout; - Timeout new_timeout; while (true) { - timeout = _timeout.get(); + Timeout timeout = _timeout.get(); result = timeout._at != MAX_VALUE; Wakeup wakeup = timeout._wakeup; - new_timeout = wakeup == null ? NOT_SET : new Timeout(MAX_VALUE, wakeup); + Timeout new_timeout = wakeup == null ? NOT_SET : new Timeout(MAX_VALUE, wakeup); if (_timeout.compareAndSet(timeout, new_timeout)) break; } @@ -166,7 +178,11 @@ public abstract class CyclicTimeout implements Destroyable @Override public String toString() { - return String.format("%s@%x:%d,%s", getClass().getSimpleName(), hashCode(), _at, _wakeup); + return String.format("%s@%x:%dms,%s", + getClass().getSimpleName(), + hashCode(), + TimeUnit.NANOSECONDS.toMillis(_at - System.nanoTime()), + _wakeup); } } @@ -200,10 +216,9 @@ public abstract class CyclicTimeout implements Destroyable @Override public void run() { - long now; - Wakeup new_wakeup; - boolean has_expired; - + long now = System.nanoTime(); + Wakeup new_wakeup = null; + boolean has_expired = false; while (true) { Timeout timeout = _timeout.get(); @@ -226,16 +241,12 @@ public abstract class CyclicTimeout implements Destroyable // Not found, we become a noop. return; - now = System.nanoTime(); - new_wakeup = null; - has_expired = false; - Timeout new_timeout; - // We are in the wakeup list! So we have to act and we know our // tail has not expired (else it would have removed us from the list). // Remove ourselves (and any prior Wakeup) from the wakeup list. wakeup = wakeup._next; + Timeout new_timeout; if (timeout._at <= now) { // We have timed out! @@ -274,7 +285,11 @@ public abstract class CyclicTimeout implements Destroyable @Override public String toString() { - return String.format("%s@%x:%d->%s", getClass().getSimpleName(), hashCode(), _at, _next); + return String.format("%s@%x:%dms->%s", + getClass().getSimpleName(), + hashCode(), + _at == MAX_VALUE ? _at : TimeUnit.NANOSECONDS.toMillis(_at - System.nanoTime()), + _next); } } } diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/MappedByteBufferPool.java b/jetty-io/src/main/java/org/eclipse/jetty/io/MappedByteBufferPool.java index a4b40322715..a07a65a9e91 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/MappedByteBufferPool.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/MappedByteBufferPool.java @@ -19,51 +19,104 @@ package org.eclipse.jetty.io; import java.nio.ByteBuffer; +import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.annotation.ManagedAttribute; +import org.eclipse.jetty.util.annotation.ManagedObject; -public class MappedByteBufferPool implements ByteBufferPool +/** + *

A ByteBuffer pool where ByteBuffers are held in queues that are held in a Map.

+ *

Given a capacity {@code factor} of 1024, the Map entry with key {@code 1} holds a + * queue of ByteBuffers each of capacity 1024, the Map entry with key {@code 2} holds a + * queue of ByteBuffers each of capacity 2048, and so on.

+ */ +@ManagedObject +public class MappedByteBufferPool extends AbstractByteBufferPool { - private final ConcurrentMap directBuffers = new ConcurrentHashMap<>(); - private final ConcurrentMap heapBuffers = new ConcurrentHashMap<>(); - private final int _factor; + private final ConcurrentMap _directBuffers = new ConcurrentHashMap<>(); + private final ConcurrentMap _heapBuffers = new ConcurrentHashMap<>(); private final Function _newBucket; + /** + * Creates a new MappedByteBufferPool with a default configuration. + */ public MappedByteBufferPool() { this(-1); } + /** + * Creates a new MappedByteBufferPool with the given capacity factor. + * + * @param factor the capacity factor + */ public MappedByteBufferPool(int factor) { - this(factor,-1,null); + this(factor, -1); } - - public MappedByteBufferPool(int factor,int maxQueue) + + /** + * Creates a new MappedByteBufferPool with the given configuration. + * + * @param factor the capacity factor + * @param maxQueueLength the maximum ByteBuffer queue length + */ + public MappedByteBufferPool(int factor, int maxQueueLength) { - this(factor,maxQueue,null); + this(factor, maxQueueLength, null); } - - public MappedByteBufferPool(int factor,int maxQueue,Function newBucket) + + /** + * Creates a new MappedByteBufferPool with the given configuration. + * + * @param factor the capacity factor + * @param maxQueueLength the maximum ByteBuffer queue length + * @param newBucket the function that creates a Bucket + */ + public MappedByteBufferPool(int factor, int maxQueueLength, Function newBucket) { - _factor = factor<=0?1024:factor; - _newBucket = newBucket!=null?newBucket:i->new Bucket(this,i*_factor,maxQueue); + this(factor, maxQueueLength, newBucket, -1, -1); + } + + /** + * Creates a new MappedByteBufferPool with the given configuration. + * + * @param factor the capacity factor + * @param maxQueueLength the maximum ByteBuffer queue length + * @param newBucket the function that creates a Bucket + * @param maxHeapMemory the max heap memory in bytes + * @param maxDirectMemory the max direct memory in bytes + */ + public MappedByteBufferPool(int factor, int maxQueueLength, Function newBucket, long maxHeapMemory, long maxDirectMemory) + { + super(factor, maxQueueLength, maxHeapMemory, maxDirectMemory); + _newBucket = newBucket != null ? newBucket : this::newBucket; + } + + private Bucket newBucket(int key) + { + return new Bucket(this, key * getCapacityFactor(), getMaxQueueLength()); } @Override public ByteBuffer acquire(int size, boolean direct) { int b = bucketFor(size); + int capacity = b * getCapacityFactor(); ConcurrentMap buffers = bucketsFor(direct); - Bucket bucket = buffers.get(b); - if (bucket==null) - return newByteBuffer(b*_factor, direct); - return bucket.acquire(direct); + if (bucket == null) + return newByteBuffer(capacity, direct); + ByteBuffer buffer = bucket.acquire(); + if (buffer == null) + return newByteBuffer(capacity, direct); + decrementMemory(buffer); + return buffer; } @Override @@ -71,37 +124,87 @@ public class MappedByteBufferPool implements ByteBufferPool { if (buffer == null) return; // nothing to do - - // validate that this buffer is from this pool - assert((buffer.capacity() % _factor) == 0); - - int b = bucketFor(buffer.capacity()); - ConcurrentMap buckets = bucketsFor(buffer.isDirect()); - Bucket bucket = buckets.computeIfAbsent(b,_newBucket); + int capacity = buffer.capacity(); + // Validate that this buffer is from this pool. + assert ((capacity % getCapacityFactor()) == 0); + + int b = bucketFor(capacity); + boolean direct = buffer.isDirect(); + ConcurrentMap buckets = bucketsFor(direct); + Bucket bucket = buckets.computeIfAbsent(b, _newBucket); bucket.release(buffer); + incrementMemory(buffer); + releaseExcessMemory(direct, this::clearOldestBucket); } + @Override public void clear() { - directBuffers.values().forEach(Bucket::clear); - directBuffers.clear(); - heapBuffers.values().forEach(Bucket::clear); - heapBuffers.clear(); + super.clear(); + _directBuffers.values().forEach(Bucket::clear); + _directBuffers.clear(); + _heapBuffers.values().forEach(Bucket::clear); + _heapBuffers.clear(); + } + + private void clearOldestBucket(boolean direct) + { + long oldest = Long.MAX_VALUE; + int index = -1; + ConcurrentMap buckets = bucketsFor(direct); + for (Map.Entry entry : buckets.entrySet()) + { + Bucket bucket = entry.getValue(); + long lastUpdate = bucket.getLastUpdate(); + if (lastUpdate < oldest) + { + oldest = lastUpdate; + index = entry.getKey(); + } + } + if (index >= 0) + { + Bucket bucket = buckets.remove(index); + // The same bucket may be concurrently + // removed, so we need this null guard. + if (bucket != null) + bucket.clear(this::decrementMemory); + } } private int bucketFor(int size) { - int bucket = size / _factor; - if (size % _factor > 0) + int factor = getCapacityFactor(); + int bucket = size / factor; + if (bucket * factor != size) ++bucket; return bucket; } + @ManagedAttribute("The number of pooled direct ByteBuffers") + public long getDirectByteBufferCount() + { + return getByteBufferCount(true); + } + + @ManagedAttribute("The number of pooled heap ByteBuffers") + public long getHeapByteBufferCount() + { + return getByteBufferCount(false); + } + + private long getByteBufferCount(boolean direct) + { + return bucketsFor(direct).values().stream() + .mapToLong(Bucket::size) + .sum(); + } + // Package local for testing ConcurrentMap bucketsFor(boolean direct) { - return direct ? directBuffers : heapBuffers; + return direct ? _directBuffers : _heapBuffers; } public static class Tagged extends MappedByteBufferPool diff --git a/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java b/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java index 97f988b1018..eabdfe6f7bc 100644 --- a/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java +++ b/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java @@ -18,76 +18,60 @@ package org.eclipse.jetty.io; -import static org.hamcrest.Matchers.greaterThanOrEqualTo; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.nio.ByteBuffer; import java.util.Arrays; +import java.util.Objects; import org.eclipse.jetty.io.ByteBufferPool.Bucket; import org.junit.jupiter.api.Test; -@SuppressWarnings("ReferenceEquality") +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.lessThan; +import static org.hamcrest.Matchers.lessThanOrEqualTo; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + public class ArrayByteBufferPoolTest { @Test - public void testMinimumRelease() throws Exception + public void testMinimumRelease() { - ArrayByteBufferPool bufferPool = new ArrayByteBufferPool(10,100,1000); + ArrayByteBufferPool bufferPool = new ArrayByteBufferPool(10, 100, 1000); ByteBufferPool.Bucket[] buckets = bufferPool.bucketsFor(true); - for (int size=1;size<=9;size++) + for (int size = 1; size <= 9; size++) { ByteBuffer buffer = bufferPool.acquire(size, true); assertTrue(buffer.isDirect()); - assertEquals(size,buffer.capacity()); - for (ByteBufferPool.Bucket bucket : buckets) - assertTrue(bucket.isEmpty()); - - bufferPool.release(buffer); - - for (ByteBufferPool.Bucket bucket : buckets) - assertTrue(bucket.isEmpty()); - } - } - - @Test - public void testMaxRelease() throws Exception - { - ArrayByteBufferPool bufferPool = new ArrayByteBufferPool(10,100,1000); - ByteBufferPool.Bucket[] buckets = bufferPool.bucketsFor(true); - - for (int size=999;size<=1001;size++) - { - bufferPool.clear(); - ByteBuffer buffer = bufferPool.acquire(size, true); - - assertTrue(buffer.isDirect()); - assertThat(buffer.capacity(),greaterThanOrEqualTo(size)); - for (ByteBufferPool.Bucket bucket : buckets) - assertTrue(bucket.isEmpty()); - - bufferPool.release(buffer); - - int pooled=0; + assertEquals(size, buffer.capacity()); for (ByteBufferPool.Bucket bucket : buckets) { - pooled+=bucket.size(); + if (bucket != null) + assertTrue(bucket.isEmpty()); + } + + bufferPool.release(buffer); + + for (ByteBufferPool.Bucket bucket : buckets) + { + if (bucket != null) + assertTrue(bucket.isEmpty()); } - assertEquals(size<=1000,1==pooled); } } @Test - public void testAcquireRelease() throws Exception + public void testMaxRelease() { - ArrayByteBufferPool bufferPool = new ArrayByteBufferPool(10,100,1000); + ArrayByteBufferPool bufferPool = new ArrayByteBufferPool(10, 100, 1000); ByteBufferPool.Bucket[] buckets = bufferPool.bucketsFor(true); - for (int size=390;size<=510;size++) + for (int size = 999; size <= 1001; size++) { bufferPool.clear(); ByteBuffer buffer = bufferPool.acquire(size, true); @@ -95,32 +79,57 @@ public class ArrayByteBufferPoolTest assertTrue(buffer.isDirect()); assertThat(buffer.capacity(), greaterThanOrEqualTo(size)); for (ByteBufferPool.Bucket bucket : buckets) - assertTrue(bucket.isEmpty()); + { + if (bucket != null) + assertTrue(bucket.isEmpty()); + } bufferPool.release(buffer); - int pooled=0; - for (ByteBufferPool.Bucket bucket : buckets) - { - if (!bucket.isEmpty()) - { - pooled+=bucket.size(); - // TODO assertThat(bucket._bufferSize,greaterThanOrEqualTo(size)); - // TODO assertThat(bucket._bufferSize,Matchers.lessThan(size+100)); - } - } - assertEquals(1,pooled); + int pooled = Arrays.stream(buckets) + .filter(Objects::nonNull) + .mapToInt(Bucket::size) + .sum(); + assertEquals(size <= 1000, 1 == pooled); } } @Test - @SuppressWarnings("ReferenceEquality") - public void testAcquireReleaseAcquire() throws Exception + public void testAcquireRelease() { - ArrayByteBufferPool bufferPool = new ArrayByteBufferPool(10,100,1000); + ArrayByteBufferPool bufferPool = new ArrayByteBufferPool(10, 100, 1000); ByteBufferPool.Bucket[] buckets = bufferPool.bucketsFor(true); - for (int size=390;size<=510;size++) + for (int size = 390; size <= 510; size++) + { + bufferPool.clear(); + ByteBuffer buffer = bufferPool.acquire(size, true); + + assertTrue(buffer.isDirect()); + assertThat(buffer.capacity(), greaterThanOrEqualTo(size)); + for (ByteBufferPool.Bucket bucket : buckets) + { + if (bucket != null) + assertTrue(bucket.isEmpty()); + } + + bufferPool.release(buffer); + + int pooled = Arrays.stream(buckets) + .filter(Objects::nonNull) + .mapToInt(Bucket::size) + .sum(); + assertEquals(1, pooled); + } + } + + @Test + public void testAcquireReleaseAcquire() + { + ArrayByteBufferPool bufferPool = new ArrayByteBufferPool(10, 100, 1000); + ByteBufferPool.Bucket[] buckets = bufferPool.bucketsFor(true); + + for (int size = 390; size <= 510; size++) { bufferPool.clear(); ByteBuffer buffer1 = bufferPool.acquire(size, true); @@ -130,45 +139,80 @@ public class ArrayByteBufferPoolTest ByteBuffer buffer3 = bufferPool.acquire(size, false); bufferPool.release(buffer3); - int pooled=0; - for (ByteBufferPool.Bucket bucket : buckets) - { - if (!bucket.isEmpty()) - { - pooled+=bucket.size(); - // TODO assertThat(bucket._bufferSize,greaterThanOrEqualTo(size)); - // TODO assertThat(bucket._bufferSize,Matchers.lessThan(size+100)); - } - } - assertEquals(1,pooled); + int pooled = Arrays.stream(buckets) + .filter(Objects::nonNull) + .mapToInt(Bucket::size) + .sum(); + assertEquals(1, pooled); - assertTrue(buffer1==buffer2); - assertTrue(buffer1!=buffer3); + assertSame(buffer1, buffer2); + assertNotSame(buffer1, buffer3); } } - @Test - public void testMaxQueue() throws Exception + public void testMaxQueue() { - ArrayByteBufferPool bufferPool = new ArrayByteBufferPool(-1,-1,-1,2); + ArrayByteBufferPool bufferPool = new ArrayByteBufferPool(-1, -1, -1, 2); ByteBuffer buffer1 = bufferPool.acquire(512, false); ByteBuffer buffer2 = bufferPool.acquire(512, false); ByteBuffer buffer3 = bufferPool.acquire(512, false); Bucket[] buckets = bufferPool.bucketsFor(false); - Arrays.asList(buckets).forEach(b->assertEquals(0,b.size())); - + Arrays.stream(buckets) + .filter(Objects::nonNull) + .forEach(b -> assertEquals(0, b.size())); + bufferPool.release(buffer1); - Bucket bucket=Arrays.asList(buckets).stream().filter(b->b.size()>0).findFirst().get(); + Bucket bucket = Arrays.stream(buckets) + .filter(Objects::nonNull) + .filter(b -> b.size() > 0) + .findFirst() + .orElseThrow(AssertionError::new); assertEquals(1, bucket.size()); bufferPool.release(buffer2); assertEquals(2, bucket.size()); - + bufferPool.release(buffer3); assertEquals(2, bucket.size()); } + @Test + public void testMaxMemory() + { + int factor = 1024; + int maxMemory = 11 * 1024; + ArrayByteBufferPool bufferPool = new ArrayByteBufferPool(-1, factor, -1, -1, -1, maxMemory); + Bucket[] buckets = bufferPool.bucketsFor(true); + + // Create the buckets - the oldest is the larger. + // 1+2+3+4=10 / maxMemory=11. + for (int i = 4; i >= 1; --i) + { + int capacity = factor * i; + ByteBuffer buffer = bufferPool.acquire(capacity, true); + bufferPool.release(buffer); + } + + // Create and release a buffer to exceed the max memory. + ByteBuffer buffer = bufferPool.newByteBuffer(2 * factor, true); + bufferPool.release(buffer); + + // Now the oldest buffer should be gone and we have: 1+2x2+3=8 + long memory = bufferPool.getMemory(true); + assertThat(memory, lessThan((long)maxMemory)); + assertNull(buckets[3]); + + // Create and release a large buffer. + // Max memory is exceeded and buckets 3 and 1 are cleared. + // We will have 2x2+7=11. + buffer = bufferPool.newByteBuffer(7 * factor, true); + bufferPool.release(buffer); + memory = bufferPool.getMemory(true); + assertThat(memory, lessThanOrEqualTo((long)maxMemory)); + assertNull(buckets[0]); + assertNull(buckets[2]); + } } diff --git a/jetty-io/src/test/java/org/eclipse/jetty/io/MappedByteBufferPoolTest.java b/jetty-io/src/test/java/org/eclipse/jetty/io/MappedByteBufferPoolTest.java index ee9589e0d96..5bd536b48cf 100644 --- a/jetty-io/src/test/java/org/eclipse/jetty/io/MappedByteBufferPoolTest.java +++ b/jetty-io/src/test/java/org/eclipse/jetty/io/MappedByteBufferPoolTest.java @@ -26,21 +26,24 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.StringUtil; import org.junit.jupiter.api.Test; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.lessThan; +import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; -import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; public class MappedByteBufferPoolTest { @Test - public void testAcquireRelease() throws Exception + public void testAcquireRelease() { MappedByteBufferPool bufferPool = new MappedByteBufferPool(); - ConcurrentMap buckets = bufferPool.bucketsFor(true); + ConcurrentMap buckets = bufferPool.bucketsFor(true); int size = 512; ByteBuffer buffer = bufferPool.acquire(size, true); @@ -56,10 +59,10 @@ public class MappedByteBufferPoolTest } @Test - public void testAcquireReleaseAcquire() throws Exception + public void testAcquireReleaseAcquire() { MappedByteBufferPool bufferPool = new MappedByteBufferPool(); - ConcurrentMap buckets = bufferPool.bucketsFor(false); + ConcurrentMap buckets = bufferPool.bucketsFor(false); ByteBuffer buffer1 = bufferPool.acquire(512, false); bufferPool.release(buffer1); @@ -76,10 +79,10 @@ public class MappedByteBufferPoolTest } @Test - public void testAcquireReleaseClear() throws Exception + public void testAcquireReleaseClear() { MappedByteBufferPool bufferPool = new MappedByteBufferPool(); - ConcurrentMap buckets = bufferPool.bucketsFor(true); + ConcurrentMap buckets = bufferPool.bucketsFor(true); ByteBuffer buffer = bufferPool.acquire(512, true); bufferPool.release(buffer); @@ -91,16 +94,14 @@ public class MappedByteBufferPoolTest assertTrue(buckets.isEmpty()); } - + /** * In a scenario where MappedByteBufferPool is being used improperly, * such as releasing a buffer that wasn't created/acquired by the * MappedByteBufferPool, an assertion is tested for. - * - * @throws Exception test failure */ @Test - public void testReleaseAssertion() throws Exception + public void testReleaseAssertion() { int factor = 1024; MappedByteBufferPool bufferPool = new MappedByteBufferPool(factor); @@ -110,8 +111,8 @@ public class MappedByteBufferPoolTest // Release a few small non-pool buffers bufferPool.release(ByteBuffer.wrap(StringUtil.getUtf8Bytes("Hello"))); - /* NOTES: - * + /* NOTES: + * * 1) This test will pass on command line maven build, as its surefire setup uses "-ea" already. * 2) In Eclipse, goto the "Run Configuration" for this test case. * Select the "Arguments" tab, and make sure "-ea" is present in the text box titled "VM arguments" @@ -123,24 +124,24 @@ public class MappedByteBufferPoolTest // Expected path. } } - + @Test public void testTagged() { MappedByteBufferPool pool = new MappedByteBufferPool.Tagged(); - ByteBuffer buffer = pool.acquire(1024,false); + ByteBuffer buffer = pool.acquire(1024, false); - assertThat(BufferUtil.toDetailString(buffer),containsString("@T00000001")); - buffer = pool.acquire(1024,false); - assertThat(BufferUtil.toDetailString(buffer),containsString("@T00000002")); + assertThat(BufferUtil.toDetailString(buffer), containsString("@T00000001")); + buffer = pool.acquire(1024, false); + assertThat(BufferUtil.toDetailString(buffer), containsString("@T00000002")); } @Test - public void testMaxQueue() throws Exception + public void testMaxQueue() { - MappedByteBufferPool bufferPool = new MappedByteBufferPool(-1,2); - ConcurrentMap buckets = bufferPool.bucketsFor(false); + MappedByteBufferPool bufferPool = new MappedByteBufferPool(-1, 2); + ConcurrentMap buckets = bufferPool.bucketsFor(false); ByteBuffer buffer1 = bufferPool.acquire(512, false); ByteBuffer buffer2 = bufferPool.acquire(512, false); @@ -149,13 +150,50 @@ public class MappedByteBufferPoolTest bufferPool.release(buffer1); assertEquals(1, buckets.size()); - Bucket bucket=buckets.values().iterator().next(); + Bucket bucket = buckets.values().iterator().next(); assertEquals(1, bucket.size()); bufferPool.release(buffer2); assertEquals(2, bucket.size()); - + bufferPool.release(buffer3); assertEquals(2, bucket.size()); } + + @Test + public void testMaxMemory() + { + int factor = 1024; + int maxMemory = 11 * 1024; + MappedByteBufferPool bufferPool = new MappedByteBufferPool(factor, -1, null, -1, maxMemory); + ConcurrentMap buckets = bufferPool.bucketsFor(true); + + // Create the buckets - the oldest is the larger. + // 1+2+3+4=10 / maxMemory=11. + for (int i = 4; i >= 1; --i) + { + int capacity = factor * i; + ByteBuffer buffer = bufferPool.acquire(capacity, true); + bufferPool.release(buffer); + } + + // Create and release a buffer to exceed the max memory. + ByteBuffer buffer = bufferPool.newByteBuffer(2 * factor, true); + bufferPool.release(buffer); + + // Now the oldest buffer should be gone and we have: 1+2x2+3=8 + long memory = bufferPool.getMemory(true); + assertThat(memory, lessThan((long)maxMemory)); + assertNull(buckets.get(4)); + + // Create and release a large buffer. + // Max memory is exceeded and buckets 3 and 1 are cleared. + // We will have 2x2+7=11. + buffer = bufferPool.newByteBuffer(7 * factor, true); + bufferPool.release(buffer); + memory = bufferPool.getMemory(true); + assertThat(memory, lessThanOrEqualTo((long)maxMemory)); + assertNull(buckets.get(1)); + assertNull(buckets.get(3)); + } } diff --git a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/JAASGroup.java b/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/JAASGroup.java deleted file mode 100644 index fe87b01d4bf..00000000000 --- a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/JAASGroup.java +++ /dev/null @@ -1,117 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. -// ------------------------------------------------------------------------ -// All rights reserved. This program and the accompanying materials -// are made available under the terms of the Eclipse Public License v1.0 -// and Apache License v2.0 which accompanies this distribution. -// -// The Eclipse Public License is available at -// http://www.eclipse.org/legal/epl-v10.html -// -// The Apache License v2.0 is available at -// http://www.opensource.org/licenses/apache2.0.php -// -// You may elect to redistribute this code under either of these licenses. -// ======================================================================== -// - -package org.eclipse.jetty.jaas; - -import java.security.Principal; -import java.security.acl.Group; -import java.util.Enumeration; -import java.util.HashSet; -import java.util.Iterator; - -public class JAASGroup implements Group -{ - public static final String ROLES = "__roles__"; - - private String _name = null; - private HashSet _members = null; - - public JAASGroup(String n) - { - this._name = n; - this._members = new HashSet(); - } - - /* ------------------------------------------------------------ */ - @Override - public synchronized boolean addMember(Principal principal) - { - return _members.add(principal); - } - - @Override - public synchronized boolean removeMember(Principal principal) - { - return _members.remove(principal); - } - - @Override - public boolean isMember(Principal principal) - { - return _members.contains(principal); - } - - @Override - public Enumeration members() - { - - class MembersEnumeration implements Enumeration - { - private Iterator itor; - - public MembersEnumeration (Iterator itor) - { - this.itor = itor; - } - - @Override - public boolean hasMoreElements () - { - return this.itor.hasNext(); - } - - - @Override - public Principal nextElement () - { - return this.itor.next(); - } - - } - - return new MembersEnumeration (_members.iterator()); - } - - @Override - public int hashCode() - { - return getName().hashCode(); - } - - @Override - public boolean equals(Object object) - { - if (! (object instanceof JAASGroup)) - return false; - - return ((JAASGroup)object).getName().equals(getName()); - } - - @Override - public String toString() - { - return getName(); - } - - @Override - public String getName() - { - - return _name; - } -} diff --git a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/StrictRoleCheckPolicy.java b/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/StrictRoleCheckPolicy.java deleted file mode 100644 index 712a4bc4911..00000000000 --- a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/StrictRoleCheckPolicy.java +++ /dev/null @@ -1,63 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. -// ------------------------------------------------------------------------ -// All rights reserved. This program and the accompanying materials -// are made available under the terms of the Eclipse Public License v1.0 -// and Apache License v2.0 which accompanies this distribution. -// -// The Eclipse Public License is available at -// http://www.eclipse.org/legal/epl-v10.html -// -// The Apache License v2.0 is available at -// http://www.opensource.org/licenses/apache2.0.php -// -// You may elect to redistribute this code under either of these licenses. -// ======================================================================== -// - -package org.eclipse.jetty.jaas; - -import java.security.Principal; -import java.security.acl.Group; -import java.util.Enumeration; - - -/* ---------------------------------------------------- */ -/** StrictRoleCheckPolicy - *

Enforces that if a runAsRole is present, then the - * role to check must be the same as that runAsRole and - * the set of static roles is ignored. - * - * - * - */ -public class StrictRoleCheckPolicy implements RoleCheckPolicy -{ - - @Override - public boolean checkRole (String roleName, Principal runAsRole, Group roles) - { - //check if this user has had any temporary role pushed onto - //them. If so, then only check if the user has that role. - if (runAsRole != null) - { - return (roleName.equals(runAsRole.getName())); - } - else - { - if (roles == null) - return false; - Enumeration rolesEnum = roles.members(); - boolean found = false; - while (rolesEnum.hasMoreElements() && !found) - { - Principal p = (Principal)rolesEnum.nextElement(); - found = roleName.equals(p.getName()); - } - return found; - } - - } - -} diff --git a/jetty-osgi/test-jetty-osgi/pom.xml b/jetty-osgi/test-jetty-osgi/pom.xml index 81207cf4b7d..d43498a3c35 100644 --- a/jetty-osgi/test-jetty-osgi/pom.xml +++ b/jetty-osgi/test-jetty-osgi/pom.xml @@ -548,7 +548,6 @@ maven-surefire-plugin - **/TestJettyOSGiBootHTTP2JDK9* diff --git a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http2.xml b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http2.xml index 2de7f348d0a..962451ec144 100644 --- a/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http2.xml +++ b/jetty-osgi/test-jetty-osgi/src/test/config/etc/jetty-http2.xml @@ -21,6 +21,13 @@ true + + + + TLSv1.3 + + + diff --git a/jetty-osgi/test-jetty-osgi/src/test/java/org/eclipse/jetty/osgi/test/TestJettyOSGiBootHTTP2Conscrypt.java b/jetty-osgi/test-jetty-osgi/src/test/java/org/eclipse/jetty/osgi/test/TestJettyOSGiBootHTTP2Conscrypt.java index 333e1b6935d..8edcaf8604b 100644 --- a/jetty-osgi/test-jetty-osgi/src/test/java/org/eclipse/jetty/osgi/test/TestJettyOSGiBootHTTP2Conscrypt.java +++ b/jetty-osgi/test-jetty-osgi/src/test/java/org/eclipse/jetty/osgi/test/TestJettyOSGiBootHTTP2Conscrypt.java @@ -31,6 +31,7 @@ import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.http2.client.HTTP2Client; import org.eclipse.jetty.http2.client.http.HttpClientTransportOverHTTP2; +import org.eclipse.jetty.util.JavaVersion; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.junit.Test; @@ -66,12 +67,12 @@ public class TestJettyOSGiBootHTTP2Conscrypt { ArrayList

Allows applications to transform upstream and downstream content.

*

Typical use cases of transformations are URL rewriting of HTML anchors - * (where the value of the href attribute of <a> elements + * (where the value of the {@code href} attribute of <a> elements * is modified by the proxy), field renaming of JSON documents, etc.

*

Applications should override {@link #newClientRequestContentTransformer(HttpServletRequest, Request)} * and/or {@link #newServerResponseContentTransformer(HttpServletRequest, HttpServletResponse, Response)} @@ -762,16 +762,23 @@ public class AsyncMiddleManServlet extends AbstractProxyServlet private static final Logger logger = Log.getLogger(GZIPContentTransformer.class); private final List buffers = new ArrayList<>(2); - private final ContentDecoder decoder = new GZIPContentDecoder(); private final ContentTransformer transformer; + private final ContentDecoder decoder; private final ByteArrayOutputStream out; private final GZIPOutputStream gzipOut; public GZIPContentTransformer(ContentTransformer transformer) + { + this(null, transformer); + } + + public GZIPContentTransformer(HttpClient httpClient, ContentTransformer transformer) { try { this.transformer = transformer; + ByteBufferPool byteBufferPool = httpClient == null ? null : httpClient.getByteBufferPool(); + this.decoder = new GZIPContentDecoder(byteBufferPool, GZIPContentDecoder.DEFAULT_BUFFER_SIZE); this.out = new ByteArrayOutputStream(); this.gzipOut = new GZIPOutputStream(out); } @@ -787,6 +794,7 @@ public class AsyncMiddleManServlet extends AbstractProxyServlet if (logger.isDebugEnabled()) logger.debug("Ungzipping {} bytes, finished={}", input.remaining(), finished); + List decodeds = Collections.emptyList(); if (!input.hasRemaining()) { if (finished) @@ -794,14 +802,19 @@ public class AsyncMiddleManServlet extends AbstractProxyServlet } else { - while (input.hasRemaining()) + decodeds = new ArrayList<>(); + while (true) { ByteBuffer decoded = decoder.decode(input); - boolean complete = finished && !input.hasRemaining(); + decodeds.add(decoded); + boolean decodeComplete = !input.hasRemaining() && !decoded.hasRemaining(); + boolean complete = finished && decodeComplete; if (logger.isDebugEnabled()) logger.debug("Ungzipped {} bytes, complete={}", decoded.remaining(), complete); if (decoded.hasRemaining() || complete) transformer.transform(decoded, complete, buffers); + if (decodeComplete) + break; } } @@ -811,6 +824,8 @@ public class AsyncMiddleManServlet extends AbstractProxyServlet buffers.clear(); output.add(result); } + + decodeds.forEach(decoder::release); } private ByteBuffer gzip(List buffers, boolean finished) throws IOException diff --git a/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ConnectHandler.java b/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ConnectHandler.java index f096401cf35..6b7d39eed9b 100644 --- a/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ConnectHandler.java +++ b/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ConnectHandler.java @@ -44,7 +44,6 @@ import org.eclipse.jetty.io.Connection; import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.io.ManagedSelector; import org.eclipse.jetty.io.MappedByteBufferPool; -import org.eclipse.jetty.io.SelectChannelEndPoint; import org.eclipse.jetty.io.SelectorManager; import org.eclipse.jetty.io.SocketChannelEndPoint; import org.eclipse.jetty.server.Handler; @@ -56,7 +55,6 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.HostPort; import org.eclipse.jetty.util.Promise; -import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler; @@ -106,6 +104,7 @@ public class ConnectHandler extends HandlerWrapper public void setScheduler(Scheduler scheduler) { + updateBean(this.scheduler,scheduler); this.scheduler = scheduler; } @@ -116,6 +115,7 @@ public class ConnectHandler extends HandlerWrapper public void setByteBufferPool(ByteBufferPool bufferPool) { + updateBean(this.bufferPool, bufferPool); this.bufferPool = bufferPool; } @@ -168,10 +168,18 @@ public class ConnectHandler extends HandlerWrapper executor = getServer().getThreadPool(); if (scheduler == null) - addBean(scheduler = new ScheduledExecutorScheduler()); + { + scheduler = getServer().getBean(Scheduler.class); + if (scheduler == null) + scheduler = new ScheduledExecutorScheduler(String.format("Proxy-Scheduler-%x", hashCode()), false); + addBean(scheduler); + } if (bufferPool == null) - addBean(bufferPool = new MappedByteBufferPool()); + { + bufferPool = new MappedByteBufferPool(); + addBean(bufferPool); + } addBean(selector = newSelectorManager()); selector.setConnectTimeout(getConnectTimeout()); diff --git a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/AsyncMiddleManServletTest.java b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/AsyncMiddleManServletTest.java index f273fc262f1..2efd189a993 100644 --- a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/AsyncMiddleManServletTest.java +++ b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/AsyncMiddleManServletTest.java @@ -18,13 +18,6 @@ package org.eclipse.jetty.proxy; -import static org.junit.jupiter.api.Assertions.assertArrayEquals; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; @@ -62,7 +55,6 @@ import org.eclipse.jetty.client.api.ContentProvider; import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.client.api.Response; -import org.eclipse.jetty.client.api.Result; import org.eclipse.jetty.client.util.BytesContentProvider; import org.eclipse.jetty.client.util.DeferredContentProvider; import org.eclipse.jetty.client.util.FutureResponseListener; @@ -87,10 +79,16 @@ import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.log.StacklessLogging; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.junit.jupiter.api.AfterEach; - import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.OS; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + public class AsyncMiddleManServletTest { private static final Logger LOG = Log.getLogger(AsyncMiddleManServletTest.class); @@ -120,7 +118,7 @@ public class AsyncMiddleManServletTest private void startProxy(AsyncMiddleManServlet proxyServlet) throws Exception { - startProxy(proxyServlet, new HashMap()); + startProxy(proxyServlet, new HashMap<>()); } private void startProxy(AsyncMiddleManServlet proxyServlet, Map initParams) throws Exception @@ -144,8 +142,8 @@ public class AsyncMiddleManServletTest proxyContext.addServlet(proxyServletHolder, "/*"); proxy.start(); - - stackless=new StacklessLogging(proxyServlet._log); + + stackless = new StacklessLogging(proxyServlet._log); } private void startClient() throws Exception @@ -219,7 +217,7 @@ public class AsyncMiddleManServletTest @Override protected ContentTransformer newClientRequestContentTransformer(HttpServletRequest clientRequest, Request proxyRequest) { - return new GZIPContentTransformer(ContentTransformer.IDENTITY); + return new GZIPContentTransformer(getHttpClient(), ContentTransformer.IDENTITY); } }); startClient(); @@ -247,7 +245,7 @@ public class AsyncMiddleManServletTest startServer(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException { response.setHeader(HttpHeader.CONTENT_ENCODING.asString(), "gzip"); response.getOutputStream().write(gzipBytes); @@ -318,7 +316,7 @@ public class AsyncMiddleManServletTest startServer(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException { response.setHeader(HttpHeader.CONTENT_ENCODING.asString(), "gzip"); @@ -412,7 +410,7 @@ public class AsyncMiddleManServletTest startServer(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException { response.setHeader(HttpHeader.CONTENT_ENCODING.asString(), "gzip"); @@ -454,7 +452,7 @@ public class AsyncMiddleManServletTest startServer(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException { // decode input stream thru gzip ByteArrayOutputStream bos = new ByteArrayOutputStream(); @@ -500,13 +498,9 @@ public class AsyncMiddleManServletTest @Override protected ContentTransformer newClientRequestContentTransformer(HttpServletRequest clientRequest, Request proxyRequest) { - return new ContentTransformer() + return (input, finished, output) -> { - @Override - public void transform(ByteBuffer input, boolean finished, List output) throws IOException - { - throw new NullPointerException("explicitly_thrown_by_test"); - } + throw new NullPointerException("explicitly_thrown_by_test"); }; } }); @@ -537,7 +531,7 @@ public class AsyncMiddleManServletTest private int count; @Override - public void transform(ByteBuffer input, boolean finished, List output) throws IOException + public void transform(ByteBuffer input, boolean finished, List output) { if (++count < 2) output.add(input); @@ -552,16 +546,12 @@ public class AsyncMiddleManServletTest final CountDownLatch latch = new CountDownLatch(1); DeferredContentProvider content = new DeferredContentProvider(); client.newRequest("localhost", serverConnector.getLocalPort()) - .content(content) - .send(new Response.CompleteListener() - { - @Override - public void onComplete(Result result) - { - if (result.isSucceeded() && result.getResponse().getStatus() == 502) - latch.countDown(); - } - }); + .content(content) + .send(result -> + { + if (result.isSucceeded() && result.getResponse().getStatus() == 502) + latch.countDown(); + }); content.offer(ByteBuffer.allocate(512)); sleep(1000); @@ -578,7 +568,7 @@ public class AsyncMiddleManServletTest testDownstreamTransformationThrows(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException { // To trigger the test failure we need that onContent() // is called twice, so the second time the test throws. @@ -597,7 +587,7 @@ public class AsyncMiddleManServletTest testDownstreamTransformationThrows(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException { // To trigger the test failure we need that onContent() // is called only once, so the the test throws from onSuccess(). @@ -621,7 +611,7 @@ public class AsyncMiddleManServletTest private int count; @Override - public void transform(ByteBuffer input, boolean finished, List output) throws IOException + public void transform(ByteBuffer input, boolean finished, List output) { if (++count < 2) output.add(input); @@ -660,7 +650,7 @@ public class AsyncMiddleManServletTest startServer(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException { OutputStream output = response.getOutputStream(); if (gzipped) @@ -694,25 +684,17 @@ public class AsyncMiddleManServletTest final CountDownLatch latch = new CountDownLatch(1); client.newRequest("localhost", serverConnector.getLocalPort()) - .onResponseContent(new Response.ContentListener() + .onResponseContent((response, content) -> { - @Override - public void onContent(Response response, ByteBuffer content) - { - // Slow down the reader so that the - // write from the proxy gets congested. - sleep(1); - } + // Slow down the reader so that the + // write from the proxy gets congested. + sleep(1); }) - .send(new Response.CompleteListener() + .send(result -> { - @Override - public void onComplete(Result result) - { - assertTrue(result.isSucceeded()); - assertEquals(200, result.getResponse().getStatus()); - latch.countDown(); - } + assertTrue(result.isSucceeded()); + assertEquals(200, result.getResponse().getStatus()); + latch.countDown(); }); assertTrue(latch.await(15, TimeUnit.SECONDS)); @@ -724,7 +706,7 @@ public class AsyncMiddleManServletTest startServer(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException { byte[] chunk = new byte[1024]; int contentLength = 2 * chunk.length; @@ -741,14 +723,10 @@ public class AsyncMiddleManServletTest @Override protected ContentTransformer newServerResponseContentTransformer(HttpServletRequest clientRequest, HttpServletResponse proxyResponse, Response serverResponse) { - return new ContentTransformer() + return (input, finished, output) -> { - @Override - public void transform(ByteBuffer input, boolean finished, List output) throws IOException - { - if (!finished) - output.add(input); - } + if (!finished) + output.add(input); }; } }); @@ -779,15 +757,11 @@ public class AsyncMiddleManServletTest DeferredContentProvider content = new DeferredContentProvider(); client.newRequest("localhost", serverConnector.getLocalPort()) .content(content) - .send(new Response.CompleteListener() + .send(result -> { - @Override - public void onComplete(Result result) - { - System.err.println(result); - if (result.getResponse().getStatus() == 500) - latch.countDown(); - } + System.err.println(result); + if (result.getResponse().getStatus() == 500) + latch.countDown(); }); content.offer(ByteBuffer.allocate(512)); sleep(1000); @@ -821,16 +795,12 @@ public class AsyncMiddleManServletTest final CountDownLatch latch = new CountDownLatch(1); DeferredContentProvider content = new DeferredContentProvider(); client.newRequest("localhost", serverConnector.getLocalPort()) - .content(content) - .send(new Response.CompleteListener() - { - @Override - public void onComplete(Result result) - { - if (result.getResponse().getStatus() == 502) - latch.countDown(); - } - }); + .content(content) + .send(result -> + { + if (result.getResponse().getStatus() == 502) + latch.countDown(); + }); content.offer(ByteBuffer.allocate(512)); sleep(1000); content.offer(ByteBuffer.allocate(512)); @@ -857,7 +827,7 @@ public class AsyncMiddleManServletTest startServer(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException { ServletOutputStream output = response.getOutputStream(); output.write(new byte[512]); @@ -898,7 +868,7 @@ public class AsyncMiddleManServletTest startServer(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException { response.getOutputStream().write(json.getBytes(StandardCharsets.UTF_8)); } @@ -916,7 +886,7 @@ public class AsyncMiddleManServletTest { InputStream input = source.getInputStream(); @SuppressWarnings("unchecked") - Map obj = (Map)JSON.parse(new InputStreamReader(input, "UTF-8")); + Map obj = (Map)JSON.parse(new InputStreamReader(input, StandardCharsets.UTF_8)); // Transform the object. obj.put(key2, obj.remove(key1)); try (OutputStream output = sink.getOutputStream()) @@ -961,7 +931,7 @@ public class AsyncMiddleManServletTest startServer(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException { // Write the content in two chunks. int chunk = data.length / 2; @@ -1021,7 +991,7 @@ public class AsyncMiddleManServletTest startServer(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException { response.getOutputStream().write(json.getBytes(StandardCharsets.UTF_8)); } @@ -1041,7 +1011,7 @@ public class AsyncMiddleManServletTest { InputStream input = source.getInputStream(); @SuppressWarnings("unchecked") - Map obj = (Map)JSON.parse(new InputStreamReader(input, "UTF-8")); + Map obj = (Map)JSON.parse(new InputStreamReader(input, StandardCharsets.UTF_8)); // Transform the object. obj.put(key2, obj.remove(key1)); try (OutputStream output = sink.getOutputStream()) @@ -1080,7 +1050,7 @@ public class AsyncMiddleManServletTest } // File deletion is delayed on windows, testing for deletion is not going to work - if(!OS.WINDOWS.isCurrentOs()) + if (!OS.WINDOWS.isCurrentOs()) { try (DirectoryStream paths = Files.newDirectoryStream(targetTestsDir, outputPrefix + "*.*")) { @@ -1097,7 +1067,7 @@ public class AsyncMiddleManServletTest startServer(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException { IO.copy(request.getInputStream(), IO.getNullStream()); } @@ -1163,7 +1133,7 @@ public class AsyncMiddleManServletTest startServer(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException { response.setHeader(HttpHeader.CONNECTION.asString(), HttpHeaderValue.CLOSE.asString()); response.setContentLength(2); @@ -1237,7 +1207,7 @@ public class AsyncMiddleManServletTest startServer(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException { response.getOutputStream().write(json.getBytes(StandardCharsets.UTF_8)); } @@ -1260,7 +1230,7 @@ public class AsyncMiddleManServletTest if (readSource) { InputStream input = source.getInputStream(); - JSON.parse(new InputStreamReader(input, "UTF-8")); + JSON.parse(new InputStreamReader(input, StandardCharsets.UTF_8)); } // No transformation. return false; @@ -1289,7 +1259,7 @@ public class AsyncMiddleManServletTest startServer(new HttpServlet() { @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response) { response.setStatus(HttpStatus.UNAUTHORIZED_401); response.setHeader(HttpHeader.WWW_AUTHENTICATE.asString(), "Basic realm=\"test\""); @@ -1304,7 +1274,7 @@ public class AsyncMiddleManServletTest return new AfterContentTransformer() { @Override - public boolean transform(Source source, Sink sink) throws IOException + public boolean transform(Source source, Sink sink) { transformed.set(true); return false; @@ -1433,7 +1403,7 @@ public class AsyncMiddleManServletTest private ByteBuffer buffer; @Override - public void transform(ByteBuffer input, boolean finished, List output) throws IOException + public void transform(ByteBuffer input, boolean finished, List output) { // Buffer only the first chunk. if (buffer == null) @@ -1495,7 +1465,7 @@ public class AsyncMiddleManServletTest startServer(new HttpServlet() { @Override - protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + protected void doGet(HttpServletRequest req, HttpServletResponse resp) { if (req.getHeader("Via") != null) resp.addHeader(PROXIED_HEADER, "true"); @@ -1503,9 +1473,11 @@ public class AsyncMiddleManServletTest } }); final String proxyTo = "http://localhost:" + serverConnector.getLocalPort(); - AsyncMiddleManServlet proxyServlet = new AsyncMiddleManServlet.Transparent() { + AsyncMiddleManServlet proxyServlet = new AsyncMiddleManServlet.Transparent() + { @Override - protected ContentTransformer newServerResponseContentTransformer(HttpServletRequest clientRequest, HttpServletResponse proxyResponse, Response serverResponse) { + protected ContentTransformer newServerResponseContentTransformer(HttpServletRequest clientRequest, HttpServletResponse proxyResponse, Response serverResponse) + { return ContentTransformer.IDENTITY; } }; @@ -1689,7 +1661,7 @@ public class AsyncMiddleManServletTest private final List buffers = new ArrayList<>(); @Override - public void transform(ByteBuffer input, boolean finished, List output) throws IOException + public void transform(ByteBuffer input, boolean finished, List output) { if (input.hasRemaining()) { @@ -1715,7 +1687,7 @@ public class AsyncMiddleManServletTest private StringBuilder head = new StringBuilder(); @Override - public void transform(ByteBuffer input, boolean finished, List output) throws IOException + public void transform(ByteBuffer input, boolean finished, List output) { if (input.hasRemaining() && head != null) { @@ -1723,12 +1695,12 @@ public class AsyncMiddleManServletTest if (lnPos == -1) { // no linefeed found, copy it all - copyHeadBytes(input,input.limit()); + copyHeadBytes(input, input.limit()); } else { // found linefeed - copyHeadBytes(input,lnPos); + copyHeadBytes(input, lnPos); output.addAll(getHeadBytes()); // mark head as sent head = null; @@ -1764,7 +1736,7 @@ public class AsyncMiddleManServletTest private List getHeadBytes() { - ByteBuffer buf = BufferUtil.toBuffer(head.toString(),StandardCharsets.UTF_8); + ByteBuffer buf = BufferUtil.toBuffer(head.toString(), StandardCharsets.UTF_8); return Collections.singletonList(buf); } } @@ -1772,7 +1744,7 @@ public class AsyncMiddleManServletTest private static class DiscardContentTransformer implements AsyncMiddleManServlet.ContentTransformer { @Override - public void transform(ByteBuffer input, boolean finished, List output) throws IOException + public void transform(ByteBuffer input, boolean finished, List output) { } } diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/LoginAuthenticator.java b/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/LoginAuthenticator.java index b0d4abf1d33..817dcc094d7 100644 --- a/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/LoginAuthenticator.java +++ b/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/LoginAuthenticator.java @@ -111,7 +111,7 @@ public abstract class LoginAuthenticator implements Authenticator s.renewId(request); s.setAttribute(Session.SESSION_CREATED_SECURE, Boolean.TRUE); if (s.isIdChanged() && (response instanceof Response)) - ((Response)response).addCookie(s.getSessionHandler().getSessionCookie(s, request.getContextPath(), request.isSecure())); + ((Response)response).replaceCookie(s.getSessionHandler().getSessionCookie(s, request.getContextPath(), request.isSecure())); if (LOG.isDebugEnabled()) LOG.debug("renew {}->{}", oldId, s.getId()); } diff --git a/jetty-server/src/main/config/etc/jetty-bytebufferpool.xml b/jetty-server/src/main/config/etc/jetty-bytebufferpool.xml new file mode 100644 index 00000000000..6da3421a938 --- /dev/null +++ b/jetty-server/src/main/config/etc/jetty-bytebufferpool.xml @@ -0,0 +1,12 @@ + + + + + + + + + + + + diff --git a/jetty-server/src/main/config/etc/jetty.xml b/jetty-server/src/main/config/etc/jetty.xml index 961f95435e5..0fae969d287 100644 --- a/jetty-server/src/main/config/etc/jetty.xml +++ b/jetty-server/src/main/config/etc/jetty.xml @@ -26,6 +26,10 @@ + + + + diff --git a/jetty-server/src/main/config/modules/bytebufferpool.mod b/jetty-server/src/main/config/modules/bytebufferpool.mod new file mode 100644 index 00000000000..7a813148b6e --- /dev/null +++ b/jetty-server/src/main/config/modules/bytebufferpool.mod @@ -0,0 +1,27 @@ +DO NOT EDIT - See: https://www.eclipse.org/jetty/documentation/current/startup-modules.html + +[description] +Configures the ByteBufferPool used by ServerConnectors. + +[xml] +etc/jetty-bytebufferpool.xml + +[ini-template] +### Server ByteBufferPool Configuration +## Minimum capacity to pool ByteBuffers +#jetty.byteBufferPool.minCapacity=0 + +## Maximum capacity to pool ByteBuffers +#jetty.byteBufferPool.maxCapacity=65536 + +## Capacity factor +#jetty.byteBufferPool.factor=1024 + +## Maximum queue length for each bucket (-1 for unbounded) +#jetty.byteBufferPool.maxQueueLength=-1 + +## Maximum heap memory retainable by the pool (-1 for unlimited) +#jetty.byteBufferPool.maxHeapMemory=-1 + +## Maximum direct memory retainable by the pool (-1 for unlimited) +#jetty.byteBufferPool.maxDirectMemory=-1 diff --git a/jetty-server/src/main/config/modules/server.mod b/jetty-server/src/main/config/modules/server.mod index 038f1836b65..dd039a01fb1 100644 --- a/jetty-server/src/main/config/modules/server.mod +++ b/jetty-server/src/main/config/modules/server.mod @@ -11,6 +11,7 @@ logging [depend] threadpool +bytebufferpool [lib] lib/servlet-api-3.1.jar diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java index d84f52c8219..1a92d1f630c 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java @@ -183,7 +183,7 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co _executor=executor!=null?executor:_server.getThreadPool(); if (scheduler==null) scheduler=_server.getBean(Scheduler.class); - _scheduler=scheduler!=null?scheduler:new ScheduledExecutorScheduler(); + _scheduler=scheduler!=null?scheduler:new ScheduledExecutorScheduler(String.format("Connector-Scheduler-%x",hashCode()),false); if (pool==null) pool=_server.getBean(ByteBufferPool.class); _byteBufferPool = pool!=null?pool:new ArrayByteBufferPool(); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 3db45d61de4..e1015d84d4e 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -1527,7 +1527,7 @@ public class Request implements HttpServletRequest if (getRemoteUser() != null) s.setAttribute(Session.SESSION_CREATED_SECURE, Boolean.TRUE); if (s.isIdChanged() && _sessionHandler.isUsingCookies()) - _channel.getResponse().addCookie(_sessionHandler.getSessionCookie(s, getContextPath(), isSecure())); + _channel.getResponse().replaceCookie(_sessionHandler.getSessionCookie(s, getContextPath(), isSecure())); } return session.getId(); @@ -1570,7 +1570,7 @@ public class Request implements HttpServletRequest _session = _sessionHandler.newHttpSession(this); HttpCookie cookie = _sessionHandler.getSessionCookie(_session,getContextPath(),isSecure()); if (cookie != null) - _channel.getResponse().addCookie(cookie); + _channel.getResponse().replaceCookie(cookie); return _session; } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java index 5afd02437c8..e74986a951e 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java @@ -24,7 +24,9 @@ import java.nio.channels.IllegalSelectorException; import java.util.Collection; import java.util.Collections; import java.util.EnumSet; +import java.util.Iterator; import java.util.List; +import java.util.ListIterator; import java.util.Locale; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Supplier; @@ -194,6 +196,94 @@ public class Response implements HttpServletResponse cookie.isHttpOnly()); } + /** + * Replace (or add) a cookie. + * Using name, path and domain, look for a matching set-cookie header and replace it. + * @param cookie The cookie to add/replace + */ + public void replaceCookie(HttpCookie cookie) + { + for (ListIterator i = _fields.listIterator(); i.hasNext();) + { + HttpField field = i.next(); + + if (field.getHeader() == HttpHeader.SET_COOKIE) + { + String old_set_cookie = field.getValue(); + String name = cookie.getName(); + if (!old_set_cookie.startsWith(name) || old_set_cookie.length()<= name.length() || old_set_cookie.charAt(name.length())!='=') + continue; + + String domain = cookie.getDomain(); + if (domain!=null) + { + if (getHttpChannel().getHttpConfiguration().getResponseCookieCompliance()==CookieCompliance.RFC2965) + { + StringBuilder buf = new StringBuilder(); + buf.append(";Domain="); + quoteOnlyOrAppend(buf,domain,isQuoteNeededForCookie(domain)); + domain = buf.toString(); + } + else + { + domain = ";Domain="+domain; + } + if (!old_set_cookie.contains(domain)) + continue; + } + else if (old_set_cookie.contains(";Domain=")) + continue; + + String path = cookie.getPath(); + if (path!=null) + { + if (getHttpChannel().getHttpConfiguration().getResponseCookieCompliance()==CookieCompliance.RFC2965) + { + StringBuilder buf = new StringBuilder(); + buf.append(";Path="); + quoteOnlyOrAppend(buf,path,isQuoteNeededForCookie(path)); + path = buf.toString(); + } + else + { + path = ";Path="+path; + } + if (!old_set_cookie.contains(path)) + continue; + } + else if (old_set_cookie.contains(";Path=")) + continue; + + if (getHttpChannel().getHttpConfiguration().getResponseCookieCompliance() == CookieCompliance.RFC2965) + i.set(new HttpField(HttpHeader.CONTENT_ENCODING.SET_COOKIE, newRFC2965SetCookie( + cookie.getName(), + cookie.getValue(), + cookie.getDomain(), + cookie.getPath(), + cookie.getMaxAge(), + cookie.getComment(), + cookie.isSecure(), + cookie.isHttpOnly(), + cookie.getVersion()) + )); + else + i.set(new HttpField(HttpHeader.CONTENT_ENCODING.SET_COOKIE, newRFC6265SetCookie( + cookie.getName(), + cookie.getValue(), + cookie.getDomain(), + cookie.getPath(), + cookie.getMaxAge(), + cookie.isSecure(), + cookie.isHttpOnly() + ))); + return; + } + } + + // Not replaced, so add normally + addCookie(cookie); + } + @Override public void addCookie(Cookie cookie) { @@ -257,6 +347,18 @@ public class Response implements HttpServletResponse final long maxAge, final boolean isSecure, final boolean isHttpOnly) + { + String set_cookie = newRFC6265SetCookie(name, value, domain, path, maxAge, isSecure, isHttpOnly); + + // add the set cookie + _fields.add(HttpHeader.SET_COOKIE, set_cookie); + + // Expire responses with set-cookie headers so they do not get cached. + _fields.put(__EXPIRES_01JAN1970); + + } + + private String newRFC6265SetCookie(String name, String value, String domain, String path, long maxAge, boolean isSecure, boolean isHttpOnly) { // Check arguments if (name == null || name.length() == 0) @@ -272,11 +374,11 @@ public class Response implements HttpServletResponse StringBuilder buf = __cookieBuilder.get(); buf.setLength(0); buf.append(name).append('=').append(value==null?"":value); - + // Append path if (path!=null && path.length()>0) buf.append(";Path=").append(path); - + // Append domain if (domain!=null && domain.length()>0) buf.append(";Domain=").append(domain); @@ -301,15 +403,9 @@ public class Response implements HttpServletResponse buf.append(";Secure"); if (isHttpOnly) buf.append(";HttpOnly"); - - // add the set cookie - _fields.add(HttpHeader.SET_COOKIE, buf.toString()); - - // Expire responses with set-cookie headers so they do not get cached. - _fields.put(__EXPIRES_01JAN1970); - + return buf.toString(); } - + /** * Format a set cookie value * @@ -333,6 +429,17 @@ public class Response implements HttpServletResponse final boolean isSecure, final boolean isHttpOnly, int version) + { + String set_cookie = newRFC2965SetCookie(name, value, domain, path, maxAge, comment, isSecure, isHttpOnly, version); + + // add the set cookie + _fields.add(HttpHeader.SET_COOKIE, set_cookie); + + // Expire responses with set-cookie headers so they do not get cached. + _fields.put(__EXPIRES_01JAN1970); + } + + private String newRFC2965SetCookie(String name, String value, String domain, String path, long maxAge, String comment, boolean isSecure, boolean isHttpOnly, int version) { // Check arguments if (name == null || name.length() == 0) @@ -347,7 +454,7 @@ public class Response implements HttpServletResponse quoteOnlyOrAppend(buf,name,quote_name); buf.append('='); - + // Append the value boolean quote_value=isQuoteNeededForCookie(value); quoteOnlyOrAppend(buf,value,quote_value); @@ -413,12 +520,7 @@ public class Response implements HttpServletResponse buf.append(";Comment="); quoteOnlyOrAppend(buf,comment,isQuoteNeededForCookie(comment)); } - - // add the set cookie - _fields.add(HttpHeader.SET_COOKIE, buf.toString()); - - // Expire responses with set-cookie headers so they do not get cached. - _fields.put(__EXPIRES_01JAN1970); + return buf.toString(); } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java index 7ca34c3a9b4..808882a8576 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java @@ -387,15 +387,18 @@ public class Server extends HandlerWrapper implements Attributes } // start connectors last - for (Connector connector : _connectors) + if (mex.size()==0) { - try + for (Connector connector : _connectors) { - connector.start(); - } - catch(Throwable e) - { - mex.add(e); + try + { + connector.start(); + } + catch (Throwable e) + { + mex.add(e); + } } } 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 236354be226..b2fd1f42cb4 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 @@ -1148,7 +1148,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu case UNAVAILABLE: baseRequest.setHandled(true); response.sendError(HttpServletResponse.SC_SERVICE_UNAVAILABLE); - return false; + return true; default: if ((DispatcherType.REQUEST.equals(dispatch) && baseRequest.isHandled())) return false; diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java index 84088146cd3..2820dca3a0c 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java @@ -38,6 +38,7 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.MimeTypes; +import org.eclipse.jetty.http.QuotedQualityCSV; import org.eclipse.jetty.server.Dispatcher; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; @@ -159,7 +160,7 @@ public class ErrorHandler extends AbstractHandler protected void generateAcceptableResponse(Request baseRequest, HttpServletRequest request, HttpServletResponse response, int code, String message) throws IOException { - List acceptable=baseRequest.getHttpFields().getQualityCSV(HttpHeader.ACCEPT); + List acceptable=baseRequest.getHttpFields().getQualityCSV(HttpHeader.ACCEPT, QuotedQualityCSV.MOST_SPECIFIC_MIME_ORDERING); if (acceptable.isEmpty() && !baseRequest.getHttpFields().contains(HttpHeader.ACCEPT)) { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/HouseKeeper.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/HouseKeeper.java index 82bc5e1f4d1..6e5faaa6b05 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/HouseKeeper.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/HouseKeeper.java @@ -121,7 +121,7 @@ public class HouseKeeper extends AbstractLifeCycle if (_scheduler == null) { - _scheduler = new ScheduledExecutorScheduler(); + _scheduler = new ScheduledExecutorScheduler(String.format("Session-HouseKeeper-%x",hashCode()),false); _ownScheduler = true; _scheduler.start(); if (LOG.isDebugEnabled()) LOG.debug("Using own scheduler for scavenging"); 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 dcd2037af3a..c2263af8ff4 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 @@ -18,8 +18,6 @@ package org.eclipse.jetty.server.session; -import static java.lang.Math.round; - import java.io.IOException; import java.util.Arrays; import java.util.Collections; @@ -67,6 +65,8 @@ import org.eclipse.jetty.util.thread.Locker.Lock; import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler; import org.eclipse.jetty.util.thread.Scheduler; +import static java.lang.Math.round; + /* ------------------------------------------------------------ */ /** * SessionHandler. @@ -523,10 +523,9 @@ public class SessionHandler extends ScopedHandler _scheduler = server.getBean(Scheduler.class); if (_scheduler == null) { - _scheduler = new ScheduledExecutorScheduler(); + _scheduler = new ScheduledExecutorScheduler(String.format("Session-Scheduler-%x",hashCode()), false); _ownScheduler = true; _scheduler.start(); - } } @@ -1658,7 +1657,7 @@ public class SessionHandler extends ScopedHandler HttpCookie cookie = access(existingSession,request.isSecure()); // Handle changed ID or max-age refresh, but only if this is not a redispatched request if ((cookie != null) && (request.getDispatcherType() == DispatcherType.ASYNC || request.getDispatcherType() == DispatcherType.REQUEST)) - baseRequest.getResponse().addCookie(cookie); + baseRequest.getResponse().replaceCookie(cookie); } if (LOG.isDebugEnabled()) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/GracefulStopTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/GracefulStopTest.java index b6a83d254a7..61226f45fb2 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/GracefulStopTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/GracefulStopTest.java @@ -18,18 +18,6 @@ package org.eclipse.jetty.server; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.lessThan; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.notNullValue; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.condition.OS.WINDOWS; - import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; @@ -54,18 +42,32 @@ import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.server.LocalConnector.LocalEndPoint; import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.server.handler.ContextHandler; +import org.eclipse.jetty.server.handler.ContextHandlerCollection; import org.eclipse.jetty.server.handler.StatisticsHandler; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.hamcrest.Matcher; import org.hamcrest.Matchers; - import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledOnOs; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.lessThan; +import static org.hamcrest.Matchers.notNullValue; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.condition.OS.WINDOWS; + public class GracefulStopTest { /** @@ -657,6 +659,60 @@ public class GracefulStopTest assertThat(response,Matchers.not(Matchers.containsString("Connection: close"))); assertTrue(latch.await(10,TimeUnit.SECONDS)); } + + @Test + public void testFailedStart() + { + Server server= new Server(); + + LocalConnector connector = new LocalConnector(server); + server.addConnector(connector); + + ContextHandlerCollection contexts = new ContextHandlerCollection(); + server.setHandler(contexts); + AtomicBoolean context0Started = new AtomicBoolean(false); + ContextHandler context0 = new ContextHandler("/zero") + { + @Override + protected void doStart() throws Exception + { + context0Started.set(true); + } + }; + ContextHandler context1 = new ContextHandler("/one") + { + @Override + protected void doStart() throws Exception + { + throw new Exception("Test start failure"); + } + }; + AtomicBoolean context2Started = new AtomicBoolean(false); + ContextHandler context2 = new ContextHandler("/two") + { + @Override + protected void doStart() throws Exception + { + context2Started.set(true); + } + }; + contexts.setHandlers(new Handler[]{context0, context1, context2}); + + try + { + server.start(); + fail(); + } + catch(Exception e) + { + assertThat(e.getMessage(),is("Test start failure")); + } + + assertTrue(server.getContainedBeans(LifeCycle.class).stream().noneMatch(LifeCycle::isRunning)); + assertTrue(server.getContainedBeans(LifeCycle.class).stream().anyMatch(LifeCycle::isFailed)); + assertTrue(context0Started.get()); + assertFalse(context2Started.get()); + } static class NoopHandler extends AbstractHandler { diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java index 812519b8210..ebf60e994ba 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java @@ -21,6 +21,7 @@ package org.eclipse.jetty.server; import static java.nio.charset.StandardCharsets.UTF_8; import static org.hamcrest.CoreMatchers.allOf; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; @@ -37,7 +38,6 @@ import java.io.IOException; import java.io.InputStreamReader; import java.io.LineNumberReader; import java.io.PrintWriter; -import java.net.HttpCookie; import java.net.Inet4Address; import java.net.InetAddress; import java.net.InetSocketAddress; @@ -59,6 +59,7 @@ import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; import org.eclipse.jetty.http.CookieCompliance; +import org.eclipse.jetty.http.HttpCookie; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; @@ -1019,7 +1020,7 @@ public class ResponseTest @Test public void testAddCookie_JavaNet() throws Exception { - HttpCookie cookie = new HttpCookie("foo", URLEncoder.encode("bar;baz", UTF_8.toString())); + java.net.HttpCookie cookie = new java.net.HttpCookie("foo", URLEncoder.encode("bar;baz", UTF_8.toString())); cookie.setPath("/secure"); assertEquals("foo=\"bar%3Bbaz\";$Path=\"/secure\"", cookie.toString()); @@ -1061,6 +1062,38 @@ public class ResponseTest assertFalse(set.hasMoreElements()); } + @Test + public void testReplaceHttpCookie() + { + Response response = getResponse(); + + response.replaceCookie(new HttpCookie("Foo","123456")); + response.replaceCookie(new HttpCookie("Foo","123456", "A", "/path")); + response.replaceCookie(new HttpCookie("Foo","123456", "B", "/path")); + + response.replaceCookie(new HttpCookie("Bar","123456")); + response.replaceCookie(new HttpCookie("Bar","123456",null, "/left")); + response.replaceCookie(new HttpCookie("Bar","123456", null, "/right")); + + response.replaceCookie(new HttpCookie("Bar","value", null, "/right")); + response.replaceCookie(new HttpCookie("Bar","value",null, "/left")); + response.replaceCookie(new HttpCookie("Bar","value")); + + response.replaceCookie(new HttpCookie("Foo","value", "B", "/path")); + response.replaceCookie(new HttpCookie("Foo","value", "A", "/path")); + response.replaceCookie(new HttpCookie("Foo","value")); + + assertThat(Collections.list(response.getHttpFields().getValues("Set-Cookie")), + contains( + "Foo=value", + "Foo=value;Path=/path;Domain=A", + "Foo=value;Path=/path;Domain=B", + "Bar=value", + "Bar=value;Path=/left", + "Bar=value;Path=/right" + )); + } + @Test public void testFlushAfterFullContent() throws Exception { diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/DoSFilter.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/DoSFilter.java index c37d529a890..7e0cc09fbc9 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/DoSFilter.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/DoSFilter.java @@ -281,7 +281,7 @@ public class DoSFilter implements Filter { try { - Scheduler result = new ScheduledExecutorScheduler(); + Scheduler result = new ScheduledExecutorScheduler(String.format("DoS-Scheduler-%x",hashCode()),false); result.start(); return result; } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java index ad1c4d4aceb..9f5483eac8a 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java @@ -96,35 +96,64 @@ public class ContainerLifeCycle extends AbstractLifeCycle implements Container, _doStarted = true; // start our managed and auto beans - for (Bean b : _beans) + try { - if (b._bean instanceof LifeCycle) + for (Bean b : _beans) { - LifeCycle l = (LifeCycle)b._bean; - switch(b._managed) + if (b._bean instanceof LifeCycle) { - case MANAGED: - if (!l.isRunning()) - start(l); - break; - - case AUTO: - if (l.isRunning()) - unmanage(b); - else - { - manage(b); - start(l); - } - break; - - default: - break; + LifeCycle l = (LifeCycle)b._bean; + switch (b._managed) + { + case MANAGED: + if (!l.isRunning()) + start(l); + break; + + case AUTO: + if (l.isRunning()) + unmanage(b); + else + { + manage(b); + start(l); + } + break; + + default: + break; + } } } - } - super.doStart(); + super.doStart(); + } + catch (Throwable t) + { + // on failure, stop any managed components that have been started + List reverse = new ArrayList<>(_beans); + Collections.reverse(reverse); + for (Bean b : reverse) + { + if (b._bean instanceof LifeCycle && b._managed == Managed.MANAGED) + { + LifeCycle l = (LifeCycle)b._bean; + if (l.isRunning()) + { + try + { + l.stop(); + } + catch(Throwable t2) + { + if (t2!=t) + t.addSuppressed(t2); + } + } + } + } + throw t; + } } /** diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/LazyListTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/LazyListTest.java index 28a49387955..0fac1551b25 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/LazyListTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/LazyListTest.java @@ -152,19 +152,19 @@ public class LazyListTest Object list=null; list=LazyList.add(list, null); assertEquals(1,LazyList.size(list)); - assertEquals(null,LazyList.get(list,0)); + assertNull(LazyList.get(list,0)); list="a"; list=LazyList.add(list, null); assertEquals(2,LazyList.size(list)); assertEquals(LazyList.get(list, 0), "a"); - assertEquals(null,LazyList.get(list,1)); + assertNull(LazyList.get(list,1)); list=LazyList.add(list, null); assertEquals(3,LazyList.size(list)); assertEquals(LazyList.get(list, 0), "a"); - assertEquals(null,LazyList.get(list,1)); - assertEquals(null,LazyList.get(list,2)); + assertNull(LazyList.get(list,1)); + assertNull(LazyList.get(list,2)); } /** @@ -254,7 +254,7 @@ public class LazyListTest Object list = LazyList.add(input, 0, null); assertNotNull(list); assertEquals(2,LazyList.size(list)); - assertEquals(null, LazyList.get(list,0)); + assertNull( LazyList.get(list,0)); assertEquals(LazyList.get(list, 1), "a"); } diff --git a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/annotations/OnMessageBinaryCallable.java b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/annotations/OnMessageBinaryCallable.java index a6d7f3a486b..1271fb13ee0 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/annotations/OnMessageBinaryCallable.java +++ b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/annotations/OnMessageBinaryCallable.java @@ -20,10 +20,8 @@ package org.eclipse.jetty.websocket.jsr356.annotations; import java.lang.reflect.Method; import java.nio.ByteBuffer; - import javax.websocket.DecodeException; import javax.websocket.Decoder; -import javax.websocket.OnMessage; import org.eclipse.jetty.websocket.jsr356.JsrSession; import org.eclipse.jetty.websocket.jsr356.annotations.Param.Role; @@ -55,12 +53,20 @@ public class OnMessageBinaryCallable extends OnMessageCallable public Object call(Object endpoint, ByteBuffer buf, boolean partialFlag) throws DecodeException { - super.args[idxMessageObject] = binaryDecoder.decode(buf); - if (idxPartialMessageFlag >= 0) + if (binaryDecoder.willDecode(buf.slice())) { - super.args[idxPartialMessageFlag] = partialFlag; + super.args[idxMessageObject] = binaryDecoder.decode(buf); + if (idxPartialMessageFlag >= 0) + { + super.args[idxPartialMessageFlag] = partialFlag; + } + return super.call(endpoint, super.args); + } + else + { + // Per JSR356, if you cannot decode, discard the message. + return null; } - return super.call(endpoint,super.args); } @Override diff --git a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/annotations/OnMessageTextCallable.java b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/annotations/OnMessageTextCallable.java index 82a8ced5eb2..9e245aca857 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/annotations/OnMessageTextCallable.java +++ b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/annotations/OnMessageTextCallable.java @@ -18,12 +18,9 @@ package org.eclipse.jetty.websocket.jsr356.annotations; -import java.io.Reader; import java.lang.reflect.Method; - import javax.websocket.DecodeException; import javax.websocket.Decoder; -import javax.websocket.OnMessage; import org.eclipse.jetty.websocket.jsr356.JsrSession; import org.eclipse.jetty.websocket.jsr356.annotations.Param.Role; @@ -55,12 +52,20 @@ public class OnMessageTextCallable extends OnMessageCallable public Object call(Object endpoint, String str, boolean partialFlag) throws DecodeException { - super.args[idxMessageObject] = textDecoder.decode(str); - if (idxPartialMessageFlag >= 0) + if (textDecoder.willDecode(str)) { - super.args[idxPartialMessageFlag] = partialFlag; + super.args[idxMessageObject] = textDecoder.decode(str); + if (idxPartialMessageFlag >= 0) + { + super.args[idxPartialMessageFlag] = partialFlag; + } + return super.call(endpoint, super.args); + } + else + { + // Per JSR356, if you cannot decode, discard the message. + return null; } - return super.call(endpoint,super.args); } @Override diff --git a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/messages/BinaryWholeMessage.java b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/messages/BinaryWholeMessage.java index e64d9b1a2a8..d4086fae34a 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/messages/BinaryWholeMessage.java +++ b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/messages/BinaryWholeMessage.java @@ -18,6 +18,7 @@ package org.eclipse.jetty.websocket.jsr356.messages; +import java.nio.ByteBuffer; import javax.websocket.DecodeException; import javax.websocket.Decoder; import javax.websocket.Decoder.Binary; @@ -54,14 +55,19 @@ public class BinaryWholeMessage extends SimpleBinaryMessage DecoderFactory.Wrapper decoder = msgWrapper.getDecoder(); Decoder.Binary binaryDecoder = (Binary)decoder.getDecoder(); - try + ByteBuffer msg = BufferUtil.toBuffer(data); + + if (binaryDecoder.willDecode(msg.slice())) { - Object obj = binaryDecoder.decode(BufferUtil.toBuffer(data)); - wholeHandler.onMessage(obj); - } - catch (DecodeException e) - { - throw new WebSocketException("Unable to decode binary data",e); + try + { + Object obj = binaryDecoder.decode(msg); + wholeHandler.onMessage(obj); + } + catch (DecodeException e) + { + throw new WebSocketException("Unable to decode binary data", e); + } } } } diff --git a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/messages/TextWholeMessage.java b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/messages/TextWholeMessage.java index 8f11d96c2a2..e3c1ca67be4 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/messages/TextWholeMessage.java +++ b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/messages/TextWholeMessage.java @@ -50,14 +50,18 @@ public class TextWholeMessage extends SimpleTextMessage DecoderFactory.Wrapper decoder = msgWrapper.getDecoder(); Decoder.Text textDecoder = (Decoder.Text)decoder.getDecoder(); - try + String msg = utf.toString(); + if (textDecoder.willDecode(msg)) { - Object obj = textDecoder.decode(utf.toString()); - wholeHandler.onMessage(obj); - } - catch (DecodeException e) - { - throw new WebSocketException("Unable to decode text data",e); + try + { + Object obj = textDecoder.decode(msg); + wholeHandler.onMessage(obj); + } + catch (DecodeException e) + { + throw new WebSocketException("Unable to decode text data", e); + } } } } diff --git a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/EchoTest.java b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/EchoTest.java index 21afb1e585b..1b0d2fb8ccd 100644 --- a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/EchoTest.java +++ b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/EchoTest.java @@ -18,9 +18,6 @@ package org.eclipse.jetty.websocket.jsr356.server; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; - import java.io.File; import java.net.URI; import java.util.ArrayList; @@ -28,7 +25,6 @@ import java.util.List; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; import java.util.stream.Stream; - import javax.websocket.ContainerProvider; import javax.websocket.WebSocketContainer; @@ -62,12 +58,17 @@ import org.eclipse.jetty.websocket.jsr356.server.samples.streaming.InputStreamSo import org.eclipse.jetty.websocket.jsr356.server.samples.streaming.ReaderParamSocket; import org.eclipse.jetty.websocket.jsr356.server.samples.streaming.ReaderSocket; import org.eclipse.jetty.websocket.jsr356.server.samples.streaming.StringReturnReaderParamSocket; +import org.hamcrest.Matcher; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.nullValue; + public class EchoTest { private static final List TESTCASES = new ArrayList<>(); @@ -84,7 +85,7 @@ public class EchoTest EchoCase.add(TESTCASES,BooleanTextSocket.class).addMessage(Boolean.FALSE).expect("false"); EchoCase.add(TESTCASES,BooleanTextSocket.class).addMessage("true").expect("true"); EchoCase.add(TESTCASES,BooleanTextSocket.class).addMessage("TRUe").expect("true"); - EchoCase.add(TESTCASES,BooleanTextSocket.class).addMessage("Apple").expect("false"); + EchoCase.add(TESTCASES,BooleanTextSocket.class).addMessage("Apple").expect(null); // fails willDecode EchoCase.add(TESTCASES,BooleanTextSocket.class).addMessage("false").expect("false"); EchoCase.add(TESTCASES,BooleanObjectTextSocket.class).addMessage(true).expect("true"); @@ -278,7 +279,8 @@ public class EchoTest for (String expected : testcase.expectedStrings) { String actual = received.poll(Timeouts.POLL_EVENT, Timeouts.POLL_EVENT_UNIT); - assertThat("Received Echo Responses",actual,containsString(expected)); + Matcher expectation = expected == null ? nullValue() : containsString(expected); + assertThat("Received Echo Responses", actual, expectation); } } finally diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/extensions/compress/PerMessageDeflateExtension.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/extensions/compress/PerMessageDeflateExtension.java index 6b8202256a5..38069b0ff2f 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/extensions/compress/PerMessageDeflateExtension.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/extensions/compress/PerMessageDeflateExtension.java @@ -25,6 +25,7 @@ import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.websocket.api.BadPayloadException; import org.eclipse.jetty.websocket.api.BatchMode; +import org.eclipse.jetty.websocket.api.ProtocolException; import org.eclipse.jetty.websocket.api.WriteCallback; import org.eclipse.jetty.websocket.api.extensions.ExtensionConfig; import org.eclipse.jetty.websocket.api.extensions.Frame; @@ -70,6 +71,12 @@ public class PerMessageDeflateExtension extends CompressExtension nextIncomingFrame(frame); return; } + + if (frame.getOpCode() == OpCode.CONTINUATION && frame.isRsv1()) + { + // Per RFC7692 we MUST Fail the websocket connection + throw new ProtocolException("Invalid RSV1 set on permessage-deflate CONTINUATION frame"); + } ByteAccumulator accumulator = newByteAccumulator(); diff --git a/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/extensions/FragmentExtensionTest.java b/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/extensions/FragmentExtensionTest.java index 1ae732538bd..eaa7c92c017 100644 --- a/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/extensions/FragmentExtensionTest.java +++ b/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/extensions/FragmentExtensionTest.java @@ -18,19 +18,14 @@ package org.eclipse.jetty.websocket.common.extensions; -import static org.hamcrest.MatcherAssert.assertThat; -import static java.nio.charset.StandardCharsets.UTF_8; -import static java.util.concurrent.TimeUnit.SECONDS; -import static org.hamcrest.Matchers.is; - import java.io.IOException; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; -import java.util.LinkedList; import java.util.List; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.LinkedBlockingDeque; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.MappedByteBufferPool; @@ -52,9 +47,13 @@ import org.eclipse.jetty.websocket.common.io.FutureWriteCallback; import org.eclipse.jetty.websocket.common.test.ByteBufferAssert; import org.eclipse.jetty.websocket.common.test.IncomingFramesCapture; import org.eclipse.jetty.websocket.common.test.OutgoingFramesCapture; - import org.junit.jupiter.api.Test; +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + @SuppressWarnings("Duplicates") public class FragmentExtensionTest { @@ -155,7 +154,7 @@ public class FragmentExtensionTest * @throws IOException on test failure */ @Test - public void testOutgoingFramesByMaxLength() throws IOException + public void testOutgoingFramesByMaxLength() throws IOException, InterruptedException { OutgoingFramesCapture capture = new OutgoingFramesCapture(); @@ -197,11 +196,11 @@ public class FragmentExtensionTest capture.assertFrameCount(len); String prefix; - LinkedList frames = capture.getFrames(); + LinkedBlockingDeque frames = capture.getFrames(); for (int i = 0; i < len; i++) { prefix = "Frame[" + i + "]"; - WebSocketFrame actualFrame = frames.get(i); + WebSocketFrame actualFrame = frames.poll(1, SECONDS); WebSocketFrame expectedFrame = expectedFrames.get(i); // System.out.printf("actual: %s%n",actualFrame); @@ -266,11 +265,11 @@ public class FragmentExtensionTest capture.assertFrameCount(len); String prefix; - LinkedList frames = capture.getFrames(); + LinkedBlockingDeque frames = capture.getFrames(); for (int i = 0; i < len; i++) { prefix = "Frame[" + i + "]"; - WebSocketFrame actualFrame = frames.get(i); + WebSocketFrame actualFrame = frames.poll(1, SECONDS); WebSocketFrame expectedFrame = expectedFrames.get(i); // Validate Frame diff --git a/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/extensions/compress/PerMessageDeflateExtensionTest.java b/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/extensions/compress/PerMessageDeflateExtensionTest.java index 831ce477813..f1c131db729 100644 --- a/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/extensions/compress/PerMessageDeflateExtensionTest.java +++ b/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/extensions/compress/PerMessageDeflateExtensionTest.java @@ -18,20 +18,20 @@ package org.eclipse.jetty.websocket.common.extensions.compress; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.MatcherAssert.assertThat; - import java.io.IOException; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.TimeUnit; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.MappedByteBufferPool; +import org.eclipse.jetty.toolchain.test.ByteBufferAssert; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.websocket.api.BatchMode; +import org.eclipse.jetty.websocket.api.ProtocolException; import org.eclipse.jetty.websocket.api.WebSocketPolicy; import org.eclipse.jetty.websocket.api.extensions.ExtensionConfig; import org.eclipse.jetty.websocket.api.extensions.Frame; @@ -42,12 +42,14 @@ import org.eclipse.jetty.websocket.common.extensions.ExtensionTool.Tester; import org.eclipse.jetty.websocket.common.frames.ContinuationFrame; import org.eclipse.jetty.websocket.common.frames.PingFrame; import org.eclipse.jetty.websocket.common.frames.TextFrame; -import org.eclipse.jetty.websocket.common.test.ByteBufferAssert; import org.eclipse.jetty.websocket.common.test.IncomingFramesCapture; import org.eclipse.jetty.websocket.common.test.OutgoingFramesCapture; - import org.junit.jupiter.api.Test; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertThrows; + /** * Client side behavioral tests for permessage-deflate extension. *

@@ -224,6 +226,56 @@ public class PerMessageDeflateExtensionTest extends AbstractExtensionTest tester.assertHasFrames("Hello"); } + /** + * Decode fragmented message (3 parts: TEXT, CONTINUATION, CONTINUATION) + */ + @Test + public void testParseFragmentedMessage_Good() + { + Tester tester = clientExtensions.newTester("permessage-deflate"); + + tester.assertNegotiated("permessage-deflate"); + + tester.parseIncomingHex(// 1 message, 3 frame + "410C", // HEADER TEXT / fin=false / rsv1=true + "F248CDC9C95700000000FFFF", + "000B", // HEADER CONTINUATION / fin=false / rsv1=false + "0ACF2FCA4901000000FFFF", + "8003", // HEADER CONTINUATION / fin=true / rsv1=false + "520400" + ); + + Frame txtFrame = new TextFrame().setPayload("Hello ").setFin(false); + Frame con1Frame = new ContinuationFrame().setPayload("World").setFin(false); + Frame con2Frame = new ContinuationFrame().setPayload("!").setFin(true); + + tester.assertHasFrames(txtFrame, con1Frame, con2Frame); + } + + /** + * Decode fragmented message (3 parts: TEXT, CONTINUATION, CONTINUATION) + *

+ * Continuation frames have RSV1 set, which MUST result in Failure + *

+ */ + @Test + public void testParseFragmentedMessage_BadRsv1() + { + Tester tester = clientExtensions.newTester("permessage-deflate"); + + tester.assertNegotiated("permessage-deflate"); + + assertThrows(ProtocolException.class, () -> + tester.parseIncomingHex(// 1 message, 3 frame + "410C", // Header TEXT / fin=false / rsv1=true + "F248CDC9C95700000000FFFF", // Payload + "400B", // Header CONTINUATION / fin=false / rsv1=true + "0ACF2FCA4901000000FFFF", // Payload + "C003", // Header CONTINUATION / fin=true / rsv1=true + "520400" // Payload + )); + } + /** * Incoming PING (Control Frame) should pass through extension unmodified */ @@ -261,6 +313,44 @@ public class PerMessageDeflateExtensionTest extends AbstractExtensionTest ByteBufferAssert.assertEquals("Frame.payload", expected, actual.getPayload().slice()); } + /** + * Incoming Text Message fragmented into 3 pieces. + */ + @Test + public void testIncomingFragmented() + { + PerMessageDeflateExtension ext = new PerMessageDeflateExtension(); + ext.setBufferPool(bufferPool); + ext.setPolicy(WebSocketPolicy.newServerPolicy()); + ExtensionConfig config = ExtensionConfig.parse("permessage-deflate"); + ext.setConfig(config); + + // Setup capture of incoming frames + IncomingFramesCapture capture = new IncomingFramesCapture(); + + // Wire up stack + ext.setNextIncomingFrames(capture); + + String payload = "Are you there?"; + Frame ping = new PingFrame().setPayload(payload); + ext.incomingFrame(ping); + + capture.assertFrameCount(1); + capture.assertHasFrame(OpCode.PING, 1); + WebSocketFrame actual = capture.getFrames().poll(); + + assertThat("Frame.opcode", actual.getOpCode(), is(OpCode.PING)); + assertThat("Frame.fin", actual.isFin(), is(true)); + assertThat("Frame.rsv1", actual.isRsv1(), is(false)); + assertThat("Frame.rsv2", actual.isRsv2(), is(false)); + assertThat("Frame.rsv3", actual.isRsv3(), is(false)); + + ByteBuffer expected = BufferUtil.toBuffer(payload, StandardCharsets.UTF_8); + assertThat("Frame.payloadLength", actual.getPayloadLength(), is(expected.remaining())); + ByteBufferAssert.assertEquals("Frame.payload", expected, actual.getPayload().slice()); + } + + /** * Verify that incoming uncompressed frames are properly passed through */ @@ -356,6 +446,58 @@ public class PerMessageDeflateExtensionTest extends AbstractExtensionTest ByteBufferAssert.assertEquals("Frame.payload", expected, actual.getPayload().slice()); } + /** + * Outgoing Fragmented Message + * @throws IOException on test failure + */ + @Test + public void testOutgoingFragmentedMessage() throws IOException, InterruptedException + { + PerMessageDeflateExtension ext = new PerMessageDeflateExtension(); + ext.setBufferPool(bufferPool); + ext.setPolicy(WebSocketPolicy.newServerPolicy()); + ExtensionConfig config = ExtensionConfig.parse("permessage-deflate"); + ext.setConfig(config); + + // Setup capture of outgoing frames + OutgoingFramesCapture capture = new OutgoingFramesCapture(); + + // Wire up stack + ext.setNextOutgoingFrames(capture); + + Frame txtFrame = new TextFrame().setPayload("Hello ").setFin(false); + Frame con1Frame = new ContinuationFrame().setPayload("World").setFin(false); + Frame con2Frame = new ContinuationFrame().setPayload("!").setFin(true); + ext.outgoingFrame(txtFrame, null, BatchMode.OFF); + ext.outgoingFrame(con1Frame, null, BatchMode.OFF); + ext.outgoingFrame(con2Frame, null, BatchMode.OFF); + + capture.assertFrameCount(3); + + WebSocketFrame capturedFrame; + + capturedFrame = capture.getFrames().poll(1, TimeUnit.SECONDS); + assertThat("Frame.opcode", capturedFrame.getOpCode(), is(OpCode.TEXT)); + assertThat("Frame.fin", capturedFrame.isFin(), is(false)); + assertThat("Frame.rsv1", capturedFrame.isRsv1(), is(true)); + assertThat("Frame.rsv2", capturedFrame.isRsv2(), is(false)); + assertThat("Frame.rsv3", capturedFrame.isRsv3(), is(false)); + + capturedFrame = capture.getFrames().poll(1, TimeUnit.SECONDS); + assertThat("Frame.opcode", capturedFrame.getOpCode(), is(OpCode.CONTINUATION)); + assertThat("Frame.fin", capturedFrame.isFin(), is(false)); + assertThat("Frame.rsv1", capturedFrame.isRsv1(), is(false)); + assertThat("Frame.rsv2", capturedFrame.isRsv2(), is(false)); + assertThat("Frame.rsv3", capturedFrame.isRsv3(), is(false)); + + capturedFrame = capture.getFrames().poll(1, TimeUnit.SECONDS); + assertThat("Frame.opcode", capturedFrame.getOpCode(), is(OpCode.CONTINUATION)); + assertThat("Frame.fin", capturedFrame.isFin(), is(true)); + assertThat("Frame.rsv1", capturedFrame.isRsv1(), is(false)); + assertThat("Frame.rsv2", capturedFrame.isRsv2(), is(false)); + assertThat("Frame.rsv3", capturedFrame.isRsv3(), is(false)); + } + @Test public void testPyWebSocket_Client_NoContextTakeover_ThreeOra() { diff --git a/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/test/OutgoingFramesCapture.java b/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/test/OutgoingFramesCapture.java index ce0960545b9..1a951efeabd 100644 --- a/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/test/OutgoingFramesCapture.java +++ b/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/test/OutgoingFramesCapture.java @@ -18,11 +18,7 @@ package org.eclipse.jetty.websocket.common.test; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.greaterThanOrEqualTo; -import static org.hamcrest.Matchers.is; - -import java.util.LinkedList; +import java.util.concurrent.LinkedBlockingDeque; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.websocket.api.BatchMode; @@ -32,10 +28,14 @@ import org.eclipse.jetty.websocket.api.extensions.OutgoingFrames; import org.eclipse.jetty.websocket.common.OpCode; import org.eclipse.jetty.websocket.common.WebSocketFrame; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.is; + public class OutgoingFramesCapture implements OutgoingFrames { - private LinkedList frames = new LinkedList<>(); + private LinkedBlockingDeque frames = new LinkedBlockingDeque<>(); public void assertFrameCount(int expectedCount) { @@ -60,11 +60,12 @@ public class OutgoingFramesCapture implements OutgoingFrames public void dump() { System.out.printf("Captured %d outgoing writes%n",frames.size()); - for (int i = 0; i < frames.size(); i++) + int i=0; + for (WebSocketFrame frame: frames) { - Frame frame = frames.get(i); System.out.printf("[%3d] %s%n",i,frame); System.out.printf(" %s%n",BufferUtil.toDetailString(frame.getPayload())); + i++; } } @@ -81,7 +82,7 @@ public class OutgoingFramesCapture implements OutgoingFrames return count; } - public LinkedList getFrames() + public LinkedBlockingDeque getFrames() { return frames; } 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 b0bfec349db..f0197c862a1 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 @@ -30,6 +30,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.concurrent.Executor; + import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -86,7 +87,7 @@ public class WebSocketServerFactory extends ContainerLifeCycle implements WebSoc private final ClassLoader contextClassloader; private final Map handshakes = new HashMap<>(); // TODO: obtain shared (per server scheduler, somehow) - private final Scheduler scheduler = new ScheduledExecutorScheduler(); + private final Scheduler scheduler = new ScheduledExecutorScheduler(String.format("WebSocket-Scheduler-%x",hashCode()),false); private final List listeners = new ArrayList<>(); private final String supportedVersions; private final WebSocketPolicy defaultPolicy; diff --git a/pom.xml b/pom.xml index 14ca21dda91..822f5fce5ed 100644 --- a/pom.xml +++ b/pom.xml @@ -25,15 +25,16 @@ 1.2 1.1.3.v20160715 8.5.35.1 + 9.4.8.Final undefined - 1.4.1 + 2.0.0 7.0 1.21 benchmarks 1.2.0 1.1.5 - 5.3.1 + 5.4.0 3.6.0 1.3.1 3.1.0 @@ -1042,6 +1043,11 @@ slf4j-simple ${slf4j.version} + + org.jboss.logging + jboss-logging + 3.3.2.Final + com.github.jnr jnr-unixsocket diff --git a/tests/test-sessions/test-file-sessions/src/test/java/org/eclipse/jetty/server/session/FileSessionDataStoreTest.java b/tests/test-sessions/test-file-sessions/src/test/java/org/eclipse/jetty/server/session/FileSessionDataStoreTest.java index 1cc47506216..4f00115af41 100644 --- a/tests/test-sessions/test-file-sessions/src/test/java/org/eclipse/jetty/server/session/FileSessionDataStoreTest.java +++ b/tests/test-sessions/test-file-sessions/src/test/java/org/eclipse/jetty/server/session/FileSessionDataStoreTest.java @@ -21,6 +21,7 @@ package org.eclipse.jetty.server.session; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; /** * FileSessionDataStoreTest @@ -68,7 +69,17 @@ public class FileSessionDataStoreTest extends AbstractSessionDataStoreTest @Override public boolean checkSessionExists(SessionData data) throws Exception { - return (FileTestHelper.getFile(data.getId()) != null); + ClassLoader old = Thread.currentThread().getContextClassLoader(); + Thread.currentThread().setContextClassLoader (_contextClassLoader); + try + { + return (FileTestHelper.getFile(data.getId()) != null); + } + finally + { + Thread.currentThread().setContextClassLoader(old); + } + } /** @@ -77,7 +88,29 @@ public class FileSessionDataStoreTest extends AbstractSessionDataStoreTest @Override public boolean checkSessionPersisted(SessionData data) throws Exception { - return FileTestHelper.checkSessionPersisted(data); + ClassLoader old = Thread.currentThread().getContextClassLoader(); + Thread.currentThread().setContextClassLoader (_contextClassLoader); + try + { + return FileTestHelper.checkSessionPersisted(data); + } + catch (Throwable e) + { + e.printStackTrace(); + throw e; + } + finally + { + Thread.currentThread().setContextClassLoader(old); + } } + @Override + @Test + public void testStoreSession() throws Exception + { + super.testStoreSession(); + } + + } diff --git a/tests/test-sessions/test-gcloud-sessions/src/test/java/org/eclipse/jetty/gcloud/session/GCloudSessionDataStoreTest.java b/tests/test-sessions/test-gcloud-sessions/src/test/java/org/eclipse/jetty/gcloud/session/GCloudSessionDataStoreTest.java index bbdf1810263..f75b7577103 100644 --- a/tests/test-sessions/test-gcloud-sessions/src/test/java/org/eclipse/jetty/gcloud/session/GCloudSessionDataStoreTest.java +++ b/tests/test-sessions/test-gcloud-sessions/src/test/java/org/eclipse/jetty/gcloud/session/GCloudSessionDataStoreTest.java @@ -96,7 +96,16 @@ public class GCloudSessionDataStoreTest extends AbstractSessionDataStoreTest @Override public boolean checkSessionPersisted(SessionData data) throws Exception { - return __testSupport.checkSessionPersisted(data); + ClassLoader old = Thread.currentThread().getContextClassLoader(); + Thread.currentThread().setContextClassLoader (_contextClassLoader); + try + { + return __testSupport.checkSessionPersisted(data); + } + finally + { + Thread.currentThread().setContextClassLoader(old); + } } } diff --git a/tests/test-sessions/test-hazelcast-sessions/src/test/java/org/eclipse/jetty/hazelcast/session/HazelcastSessionDataStoreTest.java b/tests/test-sessions/test-hazelcast-sessions/src/test/java/org/eclipse/jetty/hazelcast/session/HazelcastSessionDataStoreTest.java index 527acb6de06..a19efa11ae5 100644 --- a/tests/test-sessions/test-hazelcast-sessions/src/test/java/org/eclipse/jetty/hazelcast/session/HazelcastSessionDataStoreTest.java +++ b/tests/test-sessions/test-hazelcast-sessions/src/test/java/org/eclipse/jetty/hazelcast/session/HazelcastSessionDataStoreTest.java @@ -152,6 +152,15 @@ public class HazelcastSessionDataStoreTest extends AbstractSessionDataStoreTest @Override public boolean checkSessionPersisted(SessionData data) throws Exception { - return _testHelper.checkSessionPersisted(data); + ClassLoader old = Thread.currentThread().getContextClassLoader(); + Thread.currentThread().setContextClassLoader (_contextClassLoader); + try + { + return _testHelper.checkSessionPersisted(data); + } + finally + { + Thread.currentThread().setContextClassLoader(old); + } } } diff --git a/tests/test-sessions/test-infinispan-sessions/pom.xml b/tests/test-sessions/test-infinispan-sessions/pom.xml index e82cf97f7d1..2b8b22b0649 100644 --- a/tests/test-sessions/test-infinispan-sessions/pom.xml +++ b/tests/test-sessions/test-infinispan-sessions/pom.xml @@ -106,19 +106,19 @@ org.infinispan infinispan-core - 9.1.0.Final + ${infinispan.version} test org.infinispan infinispan-client-hotrod - 9.1.0.Final + ${infinispan.version} test org.infinispan infinispan-remote-query-client - 9.1.0.Final + ${infinispan.version} test diff --git a/tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/ClusteredSessionScavengingTest.java b/tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/ClusteredSessionScavengingTest.java index a620ef73229..45cae2c2c2e 100644 --- a/tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/ClusteredSessionScavengingTest.java +++ b/tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/ClusteredSessionScavengingTest.java @@ -22,11 +22,11 @@ package org.eclipse.jetty.server.session; import org.eclipse.jetty.session.infinispan.InfinispanSessionDataStoreFactory; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; /** * ClusteredSessionScavengingTest * - * */ public class ClusteredSessionScavengingTest extends AbstractClusteredSessionScavengingTest { @@ -48,7 +48,15 @@ public class ClusteredSessionScavengingTest extends AbstractClusteredSessionScav } - /** + @Override + @Test + public void testClusteredScavenge() + throws Exception + { + super.testClusteredScavenge(); + } + + /** * @see org.eclipse.jetty.server.session.AbstractTestBase#createSessionDataStoreFactory() */ @Override diff --git a/tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/InfinispanSessionDataStoreTest.java b/tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/InfinispanSessionDataStoreTest.java index 4a3c2618c45..45d957efa55 100644 --- a/tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/InfinispanSessionDataStoreTest.java +++ b/tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/InfinispanSessionDataStoreTest.java @@ -152,7 +152,16 @@ public class InfinispanSessionDataStoreTest extends AbstractSessionDataStoreTest @Override public boolean checkSessionPersisted(SessionData data) throws Exception { - return __testSupport.checkSessionPersisted(data); + ClassLoader old = Thread.currentThread().getContextClassLoader(); + Thread.currentThread().setContextClassLoader (_contextClassLoader); + try + { + return __testSupport.checkSessionPersisted(data); + } + finally + { + Thread.currentThread().setContextClassLoader(old); + } } } diff --git a/tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/remote/RemoteInfinispanSessionDataStoreTest.java b/tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/remote/RemoteInfinispanSessionDataStoreTest.java index 40461797143..b51acf95aed 100644 --- a/tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/remote/RemoteInfinispanSessionDataStoreTest.java +++ b/tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/remote/RemoteInfinispanSessionDataStoreTest.java @@ -96,7 +96,16 @@ public class RemoteInfinispanSessionDataStoreTest extends AbstractSessionDataSto @Override public boolean checkSessionPersisted(SessionData data) throws Exception { - return __testSupport.checkSessionPersisted(data); + ClassLoader old = Thread.currentThread().getContextClassLoader(); + Thread.currentThread().setContextClassLoader (_contextClassLoader); + try + { + return __testSupport.checkSessionPersisted(data); + } + finally + { + Thread.currentThread().setContextClassLoader(old); + } } diff --git a/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/JDBCSessionDataStoreTest.java b/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/JDBCSessionDataStoreTest.java index e8fdf69803b..feca7bdd942 100644 --- a/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/JDBCSessionDataStoreTest.java +++ b/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/JDBCSessionDataStoreTest.java @@ -89,7 +89,17 @@ public class JDBCSessionDataStoreTest extends AbstractSessionDataStoreTest @Override public boolean checkSessionPersisted(SessionData data) throws Exception { - return JdbcTestHelper.checkSessionPersisted(data); + ClassLoader old = Thread.currentThread().getContextClassLoader(); + Thread.currentThread().setContextClassLoader (_contextClassLoader); + try + { + return JdbcTestHelper.checkSessionPersisted(data); + } + finally + { + Thread.currentThread().setContextClassLoader(old); + } + } - + } diff --git a/tests/test-sessions/test-mongodb-sessions/src/test/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStoreTest.java b/tests/test-sessions/test-mongodb-sessions/src/test/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStoreTest.java index 9c4d8dea0ad..b739ee5a102 100644 --- a/tests/test-sessions/test-mongodb-sessions/src/test/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStoreTest.java +++ b/tests/test-sessions/test-mongodb-sessions/src/test/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStoreTest.java @@ -92,7 +92,16 @@ public class MongoSessionDataStoreTest extends AbstractSessionDataStoreTest @Override public boolean checkSessionPersisted(SessionData data) throws Exception { - return MongoTestHelper.checkSessionPersisted(data); + ClassLoader old = Thread.currentThread().getContextClassLoader(); + Thread.currentThread().setContextClassLoader (_contextClassLoader); + try + { + return MongoTestHelper.checkSessionPersisted(data); + } + finally + { + Thread.currentThread().setContextClassLoader(old); + } } diff --git a/tests/test-sessions/test-mongodb-sessions/src/test/java/org/eclipse/jetty/nosql/mongodb/MongoTestHelper.java b/tests/test-sessions/test-mongodb-sessions/src/test/java/org/eclipse/jetty/nosql/mongodb/MongoTestHelper.java index e0ef31ff4cb..f09cf604a80 100644 --- a/tests/test-sessions/test-mongodb-sessions/src/test/java/org/eclipse/jetty/nosql/mongodb/MongoTestHelper.java +++ b/tests/test-sessions/test-mongodb-sessions/src/test/java/org/eclipse/jetty/nosql/mongodb/MongoTestHelper.java @@ -58,7 +58,7 @@ public class MongoTestHelper try { _mongoClient = - new MongoClient( System.getProperty( "embedmongo.host" ), Integer.getInteger( "embedmongoPort" ) ); + new MongoClient( System.getProperty( "embedmongo.host" ), Integer.getInteger( "embedmongoPort" ) ); } catch ( UnknownHostException e ) { 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 7dfbc2236eb..9e9b72d2364 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 @@ -61,7 +61,6 @@ public abstract class AbstractClusteredSessionScavengingTest extends AbstractTes - @Test public void testClusteredScavenge() throws Exception diff --git a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStoreTest.java b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStoreTest.java index 288597bf681..64740eb790c 100644 --- a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStoreTest.java +++ b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStoreTest.java @@ -28,13 +28,20 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; +import java.io.File; +import java.io.FileOutputStream; +import java.io.InputStream; import java.lang.reflect.Proxy; +import java.net.URL; +import java.net.URLClassLoader; import java.util.Arrays; import java.util.HashSet; import java.util.Set; import java.util.concurrent.TimeUnit; import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.toolchain.test.IO; +import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.junit.jupiter.api.Test; /** @@ -54,6 +61,7 @@ public abstract class AbstractSessionDataStoreTest public static final long ANCIENT_TIMESTAMP = 100L; public static final long RECENT_TIMESTAMP = System.currentTimeMillis() - TimeUnit.SECONDS.toMillis(3*GRACE_PERIOD_SEC); + protected URLClassLoader _contextClassLoader; @@ -70,31 +78,86 @@ public abstract class AbstractSessionDataStoreTest /** - * Test that the store can persist a session. + * Test that the store can persist a session. The session uses an attribute + * class that is only known to the webapp classloader. This tests that + * we use the webapp loader when we serialize the session data (ie save the session). * * @throws Exception */ @Test public void testStoreSession() throws Exception { + //Use a class that would only be known to the webapp classloader + InputStream foostream = Thread.currentThread().getContextClassLoader().getResourceAsStream("Foo.clazz"); + File foodir = new File (MavenTestingUtils.getTargetDir(), "foo"); + foodir.mkdirs(); + File fooclass = new File (foodir, "Foo.class"); + IO.copy(foostream, new FileOutputStream(fooclass)); + + assertTrue(fooclass.exists()); + assertTrue(fooclass.length() != 0); + + URL[] foodirUrls = new URL[]{foodir.toURI().toURL()}; + _contextClassLoader = new URLClassLoader(foodirUrls, Thread.currentThread().getContextClassLoader()); + //create the SessionDataStore ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); - context.setContextPath("/test"); + context.setContextPath("/test"); + //use the classloader with the special class in it + context.setClassLoader(_contextClassLoader); + SessionDataStoreFactory factory = createSessionDataStoreFactory(); ((AbstractSessionDataStoreFactory)factory).setGracePeriodSec(GRACE_PERIOD_SEC); SessionDataStore store = factory.getSessionDataStore(context.getSessionHandler()); SessionContext sessionContext = new SessionContext("foo", context.getServletContext()); store.initialize(sessionContext); - + store.start(); + + ClassLoader old = Thread.currentThread().getContextClassLoader(); + SessionData data = null; + try + { + Thread.currentThread().setContextClassLoader(_contextClassLoader); + Class fooclazz = Class.forName("Foo", true, _contextClassLoader); + //create a session + long now = System.currentTimeMillis(); + data = store.newSessionData("1234", 100, now, now-1, -1);//never expires + data.setLastNode(sessionContext.getWorkerName()); + + //Make an attribute that uses the class only known to the webapp classloader + data.setAttribute("a", fooclazz.getConstructor(null).newInstance()); + } + finally + { + Thread.currentThread().setContextClassLoader(old); + } + + //store the session, using a different thread to ensure + //that the thread is adorned with the webapp classloader + //before serialization + final SessionData finalData = data; - //create a session - long now = System.currentTimeMillis(); - SessionData data = store.newSessionData("1234", 100, now, now-1, -1);//never expires - data.setAttribute("a", "b"); - data.setLastNode(sessionContext.getWorkerName()); - - store.store("1234", data); + Runnable r = new Runnable() + { + + @Override + public void run() + { + try + { + store.store("1234", finalData); + } + catch (Exception e) + { + fail(e); + } + } + }; + + Thread t = new Thread(r, "saver"); + t.start(); + t.join(TimeUnit.SECONDS.toMillis(10)); //check that the store contains all of the session data assertTrue(checkSessionPersisted(data)); @@ -148,33 +211,31 @@ public abstract class AbstractSessionDataStoreTest * * @throws Exception */ - @Test - public void testStoreObjectAttributes() throws Exception - { - //create the SessionDataStore - ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); - context.setContextPath("/test"); - SessionDataStoreFactory factory = createSessionDataStoreFactory(); - ((AbstractSessionDataStoreFactory)factory).setGracePeriodSec(GRACE_PERIOD_SEC); - SessionDataStore store = factory.getSessionDataStore(context.getSessionHandler()); - SessionContext sessionContext = new SessionContext("foo", context.getServletContext()); - store.initialize(sessionContext); - - store.start(); - - //create a session - SessionData data = store.newSessionData("1234", 100, 200, 199, -1);//never expires - TestFoo testFoo = new TestFoo(); - testFoo.setInt(33); - FooInvocationHandler handler = new FooInvocationHandler(testFoo); - Foo foo = (Foo)Proxy.newProxyInstance(Thread.currentThread().getContextClassLoader(), new Class[] {Foo.class}, handler); - data.setAttribute("foo", foo); - data.setLastNode(sessionContext.getWorkerName()); - - //test that it can be persisted - store.store("1234", data); - checkSessionPersisted(data); - } + /* + * @Test public void testStoreObjectAttributes() throws Exception { //create + * the SessionDataStore ServletContextHandler context = new + * ServletContextHandler(ServletContextHandler.SESSIONS); + * context.setContextPath("/test"); SessionDataStoreFactory factory = + * createSessionDataStoreFactory(); + * ((AbstractSessionDataStoreFactory)factory).setGracePeriodSec( + * GRACE_PERIOD_SEC); SessionDataStore store = + * factory.getSessionDataStore(context.getSessionHandler()); SessionContext + * sessionContext = new SessionContext("foo", context.getServletContext()); + * store.initialize(sessionContext); + * + * store.start(); + * + * //create a session SessionData data = store.newSessionData("1234", 100, + * 200, 199, -1);//never expires TestFoo testFoo = new TestFoo(); + * testFoo.setInt(33); FooInvocationHandler handler = new + * FooInvocationHandler(testFoo); Foo foo = + * (Foo)Proxy.newProxyInstance(Thread.currentThread().getContextClassLoader( + * ), new Class[] {Foo.class}, handler); data.setAttribute("foo", foo); + * data.setLastNode(sessionContext.getWorkerName()); + * + * //test that it can be persisted store.store("1234", data); + * checkSessionPersisted(data); } + */ /** * Test that we can load a persisted session. diff --git a/tests/test-sessions/test-sessions-common/src/main/resources/Foo.clazz b/tests/test-sessions/test-sessions-common/src/main/resources/Foo.clazz new file mode 100644 index 00000000000..5de06e0ab64 Binary files /dev/null and b/tests/test-sessions/test-sessions-common/src/main/resources/Foo.clazz differ diff --git a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/RoleCheckPolicy.java b/tests/test-sessions/test-sessions-common/src/main/resources/Foo.java similarity index 56% rename from jetty-jaas/src/main/java/org/eclipse/jetty/jaas/RoleCheckPolicy.java rename to tests/test-sessions/test-sessions-common/src/main/resources/Foo.java index d83f647f9d6..08b57983041 100644 --- a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/RoleCheckPolicy.java +++ b/tests/test-sessions/test-sessions-common/src/main/resources/Foo.java @@ -16,20 +16,26 @@ // ======================================================================== // -package org.eclipse.jetty.jaas; - -import java.security.Principal; -import java.security.acl.Group; - -public interface RoleCheckPolicy +public class Foo implements java.io.Serializable { - /* ------------------------------------------------ */ - /** Check if a role is either a runAsRole or in a set of roles - * @param roleName the role to check - * @param runAsRole a pushed role (can be null) - * @param roles a Group whose Principals are role names - * @return true if role equals runAsRole or is a member of roles. - */ - public boolean checkRole (String roleName, Principal runAsRole, Group roles); - + int myI = 0; + + public Foo() + { + } + + public void setI(int i) + { + myI = i; + } + + public int getI() + { + return myI; + } + + public boolean equals(Object o) + { + return ((Foo)o).getI() == myI; + } } diff --git a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/CreationTest.java b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/CreationTest.java index dfc3a39a362..a9fd83e4d40 100644 --- a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/CreationTest.java +++ b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/CreationTest.java @@ -19,6 +19,7 @@ package org.eclipse.jetty.server.session; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -40,11 +41,13 @@ import javax.servlet.http.HttpSession; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.client.api.Request; +import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.StacklessLogging; +import org.hamcrest.Matchers; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -238,12 +241,60 @@ public class CreationTest server1.stop(); } } - - - - - - + + + + /** + * Create and then invalidate and then create a session in the same request + * @throws Exception + */ + @Test + public void testSessionCreateInvalidateCreate() throws Exception + { + String contextPath = ""; + String servletMapping = "/server"; + int inactivePeriod = 20; + int scavengePeriod = 3; + DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); + cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT); + SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory(); + TestServer server1 = new TestServer(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory); + TestServlet servlet = new TestServlet(); + ServletHolder holder = new ServletHolder(servlet); + ServletContextHandler contextHandler = server1.addContext(contextPath); + TestContextScopeListener scopeListener = new TestContextScopeListener(); + contextHandler.addEventListener(scopeListener); + contextHandler.addServlet(holder, servletMapping); + servlet.setStore(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore()); + server1.start(); + int port1 = server1.getPort(); + + try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session"))) + { + HttpClient client = new HttpClient(); + client.start(); + String url = "http://localhost:" + port1 + contextPath + servletMapping+"?action=createinvcreate&check=false"; + + CountDownLatch synchronizer = new CountDownLatch(1); + scopeListener.setExitSynchronizer(synchronizer); + + //make a request to set up a session on the server + ContentResponse response = client.GET(url); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + //ensure request has finished being handled + synchronizer.await(5, TimeUnit.SECONDS); + + //check that the session does not exist + assertTrue(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().exists(servlet._id)); + assertThat(response.getHeaders().getValuesList(HttpHeader.SET_COOKIE).size(), Matchers.is(1)); + } + finally + { + server1.stop(); + } + } + /** * Create a session in a context, forward to another context and create a * session in it too. Check that both sessions exist after the response @@ -437,6 +488,14 @@ public class CreationTest assertNull(request.getSession(false)); assertNotNull(session); } + else if ("createinvcreate".equals(action)) + { + session.invalidate(); + assertNull(request.getSession(false)); + assertNotNull(session); + session = request.getSession(true); + _id = session.getId(); + } } } } diff --git a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionRenewTest.java b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionRenewTest.java index 6258df9559e..f3bf17f56a6 100644 --- a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionRenewTest.java +++ b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionRenewTest.java @@ -51,9 +51,6 @@ public class SessionRenewTest { protected TestServer _server; - - - /** * Tests renewing a session id when sessions are not being cached. * @throws Exception @@ -236,9 +233,7 @@ public class SessionRenewTest assertNull(session); if (((Session)afterSession).isIdChanged()) - { - ((org.eclipse.jetty.server.Response)response).addCookie(sessionManager.getSessionCookie(afterSession, request.getContextPath(), request.isSecure())); - } + ((org.eclipse.jetty.server.Response)response).replaceCookie(sessionManager.getSessionCookie(afterSession, request.getContextPath(), request.isSecure())); } } } diff --git a/tests/test-webapps/test-jaas-webapp/src/main/config/demo-base/webapps/test-jaas.xml b/tests/test-webapps/test-jaas-webapp/src/main/config/demo-base/webapps/test-jaas.xml index c56276b0a7e..eb031c87a3d 100644 --- a/tests/test-webapps/test-jaas-webapp/src/main/config/demo-base/webapps/test-jaas.xml +++ b/tests/test-webapps/test-jaas-webapp/src/main/config/demo-base/webapps/test-jaas.xml @@ -2,7 +2,7 @@ - +