Merge remote-tracking branch 'origin/jetty-10.0.x' into jetty-10.0.x-WebSocketUpgradeFilter

Jenkinsfile

@@ -3,18 +3,17 @@
 pipeline {
   agent any
   // save some io during the build
-  options { durabilityHint( 'PERFORMANCE_OPTIMIZED' ) }
+  options { durabilityHint('PERFORMANCE_OPTIMIZED') }
   stages {
-    stage( "Parallel Stage" ) {
+    stage("Parallel Stage") {
       parallel {
-        stage( "Build / Test - JDK11" ) {
-          agent {
-            node { label 'linux' }
-          }
+        stage("Build / Test - JDK11") {
+          agent { node { label 'linux' } }
           steps {
-            container( 'jetty-build' ) {
+            container('jetty-build') {
               timeout( time: 120, unit: 'MINUTES' ) {
-                mavenBuild( "jdk11", "-T3 clean install -Premote-session-tests", "maven3", true ) // -Pautobahn
+                mavenBuild( "jdk11", "-T3 clean install -Premote-session-tests -Pgcloud", "maven3",
+                            [[parserName: 'Maven'], [parserName: 'Java']] ) // -Pautobahn
                 // Collect up the jacoco execution results (only on main build)
                 jacoco inclusionPattern: '**/org/eclipse/jetty/**/*.class',
                        exclusionPattern: '' +
@@ -33,22 +32,30 @@ pipeline {
                        execPattern: '**/target/jacoco.exec',
                        classPattern: '**/target/classes',
                        sourcePattern: '**/src/main/java'
-                warnings consoleParsers: [[parserName: 'Maven'], [parserName: 'Java']]
-                junit testResults: '**/target/surefire-reports/*.xml,**/target/invoker-reports/TEST*.xml,**/target/autobahntestsuite-reports/*.xml'
               }
             }
           }
         }
 
-
         stage("Build / Test - JDK15") {
           agent { node { label 'linux' } }
           steps {
             container( 'jetty-build' ) {
               timeout( time: 120, unit: 'MINUTES' ) {
-                mavenBuild( "jdk15", "-T3 clean install -Premote-session-tests", "maven3", true )
-                warnings consoleParsers: [[parserName: 'Maven'], [parserName: 'Java']]
-                junit testResults: '**/target/surefire-reports/*.xml,**/target/invoker-reports/TEST*.xml'
+                mavenBuild( "jdk15", "clean install -T3 -Djacoco.skip=true -Premote-session-tests -Pgcloud -Djacoco.skip=true", "maven3",
+                            [[parserName: 'Maven'], [parserName: 'Java']])
+              }
+            }
+          }
+        }
+
+        stage("Build Javadoc") {
+          agent { node { label 'linux' } }
+          steps {
+            container( 'jetty-build' ) {
+              timeout( time: 40, unit: 'MINUTES' ) {
+                mavenBuild( "jdk11",
+                            "install javadoc:javadoc -DskipTests -Dpmd.skip=true -Dcheckstyle.skip=true", "maven3", false)
               }
             }
           }
@@ -69,17 +76,16 @@ pipeline {
   }
 }
 
-
 def slackNotif() {
   script {
     try {
-      if (env.BRANCH_NAME == 'jetty-10.0.x' || env.BRANCH_NAME == 'jetty-9.4.x' || env.BRANCH_NAME == 'jetty-11.0.x') {
+      if ( env.BRANCH_NAME == 'jetty-10.0.x' || env.BRANCH_NAME == 'jetty-9.4.x' || env.BRANCH_NAME == 'jetty-11.0.x') {
         //BUILD_USER = currentBuild.rawBuild.getCause(Cause.UserIdCause).getUserId()
         // by ${BUILD_USER}
         COLOR_MAP = ['SUCCESS': 'good', 'FAILURE': 'danger', 'UNSTABLE': 'danger', 'ABORTED': 'danger']
         slackSend channel: '#jenkins',
-          color: COLOR_MAP[currentBuild.currentResult],
-          message: "*${currentBuild.currentResult}:* Job ${env.JOB_NAME} build ${env.BUILD_NUMBER} - ${env.BUILD_URL}"
+                  color: COLOR_MAP[currentBuild.currentResult],
+                  message: "*${currentBuild.currentResult}:* Job ${env.JOB_NAME} build ${env.BUILD_NUMBER} - ${env.BUILD_URL}"
       }
     } catch (Exception e) {
       e.printStackTrace()
@@ -95,24 +101,27 @@ def slackNotif() {
  *
  * @param jdk the jdk tool name (in jenkins) to use for this build
  * @param cmdline the command line in "<profiles> <goals> <properties>"`format.
- * @paran mvnName maven installation to use
  * @return the Jenkinsfile step representing a maven build
  */
-def mavenBuild(jdk, cmdline, mvnName, junitPublishDisabled) {
-  def localRepo = ".repository"
-  def mavenOpts = '-Xms1g -Xmx4g -Djava.awt.headless=true'
-
-  withMaven(
-          maven: mvnName,
-          jdk: "$jdk",
-          publisherStrategy: 'EXPLICIT',
-          options: [junitPublisher(disabled: junitPublishDisabled), mavenLinkerPublisher(disabled: false), pipelineGraphPublisher(disabled: false)],
-          mavenOpts: mavenOpts,
-          mavenLocalRepo: localRepo) {
-    // Some common Maven command line + provided command line
-    sh "mvn -Pci -V -B -e -fae -Dmaven.test.failure.ignore=true -Djetty.testtracker.log=true $cmdline -Dunix.socket.tmp=" + env.JENKINS_HOME
+def mavenBuild(jdk, cmdline, mvnName, consoleParsers) {
+  script {
+    try {
+      withEnv(["JAVA_HOME=${ tool "$jdk" }",
+               "PATH+MAVEN=${ tool "$jdk" }/bin:${tool "$mvnName"}/bin",
+               "MAVEN_OPTS=-Xms2g -Xmx4g -Djava.awt.headless=true"]) {
+        configFileProvider(
+                [configFile(fileId: 'oss-settings.xml', variable: 'GLOBAL_MVN_SETTINGS')]) {
+          sh "mvn -s $GLOBAL_MVN_SETTINGS -DsettingsPath=$GLOBAL_MVN_SETTINGS -Pci -V -B -e -Djetty.testtracker.log=true $cmdline -Dunix.socket.tmp=" +
+                     env.JENKINS_HOME
+        }
+      }
+    } finally {
+      junit testResults: '**/target/surefire-reports/*.xml,**/target/invoker-reports/TEST*.xml,**/h2spec-reports/*.xml', allowEmptyResults: true
+      if(consoleParsers!=null) {
+        warnings consoleParsers: consoleParsers
+      }
+    }
   }
 }
 
-
 // vim: et:ts=2:sw=2:ft=groovy

Jenkinsfile-autobahn

@@ -0,0 +1,88 @@
+#!groovy
+
+pipeline {
+  agent any
+  triggers {
+    pollSCM('@daily')
+  }
+  options {
+    buildDiscarder logRotator( numToKeepStr: '50' )
+    // save some io during the build
+    durabilityHint( 'PERFORMANCE_OPTIMIZED' )
+  }
+
+  stages {
+    stage( "Build / Test - JDK11" ) {
+      agent {
+        node { label 'linux' }
+      }
+      steps {
+        container( 'jetty-build' ) {
+          timeout( time: 120, unit: 'MINUTES' ) {
+            mavenBuild( "jdk11", "-T3 clean install -Djacoco.skip=true -Pautobahn", "maven3", true ) //
+            junit testResults: '**/target/surefire-reports/*.xml,**/target/invoker-reports/TEST*.xml,**/target/autobahntestsuite-reports/*.xml'
+          }
+        }
+      }
+    }
+  }
+  post {
+    failure {
+      slackNotif()
+    }
+    unstable {
+      slackNotif()
+    }
+    fixed {
+      slackNotif()
+    }
+  }
+}
+
+
+def slackNotif() {
+  script {
+    try {
+      if (env.BRANCH_NAME == 'jetty-10.0.x' || env.BRANCH_NAME == 'jetty-9.4.x' || env.BRANCH_NAME == 'jetty-11.0.x') {
+        //BUILD_USER = currentBuild.rawBuild.getCause(Cause.UserIdCause).getUserId()
+        // by ${BUILD_USER}
+        COLOR_MAP = ['SUCCESS': 'good', 'FAILURE': 'danger', 'UNSTABLE': 'danger', 'ABORTED': 'danger']
+        slackSend channel: '#jenkins',
+          color: COLOR_MAP[currentBuild.currentResult],
+          message: "*${currentBuild.currentResult}:* Job ${env.JOB_NAME} build ${env.BUILD_NUMBER} - ${env.BUILD_URL}"
+      }
+    } catch (Exception e) {
+      e.printStackTrace()
+      echo "skip failure slack notification: " + e.getMessage()
+    }
+  }
+}
+
+/**
+ * To other developers, if you are using this method above, please use the following syntax.
+ *
+ * mavenBuild("<jdk>", "<profiles> <goals> <plugins> <properties>"
+ *
+ * @param jdk the jdk tool name (in jenkins) to use for this build
+ * @param cmdline the command line in "<profiles> <goals> <properties>"`format.
+ * @paran mvnName maven installation to use
+ * @return the Jenkinsfile step representing a maven build
+ */
+def mavenBuild(jdk, cmdline, mvnName, junitPublishDisabled) {
+  def localRepo = ".repository"
+  def mavenOpts = '-Xms1g -Xmx4g -Djava.awt.headless=true'
+
+  withMaven(
+          maven: mvnName,
+          jdk: "$jdk",
+          publisherStrategy: 'EXPLICIT',
+          options: [junitPublisher(disabled: junitPublishDisabled), mavenLinkerPublisher(disabled: false), pipelineGraphPublisher(disabled: false)],
+          mavenOpts: mavenOpts,
+          mavenLocalRepo: localRepo) {
+    // Some common Maven command line + provided command line
+    sh "mvn -Premote-session-tests -Pci -V -B -e -fae -Dmaven.test.failure.ignore=true -Djetty.testtracker.log=true $cmdline -Dunix.socket.tmp=" + env.JENKINS_HOME
+  }
+}
+
+
+// vim: et:ts=2:sw=2:ft=groovy

apache-jstl/pom.xml

@@ -72,6 +72,12 @@
       <scope>test</scope>
     </dependency>
 
+    <dependency>
+      <groupId>org.slf4j</groupId>
+      <artifactId>slf4j-simple</artifactId>
+      <scope>test</scope>
+    </dependency>
+
   </dependencies>
 
 </project>

build-resources/pom.xml

@@ -25,6 +25,7 @@
         <plugin>
           <groupId>org.apache.maven.plugins</groupId>
           <artifactId>maven-javadoc-plugin</artifactId>
+          <version>3.2.0</version>
           <configuration>
             <!-- No point building javadoc for this project -->
             <skip>true</skip>

demos/demo-jaas-webapp/src/main/config/modules/demo-jaas.mod

@@ -14,7 +14,6 @@ jdbc
 jsp
 annotations
 ext
-demo-realm
 
 [files]
 basehome:modules/demo.d/demo-jaas.xml|webapps/demo-jaas.xml
@@ -22,6 +21,6 @@ basehome:modules/demo.d/demo-login.conf|etc/demo-login.conf
 basehome:modules/demo.d/demo-login.properties|etc/demo-login.properties
 maven://org.eclipse.jetty.demos/demo-jaas-webapp/${jetty.version}/war|webapps/demo-jaas.war
 
-[ini-template]
+[ini]
 # Enable security via jaas, and configure it
-jetty.jaas.login.conf=etc/demo-login.conf
+jetty.jaas.login.conf?=etc/demo-login.conf

demos/embedded/src/test/java/org/eclipse/jetty/demos/ServerWithJMXTest.java

@@ -27,10 +27,12 @@ import org.eclipse.jetty.jmx.MBeanContainer;
 import org.eclipse.jetty.server.Server;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+@Disabled
 public class ServerWithJMXTest extends AbstractEmbeddedTest
 {
     private Server server;

jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java

@@ -521,6 +521,12 @@ public class HttpClient extends ContainerLifeCycle
         return new Origin(scheme, host, port, request.getTag(), protocol);
     }
 
+    /**
+     * <p>Returns, creating it if absent, the destination with the given origin.</p>
+     *
+     * @param origin the origin that identifies the destination
+     * @return the destination for the given origin
+     */
     public HttpDestination resolveDestination(Origin origin)
     {
         return destinations.computeIfAbsent(origin, o ->

jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java

@@ -163,8 +163,14 @@ public abstract class HttpConnection implements IConnection, Attachable
         HttpFields headers = request.getHeaders();
         if (version.getVersion() <= 11)
         {
-            if (!headers.contains(HttpHeader.HOST))
-                request.addHeader(getHttpDestination().getHostField());
+            if (!headers.contains(HttpHeader.HOST.asString()))
+            {
+                URI uri = request.getURI();
+                if (uri != null)
+                    request.addHeader(new HttpField(HttpHeader.HOST, uri.getAuthority()));
+                else
+                    request.addHeader(getHttpDestination().getHostField());
+            }
         }
 
         // Add content headers

jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java

@@ -243,15 +243,13 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
         abort(x);
     }
 
+    public void send(Request request, Response.CompleteListener listener)
+    {
+        ((HttpRequest)request).sendAsync(this, listener);
+    }
+
     protected void send(HttpRequest request, List<Response.ResponseListener> listeners)
     {
-        if (!getScheme().equalsIgnoreCase(request.getScheme()))
-            throw new IllegalArgumentException("Invalid request scheme " + request.getScheme() + " for destination " + this);
-        if (!getHost().equalsIgnoreCase(request.getHost()))
-            throw new IllegalArgumentException("Invalid request host " + request.getHost() + " for destination " + this);
-        int port = request.getPort();
-        if (port >= 0 && getPort() != port)
-            throw new IllegalArgumentException("Invalid request port " + port + " for destination " + this);
         send(new HttpExchange(this, request, listeners));
     }
 

jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java

@@ -40,6 +40,7 @@ import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.BiConsumer;
 import java.util.function.BiFunction;
 import java.util.function.Consumer;
 import java.util.function.LongConsumer;
@@ -767,7 +768,7 @@ public class HttpRequest implements Request
     public ContentResponse send() throws InterruptedException, TimeoutException, ExecutionException
     {
         FutureResponseListener listener = new FutureResponseListener(this);
-        send(this, listener);
+        send(listener);
 
         try
         {
@@ -806,15 +807,20 @@ public class HttpRequest implements Request
     @Override
     public void send(Response.CompleteListener listener)
     {
-        send(this, listener);
+        sendAsync(client::send, listener);
     }
 
-    private void send(HttpRequest request, Response.CompleteListener listener)
+    void sendAsync(HttpDestination destination, Response.CompleteListener listener)
+    {
+        sendAsync(destination::send, listener);
+    }
+
+    private void sendAsync(BiConsumer<HttpRequest, List<Response.ResponseListener>> sender, Response.CompleteListener listener)
     {
         if (listener != null)
             responseListeners.add(listener);
         sent();
-        client.send(request, responseListeners);
+        sender.accept(this, responseListeners);
     }
 
     void sent()

jetty-documentation/src/main/asciidoc/old_docs/troubleshooting/security-reports.adoc

@@ -21,7 +21,7 @@
 
 ==== List of Security Reports
 
-A current list of Jetty security reports can be viewed on the link:https://www.eclipse.org/jetty/security-reports.htmlhttps://www.eclipse.org/jetty/security-reports.html[Project Home Page.]
+A current list of Jetty security reports can be viewed on the link:https://www.eclipse.org/jetty/security-reports.html[Project Home Page.]
 
 ==== Reporting Security Issues
 

jetty-home/pom.xml

@@ -655,6 +655,11 @@
       <artifactId>websocket-jetty-server</artifactId>
       <version>${project.version}</version>
     </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty.websocket</groupId>
+      <artifactId>websocket-jetty-client</artifactId>
+      <version>${project.version}</version>
+    </dependency>
     <dependency>
       <groupId>org.eclipse.jetty.websocket</groupId>
       <artifactId>websocket-javax-server</artifactId>

jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java

@@ -22,6 +22,8 @@ import java.io.IOException;
 import java.nio.BufferOverflowException;
 import java.nio.ByteBuffer;
 import java.util.Arrays;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import org.eclipse.jetty.http.HttpTokens.EndOfContent;
 import org.eclipse.jetty.util.ArrayTrie;
@@ -636,17 +638,23 @@ public class HttpGenerator
 
                         case CONNECTION:
                         {
-                            putTo(field, header);
+                            boolean keepAlive = field.contains(HttpHeaderValue.KEEP_ALIVE.asString());
+                            if (keepAlive && _info.getHttpVersion() == HttpVersion.HTTP_1_0 && _persistent == null)
+                            {
+                                _persistent = true;
+                            }
                             if (field.contains(HttpHeaderValue.CLOSE.asString()))
                             {
                                 close = true;
                                 _persistent = false;
                             }
-
-                            if (_info.getHttpVersion() == HttpVersion.HTTP_1_0 && _persistent == null && field.contains(HttpHeaderValue.KEEP_ALIVE.asString()))
+                            if (keepAlive && _persistent == Boolean.FALSE)
                             {
-                                _persistent = true;
+                                field = new HttpField(HttpHeader.CONNECTION,
+                                    Stream.of(field.getValues()).filter(s -> !HttpHeaderValue.KEEP_ALIVE.is(s))
+                                        .collect(Collectors.joining(", ")));
                             }
+                            putTo(field, header);
                             break;
                         }
 

jetty-http/src/main/java/org/eclipse/jetty/http/HttpHeader.java

@@ -128,14 +128,14 @@ public enum HttpHeader
     /**
      * HTTP2 Fields.
      */
-    C_METHOD(":method"),
-    C_SCHEME(":scheme"),
-    C_AUTHORITY(":authority"),
-    C_PATH(":path"),
-    C_STATUS(":status"),
+    C_METHOD(":method", true),
+    C_SCHEME(":scheme", true),
+    C_AUTHORITY(":authority", true),
+    C_PATH(":path", true),
+    C_STATUS(":status", true),
     C_PROTOCOL(":protocol"),
 
-    UNKNOWN("::UNKNOWN::");
+    UNKNOWN("::UNKNOWN::", true);
 
     public static final Trie<HttpHeader> CACHE = new ArrayTrie<>(630);
 
@@ -154,14 +154,21 @@ public enum HttpHeader
     private final byte[] _bytes;
     private final byte[] _bytesColonSpace;
     private final ByteBuffer _buffer;
+    private final boolean _pseudo;
 
     HttpHeader(String s)
+    {
+        this(s, false);
+    }
+
+    HttpHeader(String s, boolean pseudo)
     {
         _string = s;
         _lowerCase = StringUtil.asciiToLowerCase(s);
         _bytes = StringUtil.getBytes(s);
         _bytesColonSpace = StringUtil.getBytes(s + ": ");
         _buffer = ByteBuffer.wrap(_bytes);
+        _pseudo = pseudo;
     }
 
     public String lowerCaseName()
@@ -189,6 +196,14 @@ public enum HttpHeader
         return _string.equalsIgnoreCase(s);
     }
 
+    /**
+     * @return True if the header is a HTTP2 Pseudo header (eg ':path')
+     */
+    public boolean isPseudo()
+    {
+        return _pseudo;
+    }
+
     public String asString()
     {
         return _string;

jetty-http/src/main/java/org/eclipse/jetty/http/HttpMethod.java

@@ -26,137 +26,72 @@ import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.Trie;
 
 /**
- *
+ * Known HTTP Methods
  */
 public enum HttpMethod
 {
-    GET,
-    POST,
-    HEAD,
-    PUT,
-    OPTIONS,
-    DELETE,
-    TRACE,
-    CONNECT,
-    MOVE,
-    PROXY,
-    PRI;
+    // From https://www.iana.org/assignments/http-methods/http-methods.xhtml
+    ACL(Type.IDEMPOTENT),
+    BASELINE_CONTROL(Type.IDEMPOTENT),
+    BIND(Type.IDEMPOTENT),
+    CHECKIN(Type.IDEMPOTENT),
+    CHECKOUT(Type.IDEMPOTENT),
+    CONNECT(Type.NORMAL),
+    COPY(Type.IDEMPOTENT),
+    DELETE(Type.IDEMPOTENT),
+    GET(Type.SAFE),
+    HEAD(Type.SAFE),
+    LABEL(Type.IDEMPOTENT),
+    LINK(Type.IDEMPOTENT),
+    LOCK(Type.NORMAL),
+    MERGE(Type.IDEMPOTENT),
+    MKACTIVITY(Type.IDEMPOTENT),
+    MKCALENDAR(Type.IDEMPOTENT),
+    MKCOL(Type.IDEMPOTENT),
+    MKREDIRECTREF(Type.IDEMPOTENT),
+    MKWORKSPACE(Type.IDEMPOTENT),
+    MOVE(Type.IDEMPOTENT),
+    OPTIONS(Type.SAFE),
+    ORDERPATCH(Type.IDEMPOTENT),
+    PATCH(Type.NORMAL),
+    POST(Type.NORMAL),
+    PRI(Type.SAFE),
+    PROPFIND(Type.SAFE),
+    PROPPATCH(Type.IDEMPOTENT),
+    PUT(Type.IDEMPOTENT),
+    REBIND(Type.IDEMPOTENT),
+    REPORT(Type.SAFE),
+    SEARCH(Type.SAFE),
+    TRACE(Type.SAFE),
+    UNBIND(Type.IDEMPOTENT),
+    UNCHECKOUT(Type.IDEMPOTENT),
+    UNLINK(Type.IDEMPOTENT),
+    UNLOCK(Type.IDEMPOTENT),
+    UPDATE(Type.IDEMPOTENT),
+    UPDATEREDIRECTREF(Type.IDEMPOTENT),
+    VERSION_CONTROL(Type.IDEMPOTENT),
 
-    /**
-     * Optimized lookup to find a method name and trailing space in a byte array.
-     *
-     * @param bytes Array containing ISO-8859-1 characters
-     * @param position The first valid index
-     * @param limit The first non valid index
-     * @return An HttpMethod if a match or null if no easy match.
-     */
-    public static HttpMethod lookAheadGet(byte[] bytes, final int position, int limit)
+    // Other methods
+    PROXY(Type.NORMAL);
+
+    // The type of the method
+    private enum Type
     {
-        int length = limit - position;
-        if (length < 4)
-            return null;
-        switch (bytes[position])
-        {
-            case 'G':
-                if (bytes[position + 1] == 'E' && bytes[position + 2] == 'T' && bytes[position + 3] == ' ')
-                    return GET;
-                break;
-            case 'P':
-                if (bytes[position + 1] == 'O' && bytes[position + 2] == 'S' && bytes[position + 3] == 'T' && length >= 5 && bytes[position + 4] == ' ')
-                    return POST;
-                if (bytes[position + 1] == 'R' && bytes[position + 2] == 'O' && bytes[position + 3] == 'X' && length >= 6 && bytes[position + 4] == 'Y' && bytes[position + 5] == ' ')
-                    return PROXY;
-                if (bytes[position + 1] == 'U' && bytes[position + 2] == 'T' && bytes[position + 3] == ' ')
-                    return PUT;
-                if (bytes[position + 1] == 'R' && bytes[position + 2] == 'I' && bytes[position + 3] == ' ')
-                    return PRI;
-                break;
-            case 'H':
-                if (bytes[position + 1] == 'E' && bytes[position + 2] == 'A' && bytes[position + 3] == 'D' && length >= 5 && bytes[position + 4] == ' ')
-                    return HEAD;
-                break;
-            case 'O':
-                if (bytes[position + 1] == 'P' && bytes[position + 2] == 'T' && bytes[position + 3] == 'I' && length >= 8 &&
-                    bytes[position + 4] == 'O' && bytes[position + 5] == 'N' && bytes[position + 6] == 'S' && bytes[position + 7] == ' ')
-                    return OPTIONS;
-                break;
-            case 'D':
-                if (bytes[position + 1] == 'E' && bytes[position + 2] == 'L' && bytes[position + 3] == 'E' && length >= 7 &&
-                    bytes[position + 4] == 'T' && bytes[position + 5] == 'E' && bytes[position + 6] == ' ')
-                    return DELETE;
-                break;
-            case 'T':
-                if (bytes[position + 1] == 'R' && bytes[position + 2] == 'A' && bytes[position + 3] == 'C' && length >= 6 &&
-                    bytes[position + 4] == 'E' && bytes[position + 5] == ' ')
-                    return TRACE;
-                break;
-            case 'C':
-                if (bytes[position + 1] == 'O' && bytes[position + 2] == 'N' && bytes[position + 3] == 'N' && length >= 8 &&
-                    bytes[position + 4] == 'E' && bytes[position + 5] == 'C' && bytes[position + 6] == 'T' && bytes[position + 7] == ' ')
-                    return CONNECT;
-                break;
-            case 'M':
-                if (bytes[position + 1] == 'O' && bytes[position + 2] == 'V' && bytes[position + 3] == 'E' && length >= 5 && bytes[position + 4] == ' ')
-                    return MOVE;
-                break;
-
-            default:
-                break;
-        }
-        return null;
+        NORMAL,
+        IDEMPOTENT,
+        SAFE
     }
 
-    /**
-     * Optimized lookup to find a method name and trailing space in a byte array.
-     *
-     * @param buffer buffer containing ISO-8859-1 characters, it is not modified.
-     * @return An HttpMethod if a match or null if no easy match.
-     */
-    public static HttpMethod lookAheadGet(ByteBuffer buffer)
-    {
-        if (buffer.hasArray())
-            return lookAheadGet(buffer.array(), buffer.arrayOffset() + buffer.position(), buffer.arrayOffset() + buffer.limit());
-
-        int l = buffer.remaining();
-        if (l >= 4)
-        {
-            HttpMethod m = CACHE.getBest(buffer, 0, l);
-            if (m != null)
-            {
-                int ml = m.asString().length();
-                if (l > ml && buffer.get(buffer.position() + ml) == ' ')
-                    return m;
-            }
-        }
-        return null;
-    }
-
-    public static final Trie<HttpMethod> INSENSITIVE_CACHE = new ArrayTrie<>();
-
-    static
-    {
-        for (HttpMethod method : HttpMethod.values())
-        {
-            INSENSITIVE_CACHE.put(method.toString(), method);
-        }
-    }
-
-    public static final Trie<HttpMethod> CACHE = new ArrayTernaryTrie<>(false);
-
-    static
-    {
-        for (HttpMethod method : HttpMethod.values())
-        {
-            CACHE.put(method.toString(), method);
-        }
-    }
-
-    private final ByteBuffer _buffer;
+    private final String _method;
     private final byte[] _bytes;
+    private final ByteBuffer _buffer;
+    private final Type _type;
 
-    HttpMethod()
+    HttpMethod(Type type)
     {
-        _bytes = StringUtil.getBytes(toString());
+        _method = name().replace('_', '-');
+        _type = type;
+        _bytes = StringUtil.getBytes(_method);
         _buffer = ByteBuffer.wrap(_bytes);
     }
 
@@ -170,6 +105,28 @@ public enum HttpMethod
         return toString().equalsIgnoreCase(s);
     }
 
+    /**
+     * An HTTP method is safe if it doesn't alter the state of the server.
+     * In other words, a method is safe if it leads to a read-only operation.
+     * Several common HTTP methods are safe: GET , HEAD , or OPTIONS .
+     * All safe methods are also idempotent, but not all idempotent methods are safe
+     * @return if the method is safe.
+     */
+    public boolean isSafe()
+    {
+        return _type == Type.SAFE;
+    }
+
+    /**
+     * An idempotent HTTP method is an HTTP method that can be called many times without different outcomes.
+     * It would not matter if the method is called only once, or ten times over. The result should be the same.
+     * @return true if the method is idempotent.
+     */
+    public boolean isIdempotent()
+    {
+        return _type.ordinal() >= Type.IDEMPOTENT.ordinal();
+    }
+
     public ByteBuffer asBuffer()
     {
         return _buffer.asReadOnlyBuffer();
@@ -177,11 +134,94 @@ public enum HttpMethod
 
     public String asString()
     {
-        return toString();
+        return _method;
+    }
+
+    public String toString()
+    {
+        return _method;
+    }
+
+    public static final Trie<HttpMethod> INSENSITIVE_CACHE = new ArrayTrie<>(252);
+    public static final Trie<HttpMethod> CACHE = new ArrayTernaryTrie<>(false, 300);
+    public static final Trie<HttpMethod> LOOK_AHEAD = new ArrayTernaryTrie<>(false, 330);
+    public static final int ACL_AS_INT = ('A' & 0xff) << 24 | ('C' & 0xFF) << 16 | ('L' & 0xFF) << 8 | (' ' & 0xFF);
+    public static final int GET_AS_INT = ('G' & 0xff) << 24 | ('E' & 0xFF) << 16 | ('T' & 0xFF) << 8 | (' ' & 0xFF);
+    public static final int PRI_AS_INT = ('P' & 0xff) << 24 | ('R' & 0xFF) << 16 | ('I' & 0xFF) << 8 | (' ' & 0xFF);
+    public static final int PUT_AS_INT = ('P' & 0xff) << 24 | ('U' & 0xFF) << 16 | ('T' & 0xFF) << 8 | (' ' & 0xFF);
+    public static final int POST_AS_INT = ('P' & 0xff) << 24 | ('O' & 0xFF) << 16 | ('S' & 0xFF) << 8 | ('T' & 0xFF);
+    public static final int HEAD_AS_INT = ('H' & 0xff) << 24 | ('E' & 0xFF) << 16 | ('A' & 0xFF) << 8 | ('D' & 0xFF);
+    static
+    {
+        for (HttpMethod method : HttpMethod.values())
+        {
+            if (!INSENSITIVE_CACHE.put(method.asString(), method))
+                throw new IllegalStateException("INSENSITIVE_CACHE too small: " + method);
+
+            if (!CACHE.put(method.asString(), method))
+                throw new IllegalStateException("CACHE too small: " + method);
+
+            if (!LOOK_AHEAD.put(method.asString() + ' ', method))
+                throw new IllegalStateException("LOOK_AHEAD too small: " + method);
+        }
     }
 
     /**
-     * Converts the given String parameter to an HttpMethod
+     * Optimized lookup to find a method name and trailing space in a byte array.
+     *
+     * @param bytes Array containing ISO-8859-1 characters
+     * @param position The first valid index
+     * @param limit The first non valid index
+     * @return An HttpMethod if a match or null if no easy match.
+     * @deprecated Not used
+     */
+    @Deprecated
+    public static HttpMethod lookAheadGet(byte[] bytes, final int position, int limit)
+    {
+        return LOOK_AHEAD.getBest(bytes, position, limit - position);
+    }
+
+    /**
+     * Optimized lookup to find a method name and trailing space in a byte array.
+     *
+     * @param buffer buffer containing ISO-8859-1 characters, it is not modified.
+     * @return An HttpMethod if a match or null if no easy match.
+     */
+    public static HttpMethod lookAheadGet(ByteBuffer buffer)
+    {
+        int len = buffer.remaining();
+        // Short cut for 3 char methods, mostly for GET optimisation
+        if (len > 3)
+        {
+            switch (buffer.getInt(buffer.position()))
+            {
+                case ACL_AS_INT:
+                    return ACL;
+                case GET_AS_INT:
+                    return GET;
+                case PRI_AS_INT:
+                    return PRI;
+                case PUT_AS_INT:
+                    return PUT;
+                case POST_AS_INT:
+                    if (len > 4 && buffer.get(buffer.position() + 4) == ' ')
+                        return POST;
+                    break;
+                case HEAD_AS_INT:
+                    if (len > 4 && buffer.get(buffer.position() + 4) == ' ')
+                        return HEAD;
+                    break;
+                default:
+                    break;
+            }
+        }
+        return LOOK_AHEAD.getBest(buffer, 0, len);
+    }
+
+    /**
+     * Converts the given String parameter to an HttpMethod.
+     * The string may differ from the Enum name as a '-'  in the method
+     * name is represented as a '_' in the Enum name.
      *
      * @param method the String to get the equivalent HttpMethod from
      * @return the HttpMethod or null if the parameter method is unknown

jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java

@@ -107,6 +107,7 @@ public class HttpParser
      * </ul>
      */
     public static final Trie<HttpField> CACHE = new ArrayTrie<>(2048);
+    private static final Trie<HttpField> NO_CACHE = Trie.empty(true);
 
     // States
     public enum FieldState
@@ -155,6 +156,7 @@ public class HttpParser
     private final ComplianceViolation.Listener _complianceListener;
     private final int _maxHeaderBytes;
     private final HttpCompliance _complianceMode;
+    private final Utf8StringBuilder _uri = new Utf8StringBuilder(INITIAL_URI_LENGTH);
     private HttpField _field;
     private HttpHeader _header;
     private String _headerString;
@@ -169,7 +171,6 @@ public class HttpParser
     private HttpMethod _method;
     private String _methodString;
     private HttpVersion _version;
-    private Utf8StringBuilder _uri = new Utf8StringBuilder(INITIAL_URI_LENGTH); // Tune?
     private EndOfContent _endOfContent;
     private boolean _hasContentLength;
     private boolean _hasTransferEncoding;
@@ -234,7 +235,7 @@ public class HttpParser
         // Add headers with null values so HttpParser can avoid looking up name again for unknown values
         for (HttpHeader h : HttpHeader.values())
         {
-            if (!CACHE.put(new HttpField(h, (String)null)))
+            if (!h.isPseudo() && !CACHE.put(new HttpField(h, (String)null)))
                 throw new IllegalStateException("CACHE FULL");
         }
     }
@@ -874,11 +875,6 @@ public class HttpParser
                             }
                             checkVersion();
 
-                            // Should we try to cache header fields?
-                            int headerCache = getHeaderCacheSize();
-                            if (_fieldCache == null && _version.getVersion() >= HttpVersion.HTTP_1_1.getVersion() && headerCache > 0)
-                                _fieldCache = new ArrayTernaryTrie<>(headerCache);
-
                             setState(State.HEADER);
 
                             _requestHandler.startRequest(_methodString, _uri.toString(), _version);
@@ -951,7 +947,7 @@ public class HttpParser
             // Handle known headers
             if (_header != null)
             {
-                boolean addToConnectionTrie = false;
+                boolean addToFieldCache = false;
                 switch (_header)
                 {
                     case CONTENT_LENGTH:
@@ -1023,14 +1019,16 @@ public class HttpParser
                             _field = new HostPortHttpField(_header,
                                 CASE_SENSITIVE_FIELD_NAME.isAllowedBy(_complianceMode) ? _headerString : _header.asString(),
                                 _valueString);
-                            addToConnectionTrie = _fieldCache != null;
+                            addToFieldCache = true;
                         }
                         break;
 
                     case CONNECTION:
                         // Don't cache headers if not persistent
-                        if (HttpHeaderValue.CLOSE.is(_valueString) || new QuotedCSV(_valueString).getValues().stream().anyMatch(HttpHeaderValue.CLOSE::is))
-                            _fieldCache = null;
+                        if (_field == null)
+                            _field = new HttpField(_header, caseInsensitiveHeader(_headerString, _header.asString()), _valueString);
+                        if (getHeaderCacheSize() > 0 && _field.contains(HttpHeaderValue.CLOSE.asString()))
+                            _fieldCache = NO_CACHE;
                         break;
 
                     case AUTHORIZATION:
@@ -1041,18 +1039,29 @@ public class HttpParser
                     case COOKIE:
                     case CACHE_CONTROL:
                     case USER_AGENT:
-                        addToConnectionTrie = _fieldCache != null && _field == null;
+                        addToFieldCache = _field == null;
                         break;
 
                     default:
                         break;
                 }
 
-                if (addToConnectionTrie && !_fieldCache.isFull() && _header != null && _valueString != null)
+                // Cache field?
+                if (addToFieldCache && _header != null && _valueString != null)
                 {
-                    if (_field == null)
-                        _field = new HttpField(_header, caseInsensitiveHeader(_headerString, _header.asString()), _valueString);
-                    _fieldCache.put(_field);
+                    if (_fieldCache == null)
+                    {
+                        _fieldCache = (getHeaderCacheSize() > 0 && (_version != null && _version == HttpVersion.HTTP_1_1))
+                            ? new ArrayTernaryTrie<>(getHeaderCacheSize())
+                            : NO_CACHE;
+                    }
+
+                    if (!_fieldCache.isFull())
+                    {
+                        if (_field == null)
+                            _field = new HttpField(_header, caseInsensitiveHeader(_headerString, _header.asString()), _valueString);
+                        _fieldCache.put(_field);
+                    }
                 }
             }
             _handler.parsedHeader(_field != null ? _field : new HttpField(_header, _headerString, _valueString));

jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerTest.java

@@ -870,4 +870,19 @@ public class HttpGeneratorServerTest
         assertThat(headers, containsString(HttpHeaderValue.KEEP_ALIVE.asString()));
         assertThat(headers, containsString(customValue));
     }
+
+    @Test
+    public void testKeepAliveWithClose() throws Exception
+    {
+        HttpGenerator generator = new HttpGenerator();
+        HttpFields.Mutable fields = HttpFields.build();
+        fields.put(HttpHeader.CONNECTION, HttpHeaderValue.KEEP_ALIVE.asString() + ", other, " + HttpHeaderValue.CLOSE.asString());
+        MetaData.Response info = new MetaData.Response(HttpVersion.HTTP_1_0, 200, "OK", fields, -1);
+        ByteBuffer header = BufferUtil.allocate(4096);
+        HttpGenerator.Result result = generator.generateResponse(info, false, header, null, null, true);
+        assertSame(HttpGenerator.Result.FLUSH, result);
+        String headers = BufferUtil.toString(header);
+        assertThat(headers, containsString("Connection: other, close\r\n"));
+        assertThat(headers, not(containsString("keep-alive")));
+    }
 }

jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java

@@ -22,6 +22,7 @@ import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Locale;
 
 import org.eclipse.jetty.http.HttpParser.State;
 import org.eclipse.jetty.logging.StacklessLogging;
@@ -31,6 +32,8 @@ import org.hamcrest.Matchers;
 import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import static org.eclipse.jetty.http.HttpCompliance.Violation.CASE_INSENSITIVE_METHOD;
 import static org.eclipse.jetty.http.HttpCompliance.Violation.CASE_SENSITIVE_FIELD_NAME;
@@ -81,12 +84,20 @@ public class HttpParserTest
     @Test
     public void testHttpMethod()
     {
-        assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer("Wibble ")));
-        assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer("GET")));
-        assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer("MO")));
+        for (HttpMethod m : HttpMethod.values())
+        {
+            assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer(m.asString().substring(0,2))));
+            assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer(m.asString())));
+            assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer(m.asString() + "FOO")));
+            assertEquals(m, HttpMethod.lookAheadGet(BufferUtil.toBuffer(m.asString() + " ")));
+            assertEquals(m, HttpMethod.lookAheadGet(BufferUtil.toBuffer(m.asString() + " /foo/bar")));
 
-        assertEquals(HttpMethod.GET, HttpMethod.lookAheadGet(BufferUtil.toBuffer("GET ")));
-        assertEquals(HttpMethod.MOVE, HttpMethod.lookAheadGet(BufferUtil.toBuffer("MOVE ")));
+            assertNull(HttpMethod.lookAheadGet(m.asString().substring(0,2).getBytes(), 0,2));
+            assertNull(HttpMethod.lookAheadGet(m.asString().getBytes(), 0, m.asString().length()));
+            assertNull(HttpMethod.lookAheadGet((m.asString() + "FOO").getBytes(), 0, m.asString().length() + 3));
+            assertEquals(m, HttpMethod.lookAheadGet(("\n" + m.asString() + " ").getBytes(), 1, m.asString().length() + 2));
+            assertEquals(m, HttpMethod.lookAheadGet(("\n" + m.asString() + " /foo").getBytes(), 1, m.asString().length() + 6));
+        }
 
         ByteBuffer b = BufferUtil.allocateDirect(128);
         BufferUtil.append(b, BufferUtil.toBuffer("GET"));
@@ -96,6 +107,15 @@ public class HttpParserTest
         assertEquals(HttpMethod.GET, HttpMethod.lookAheadGet(b));
     }
 
+    @ParameterizedTest
+    @ValueSource(strings = {"GET", "POST", "VERSION-CONTROL"})
+    public void httpMethodNameTest(String methodName)
+    {
+        HttpMethod method = HttpMethod.fromString(methodName);
+        assertNotNull(method, "Method should have been found: " + methodName);
+        assertEquals(methodName.toUpperCase(Locale.US), method.toString());
+    }
+
     @Test
     public void testLineParseMockIP()
     {

jetty-http2/http2-server/pom.xml

@@ -33,6 +33,8 @@
           <skip>${skipTests}</skip>
           <junitPackage>org.eclipse.jetty.h2spec</junitPackage>
           <skipNoDockerAvailable>true</skipNoDockerAvailable>
+          <reportsDirectory>${project.build.directory}/h2spec-reports</reportsDirectory>
+          <testFailureIgnore>true</testFailureIgnore>
           <excludeSpecs>
             <excludeSpec>3.5 - Sends invalid connection preface</excludeSpec>
           </excludeSpecs>

jetty-jaas/pom.xml

@@ -66,10 +66,14 @@
     </dependency>
     <dependency>
       <groupId>org.apache.directory.server</groupId>
-      <artifactId>apacheds-all</artifactId>
+      <artifactId>apacheds-test-framework</artifactId>
       <version>${apacheds.version}</version>
       <scope>test</scope>
       <exclusions>
+        <exclusion>
+          <groupId>junit</groupId>
+          <artifactId>junit</artifactId>
+        </exclusion>
         <!-- exclude additional LDIF schema files to avoid conflicts through
           multiple copies -->
         <exclusion>
@@ -118,6 +122,11 @@
         </exclusion>
       </exclusions>
     </dependency>
+    <dependency>
+      <groupId>org.apache.directory.api</groupId>
+      <artifactId>api-ldap-schema-data</artifactId>
+      <version>2.0.0</version>
+    </dependency>
     <!-- because directory server do not have yet junit5 extensions -->
     <dependency>
       <groupId>org.junit.vintage</groupId>

jetty-jaas/src/main/java/module-info.java

@@ -27,4 +27,5 @@ module org.eclipse.jetty.jaas
 
     // Only required if using JDBCLoginModule.
     requires static java.sql;
+    requires org.eclipse.jetty.util;
 }

jetty-jaas/src/main/java/org/eclipse/jetty/jaas/JAASLoginService.java

@@ -20,15 +20,16 @@ package org.eclipse.jetty.jaas;
 
 import java.io.IOException;
 import java.security.Principal;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Set;
 import javax.security.auth.Subject;
 import javax.security.auth.callback.Callback;
 import javax.security.auth.callback.CallbackHandler;
 import javax.security.auth.callback.NameCallback;
-import javax.security.auth.callback.PasswordCallback;
 import javax.security.auth.callback.UnsupportedCallbackException;
 import javax.security.auth.login.Configuration;
 import javax.security.auth.login.FailedLoginException;
@@ -37,9 +38,6 @@ import javax.security.auth.login.LoginException;
 import javax.servlet.ServletRequest;
 
 import org.eclipse.jetty.jaas.callback.DefaultCallbackHandler;
-import org.eclipse.jetty.jaas.callback.ObjectCallback;
-import org.eclipse.jetty.jaas.callback.RequestParameterCallback;
-import org.eclipse.jetty.jaas.callback.ServletRequestCallback;
 import org.eclipse.jetty.security.DefaultIdentityService;
 import org.eclipse.jetty.security.IdentityService;
 import org.eclipse.jetty.security.LoginService;
@@ -47,7 +45,7 @@ import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.UserIdentity;
 import org.eclipse.jetty.util.ArrayUtil;
 import org.eclipse.jetty.util.Loader;
-import org.eclipse.jetty.util.component.AbstractLifeCycle;
+import org.eclipse.jetty.util.component.ContainerLifeCycle;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -58,13 +56,14 @@ import org.slf4j.LoggerFactory;
  * Implementation of jetty's LoginService that works with JAAS for
  * authorization and authentication.
  */
-public class JAASLoginService extends AbstractLifeCycle implements LoginService
+public class JAASLoginService extends ContainerLifeCycle implements LoginService
 {
     private static final Logger LOG = LoggerFactory.getLogger(JAASLoginService.class);
 
     public static final String DEFAULT_ROLE_CLASS_NAME = "org.eclipse.jetty.jaas.JAASRole";
     public static final String[] DEFAULT_ROLE_CLASS_NAMES = {DEFAULT_ROLE_CLASS_NAME};
-
+    public static final ThreadLocal<JAASLoginService> INSTANCE = new ThreadLocal<>();
+    
     protected String[] _roleClassNames = DEFAULT_ROLE_CLASS_NAMES;
     protected String _callbackHandlerClass;
     protected String _realmName;
@@ -183,6 +182,7 @@ public class JAASLoginService extends AbstractLifeCycle implements LoginService
     {
         if (_identityService == null)
             _identityService = new DefaultIdentityService();
+        addBean(new PropertyUserStoreManager());
         super.doStart();
     }
 
@@ -193,59 +193,27 @@ public class JAASLoginService extends AbstractLifeCycle implements LoginService
         {
             CallbackHandler callbackHandler = null;
             if (_callbackHandlerClass == null)
-            {
-                callbackHandler = new CallbackHandler()
-                {
-                    @Override
-                    public void handle(Callback[] callbacks) throws IOException, UnsupportedCallbackException
-                    {
-                        for (Callback callback : callbacks)
-                        {
-                            if (callback instanceof NameCallback)
-                            {
-                                ((NameCallback)callback).setName(username);
-                            }
-                            else if (callback instanceof PasswordCallback)
-                            {
-                                ((PasswordCallback)callback).setPassword(credentials.toString().toCharArray());
-                            }
-                            else if (callback instanceof ObjectCallback)
-                            {
-                                ((ObjectCallback)callback).setObject(credentials);
-                            }
-                            else if (callback instanceof RequestParameterCallback)
-                            {
-                                RequestParameterCallback rpc = (RequestParameterCallback)callback;
-                                if (request != null)
-                                    rpc.setParameterValues(Arrays.asList(request.getParameterValues(rpc.getParameterName())));
-                            }
-                            else if (callback instanceof ServletRequestCallback)
-                            {
-                                ((ServletRequestCallback)callback).setRequest(request);
-                            }
-                            else
-                                throw new UnsupportedCallbackException(callback);
-                        }
-                    }
-                };
-            }
+                callbackHandler = new DefaultCallbackHandler();
             else
             {
                 Class<?> clazz = Loader.loadClass(_callbackHandlerClass);
                 callbackHandler = (CallbackHandler)clazz.getDeclaredConstructor().newInstance();
-                if (DefaultCallbackHandler.class.isAssignableFrom(clazz))
-                {
-                    DefaultCallbackHandler dch = (DefaultCallbackHandler)callbackHandler;
-                    if (request instanceof Request)
-                        dch.setRequest((Request)request);
-                    dch.setCredential(credentials);
-                    dch.setUserName(username);
-                }
+            }
+            
+            if (callbackHandler instanceof DefaultCallbackHandler)
+            {
+                DefaultCallbackHandler dch = (DefaultCallbackHandler)callbackHandler;
+                if (request instanceof Request)
+                    dch.setRequest((Request)request);
+                dch.setCredential(credentials);
+                dch.setUserName(username);
             }
 
             //set up the login context
             Subject subject = new Subject();
-            LoginContext loginContext = (_configuration == null ? new LoginContext(_loginModuleName, subject, callbackHandler)
+            INSTANCE.set(this);
+            LoginContext loginContext = 
+                (_configuration == null ? new LoginContext(_loginModuleName, subject, callbackHandler)
                 : new LoginContext(_loginModuleName, subject, callbackHandler, _configuration));
 
             loginContext.login();
@@ -263,8 +231,14 @@ public class JAASLoginService extends AbstractLifeCycle implements LoginService
         }
         catch (Exception e)
         {
-            LOG.trace("IGNORED", e);
+            if (LOG.isDebugEnabled())
+                LOG.debug("Login error", e);
         }
+        finally
+        {
+            INSTANCE.remove();
+        }
+        
         return null;
     }
 
@@ -306,52 +280,36 @@ public class JAASLoginService extends AbstractLifeCycle implements LoginService
     protected String[] getGroups(Subject subject)
     {
         Collection<String> groups = new LinkedHashSet<>();
-        Set<Principal> principals = subject.getPrincipals();
-        for (Principal principal : principals)
+        for (Principal principal : subject.getPrincipals())
         {
-            Class<?> c = principal.getClass();
-            while (c != null)
-            {
-                if (roleClassNameMatches(c.getName()))
-                {
-                    groups.add(principal.getName());
-                    break;
-                }
-
-                boolean added = false;
-                for (Class<?> ci : c.getInterfaces())
-                {
-                    if (roleClassNameMatches(ci.getName()))
-                    {
-                        groups.add(principal.getName());
-                        added = true;
-                        break;
-                    }
-                }
-
-                if (!added)
-                {
-                    c = c.getSuperclass();
-                }
-                else
-                    break;
-            }
+            if (isRoleClass(principal.getClass(), Arrays.asList(getRoleClassNames())))
+                groups.add(principal.getName());
         }
 
         return groups.toArray(new String[groups.size()]);
     }
-
-    private boolean roleClassNameMatches(String classname)
+    
+    /**
+     * Check whether the class, its superclasses or any interfaces they implement
+     * is one of the classes that represents a role.
+     * 
+     * @param clazz the class to check
+     * @param roleClassNames the list of classnames that represent roles
+     * @return true if the class is a role class
+     */
+    private static boolean isRoleClass(Class<?> clazz, List<String> roleClassNames)
     {
-        boolean result = false;
-        for (String roleClassName : getRoleClassNames())
+        Class<?> c = clazz;
+        
+        //add the class, its interfaces and superclasses to the list to test
+        List<String> classnames = new ArrayList<>();
+        while (c != null)
         {
-            if (roleClassName.equals(classname))
-            {
-                result = true;
-                break;
-            }
+            classnames.add(c.getName());
+            Arrays.stream(c.getInterfaces()).map(Class::getName).forEach(classnames::add);
+            c = c.getSuperclass();
         }
-        return result;
+        
+        return roleClassNames.stream().anyMatch(classnames::contains);
     }
 }

jetty-jaas/src/main/java/org/eclipse/jetty/jaas/JAASUserPrincipal.java

@@ -26,7 +26,7 @@ import javax.security.auth.login.LoginContext;
  * JAASUserPrincipal
  * <p>
  * Implements the JAAS version of the
- * org.eclipse.jetty.http.UserPrincipal interface.
+ * org.eclipse.jetty.security.UserPrincipal interface.
  */
 public class JAASUserPrincipal implements Principal
 {

jetty-jaas/src/main/java/org/eclipse/jetty/jaas/PropertyUserStoreManager.java

@@ -0,0 +1,99 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under
+// the terms of the Eclipse Public License 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0
+//
+// This Source Code may also be made available under the following
+// Secondary Licenses when the conditions for such availability set
+// forth in the Eclipse Public License, v. 2.0 are satisfied:
+// the Apache License v2.0 which is available at
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.jaas;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+
+import org.eclipse.jetty.security.PropertyUserStore;
+import org.eclipse.jetty.util.component.AbstractLifeCycle;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * PropertyUserStoreManager
+ *
+ * Maintains a map of PropertyUserStores, keyed off the location of the property file containing
+ * the authentication and authorization information.
+ * 
+ * This class is used to enable the PropertyUserStores to be cached and shared. This is essential
+ * for the PropertyFileLoginModules, whose lifecycle is controlled by the JAAS api and instantiated
+ * afresh whenever a user needs to be authenticated. Without this class, every PropertyFileLoginModule
+ * instantiation would re-read and reload in all the user information just to authenticate a single user.
+ */
+public class PropertyUserStoreManager extends AbstractLifeCycle
+{
+    private static final Logger LOG = LoggerFactory.getLogger(PropertyUserStoreManager.class);
+    /**
+     * Map of user authentication and authorization information loaded in from a property file.
+     * The map is keyed off the location of the file.
+     */
+    private Map<String, PropertyUserStore> _propertyUserStores; 
+
+    public PropertyUserStore getPropertyUserStore(String file)
+    {
+        synchronized (this)
+        {
+            if (_propertyUserStores == null)
+                return null;
+            
+            return _propertyUserStores.get(file);
+        }
+    }
+    
+    public PropertyUserStore addPropertyUserStore(String file, PropertyUserStore store)
+    {
+        synchronized (this)
+        {
+            Objects.requireNonNull(_propertyUserStores);
+            PropertyUserStore existing = _propertyUserStores.get(file);
+            if (existing != null)
+                return existing;
+            
+            _propertyUserStores.put(file, store);
+            return store;
+        }
+    }
+
+    @Override
+    protected void doStart() throws Exception
+    {
+        _propertyUserStores = new HashMap<String, PropertyUserStore>();
+        super.doStart();
+    }
+
+    @Override
+    protected void doStop() throws Exception
+    {
+        for (Map.Entry<String,PropertyUserStore> entry: _propertyUserStores.entrySet())
+        {
+            try
+            {
+                entry.getValue().stop();
+            }
+            catch (Exception e)
+            {
+                LOG.warn("Error stopping PropertyUserStore at {}", entry.getKey(), e);
+            }
+        }
+        _propertyUserStores = null;
+        super.doStop();
+    }
+}

jetty-jaas/src/main/java/org/eclipse/jetty/jaas/callback/DefaultCallbackHandler.java

@@ -26,7 +26,6 @@ import javax.security.auth.callback.PasswordCallback;
 import javax.security.auth.callback.UnsupportedCallbackException;
 
 import org.eclipse.jetty.server.Request;
-import org.eclipse.jetty.util.security.Password;
 
 /**
  * DefaultCallbackHandler
@@ -47,39 +46,34 @@ public class DefaultCallbackHandler extends AbstractCallbackHandler
     public void handle(Callback[] callbacks)
         throws IOException, UnsupportedCallbackException
     {
-        for (int i = 0; i < callbacks.length; i++)
+        for (Callback callback : callbacks)
         {
-            if (callbacks[i] instanceof NameCallback)
+            if (callback instanceof NameCallback)
             {
-                ((NameCallback)callbacks[i]).setName(getUserName());
+                ((NameCallback)callback).setName(getUserName());
             }
-            else if (callbacks[i] instanceof ObjectCallback)
+            else if (callback instanceof ObjectCallback)
             {
-                ((ObjectCallback)callbacks[i]).setObject(getCredential());
+                ((ObjectCallback)callback).setObject(getCredential());
             }
-            else if (callbacks[i] instanceof PasswordCallback)
+            else if (callback instanceof PasswordCallback)
             {
-                if (getCredential() instanceof Password)
-                    ((PasswordCallback)callbacks[i]).setPassword(((Password)getCredential()).toString().toCharArray());
-                else if (getCredential() instanceof String)
+                ((PasswordCallback)callback).setPassword(getCredential().toString().toCharArray());
+            }
+            else if (callback instanceof RequestParameterCallback)
+            {
+                if (_request != null)
                 {
-                    ((PasswordCallback)callbacks[i]).setPassword(((String)getCredential()).toCharArray());
+                    RequestParameterCallback rpc = (RequestParameterCallback)callback;
+                    rpc.setParameterValues(Arrays.asList(_request.getParameterValues(rpc.getParameterName())));
                 }
-                else
-                    throw new UnsupportedCallbackException(callbacks[i], "User supplied credentials cannot be converted to char[] for PasswordCallback: try using an ObjectCallback instead");
             }
-            else if (callbacks[i] instanceof RequestParameterCallback)
+            else if (callback instanceof ServletRequestCallback)
             {
-                RequestParameterCallback callback = (RequestParameterCallback)callbacks[i];
-                callback.setParameterValues(Arrays.asList(_request.getParameterValues(callback.getParameterName())));
-            }
-            else if (callbacks[i] instanceof ServletRequestCallback)
-            {
-                ((ServletRequestCallback)callbacks[i]).setRequest(_request);
+                ((ServletRequestCallback)callback).setRequest(_request);
             }
             else
-                throw new UnsupportedCallbackException(callbacks[i]);
+                throw new UnsupportedCallbackException(callback);
         }
     }
 }
-

jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/AbstractDatabaseLoginModule.java

@@ -27,6 +27,7 @@ import java.util.Map;
 import javax.security.auth.Subject;
 import javax.security.auth.callback.CallbackHandler;
 
+import org.eclipse.jetty.security.UserPrincipal;
 import org.eclipse.jetty.util.security.Credential;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -57,11 +58,11 @@ public abstract class AbstractDatabaseLoginModule extends AbstractLoginModule
      */
     public abstract Connection getConnection() throws Exception;
 
-    public class JDBCUserInfo extends UserInfo
+    public class JDBCUser extends JAASUser
     {
-        public JDBCUserInfo(String userName, Credential credential)
+        public JDBCUser(UserPrincipal user)
         {
-            super(userName, credential);
+            super(user);
         }
 
         @Override
@@ -79,7 +80,7 @@ public abstract class AbstractDatabaseLoginModule extends AbstractLoginModule
      * @throws Exception if unable to get the user info
      */
     @Override
-    public UserInfo getUserInfo(String userName)
+    public JAASUser getUser(String userName)
         throws Exception
     {
         try (Connection connection = getConnection())
@@ -100,11 +101,9 @@ public abstract class AbstractDatabaseLoginModule extends AbstractLoginModule
             }
 
             if (dbCredential == null)
-            {
                 return null;
-            }
 
-            return new JDBCUserInfo(userName, Credential.getCredential(dbCredential));
+            return new JDBCUser(new UserPrincipal(userName, Credential.getCredential(dbCredential)));
         }
     }
 

jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/AbstractLoginModule.java

@@ -19,11 +19,11 @@
 package org.eclipse.jetty.jaas.spi;
 
 import java.io.IOException;
-import java.security.Principal;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 import javax.security.auth.Subject;
 import javax.security.auth.callback.Callback;
 import javax.security.auth.callback.CallbackHandler;
@@ -34,9 +34,10 @@ import javax.security.auth.login.FailedLoginException;
 import javax.security.auth.login.LoginException;
 import javax.security.auth.spi.LoginModule;
 
-import org.eclipse.jetty.jaas.JAASPrincipal;
 import org.eclipse.jetty.jaas.JAASRole;
 import org.eclipse.jetty.jaas.callback.ObjectCallback;
+import org.eclipse.jetty.security.UserPrincipal;
+import org.eclipse.jetty.util.thread.AutoLock;
 
 /**
  * AbstractLoginModule
@@ -50,35 +51,22 @@ public abstract class AbstractLoginModule implements LoginModule
 
     private boolean authState = false;
     private boolean commitState = false;
-    private JAASUserInfo currentUser;
+    private JAASUser currentUser;
     private Subject subject;
 
-    /**
-     * JAASUserInfo
-     *
-     * This class unites the UserInfo data with jaas concepts
-     * such as Subject and Principals
-     */
-    public class JAASUserInfo
+    public abstract static class JAASUser
     {
-        private UserInfo user;
-        private Principal principal;
-        private List<JAASRole> roles;
-
-        public JAASUserInfo(UserInfo u)
+        private final UserPrincipal _user;
+        private List<JAASRole> _roles;
+        
+        public JAASUser(UserPrincipal u)
         {
-            this.user = u;
-            this.principal = new JAASPrincipal(u.getUserName());
+            _user = u;
         }
 
         public String getUserName()
         {
-            return this.user.getUserName();
-        }
-
-        public Principal getPrincipal()
-        {
-            return this.principal;
+            return _user.getName();
         }
 
         /**
@@ -86,12 +74,12 @@ public abstract class AbstractLoginModule implements LoginModule
          */
         public void setJAASInfo(Subject subject)
         {
-            subject.getPrincipals().add(this.principal);
-            if (this.user.getCredential() != null)
-            {
-                subject.getPrivateCredentials().add(this.user.getCredential());
-            }
-            subject.getPrincipals().addAll(roles);
+            if (_user == null)
+                return;
+
+            _user.configureSubject(subject);
+            if (_roles != null)
+                subject.getPrincipals().addAll(_roles);
         }
 
         /**
@@ -99,35 +87,29 @@ public abstract class AbstractLoginModule implements LoginModule
          */
         public void unsetJAASInfo(Subject subject)
         {
-            subject.getPrincipals().remove(this.principal);
-            if (this.user.getCredential() != null)
-            {
-                subject.getPrivateCredentials().remove(this.user.getCredential());
-            }
-            subject.getPrincipals().removeAll(this.roles);
+            if (_user == null)
+                return;
+            _user.deconfigureSubject(subject);
+            if (_roles != null)
+                subject.getPrincipals().removeAll(_roles);
         }
 
         public boolean checkCredential(Object suppliedCredential)
         {
-            return this.user.checkCredential(suppliedCredential);
+            return _user.authenticate(suppliedCredential);
         }
 
         public void fetchRoles() throws Exception
         {
-            this.user.fetchRoles();
-            this.roles = new ArrayList<JAASRole>();
-            if (this.user.getRoleNames() != null)
-            {
-                Iterator<String> itor = this.user.getRoleNames().iterator();
-                while (itor.hasNext())
-                {
-                    this.roles.add(new JAASRole((String)itor.next()));
-                }
-            }
+            List<String> rolenames = doFetchRoles();
+            if (rolenames != null)
+                _roles = rolenames.stream().map(JAASRole::new).collect(Collectors.toList());
         }
+        
+        public abstract List<String> doFetchRoles() throws Exception;
     }
 
-    public abstract UserInfo getUserInfo(String username) throws Exception;
+    public abstract JAASUser getUser(String username) throws Exception;
 
     public Subject getSubject()
     {
@@ -139,12 +121,12 @@ public abstract class AbstractLoginModule implements LoginModule
         this.subject = s;
     }
 
-    public JAASUserInfo getCurrentUser()
+    public JAASUser getCurrentUser()
     {
         return this.currentUser;
     }
 
-    public void setCurrentUser(JAASUserInfo u)
+    public void setCurrentUser(JAASUser u)
     {
         this.currentUser = u;
     }
@@ -252,15 +234,15 @@ public abstract class AbstractLoginModule implements LoginModule
                 throw new FailedLoginException();
             }
 
-            UserInfo userInfo = getUserInfo(webUserName);
+            JAASUser user = getUser(webUserName);
 
-            if (userInfo == null)
+            if (user == null)
             {
                 setAuthenticated(false);
                 throw new FailedLoginException();
             }
 
-            currentUser = new JAASUserInfo(userInfo);
+            currentUser = user;
             setAuthenticated(currentUser.checkCredential(webCredential));
 
             if (isAuthenticated())

jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/LdapLoginModule.java

@@ -45,6 +45,7 @@ import javax.security.auth.login.FailedLoginException;
 import javax.security.auth.login.LoginException;
 
 import org.eclipse.jetty.jaas.callback.ObjectCallback;
+import org.eclipse.jetty.security.UserPrincipal;
 import org.eclipse.jetty.util.TypeUtil;
 import org.eclipse.jetty.util.security.Credential;
 import org.slf4j.Logger;
@@ -179,18 +180,13 @@ public class LdapLoginModule extends AbstractLoginModule
 
     private DirContext _rootContext;
 
-    public class LDAPUserInfo extends UserInfo
+    public class LDAPUser extends JAASUser
     {
         Attributes attributes;
 
-        /**
-         * @param userName the user name
-         * @param credential the credential
-         * @param attributes the user {@link Attributes}
-         */
-        public LDAPUserInfo(String userName, Credential credential, Attributes attributes)
+        public LDAPUser(UserPrincipal user, Attributes attributes)
         {
-            super(userName, credential);
+            super(user);
             this.attributes = attributes;
         }
 
@@ -201,6 +197,25 @@ public class LdapLoginModule extends AbstractLoginModule
         }
     }
 
+    public class LDAPBindingUser extends JAASUser
+    {   
+        DirContext _context;
+        String _userDn;
+
+        public LDAPBindingUser(UserPrincipal user, DirContext context, String userDn)
+        {
+            super(user);
+            _context = context;
+            _userDn = userDn;
+        }
+
+        @Override
+        public List<String> doFetchRoles() throws Exception
+        {
+            return getUserRolesByDn(_context, _userDn);
+        }
+    }
+
     /**
      * get the available information about the user
      * <p>
@@ -214,19 +229,17 @@ public class LdapLoginModule extends AbstractLoginModule
      * @throws Exception if unable to get the user info
      */
     @Override
-    public UserInfo getUserInfo(String username) throws Exception
+    public JAASUser getUser(String username) throws Exception
     {
         Attributes attributes = getUserAttributes(username);
         String pwdCredential = getUserCredentials(attributes);
 
         if (pwdCredential == null)
-        {
             return null;
-        }
 
         pwdCredential = convertCredentialLdapToJetty(pwdCredential);
         Credential credential = Credential.getCredential(pwdCredential);
-        return new LDAPUserInfo(username, credential, attributes);
+        return new LDAPUser(new UserPrincipal(username, credential), attributes);
     }
 
     protected String doRFC2254Encoding(String inputString)
@@ -421,7 +434,7 @@ public class LdapLoginModule extends AbstractLoginModule
             else
             {
                 // This sets read and the credential
-                UserInfo userInfo = getUserInfo(webUserName);
+                JAASUser userInfo = getUser(webUserName);
 
                 if (userInfo == null)
                 {
@@ -429,7 +442,7 @@ public class LdapLoginModule extends AbstractLoginModule
                     return false;
                 }
 
-                setCurrentUser(new JAASUserInfo(userInfo));
+                setCurrentUser(userInfo);
 
                 if (webCredential instanceof String)
                     authed = credentialLogin(Credential.getCredential((String)webCredential));
@@ -520,12 +533,8 @@ public class LdapLoginModule extends AbstractLoginModule
         try
         {
             DirContext dirContext = new InitialDirContext(environment);
-            List<String> roles = getUserRolesByDn(dirContext, userDn);
-
-            UserInfo userInfo = new UserInfo(username, null, roles);
-            setCurrentUser(new JAASUserInfo(userInfo));
+            setCurrentUser(new LDAPBindingUser(new UserPrincipal(username, null), dirContext, userDn));
             setAuthenticated(true);
-
             return true;
         }
         catch (javax.naming.AuthenticationException e)

jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/PropertyFileLoginModule.java

@@ -18,16 +18,19 @@
 
 package org.eclipse.jetty.jaas.spi;
 
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.stream.Collectors;
 import javax.security.auth.Subject;
 import javax.security.auth.callback.CallbackHandler;
 
-import org.eclipse.jetty.security.AbstractLoginService;
+import org.eclipse.jetty.jaas.JAASLoginService;
+import org.eclipse.jetty.jaas.PropertyUserStoreManager;
 import org.eclipse.jetty.security.PropertyUserStore;
+import org.eclipse.jetty.security.RolePrincipal;
+import org.eclipse.jetty.security.UserPrincipal;
 import org.eclipse.jetty.server.UserIdentity;
 import org.eclipse.jetty.util.security.Credential;
 import org.slf4j.Logger;
@@ -39,16 +42,13 @@ import org.slf4j.LoggerFactory;
 public class PropertyFileLoginModule extends AbstractLoginModule
 {
     public static final String DEFAULT_FILENAME = "realm.properties";
-
     private static final Logger LOG = LoggerFactory.getLogger(PropertyFileLoginModule.class);
 
-    private static ConcurrentHashMap<String, PropertyUserStore> _propertyUserStores = new ConcurrentHashMap<String, PropertyUserStore>();
-
-    private int _refreshInterval = 0;
-    private String _filename = DEFAULT_FILENAME;
+    private PropertyUserStore _store;
 
     /**
-     * Read contents of the configured property file.
+     * Use a PropertyUserStore to read the authentication and authorizaton information contained in
+     * the file named by the option "file".
      *
      * @param subject the subject
      * @param callbackHandler the callback handler
@@ -64,68 +64,83 @@ public class PropertyFileLoginModule extends AbstractLoginModule
         setupPropertyUserStore(options);
     }
 
+    /**
+     * Get an existing, or create a new PropertyUserStore to read the 
+     * authentication and authorization information from the file named by
+     * the option "file".
+     * 
+     * @param options configuration options
+     */
     private void setupPropertyUserStore(Map<String, ?> options)
     {
-        parseConfig(options);
+        String filename = (String)options.get("file");
+        filename = (filename == null ? DEFAULT_FILENAME : filename);
 
-        if (_propertyUserStores.get(_filename) == null)
+        PropertyUserStoreManager mgr = JAASLoginService.INSTANCE.get().getBean(PropertyUserStoreManager.class);
+        if (mgr == null)
+            throw new IllegalStateException("No PropertyUserStoreManager");
+
+        _store = mgr.getPropertyUserStore(filename);
+        if (_store == null)
         {
-            PropertyUserStore propertyUserStore = new PropertyUserStore();
-            propertyUserStore.setConfig(_filename);
-
-            PropertyUserStore prev = _propertyUserStores.putIfAbsent(_filename, propertyUserStore);
-            if (prev == null)
+            boolean hotReload = false;  
+            String tmp = (String)options.get("hotReload");
+            if (tmp != null)
+                hotReload = Boolean.parseBoolean(tmp);
+            else
             {
-                LOG.debug("setupPropertyUserStore: Starting new PropertyUserStore. PropertiesFile: {} refreshInterval: {}", _filename, _refreshInterval);
-
-                try
+                //refreshInterval is deprecated, use hotReload instead
+                tmp = (String)options.get("refreshInterval");
+                if (tmp != null)
                 {
-                    propertyUserStore.start();
-                }
-                catch (Exception e)
-                {
-                    LOG.warn("Exception while starting propertyUserStore: ", e);
+                    LOG.warn("Use 'hotReload' boolean property instead of 'refreshInterval'");
+                    try
+                    {
+                        hotReload = (Integer.parseInt(tmp) > 0);
+                    }
+                    catch (NumberFormatException e)
+                    {
+                        LOG.warn("'refreshInterval' is not an integer");
+                    }
                 }
             }
+            PropertyUserStore newStore = new PropertyUserStore();
+            newStore.setConfig(filename);
+            newStore.setHotReload(hotReload);
+            _store = mgr.addPropertyUserStore(filename, newStore);
+            try
+            {
+                _store.start();
+            }
+            catch (Exception e)
+            {
+                LOG.warn("Exception starting propertyUserStore {} ", filename, e);
+            }
         }
     }
 
-    private void parseConfig(Map<String, ?> options)
-    {
-        String tmp = (String)options.get("file");
-        _filename = (tmp == null ? DEFAULT_FILENAME : tmp);
-        tmp = (String)options.get("refreshInterval");
-        _refreshInterval = (tmp == null ? _refreshInterval : Integer.parseInt(tmp));
-    }
-
     /**
      * @param userName the user name
      * @throws Exception if unable to get the user information
      */
     @Override
-    public UserInfo getUserInfo(String userName) throws Exception
+    public JAASUser getUser(String userName) throws Exception
     {
-        PropertyUserStore propertyUserStore = _propertyUserStores.get(_filename);
-        if (propertyUserStore == null)
-            throw new IllegalStateException("PropertyUserStore should never be null here!");
-
         if (LOG.isDebugEnabled())
-            LOG.debug("Checking PropertyUserStore {} for {}", _filename, userName);
-        UserIdentity userIdentity = propertyUserStore.getUserIdentity(userName);
-        if (userIdentity == null)
+            LOG.debug("Checking PropertyUserStore {} for {}", _store.getConfig(), userName);
+        UserPrincipal up = _store.getUserPrincipal(userName);
+        if (up == null)
             return null;
 
-        //TODO in future versions change the impl of PropertyUserStore so its not
-        //storing Subjects etc, just UserInfo
-        Set<AbstractLoginService.RolePrincipal> principals = userIdentity.getSubject().getPrincipals(AbstractLoginService.RolePrincipal.class);
-
-        List<String> roles = principals.stream()
-            .map(AbstractLoginService.RolePrincipal::getName)
-            .collect(Collectors.toList());
-
-        Credential credential = (Credential)userIdentity.getSubject().getPrivateCredentials().iterator().next();
-        if (LOG.isDebugEnabled())
-            LOG.debug("Found: {} in PropertyUserStore {}", userName,  _filename);
-        return new UserInfo(userName, credential, roles);
+        List<RolePrincipal> rps = _store.getRolePrincipals(userName);
+        List<String> roles = rps == null ? Collections.emptyList() : rps.stream().map(RolePrincipal::getName).collect(Collectors.toList());
+        return new JAASUser(up)
+        {
+            @Override
+            public List<String> doFetchRoles()
+            {
+                return roles;
+            }
+        };
     }
 }

jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/UserInfo.java

@@ -1,113 +0,0 @@
-//
-// ========================================================================
-// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
-//
-// This program and the accompanying materials are made available under
-// the terms of the Eclipse Public License 2.0 which is available at
-// https://www.eclipse.org/legal/epl-2.0
-//
-// This Source Code may also be made available under the following
-// Secondary Licenses when the conditions for such availability set
-// forth in the Eclipse Public License, v. 2.0 are satisfied:
-// the Apache License v2.0 which is available at
-// https://www.apache.org/licenses/LICENSE-2.0
-//
-// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
-// ========================================================================
-//
-
-package org.eclipse.jetty.jaas.spi;
-
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
-
-import org.eclipse.jetty.util.security.Credential;
-import org.eclipse.jetty.util.thread.AutoLock;
-
-/**
- * UserInfo
- *
- * This is the information read from the external source
- * about a user.
- *
- * Can be cached.
- */
-public class UserInfo
-{
-    private final AutoLock _lock = new AutoLock();
-    private String _userName;
-    private Credential _credential;
-    protected List<String> _roleNames = new ArrayList<>();
-    protected boolean _rolesLoaded = false;
-
-    /**
-     * @param userName the user name
-     * @param credential the credential
-     * @param roleNames a {@link List} of role name
-     */
-    public UserInfo(String userName, Credential credential, List<String> roleNames)
-    {
-        _userName = userName;
-        _credential = credential;
-        if (roleNames != null)
-        {
-            _roleNames.addAll(roleNames);
-            _rolesLoaded = true;
-        }
-    }
-
-    /**
-     * @param userName the user name
-     * @param credential the credential
-     */
-    public UserInfo(String userName, Credential credential)
-    {
-        this(userName, credential, null);
-    }
-
-    /**
-     * Should be overridden by subclasses to obtain
-     * role info
-     *
-     * @return List of role associated to the user
-     * @throws Exception if the roles cannot be retrieved
-     */
-    public List<String> doFetchRoles()
-        throws Exception
-    {
-        return Collections.emptyList();
-    }
-
-    public void fetchRoles() throws Exception
-    {
-        try (AutoLock l = _lock.lock())
-        {
-            if (!_rolesLoaded)
-            {
-                _roleNames.addAll(doFetchRoles());
-                _rolesLoaded = true;
-            }
-        }
-    }
-
-    public String getUserName()
-    {
-        return this._userName;
-    }
-
-    public List<String> getRoleNames()
-    {
-        return Collections.unmodifiableList(_roleNames);
-    }
-
-    public boolean checkCredential(Object suppliedCredential)
-    {
-        return _credential.check(suppliedCredential);
-    }
-
-    protected Credential getCredential()
-    {
-        return _credential;
-    }
-}

jetty-jaas/src/test/java/org/eclipse/jetty/jaas/JAASLoginServiceTest.java

@@ -19,6 +19,7 @@
 package org.eclipse.jetty.jaas;
 
 import java.security.Principal;
+import java.util.Arrays;
 import java.util.Collections;
 import javax.security.auth.Subject;
 import javax.security.auth.login.AppConfigurationEntry;
@@ -29,25 +30,17 @@ import org.eclipse.jetty.security.DefaultIdentityService;
 import org.eclipse.jetty.server.Request;
 import org.junit.jupiter.api.Test;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /**
  * JAASLoginServiceTest
  */
 public class JAASLoginServiceTest
 {
-    public static class TestConfiguration extends Configuration
-    {
-        AppConfigurationEntry _entry = new AppConfigurationEntry(TestLoginModule.class.getCanonicalName(), LoginModuleControlFlag.REQUIRED, Collections.emptyMap());
-
-        @Override
-        public AppConfigurationEntry[] getAppConfigurationEntry(String name)
-        {
-            return new AppConfigurationEntry[]{_entry};
-        }
-    }
-
     interface SomeRole
     {
 
@@ -94,18 +87,31 @@ public class JAASLoginServiceTest
     @Test
     public void testServletRequestCallback() throws Exception
     {
+        Configuration config = new Configuration()
+            {
+                @Override
+                public AppConfigurationEntry[] getAppConfigurationEntry(String name)
+                {
+                    return new AppConfigurationEntry[] {
+                        new AppConfigurationEntry(TestLoginModule.class.getCanonicalName(), 
+                            LoginModuleControlFlag.REQUIRED, 
+                            Collections.emptyMap())
+                    };
+                }
+            };
+        
         //Test with the DefaultCallbackHandler
         JAASLoginService ls = new JAASLoginService("foo");
         ls.setCallbackHandlerClass("org.eclipse.jetty.jaas.callback.DefaultCallbackHandler");
         ls.setIdentityService(new DefaultIdentityService());
-        ls.setConfiguration(new TestConfiguration());
+        ls.setConfiguration(config);
         Request request = new Request(null, null);
         ls.login("aaardvaark", "aaa", request);
 
         //Test with the fallback CallbackHandler
         ls = new JAASLoginService("foo");
         ls.setIdentityService(new DefaultIdentityService());
-        ls.setConfiguration(new TestConfiguration());
+        ls.setConfiguration(config);
         ls.login("aaardvaark", "aaa", request);
     }
 
@@ -137,12 +143,8 @@ public class JAASLoginServiceTest
         subject.getPrincipals().add(new AnotherTestRole("z"));
 
         String[] groups = ls.getGroups(subject);
-        assertEquals(3, groups.length);
-        for (String g : groups)
-        {
-            assertTrue(g.equals("x") || g.equals("y") || g.equals("z"));
-        }
-
+        assertThat(Arrays.asList(groups), containsInAnyOrder("x", "y", "z"));
+        
         //test a custom role class
         ls.setRoleClassNames(new String[]{AnotherTestRole.class.getName()});
         Subject subject2 = new Subject();
@@ -150,8 +152,9 @@ public class JAASLoginServiceTest
         subject2.getPrincipals().add(new TestRole("x"));
         subject2.getPrincipals().add(new TestRole("y"));
         subject2.getPrincipals().add(new AnotherTestRole("z"));
-        assertEquals(1, ls.getGroups(subject2).length);
-        assertEquals("z", ls.getGroups(subject2)[0]);
+        String[] s2groups = ls.getGroups(subject2);
+        assertThat(s2groups, is(notNullValue()));
+        assertThat(Arrays.asList(s2groups), containsInAnyOrder("z"));
 
         //test a custom role class that implements an interface
         ls.setRoleClassNames(new String[]{SomeRole.class.getName()});
@@ -160,11 +163,9 @@ public class JAASLoginServiceTest
         subject3.getPrincipals().add(new TestRole("x"));
         subject3.getPrincipals().add(new TestRole("y"));
         subject3.getPrincipals().add(new AnotherTestRole("z"));
-        assertEquals(3, ls.getGroups(subject3).length);
-        for (String g : groups)
-        {
-            assertTrue(g.equals("x") || g.equals("y") || g.equals("z"));
-        }
+        String[] s3groups = ls.getGroups(subject3);
+        assertThat(s3groups, is(notNullValue()));
+        assertThat(Arrays.asList(s3groups), containsInAnyOrder("x", "y", "z"));
 
         //test a class that doesn't match
         ls.setRoleClassNames(new String[]{NotTestRole.class.getName()});

jetty-jaas/src/test/java/org/eclipse/jetty/jaas/TestLoginModule.java

@@ -18,12 +18,14 @@
 
 package org.eclipse.jetty.jaas;
 
+import java.util.Collections;
+import java.util.List;
 import javax.security.auth.callback.Callback;
 import javax.security.auth.login.LoginException;
 
 import org.eclipse.jetty.jaas.callback.ServletRequestCallback;
 import org.eclipse.jetty.jaas.spi.AbstractLoginModule;
-import org.eclipse.jetty.jaas.spi.UserInfo;
+import org.eclipse.jetty.security.UserPrincipal;
 import org.eclipse.jetty.util.ArrayUtil;
 import org.eclipse.jetty.util.security.Password;
 
@@ -34,9 +36,16 @@ public class TestLoginModule extends AbstractLoginModule
     public ServletRequestCallback _callback = new ServletRequestCallback();
 
     @Override
-    public UserInfo getUserInfo(String username) throws Exception
-    {
-        return new UserInfo(username, new Password("aaa"));
+    public JAASUser getUser(String username) throws Exception
+    {        
+        return new JAASUser(new UserPrincipal(username, new Password("aaa")))
+        {
+            @Override
+            public List<String> doFetchRoles() throws Exception
+            {
+                return Collections.emptyList();
+            }
+        };
     }
 
     @Override

jetty-jaas/src/test/java/org/eclipse/jetty/jaas/spi/PropertyFileLoginModuleTest.java

@@ -19,34 +19,72 @@
 package org.eclipse.jetty.jaas.spi;
 
 import java.io.File;
-import java.util.HashMap;
-import javax.security.auth.Subject;
+import java.util.Collections;
+import javax.security.auth.login.AppConfigurationEntry;
+import javax.security.auth.login.AppConfigurationEntry.LoginModuleControlFlag;
+import javax.security.auth.login.Configuration;
 
-import org.eclipse.jetty.jaas.callback.DefaultCallbackHandler;
+import org.eclipse.jetty.jaas.JAASLoginService;
+import org.eclipse.jetty.jaas.PropertyUserStoreManager;
+import org.eclipse.jetty.security.DefaultIdentityService;
+import org.eclipse.jetty.security.PropertyUserStore;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.UserIdentity;
 import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
 import org.junit.jupiter.api.Test;
 
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.contains;
-import static org.hamcrest.Matchers.containsInAnyOrder;
-import static org.hamcrest.Matchers.not;
-import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
 
 public class PropertyFileLoginModuleTest
 {
     @Test
-    public void testRoles()
-        throws Exception
+    public void testPropertyFileLoginModule() throws Exception
     {
-        File file = MavenTestingUtils.getTestResourceFile("login.properties");
-        PropertyFileLoginModule module = new PropertyFileLoginModule();
-        Subject subject = new Subject();
-        HashMap<String, String> options = new HashMap<>();
-        options.put("file", file.getCanonicalPath());
-        module.initialize(subject, new DefaultCallbackHandler(), new HashMap<String, String>(), options);
-        UserInfo fred = module.getUserInfo("fred");
-        assertEquals("fred", fred.getUserName());
-        assertThat(fred.getRoleNames(), containsInAnyOrder("role1", "role2", "role3"));
-        assertThat(fred.getRoleNames(), not(contains("fred")));
+        //configure for PropertyFileLoginModule
+        File loginProperties = MavenTestingUtils.getTestResourceFile("login.properties");
+
+        Configuration testConfig = new Configuration()
+        {
+            @Override
+            public AppConfigurationEntry[] getAppConfigurationEntry(String name)
+            { 
+                return new AppConfigurationEntry[]{new AppConfigurationEntry(PropertyFileLoginModule.class.getName(), 
+                                                                             LoginModuleControlFlag.REQUIRED,
+                                                                             Collections.singletonMap("file", loginProperties.getAbsolutePath()))};
+            }
+        };
+
+        JAASLoginService ls = new JAASLoginService("foo");
+        ls.setCallbackHandlerClass("org.eclipse.jetty.jaas.callback.DefaultCallbackHandler");
+        ls.setIdentityService(new DefaultIdentityService());
+        ls.setConfiguration(testConfig);
+        ls.start();
+
+        //test that the manager is created when the JAASLoginService starts
+        PropertyUserStoreManager mgr = ls.getBean(PropertyUserStoreManager.class);
+        assertThat(mgr, notNullValue());
+
+        //test the PropertyFileLoginModule authentication and authorization
+        Request request = new Request(null, null);
+        UserIdentity uid = ls.login("fred", "pwd", request);
+        assertThat(uid.isUserInRole("role1", null), is(true));
+        assertThat(uid.isUserInRole("role2", null), is(true));
+        assertThat(uid.isUserInRole("role3", null), is(true));
+        assertThat(uid.isUserInRole("role4", null), is(false));
+
+        //Test that the PropertyUserStore is created by the PropertyFileLoginModule
+        PropertyUserStore store = mgr.getPropertyUserStore(loginProperties.getAbsolutePath());
+        assertThat(store, is(notNullValue()));
+        assertThat(store.isRunning(), is(true));
+        assertThat(store.isHotReload(), is(false));
+
+        //test that the PropertyUserStoreManager is stopped and all PropertyUserStores stopped
+        ls.stop();
+        assertThat(mgr.isStopped(), is(true));
+        assertThat(mgr.getPropertyUserStore(loginProperties.getAbsolutePath()), is(nullValue()));
+        assertThat(store.isStopped(), is(true));
     }
 }

jetty-jaspi/src/test/java/org/eclipse/jetty/security/jaspi/JaspiTest.java

@@ -19,9 +19,12 @@
 package org.eclipse.jetty.security.jaspi;
 
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.Base64;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -29,6 +32,8 @@ import javax.servlet.http.HttpServletResponse;
 import org.eclipse.jetty.security.AbstractLoginService;
 import org.eclipse.jetty.security.ConstraintMapping;
 import org.eclipse.jetty.security.ConstraintSecurityHandler;
+import org.eclipse.jetty.security.RolePrincipal;
+import org.eclipse.jetty.security.UserPrincipal;
 import org.eclipse.jetty.server.LocalConnector;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.Server;
@@ -55,7 +60,7 @@ public class JaspiTest
     public class TestLoginService extends AbstractLoginService
     {
         protected Map<String, UserPrincipal> _users = new HashMap<>();
-        protected Map<String, String[]> _roles = new HashMap();
+        protected Map<String, List<RolePrincipal>> _roles = new HashMap<>();
 
         public TestLoginService(String name)
         {
@@ -66,11 +71,15 @@ public class JaspiTest
         {
             UserPrincipal userPrincipal = new UserPrincipal(username, credential);
             _users.put(username, userPrincipal);
-            _roles.put(username, roles);
+            if (roles != null)
+            {
+                List<RolePrincipal> rps = Arrays.stream(roles).map(RolePrincipal::new).collect(Collectors.toList());
+                _roles.put(username, rps);
+            }
         }
 
         @Override
-        protected String[] loadRoleInfo(UserPrincipal user)
+        protected List<RolePrincipal> loadRoleInfo(UserPrincipal user)
         {
             return _roles.get(user.getName());
         }

jetty-jmh/src/main/java/org/eclipse/jetty/http/jmh/HttpMethodBenchmark.java

@@ -0,0 +1,127 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under
+// the terms of the Eclipse Public License 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0
+//
+// This Source Code may also be made available under the following
+// Secondary Licenses when the conditions for such availability set
+// forth in the Eclipse Public License, v. 2.0 are satisfied:
+// the Apache License v2.0 which is available at
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.http.jmh;
+
+import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import org.eclipse.jetty.http.HttpMethod;
+import org.eclipse.jetty.util.BufferUtil;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Threads;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.profile.GCProfiler;
+import org.openjdk.jmh.runner.Runner;
+import org.openjdk.jmh.runner.RunnerException;
+import org.openjdk.jmh.runner.options.Options;
+import org.openjdk.jmh.runner.options.OptionsBuilder;
+
+@State(Scope.Benchmark)
+@Threads(4)
+@Warmup(iterations = 5, time = 2000, timeUnit = TimeUnit.MILLISECONDS)
+@Measurement(iterations = 5, time = 2000, timeUnit = TimeUnit.MILLISECONDS)
+public class HttpMethodBenchmark
+{
+    private static final ByteBuffer GET = BufferUtil.toBuffer("GET / HTTP/1.1\r\n\r\n");
+    private static final ByteBuffer POST = BufferUtil.toBuffer("POST / HTTP/1.1\r\n\r\n");
+    private static final ByteBuffer MOVE = BufferUtil.toBuffer("MOVE / HTTP/1.1\r\n\r\n");
+    private static final Map<String, HttpMethod> MAP = new HashMap<>();
+
+    static
+    {
+        for (HttpMethod m : HttpMethod.values())
+            MAP.put(m.asString(), m);
+    }
+
+    @Benchmark
+    @BenchmarkMode({Mode.Throughput})
+    public HttpMethod testTrieGetBest() throws Exception
+    {
+        return HttpMethod.LOOK_AHEAD.getBest(GET, 0, GET.remaining());
+    }
+
+    @Benchmark
+    @BenchmarkMode({Mode.Throughput})
+    public HttpMethod testIntSwitch() throws Exception
+    {
+        switch (GET.getInt(0))
+        {
+            case HttpMethod.ACL_AS_INT:
+                return HttpMethod.ACL;
+            case HttpMethod.GET_AS_INT:
+                return HttpMethod.GET;
+            case HttpMethod.PRI_AS_INT:
+                return HttpMethod.PRI;
+            case HttpMethod.PUT_AS_INT:
+                return HttpMethod.PUT;
+            default:
+                return null;
+        }
+    }
+
+    @Benchmark
+    @BenchmarkMode({Mode.Throughput})
+    public HttpMethod testMapGet() throws Exception
+    {
+        for (int i = 0; i < GET.remaining(); i++)
+        {
+            if (GET.get(i) == (byte)' ')
+                return MAP.get(BufferUtil.toString(GET, 0, i, StandardCharsets.US_ASCII));
+        }
+        return null;
+    }
+
+    @Benchmark
+    @BenchmarkMode({Mode.Throughput})
+    public HttpMethod testHttpMethodPost() throws Exception
+    {
+        return HttpMethod.lookAheadGet(POST);
+    }
+
+    @Benchmark
+    @BenchmarkMode({Mode.Throughput})
+    public HttpMethod testHttpMethodMove() throws Exception
+    {
+        return HttpMethod.lookAheadGet(MOVE);
+    }
+
+    public static void main(String[] args) throws RunnerException
+    {
+        Options opt = new OptionsBuilder()
+            .include(HttpMethodBenchmark.class.getSimpleName())
+            .warmupIterations(10)
+            .measurementIterations(10)
+            .addProfiler(GCProfiler.class)
+            .forks(1)
+            .threads(1)
+            .build();
+
+        new Runner(opt).run();
+    }
+}
+
+

jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java

@@ -30,7 +30,6 @@ import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
 
 import org.eclipse.jetty.http.HttpMethod;
-import org.eclipse.jetty.http.HttpVersion;
 import org.eclipse.jetty.http.MimeTypes;
 import org.eclipse.jetty.security.LoginService;
 import org.eclipse.jetty.security.ServerAuthException;
@@ -300,7 +299,7 @@ public class OpenIdAuthenticator extends LoginAuthenticator
                             LOG.debug("authenticated {}->{}", openIdAuth, nuri);
 
                         response.setContentLength(0);
-                        baseResponse.sendRedirect(getRedirectCode(baseRequest.getHttpVersion()), nuri);
+                        baseResponse.sendRedirect(nuri, true);
                         return openIdAuth;
                     }
                 }
@@ -392,7 +391,7 @@ public class OpenIdAuthenticator extends LoginAuthenticator
             String challengeUri = getChallengeUri(request);
             if (LOG.isDebugEnabled())
                 LOG.debug("challenge {}->{}", session.getId(), challengeUri);
-            baseResponse.sendRedirect(getRedirectCode(baseRequest.getHttpVersion()), challengeUri);
+            baseResponse.sendRedirect(challengeUri, true);
 
             return Authentication.SEND_CONTINUE;
         }
@@ -436,10 +435,9 @@ public class OpenIdAuthenticator extends LoginAuthenticator
             {
                 String query = URIUtil.addQueries(ERROR_PARAMETER + "=" + UrlEncoded.encodeString(message), _errorQuery);
                 redirectUri = URIUtil.addPathQuery(URIUtil.addPaths(request.getContextPath(), _errorPath), query);
-                baseResponse.sendRedirect(getRedirectCode(baseRequest.getHttpVersion()), redirectUri);
             }
 
-            baseResponse.sendRedirect(getRedirectCode(baseRequest.getHttpVersion()), redirectUri);
+            baseResponse.sendRedirect(redirectUri, true);
         }
     }
 
@@ -461,12 +459,6 @@ public class OpenIdAuthenticator extends LoginAuthenticator
         return pathInContext != null && (pathInContext.equals(_errorPath));
     }
 
-    private static int getRedirectCode(HttpVersion httpVersion)
-    {
-        return (httpVersion.getVersion() < HttpVersion.HTTP_1_1.getVersion()
-            ? HttpServletResponse.SC_MOVED_TEMPORARILY : HttpServletResponse.SC_SEE_OTHER);
-    }
-
     private String getRedirectUri(HttpServletRequest request)
     {
         final StringBuffer redirectUri = new StringBuffer(128);

jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/annotations/AnnotationConfiguration.java

@@ -30,6 +30,7 @@ import org.eclipse.jetty.osgi.boot.OSGiWebappConstants;
 import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.resource.Resource;
 import org.eclipse.jetty.util.statistic.CounterStatistic;
+import org.eclipse.jetty.webapp.Configuration;
 import org.eclipse.jetty.webapp.WebAppContext;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.Constants;
@@ -74,6 +75,12 @@ public class AnnotationConfiguration extends org.eclipse.jetty.annotations.Annot
     {
     }
 
+    @Override
+    public Class<? extends Configuration> replaces()
+    {
+        return org.eclipse.jetty.annotations.AnnotationConfiguration.class;
+    }
+
     /**
      * This parser scans the bundles using the OSGi APIs instead of assuming a jar.
      */

jetty-plus/src/main/java/org/eclipse/jetty/plus/security/DataSourceLoginService.java

@@ -27,6 +27,7 @@ import java.sql.Statement;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Locale;
+import java.util.stream.Collectors;
 import javax.naming.InitialContext;
 import javax.naming.NameNotFoundException;
 import javax.naming.NamingException;
@@ -35,16 +36,17 @@ import javax.sql.DataSource;
 import org.eclipse.jetty.plus.jndi.NamingEntryUtil;
 import org.eclipse.jetty.security.AbstractLoginService;
 import org.eclipse.jetty.security.IdentityService;
+import org.eclipse.jetty.security.RolePrincipal;
+import org.eclipse.jetty.security.UserPrincipal;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.util.security.Credential;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
- * DataSourceUserRealm
+ * DataSourceLoginService
  * <p>
- * Obtain user/password/role information from a database
- * via jndi DataSource.
+ * Obtain user/password/role information from a database via jndi DataSource.
  */
 public class DataSourceLoginService extends AbstractLoginService
 {
@@ -264,7 +266,7 @@ public class DataSourceLoginService extends AbstractLoginService
     }
 
     @Override
-    public String[] loadRoleInfo(UserPrincipal user)
+    public List<RolePrincipal> loadRoleInfo(UserPrincipal user)
     {
         DBUserPrincipal dbuser = (DBUserPrincipal)user;
 
@@ -280,11 +282,9 @@ public class DataSourceLoginService extends AbstractLoginService
                 try (ResultSet rs2 = statement2.executeQuery())
                 {
                     while (rs2.next())
-                    {
                         roles.add(rs2.getString(_roleTableRoleField));
-                    }
 
-                    return roles.toArray(new String[roles.size()]);
+                    return roles.stream().map(RolePrincipal::new).collect(Collectors.toList());
                 }
             }
         }

jetty-security/src/main/java/org/eclipse/jetty/security/AbstractLoginService.java

@@ -18,19 +18,21 @@
 
 package org.eclipse.jetty.security;
 
-import java.io.Serializable;
-import java.security.Principal;
+import java.util.ArrayList;
+import java.util.List;
 import javax.security.auth.Subject;
 import javax.servlet.ServletRequest;
 
 import org.eclipse.jetty.server.UserIdentity;
 import org.eclipse.jetty.util.component.ContainerLifeCycle;
-import org.eclipse.jetty.util.security.Credential;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
  * AbstractLoginService
+ * 
+ * Base class for LoginServices that allows subclasses to provide the user authentication and authorization information,
+ * but provides common behaviour such as handling authentication. 
  */
 public abstract class AbstractLoginService extends ContainerLifeCycle implements LoginService
 {
@@ -40,65 +42,7 @@ public abstract class AbstractLoginService extends ContainerLifeCycle implements
     protected String _name;
     protected boolean _fullValidate = false;
 
-    /**
-     * RolePrincipal
-     */
-    public static class RolePrincipal implements Principal, Serializable
-    {
-        private static final long serialVersionUID = 2998397924051854402L;
-        private final String _roleName;
-
-        public RolePrincipal(String name)
-        {
-            _roleName = name;
-        }
-
-        @Override
-        public String getName()
-        {
-            return _roleName;
-        }
-    }
-
-    /**
-     * UserPrincipal
-     */
-    public static class UserPrincipal implements Principal, Serializable
-    {
-        private static final long serialVersionUID = -6226920753748399662L;
-        private final String _name;
-        private final Credential _credential;
-
-        public UserPrincipal(String name, Credential credential)
-        {
-            _name = name;
-            _credential = credential;
-        }
-
-        public boolean authenticate(Object credentials)
-        {
-            return _credential != null && _credential.check(credentials);
-        }
-
-        public boolean authenticate(Credential c)
-        {
-            return (_credential != null && c != null && _credential.equals(c));
-        }
-
-        @Override
-        public String getName()
-        {
-            return _name;
-        }
-
-        @Override
-        public String toString()
-        {
-            return _name;
-        }
-    }
-
-    protected abstract String[] loadRoleInfo(UserPrincipal user);
+    protected abstract List<RolePrincipal> loadRoleInfo(UserPrincipal user);
 
     protected abstract UserPrincipal loadUserInfo(String username);
 
@@ -155,18 +99,22 @@ public abstract class AbstractLoginService extends ContainerLifeCycle implements
         if (userPrincipal != null && userPrincipal.authenticate(credentials))
         {
             //safe to load the roles
-            String[] roles = loadRoleInfo(userPrincipal);
+            List<RolePrincipal> roles = loadRoleInfo(userPrincipal);
 
+            List<String> roleNames = new ArrayList<>();
             Subject subject = new Subject();
-            subject.getPrincipals().add(userPrincipal);
-            subject.getPrivateCredentials().add(userPrincipal._credential);
+            userPrincipal.configureSubject(subject);
             if (roles != null)
-                for (String role : roles)
+            {
+                roles.forEach(p -> 
                 {
-                    subject.getPrincipals().add(new RolePrincipal(role));
-                }
+                    p.configureForSubject(subject);
+                    roleNames.add(p.getName());
+                });
+            }
+  
             subject.setReadOnly();
-            return _identityService.newUserIdentity(subject, userPrincipal, roles);
+            return _identityService.newUserIdentity(subject, userPrincipal, roleNames.toArray(new String[0]));
         }
 
         return null;
@@ -185,10 +133,10 @@ public abstract class AbstractLoginService extends ContainerLifeCycle implements
 
         if (user.getUserPrincipal() instanceof UserPrincipal)
         {
-            return fresh.authenticate(((UserPrincipal)user.getUserPrincipal())._credential);
+            return fresh.authenticate(((UserPrincipal)user.getUserPrincipal()));
         }
 
-        throw new IllegalStateException("UserPrincipal not KnownUser"); //can't validate
+        throw new IllegalStateException("UserPrincipal not known"); //can't validate
     }
 
     @Override
@@ -201,7 +149,6 @@ public abstract class AbstractLoginService extends ContainerLifeCycle implements
     public void logout(UserIdentity user)
     {
         //Override in subclasses
-
     }
 
     public boolean isFullValidate()

jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java

@@ -634,7 +634,8 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
         if (dataConstraint == null || dataConstraint == UserDataConstraint.None)
             return true;
 
-        HttpConfiguration httpConfig = Request.getBaseRequest(request).getHttpChannel().getHttpConfiguration();
+        Request baseRequest = Request.getBaseRequest(request);
+        HttpConfiguration httpConfig = baseRequest.getHttpChannel().getHttpConfiguration();
 
         if (dataConstraint == UserDataConstraint.Confidential || dataConstraint == UserDataConstraint.Integral)
         {
@@ -648,7 +649,7 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
 
                 String url = URIUtil.newURI(scheme, request.getServerName(), port, request.getRequestURI(), request.getQueryString());
                 response.setContentLength(0);
-                response.sendRedirect(url);
+                response.sendRedirect(url, true);
             }
             else
                 response.sendError(HttpStatus.FORBIDDEN_403, "!Secure");

jetty-security/src/main/java/org/eclipse/jetty/security/HashLoginService.java

@@ -19,20 +19,13 @@
 package org.eclipse.jetty.security;
 
 import java.util.List;
-import java.util.Set;
-import java.util.stream.Collectors;
 
-import org.eclipse.jetty.server.UserIdentity;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
- * Properties User Realm.
- * <p>
- * An implementation of UserRealm that stores users and roles in-memory in HashMaps.
- * <p>
- * Typically these maps are populated by calling the load() method or passing a properties resource to the constructor. The format of the properties file is:
- *
+ * An implementation of a LoginService that stores users and roles in-memory in HashMaps.
+ * The source of the users and roles information is a properties file formatted like so:
  * <pre>
  *  username: password [,rolename ...]
  * </pre>
@@ -72,7 +65,7 @@ public class HashLoginService extends AbstractLoginService
     }
 
     /**
-     * Load realm users from properties file.
+     * Load users from properties file.
      * <p>
      * The property file maps usernames to password specs followed by an optional comma separated list of role names.
      * </p>
@@ -121,41 +114,21 @@ public class HashLoginService extends AbstractLoginService
     }
 
     @Override
-    protected String[] loadRoleInfo(UserPrincipal user)
+    protected List<RolePrincipal> loadRoleInfo(UserPrincipal user)
     {
-        UserIdentity id = _userStore.getUserIdentity(user.getName());
-        if (id == null)
-            return null;
-
-        Set<RolePrincipal> roles = id.getSubject().getPrincipals(RolePrincipal.class);
-        if (roles == null)
-            return null;
-
-        List<String> list = roles.stream()
-            .map(rolePrincipal -> rolePrincipal.getName())
-            .collect(Collectors.toList());
-
-        return list.toArray(new String[roles.size()]);
+        return _userStore.getRolePrincipals(user.getName());
     }
 
     @Override
     protected UserPrincipal loadUserInfo(String userName)
     {
-        UserIdentity id = _userStore.getUserIdentity(userName);
-        if (id != null)
-        {
-            return (UserPrincipal)id.getUserPrincipal();
-        }
-
-        return null;
+        return _userStore.getUserPrincipal(userName);
     }
 
     @Override
     protected void doStart() throws Exception
     {
         super.doStart();
-
-        // can be null so we switch to previous behaviour using PropertyUserStore
         if (_userStore == null)
         {
             if (LOG.isDebugEnabled())
@@ -179,7 +152,6 @@ public class HashLoginService extends AbstractLoginService
     }
 
     /**
-     * To facilitate testing.
      *
      * @return true if a UserStore has been created from a config, false if a UserStore was provided.
      */

jetty-security/src/main/java/org/eclipse/jetty/security/JDBCLoginService.java

@@ -18,7 +18,6 @@
 
 package org.eclipse.jetty.security;
 
-import java.io.IOException;
 import java.io.InputStream;
 import java.sql.Connection;
 import java.sql.DriverManager;
@@ -28,7 +27,7 @@ import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Properties;
-import javax.servlet.ServletRequest;
+import java.util.stream.Collectors;
 
 import org.eclipse.jetty.util.Loader;
 import org.eclipse.jetty.util.resource.Resource;
@@ -37,17 +36,8 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
- * HashMapped User Realm with JDBC as data source.
- * The {@link #login(String, Object, ServletRequest)} method checks the inherited Map for the user. If the user is not
- * found, it will fetch details from the database and populate the inherited
- * Map. It then calls the superclass {@link #login(String, Object, ServletRequest)} method to perform the actual
- * authentication. Periodically (controlled by configuration parameter),
- * internal hashes are cleared. Caching can be disabled by setting cache refresh
- * interval to zero. Uses one database connection that is initialized at
- * startup. Reconnect on failures.
- * <p>
- * An example properties file for configuration is in
- * <code>${jetty.home}/etc/jdbcRealm.properties</code>
+ * JDBC as a source of user authentication and authorization information.
+ * Uses one database connection that is lazily initialized. Reconnect on failures.
  */
 public class JDBCLoginService extends AbstractLoginService
 {
@@ -61,16 +51,18 @@ public class JDBCLoginService extends AbstractLoginService
     protected String _userTableKey;
     protected String _userTablePasswordField;
     protected String _roleTableRoleField;
-    protected Connection _con;
     protected String _userSql;
     protected String _roleSql;
+    protected Connection _con;
 
     /**
-     * JDBCKnownUser
+     * JDBCUserPrincipal
+     * 
+     * A UserPrincipal with extra jdbc key info.
      */
     public class JDBCUserPrincipal extends UserPrincipal
     {
-        int _userKey;
+        final int _userKey;
 
         public JDBCUserPrincipal(String name, Credential credential, int key)
         {
@@ -85,25 +77,21 @@ public class JDBCLoginService extends AbstractLoginService
     }
 
     public JDBCLoginService()
-        throws IOException
     {
     }
 
     public JDBCLoginService(String name)
-        throws IOException
     {
         setName(name);
     }
 
     public JDBCLoginService(String name, String config)
-        throws IOException
     {
         setName(name);
         setConfig(config);
     }
 
     public JDBCLoginService(String name, IdentityService identityService, String config)
-        throws IOException
     {
         setName(name);
         setIdentityService(identityService);
@@ -171,19 +159,12 @@ public class JDBCLoginService extends AbstractLoginService
     }
 
     /**
-     * (re)Connect to database with parameters setup by loadConfig()
+     * Connect to database with parameters setup by loadConfig()
      */
-    public void connectDatabase()
+    public Connection connectDatabase()
+        throws SQLException
     {
-        try
-        {
-            Class.forName(_jdbcDriver);
-            _con = DriverManager.getConnection(_url, _userName, _password);
-        }
-        catch (Exception e)
-        {
-            LOG.warn("UserRealm {} could not connect to database; will try later", getName(), e);
-        }
+        return DriverManager.getConnection(_url, _userName, _password);
     }
 
     @Override
@@ -192,10 +173,7 @@ public class JDBCLoginService extends AbstractLoginService
         try
         {
             if (null == _con)
-                connectDatabase();
-
-            if (null == _con)
-                throw new SQLException("Can't connect to database");
+                _con = connectDatabase();
 
             try (PreparedStatement stat1 = _con.prepareStatement(_userSql))
             {
@@ -214,7 +192,7 @@ public class JDBCLoginService extends AbstractLoginService
         }
         catch (SQLException e)
         {
-            LOG.warn("UserRealm {} could not load user information from database", getName(), e);
+            LOG.warn("LoginService {} could not load user {}", getName(), username, e);
             closeConnection();
         }
 
@@ -222,17 +200,17 @@ public class JDBCLoginService extends AbstractLoginService
     }
 
     @Override
-    public String[] loadRoleInfo(UserPrincipal user)
+    public List<RolePrincipal> loadRoleInfo(UserPrincipal user)
     {
+        if (user == null)
+            return null;
+
         JDBCUserPrincipal jdbcUser = (JDBCUserPrincipal)user;
 
         try
         {
             if (null == _con)
-                connectDatabase();
-
-            if (null == _con)
-                throw new SQLException("Can't connect to database");
+                _con = connectDatabase();
 
             List<String> roles = new ArrayList<String>();
 
@@ -242,16 +220,15 @@ public class JDBCLoginService extends AbstractLoginService
                 try (ResultSet rs2 = stat2.executeQuery())
                 {
                     while (rs2.next())
-                    {
                         roles.add(rs2.getString(_roleTableRoleField));
-                    }
-                    return roles.toArray(new String[roles.size()]);
+                                        
+                    return roles.stream().map(RolePrincipal::new).collect(Collectors.toList());
                 }
             }
         }
         catch (SQLException e)
         {
-            LOG.warn("UserRealm {} could not load user information from database", getName(), e);
+            LOG.warn("LoginService {} could not load roles for user {}", getName(), user.getName(), e);
             closeConnection();
         }
 
@@ -273,7 +250,7 @@ public class JDBCLoginService extends AbstractLoginService
         if (_con != null)
         {
             if (LOG.isDebugEnabled())
-                LOG.debug("Closing db connection for JDBCUserRealm");
+                LOG.debug("Closing db connection for JDBCLoginService");
             try
             {
                 _con.close();

jetty-security/src/main/java/org/eclipse/jetty/security/PropertyUserStore.java

@@ -206,7 +206,7 @@ public class PropertyUserStore extends UserStore implements PathWatcher.Listener
     @Override
     public String toString()
     {
-        return String.format("%s@%x[users.count=%d,identityService=%s]", getClass().getSimpleName(), hashCode(), getKnownUserIdentities().size(), getIdentityService());
+        return String.format("%s[cfg=%s]", super.toString(), _configPath);
     }
 
     protected void loadUsers() throws IOException
@@ -251,7 +251,7 @@ public class PropertyUserStore extends UserStore implements PathWatcher.Listener
             }
         }
 
-        List<String> currentlyKnownUsers = new ArrayList<>(getKnownUserIdentities().keySet());
+        List<String> currentlyKnownUsers = new ArrayList<>(_users.keySet());
         // if its not the initial load then we want to process removed users
         if (!_firstLoad)
         {

jetty-security/src/main/java/org/eclipse/jetty/security/RolePrincipal.java

@@ -0,0 +1,52 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under
+// the terms of the Eclipse Public License 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0
+//
+// This Source Code may also be made available under the following
+// Secondary Licenses when the conditions for such availability set
+// forth in the Eclipse Public License, v. 2.0 are satisfied:
+// the Apache License v2.0 which is available at
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.security;
+
+import java.io.Serializable;
+import java.security.Principal;
+import javax.security.auth.Subject;
+
+/**
+ * RolePrincipal
+ * 
+ * Represents a role. This class can be added to a Subject to represent a role that the
+ * Subject has.
+ * 
+ */
+public class RolePrincipal implements Principal, Serializable
+{
+    private static final long serialVersionUID = 2998397924051854402L;
+    private final String _roleName;
+
+    public RolePrincipal(String name)
+    {
+        _roleName = name;
+    }
+
+    @Override
+    public String getName()
+    {
+        return _roleName;
+    }
+    
+    public void configureForSubject(Subject subject)
+    {
+        subject.getPrincipals().add(this);
+    }
+}

jetty-security/src/main/java/org/eclipse/jetty/security/UserPrincipal.java

@@ -0,0 +1,92 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under
+// the terms of the Eclipse Public License 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0
+//
+// This Source Code may also be made available under the following
+// Secondary Licenses when the conditions for such availability set
+// forth in the Eclipse Public License, v. 2.0 are satisfied:
+// the Apache License v2.0 which is available at
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.security;
+
+import java.io.Serializable;
+import java.security.Principal;
+import javax.security.auth.Subject;
+
+import org.eclipse.jetty.util.security.Credential;
+
+/**
+ * UserPrincipal
+ * 
+ * Represents a user with a credential.
+ * Instances of this class can be added to a Subject to
+ * present the user, while the credentials can be added 
+ * directly to the Subject.
+ */
+public class UserPrincipal implements Principal, Serializable
+{
+    private static final long serialVersionUID = -6226920753748399662L;
+    private final String _name;
+    protected final Credential _credential;
+
+    public UserPrincipal(String name, Credential credential)
+    {
+        _name = name;
+        _credential = credential;
+    }
+
+    public boolean authenticate(Object credentials)
+    {
+        return _credential != null && _credential.check(credentials);
+    }
+
+    public boolean authenticate(Credential c)
+    {
+        return (_credential != null && c != null && _credential.equals(c));
+    }
+    
+    public boolean authenticate(UserPrincipal u)
+    {
+        return (u != null && authenticate(u._credential));
+    }
+
+    public void configureSubject(Subject subject)
+    {
+        if (subject == null)
+            return;
+
+        subject.getPrincipals().add(this);
+        if (_credential != null)
+            subject.getPrivateCredentials().add(_credential); 
+    }
+
+    public void deconfigureSubject(Subject subject)
+    {
+        if (subject == null)
+            return;
+        subject.getPrincipals().remove(this);
+        if (_credential != null)
+            subject.getPrivateCredentials().remove(_credential);
+    }
+
+    @Override
+    public String getName()
+    {
+        return _name;
+    }
+
+    @Override
+    public String toString()
+    {
+        return _name;
+    }
+}

jetty-security/src/main/java/org/eclipse/jetty/security/UserStore.java

@@ -18,59 +18,75 @@
 
 package org.eclipse.jetty.security;
 
-import java.security.Principal;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
-import javax.security.auth.Subject;
+import java.util.stream.Collectors;
 
-import org.eclipse.jetty.server.UserIdentity;
 import org.eclipse.jetty.util.component.AbstractLifeCycle;
 import org.eclipse.jetty.util.security.Credential;
 
 /**
- * Base class to store User
+ * Store of user authentication and authorization information.
+ * 
  */
 public class UserStore extends AbstractLifeCycle
 {
-    private IdentityService _identityService = new DefaultIdentityService();
-    private final Map<String, UserIdentity> _knownUserIdentities = new ConcurrentHashMap<>();
+    protected final Map<String, User> _users = new ConcurrentHashMap<>();
+    
+    protected class User
+    {
+        protected UserPrincipal _userPrincipal;
+        protected List<RolePrincipal> _rolePrincipals = Collections.emptyList();
+        
+        protected User(String username, Credential credential, String[] roles)
+        {
+            _userPrincipal = new UserPrincipal(username, credential);
 
+            _rolePrincipals = Collections.emptyList();
+            
+            if (roles != null)
+                _rolePrincipals = Arrays.stream(roles).map(RolePrincipal::new).collect(Collectors.toList());
+        }
+        
+        protected UserPrincipal getUserPrincipal()
+        {
+            return _userPrincipal;
+        }
+        
+        protected List<RolePrincipal> getRolePrincipals()
+        {
+            return _rolePrincipals;
+        }
+    }
+    
     public void addUser(String username, Credential credential, String[] roles)
     {
-        Principal userPrincipal = new AbstractLoginService.UserPrincipal(username, credential);
-        Subject subject = new Subject();
-        subject.getPrincipals().add(userPrincipal);
-        subject.getPrivateCredentials().add(credential);
-
-        if (roles != null)
-        {
-            for (String role : roles)
-            {
-                subject.getPrincipals().add(new AbstractLoginService.RolePrincipal(role));
-            }
-        }
-
-        subject.setReadOnly();
-        _knownUserIdentities.put(username, _identityService.newUserIdentity(subject, userPrincipal, roles));
+        _users.put(username, new User(username, credential, roles));
     }
 
     public void removeUser(String username)
     {
-        _knownUserIdentities.remove(username);
+        _users.remove(username);
+    }
+    
+    public UserPrincipal getUserPrincipal(String username)
+    {
+        User user = _users.get(username);
+        return (user == null ? null : user.getUserPrincipal());
+    }
+    
+    public List<RolePrincipal> getRolePrincipals(String username)
+    {
+        User user = _users.get(username);
+        return (user == null ? null : user.getRolePrincipals());
     }
 
-    public UserIdentity getUserIdentity(String userName)
+    @Override
+    public String toString()
     {
-        return _knownUserIdentities.get(userName);
-    }
-
-    public IdentityService getIdentityService()
-    {
-        return _identityService;
-    }
-
-    public Map<String, UserIdentity> getKnownUserIdentities()
-    {
-        return _knownUserIdentities;
+        return String.format("%s@%x[users.count=%d]", getClass().getSimpleName(), hashCode(), _users.size());
     }
 }

jetty-security/src/main/java/org/eclipse/jetty/security/authentication/FormAuthenticator.java

@@ -35,7 +35,6 @@ import javax.servlet.http.HttpSession;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpHeaderValue;
 import org.eclipse.jetty.http.HttpMethod;
-import org.eclipse.jetty.http.HttpVersion;
 import org.eclipse.jetty.http.MimeTypes;
 import org.eclipse.jetty.security.ServerAuthException;
 import org.eclipse.jetty.security.UserAuthentication;
@@ -288,8 +287,7 @@ public class FormAuthenticator extends LoginAuthenticator
                     LOG.debug("authenticated {}->{}", formAuth, nuri);
 
                     response.setContentLength(0);
-                    int redirectCode = (baseRequest.getHttpVersion().getVersion() < HttpVersion.HTTP_1_1.getVersion() ? HttpServletResponse.SC_MOVED_TEMPORARILY : HttpServletResponse.SC_SEE_OTHER);
-                    baseResponse.sendRedirect(redirectCode, response.encodeRedirectURL(nuri));
+                    baseResponse.sendRedirect(response.encodeRedirectURL(nuri), true);
                     return formAuth;
                 }
 
@@ -313,8 +311,7 @@ public class FormAuthenticator extends LoginAuthenticator
                 else
                 {
                     LOG.debug("auth failed {}->{}", username, _formErrorPage);
-                    int redirectCode = (baseRequest.getHttpVersion().getVersion() < HttpVersion.HTTP_1_1.getVersion() ? HttpServletResponse.SC_MOVED_TEMPORARILY : HttpServletResponse.SC_SEE_OTHER);
-                    baseResponse.sendRedirect(redirectCode, response.encodeRedirectURL(URIUtil.addPaths(request.getContextPath(), _formErrorPage)));
+                    baseResponse.sendRedirect(response.encodeRedirectURL(URIUtil.addPaths(request.getContextPath(), _formErrorPage)), true);
                 }
 
                 return Authentication.SEND_FAILURE;
@@ -407,8 +404,7 @@ public class FormAuthenticator extends LoginAuthenticator
             else
             {
                 LOG.debug("challenge {}->{}", session.getId(), _formLoginPage);
-                int redirectCode = (baseRequest.getHttpVersion().getVersion() < HttpVersion.HTTP_1_1.getVersion() ? HttpServletResponse.SC_MOVED_TEMPORARILY : HttpServletResponse.SC_SEE_OTHER);
-                baseResponse.sendRedirect(redirectCode, response.encodeRedirectURL(URIUtil.addPaths(request.getContextPath(), _formLoginPage)));
+                baseResponse.sendRedirect(response.encodeRedirectURL(URIUtil.addPaths(request.getContextPath(), _formLoginPage)), true);
             }
             return Authentication.SEND_CONTINUE;
         }

jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java

@@ -77,6 +77,7 @@ import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.in;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.nullValue;
 import static org.hamcrest.Matchers.startsWith;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -468,9 +469,6 @@ public class ConstraintTest
             )
         ));
 
-//        rawResponse = _connector.getResponse("GET /ctx/noauth/info HTTP/1.0\r\n\r\n");
-//        assertThat(rawResponse, startsWith("HTTP/1.1 200 OK"));
-
         scenarios.add(Arguments.of(
             new Scenario(
                 "GET /ctx/forbid/info HTTP/1.0\r\n\r\n",
@@ -478,9 +476,6 @@ public class ConstraintTest
             )
         ));
 
-//        rawResponse = _connector.getResponse("GET /ctx/forbid/info HTTP/1.0\r\n\r\n");
-//        assertThat(rawResponse, startsWith("HTTP/1.1 403 Forbidden"));
-
         scenarios.add(Arguments.of(
             new Scenario(
                 "GET /ctx/auth/info HTTP/1.0\r\n\r\n",
@@ -493,9 +488,39 @@ public class ConstraintTest
             )
         ));
 
-//        rawResponse = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n\r\n");
-//        assertThat(rawResponse, startsWith("HTTP/1.1 401 Unauthorized"));
-//        assertThat(rawResponse, containsString("WWW-Authenticate: basic realm=\"TestRealm\""));
+        scenarios.add(Arguments.of(
+            new Scenario(
+                "POST /ctx/auth/info HTTP/1.1\r\n" +
+                    "Host: test\r\n" +
+                    "Content-Length: 10\r\n" +
+                    "\r\n" +
+                    "0123456789",
+                HttpStatus.UNAUTHORIZED_401,
+                (response) ->
+                {
+                    String authHeader = response.get(HttpHeader.WWW_AUTHENTICATE);
+                    assertThat(response.toString(), authHeader, containsString("basic realm=\"TestRealm\""));
+                    assertThat(response.get(HttpHeader.CONNECTION), nullValue());
+                }
+            )
+        ));
+
+        scenarios.add(Arguments.of(
+            new Scenario(
+                "POST /ctx/auth/info HTTP/1.1\r\n" +
+                    "Host: test\r\n" +
+                    "Content-Length: 10\r\n" +
+                    "\r\n" +
+                    "012345",
+                HttpStatus.UNAUTHORIZED_401,
+                (response) ->
+                {
+                    String authHeader = response.get(HttpHeader.WWW_AUTHENTICATE);
+                    assertThat(response.toString(), authHeader, containsString("basic realm=\"TestRealm\""));
+                    assertThat(response.get(HttpHeader.CONNECTION), is("close"));
+                }
+            )
+        ));
 
         scenarios.add(Arguments.of(
             new Scenario(
@@ -511,12 +536,6 @@ public class ConstraintTest
             )
         ));
 
-//        rawResponse = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n" +
-//                "Authorization: Basic " + authBase64("user:wrong") + "\r\n" +
-//                "\r\n");
-//        assertThat(rawResponse, startsWith("HTTP/1.1 401 Unauthorized"));
-//        assertThat(rawResponse, containsString("WWW-Authenticate: basic realm=\"TestRealm\""));
-
         scenarios.add(Arguments.of(
             new Scenario(
                 "GET /ctx/auth/info HTTP/1.0\r\n" +
@@ -526,10 +545,16 @@ public class ConstraintTest
             )
         ));
 
-//        rawResponse = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n" +
-//                "Authorization: Basic " + authBase64("user:password") + "\r\n" +
-//                "\r\n");
-//        assertThat(rawResponse, startsWith("HTTP/1.1 200 OK"));
+        scenarios.add(Arguments.of(
+            new Scenario(
+                "POST /ctx/auth/info HTTP/1.0\r\n" +
+                    "Content-Length: 10\r\n" +
+                    "Authorization: Basic " + authBase64("user:password") + "\r\n" +
+                    "\r\n" +
+                    "0123456789",
+                HttpStatus.OK_200
+            )
+        ));
 
         // == test admin
         scenarios.add(Arguments.of(
@@ -544,10 +569,6 @@ public class ConstraintTest
             )
         ));
 
-//        rawResponse = _connector.getResponse("GET /ctx/admin/info HTTP/1.0\r\n\r\n");
-//        assertThat(rawResponse, startsWith("HTTP/1.1 401 Unauthorized"));
-//        assertThat(rawResponse, containsString("WWW-Authenticate: basic realm=\"TestRealm\""));
-
         scenarios.add(Arguments.of(
             new Scenario(
                 "GET /ctx/admin/info HTTP/1.0\r\n" +
@@ -1007,6 +1028,63 @@ public class ConstraintTest
         assertThat(response, containsString("!role"));
     }
 
+    @Test
+    public void testNonFormPostRedirectHttp10() throws Exception
+    {
+        _security.setAuthenticator(new FormAuthenticator("/testLoginPage", "/testErrorPage", false));
+        _server.start();
+
+        String response = _connector.getResponse("POST /ctx/auth/info HTTP/1.0\r\n" +
+            "Content-Type: text/plain\r\n" +
+            "Connection: keep-alive\r\n" +
+            "Content-Length: 10\r\n" +
+            "\r\n" +
+            "0123456789\r\n");
+        assertThat(response, containsString(" 302 Found"));
+        assertThat(response, containsString("/ctx/testLoginPage"));
+        assertThat(response, not(containsString("Connection: close")));
+        assertThat(response, containsString("Connection: keep-alive"));
+
+        response = _connector.getResponse("POST /ctx/auth/info HTTP/1.0\r\n" +
+            "Host: localhost\r\n" +
+            "Content-Type: text/plain\r\n" +
+            "Connection: keep-alive\r\n" +
+            "Content-Length: 10\r\n" +
+            "\r\n" +
+            "012345\r\n");
+        assertThat(response, containsString(" 302 Found"));
+        assertThat(response, containsString("/ctx/testLoginPage"));
+        assertThat(response, not(containsString("Connection: keep-alive")));
+    }
+
+    @Test
+    public void testNonFormPostRedirectHttp11() throws Exception
+    {
+        _security.setAuthenticator(new FormAuthenticator("/testLoginPage", "/testErrorPage", false));
+        _server.start();
+
+        String response = _connector.getResponse("POST /ctx/auth/info HTTP/1.1\r\n" +
+            "Host: test\r\n" +
+            "Content-Type: text/plain\r\n" +
+            "Content-Length: 10\r\n" +
+            "\r\n" +
+            "0123456789\r\n");
+        assertThat(response, containsString(" 303 See Other"));
+        assertThat(response, containsString("/ctx/testLoginPage"));
+        assertThat(response, not(containsString("Connection: close")));
+
+        response = _connector.getResponse("POST /ctx/auth/info HTTP/1.1\r\n" +
+            "Host: test\r\n" +
+            "Host: localhost\r\n" +
+            "Content-Type: text/plain\r\n" +
+            "Content-Length: 10\r\n" +
+            "\r\n" +
+            "012345\r\n");
+        assertThat(response, containsString(" 303 See Other"));
+        assertThat(response, containsString("/ctx/testLoginPage"));
+        assertThat(response, containsString("Connection: close"));
+    }
+
     @Test
     public void testFormNoCookies() throws Exception
     {

jetty-security/src/test/java/org/eclipse/jetty/security/PropertyUserStoreTest.java

@@ -191,9 +191,9 @@ public class PropertyUserStoreTest
 
         store.start();
 
-        assertThat("Failed to retrieve UserIdentity directly from PropertyUserStore", store.getUserIdentity("tom"), notNullValue());
-        assertThat("Failed to retrieve UserIdentity directly from PropertyUserStore", store.getUserIdentity("dick"), notNullValue());
-        assertThat("Failed to retrieve UserIdentity directly from PropertyUserStore", store.getUserIdentity("harry"), notNullValue());
+        assertThat("Failed to retrieve user directly from PropertyUserStore", store.getUserPrincipal("tom"), notNullValue());
+        assertThat("Failed to retrieve user directly from PropertyUserStore", store.getUserPrincipal("dick"), notNullValue());
+        assertThat("Failed to retrieve user directly from PropertyUserStore", store.getUserPrincipal("harry"), notNullValue());
         userCount.assertThatCount(is(3));
         userCount.awaitCount(3);
     }
@@ -224,12 +224,12 @@ public class PropertyUserStoreTest
 
         store.start();
 
-        assertThat("Failed to retrieve UserIdentity directly from PropertyUserStore", //
-            store.getUserIdentity("tom"), notNullValue());
-        assertThat("Failed to retrieve UserIdentity directly from PropertyUserStore", //
-            store.getUserIdentity("dick"), notNullValue());
-        assertThat("Failed to retrieve UserIdentity directly from PropertyUserStore", //
-            store.getUserIdentity("harry"), notNullValue());
+        assertThat("Failed to retrieve user directly from PropertyUserStore", //
+            store.getUserPrincipal("tom"), notNullValue());
+        assertThat("Failed to retrieve user directly from PropertyUserStore", //
+            store.getUserPrincipal("dick"), notNullValue());
+        assertThat("Failed to retrieve user directly from PropertyUserStore", //
+            store.getUserPrincipal("harry"), notNullValue());
         userCount.assertThatCount(is(3));
         userCount.awaitCount(3);
     }
@@ -264,7 +264,7 @@ public class PropertyUserStoreTest
         addAdditionalUser(usersFile, "skip: skip, roleA\n");
         userCount.awaitCount(4);
         assertThat(loadCount.get(), is(2));
-        assertThat(store.getUserIdentity("skip"), notNullValue());
+        assertThat(store.getUserPrincipal("skip"), notNullValue());
         userCount.assertThatCount(is(4));
         userCount.assertThatUsers(hasItem("skip"));
 

jetty-security/src/test/java/org/eclipse/jetty/security/TestLoginService.java

@@ -44,26 +44,14 @@ public class TestLoginService extends AbstractLoginService
     }
 
     @Override
-    protected String[] loadRoleInfo(UserPrincipal user)
+    protected List<RolePrincipal> loadRoleInfo(UserPrincipal user)
     {
-        UserIdentity userIdentity = userStore.getUserIdentity(user.getName());
-        Set<RolePrincipal> roles = userIdentity.getSubject().getPrincipals(RolePrincipal.class);
-        if (roles == null)
-            return null;
-
-        List<String> list = new ArrayList<>();
-        for (RolePrincipal r : roles)
-        {
-            list.add(r.getName());
-        }
-
-        return list.toArray(new String[roles.size()]);
+        return userStore.getRolePrincipals(user.getName());
     }
 
     @Override
     protected UserPrincipal loadUserInfo(String username)
     {
-        UserIdentity userIdentity = userStore.getUserIdentity(username);
-        return userIdentity == null ? null : (UserPrincipal)userIdentity.getUserPrincipal();
+        return userStore.getUserPrincipal(username);
     }
 }

jetty-security/src/test/java/org/eclipse/jetty/security/UserStoreTest.java

@@ -19,10 +19,7 @@
 package org.eclipse.jetty.security;
 
 import java.util.List;
-import java.util.Set;
-import java.util.stream.Collectors;
 
-import org.eclipse.jetty.server.UserIdentity;
 import org.eclipse.jetty.util.security.Credential;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -44,30 +41,21 @@ public class UserStoreTest
     @Test
     public void addUser()
     {
-        this.userStore.addUser("foo", Credential.getCredential("beer"), new String[]{"pub"});
-        assertEquals(1, this.userStore.getKnownUserIdentities().size());
-        UserIdentity userIdentity = this.userStore.getUserIdentity("foo");
-        assertNotNull(userIdentity);
-        assertEquals("foo", userIdentity.getUserPrincipal().getName());
-        Set<AbstractLoginService.RolePrincipal>
-            roles = userIdentity.getSubject().getPrincipals(AbstractLoginService.RolePrincipal.class);
-        List<String> list = roles.stream()
-            .map(rolePrincipal -> rolePrincipal.getName())
-            .collect(Collectors.toList());
-        assertEquals(1, list.size());
-        assertEquals("pub", list.get(0));
+        userStore.addUser("foo", Credential.getCredential("beer"), new String[]{"pub"});
+        assertNotNull(userStore.getUserPrincipal("foo"));
+
+        List<RolePrincipal> rps = userStore.getRolePrincipals("foo");
+        assertNotNull(rps);
+        assertNotNull(rps.get(0));
+        assertEquals("pub", rps.get(0).getName());
     }
 
     @Test
     public void removeUser()
     {
         this.userStore.addUser("foo", Credential.getCredential("beer"), new String[]{"pub"});
-        assertEquals(1, this.userStore.getKnownUserIdentities().size());
-        UserIdentity userIdentity = this.userStore.getUserIdentity("foo");
-        assertNotNull(userIdentity);
-        assertEquals("foo", userIdentity.getUserPrincipal().getName());
+        assertNotNull(userStore.getUserPrincipal("foo"));
         userStore.removeUser("foo");
-        userIdentity = this.userStore.getUserIdentity("foo");
-        assertNull(userIdentity);
+        assertNull(userStore.getUserPrincipal("foo"));
     }
 }

jetty-server/src/main/config/modules/stats.mod

@@ -8,6 +8,10 @@ server
 
 [depend]
 server
+servlet
+
+[lib]
+lib/jetty-util-ajax-${jetty.version}.jar
 
 [xml]
 etc/jetty-stats.xml

jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java

@@ -30,14 +30,18 @@ import java.util.concurrent.atomic.AtomicLong;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import javax.servlet.DispatcherType;
 import javax.servlet.RequestDispatcher;
 import javax.servlet.ServletException;
 
 import org.eclipse.jetty.http.BadMessageException;
+import org.eclipse.jetty.http.HttpField;
 import org.eclipse.jetty.http.HttpFields;
 import org.eclipse.jetty.http.HttpGenerator;
 import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.http.HttpHeaderValue;
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.HttpVersion;
 import org.eclipse.jetty.http.MetaData;
@@ -51,6 +55,7 @@ import org.eclipse.jetty.server.handler.ErrorHandler;
 import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.SharedBlockingCallback.Blocker;
+import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.thread.Scheduler;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -422,7 +427,16 @@ public abstract class HttpChannel implements Runnable, HttpOutput.Interceptor
                             // the following is needed as you cannot trust the response code and reason
                             // as those could have been modified after calling sendError
                             Integer code = (Integer)_request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE);
-                            _response.setStatus(code != null ? code : HttpStatus.INTERNAL_SERVER_ERROR_500);
+                            if (code == null)
+                                code = HttpStatus.INTERNAL_SERVER_ERROR_500;
+                            _response.setStatus(code);
+
+                            // The handling of the original dispatch failed and we are now going to either generate
+                            // and error response ourselves or dispatch for an error page.  If there is content left over
+                            // from the failed dispatch, then we try to consume it here and if we fail we add a
+                            // Connection:close.  This can't be deferred to COMPLETE as the response will be committed
+                            // by then.
+                            ensureConsumeAllOrNotPersistent();
 
                             ContextHandler.Context context = (ContextHandler.Context)_request.getAttribute(ErrorHandler.ERROR_CONTEXT);
                             ErrorHandler errorHandler = ErrorHandler.getErrorHandler(getServer(), context == null ? null : context.getContextHandler());
@@ -496,10 +510,18 @@ public abstract class HttpChannel implements Runnable, HttpOutput.Interceptor
 
                     case COMPLETE:
                     {
-                        if (!_response.isCommitted() && !_request.isHandled() && !_response.getHttpOutput().isClosed())
+                        if (!_response.isCommitted())
                         {
-                            _response.sendError(HttpStatus.NOT_FOUND_404);
-                            break;
+                            if (!_request.isHandled() && !_response.getHttpOutput().isClosed())
+                            {
+                                // The request was not actually handled
+                                _response.sendError(HttpStatus.NOT_FOUND_404);
+                                break;
+                            }
+
+                            // Indicate Connection:close if we can't consume all.
+                            if (_response.getStatus() >= 200)
+                                ensureConsumeAllOrNotPersistent();
                         }
 
                         // RFC 7230, section 3.3.
@@ -514,12 +536,7 @@ public abstract class HttpChannel implements Runnable, HttpOutput.Interceptor
                         // If send error is called we need to break.
                         if (checkAndPrepareUpgrade())
                             break;
-
-                        // TODO Currently a blocking/aborting consumeAll is done in the handling of the TERMINATED
-                        // TODO Action triggered by the completed callback below.  It would be possible to modify the
-                        // TODO callback to do a non-blocking consumeAll at this point and only call completed when
-                        // TODO that is done.
-
+                        
                         // Set a close callback on the HttpOutput to make it an async callback
                         _response.completeOutput(Callback.from(() -> _state.completed(null), _state::completed));
 
@@ -548,6 +565,66 @@ public abstract class HttpChannel implements Runnable, HttpOutput.Interceptor
         return !suspended;
     }
 
+    public void ensureConsumeAllOrNotPersistent()
+    {
+        switch (_request.getHttpVersion())
+        {
+            case HTTP_1_0:
+                if (_request.getHttpInput().consumeAll())
+                    return;
+
+                // Remove any keep-alive value in Connection headers
+                _response.getHttpFields().computeField(HttpHeader.CONNECTION, (h, fields) ->
+                {
+                    if (fields == null || fields.isEmpty())
+                        return null;
+                    String v = fields.stream()
+                        .flatMap(field -> Stream.of(field.getValues()).filter(s -> !HttpHeaderValue.KEEP_ALIVE.is(s)))
+                        .collect(Collectors.joining(", "));
+                    if (StringUtil.isEmpty(v))
+                        return null;
+
+                    return new HttpField(HttpHeader.CONNECTION, v);
+                });
+                break;
+
+            case HTTP_1_1:
+                if (_request.getHttpInput().consumeAll())
+                    return;
+
+                // Add close value to Connection headers
+                _response.getHttpFields().computeField(HttpHeader.CONNECTION, (h, fields) ->
+                {
+                    if (fields == null || fields.isEmpty())
+                        return HttpConnection.CONNECTION_CLOSE;
+
+                    if (fields.stream().anyMatch(f -> f.contains(HttpHeaderValue.CLOSE.asString())))
+                    {
+                        if (fields.size() == 1)
+                        {
+                            HttpField f = fields.get(0);
+                            if (HttpConnection.CONNECTION_CLOSE.equals(f))
+                                return f;
+                        }
+
+                        return new HttpField(HttpHeader.CONNECTION, fields.stream()
+                            .flatMap(field -> Stream.of(field.getValues()).filter(s -> !HttpHeaderValue.KEEP_ALIVE.is(s)))
+                            .collect(Collectors.joining(", ")));
+                    }
+
+                    return new HttpField(HttpHeader.CONNECTION,
+                        Stream.concat(fields.stream()
+                        .flatMap(field -> Stream.of(field.getValues()).filter(s -> !HttpHeaderValue.KEEP_ALIVE.is(s))),
+                        Stream.of(HttpHeaderValue.CLOSE.asString()))
+                        .collect(Collectors.joining(", ")));
+                });
+                break;
+
+            default:
+                break;
+        }
+    }
+
     /**
      * @param message the error message.
      * @return true if we have sent an error, false if we have aborted.

jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java

@@ -904,8 +904,6 @@ public class HttpChannelState
                 default:
                     throw new IllegalStateException(getStatusStringLocked());
             }
-            if (_outputState != OutputState.OPEN)
-                throw new IllegalStateException("Response is " + _outputState);
 
             response.setStatus(code);
             response.errorClose();

jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java

@@ -199,7 +199,7 @@ public class HttpConfiguration implements Dumpable
         return _responseHeaderSize;
     }
 
-    @ManagedAttribute("The maximum allowed size in bytes for an HTTP header field cache")
+    @ManagedAttribute("The maximum allowed size in Trie nodes for an HTTP header field cache")
     public int getHeaderCacheSize()
     {
         return _headerCacheSize;
@@ -423,7 +423,8 @@ public class HttpConfiguration implements Dumpable
     }
 
     /**
-     * @param headerCacheSize The size in bytes of the header field cache.
+     * @param headerCacheSize The size of the header field cache, in terms of unique characters branches
+     * in the lookup {@link Trie} and associated data structures.
      */
     public void setHeaderCacheSize(int headerCacheSize)
     {

jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java

@@ -428,28 +428,12 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
             // close to seek EOF
             _parser.close();
         }
-        else if (_parser.inContentState() && _generator.isPersistent())
+        // else abort if we can't consume all
+        else if (_generator.isPersistent() && !_input.consumeAll())
         {
-            // Try to progress without filling.
-            parseRequestBuffer();
-            if (_parser.inContentState())
-            {
-                // If we are async, then we have problems to complete neatly
-                if (_input.isAsync())
-                {
-                    if (LOG.isDebugEnabled())
-                        LOG.debug("{}unconsumed input while async {}", _parser.isChunking() ? "Possible " : "", this);
-                    _channel.abort(new IOException("unconsumed input"));
-                }
-                else
-                {
-                    if (LOG.isDebugEnabled())
-                        LOG.debug("{}unconsumed input {}", _parser.isChunking() ? "Possible " : "", this);
-                    // Complete reading the request
-                    if (!_input.consumeAll())
-                        _channel.abort(new IOException("unconsumed input"));
-                }
-            }
+            if (LOG.isDebugEnabled())
+                LOG.debug("unconsumed input {} {}", this, _parser);
+            _channel.abort(new IOException("unconsumed input"));
         }
 
         // Reset the channel, parsers and generator

jetty-server/src/main/java/org/eclipse/jetty/server/Request.java

@@ -204,8 +204,8 @@ public class Request implements HttpServletRequest
     private String _method;
     private String _pathInContext;
     private ServletPathMapping _servletPathMapping;
-    private boolean _secure;
     private Object _asyncNotSupportedSource = null;
+    private boolean _secure;
     private boolean _newContext;
     private boolean _cookiesExtracted = false;
     private boolean _handled = false;
@@ -220,12 +220,12 @@ public class Request implements HttpServletRequest
     private Cookies _cookies;
     private DispatcherType _dispatcherType;
     private int _inputState = INPUT_NONE;
+    private BufferedReader _reader;
+    private String _readerEncoding;
     private MultiMap<String> _queryParameters;
     private MultiMap<String> _contentParameters;
     private MultiMap<String> _parameters;
     private Charset _queryEncoding;
-    private BufferedReader _reader;
-    private String _readerEncoding;
     private InetSocketAddress _remote;
     private String _requestedSessionId;
     private UserIdentity.Scope _scope;
@@ -1751,16 +1751,9 @@ public class Request implements HttpServletRequest
 
     protected void recycle()
     {
-        _metaData = null;
-        _httpFields = null;
-        _trailers = null;
-        _method = null;
-        _uri = null;
-
         if (_context != null)
             throw new IllegalStateException("Request in context!");
-
-        if (_inputState == INPUT_READER)
+        if (_reader != null && _inputState == INPUT_READER)
         {
             try
             {
@@ -1774,17 +1767,27 @@ public class Request implements HttpServletRequest
             {
                 LOG.trace("IGNORED", e);
                 _reader = null;
+                _readerEncoding = null;
             }
         }
 
-        _dispatcherType = null;
-        setAuthentication(Authentication.NOT_CHECKED);
         getHttpChannelState().recycle();
-        if (_async != null)
-            _async.reset();
-        _async = null;
+        _requestAttributeListeners.clear();
+        _input.recycle();
+        _metaData = null;
+        _httpFields = null;
+        _trailers = null;
+        _uri = null;
+        _method = null;
+        _pathInContext = null;
+        _servletPathMapping = null;
         _asyncNotSupportedSource = null;
+        _secure = false;
+        _newContext = false;
+        _cookiesExtracted = false;
         _handled = false;
+        _contentParamsExtracted = false;
+        _requestedSessionIdFromCookie = false;
         _attributes = Attributes.unwrap(_attributes);
         if (_attributes != null)
         {
@@ -1793,33 +1796,32 @@ public class Request implements HttpServletRequest
             else
                 _attributes = null;
         }
+        setAuthentication(Authentication.NOT_CHECKED);
         _contentType = null;
         _characterEncoding = null;
-        _pathInContext = null;
-        if (_cookies != null)
-            _cookies.reset();
-        _cookiesExtracted = false;
         _context = null;
         _errorContext = null;
-        _newContext = false;
-        _queryEncoding = null;
-        _requestedSessionId = null;
-        _requestedSessionIdFromCookie = false;
-        _secure = false;
-        _session = null;
-        _sessionHandler = null;
-        _scope = null;
-        _timeStamp = 0;
+        if (_cookies != null)
+            _cookies.reset();
+        _dispatcherType = null;
+        _inputState = INPUT_NONE;
+        // _reader can be reused
+        // _readerEncoding can be reused
         _queryParameters = null;
         _contentParameters = null;
         _parameters = null;
-        _contentParamsExtracted = false;
-        _inputState = INPUT_NONE;
-        _multiParts = null;
+        _queryEncoding = null;
         _remote = null;
+        _requestedSessionId = null;
+        _scope = null;
+        _session = null;
+        _sessionHandler = null;
+        _timeStamp = 0;
+        _multiParts = null;
+        if (_async != null)
+            _async.reset();
+        _async = null;
         _sessions = null;
-        _input.recycle();
-        _requestAttributeListeners.clear();
     }
 
     @Override

jetty-server/src/main/java/org/eclipse/jetty/server/Response.java

@@ -122,17 +122,20 @@ public class Response implements HttpServletResponse
 
     protected void recycle()
     {
+        // _channel need not be recycled
+        _fields.clear();
+        _errorSentAndIncludes.set(0);
+        _out.recycle();
         _status = HttpStatus.OK_200;
         _reason = null;
         _locale = null;
         _mimeType = null;
         _characterEncoding = null;
+        _encodingFrom = EncodingFrom.NOT_SET;
         _contentType = null;
         _outputType = OutputType.NONE;
+        // _writer does not need to be recycled
         _contentLength = -1;
-        _out.recycle();
-        _fields.clear();
-        _encodingFrom = EncodingFrom.NOT_SET;
         _trailers = null;
     }
 
@@ -495,7 +498,37 @@ public class Response implements HttpServletResponse
      */
     public void sendRedirect(int code, String location) throws IOException
     {
-        if ((code < HttpServletResponse.SC_MULTIPLE_CHOICES) || (code >= HttpServletResponse.SC_BAD_REQUEST))
+        sendRedirect(code, location, false);
+    }
+
+    /**
+     * Sends a response with a HTTP version appropriate 30x redirection.
+     *
+     * @param location the location to send in {@code Location} headers
+     * @param consumeAll if True, consume any HTTP/1 request input before doing the redirection. If the input cannot
+     * be consumed without blocking, then add a `Connection: close` header to the response.
+     * @throws IOException if unable to send the redirect
+     */
+    public void sendRedirect(String location, boolean consumeAll) throws IOException
+    {
+        sendRedirect(getHttpChannel().getRequest().getHttpVersion().getVersion() < HttpVersion.HTTP_1_1.getVersion()
+            ? HttpServletResponse.SC_MOVED_TEMPORARILY : HttpServletResponse.SC_SEE_OTHER, location, consumeAll);
+    }
+
+    /**
+     * Sends a response with a given redirection code.
+     *
+     * @param code the redirect status code
+     * @param location the location to send in {@code Location} headers
+     * @param consumeAll if True, consume any HTTP/1 request input before doing the redirection. If the input cannot
+     * be consumed without blocking, then add a `Connection: close` header to the response.
+     * @throws IOException if unable to send the redirect
+     */
+    public void sendRedirect(int code, String location, boolean consumeAll) throws IOException
+    {
+        if (consumeAll)
+            getHttpChannel().ensureConsumeAllOrNotPersistent();
+        if (!HttpStatus.isRedirection(code))
             throw new IllegalArgumentException("Not a 3xx redirect code");
 
         if (!isMutable())

jetty-server/src/main/java/org/eclipse/jetty/server/SecureRequestCustomizer.java

@@ -338,10 +338,11 @@ public class SecureRequestCustomizer implements HttpConfiguration.Customizer
 
             try
             {
-                _certs = getSslSessionData().getCerts();
+                SslSessionData sslSessionData = getSslSessionData();
+                _certs = sslSessionData.getCerts();
                 _cipherSuite = _session.getCipherSuite();
-                _keySize = getSslSessionData().getKeySize();
-                _sessionId = getSslSessionData().getIdStr();
+                _keySize = sslSessionData.getKeySize();
+                _sessionId = sslSessionData.getIdStr();
                 _sessionAttribute = getSslSessionAttribute();
             }
             catch (Exception e)

jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java

@@ -1191,10 +1191,11 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu
         {
             // context request must end with /
             baseRequest.setHandled(true);
-            if (baseRequest.getQueryString() != null)
-                response.sendRedirect(baseRequest.getRequestURI() + "/?" + baseRequest.getQueryString());
-            else
-                response.sendRedirect(baseRequest.getRequestURI() + "/");
+            String queryString = baseRequest.getQueryString();
+            baseRequest.getResponse().sendRedirect(
+                HttpServletResponse.SC_MOVED_TEMPORARILY,
+                baseRequest.getRequestURI() + (queryString == null ? "/" : ("/?" + queryString)),
+                true);
             return false;
         }
 

jetty-server/src/main/java/org/eclipse/jetty/server/handler/SecuredRedirectHandler.java

@@ -64,7 +64,7 @@ public class SecuredRedirectHandler extends HandlerWrapper
             String secureScheme = httpConfig.getSecureScheme();
             String url = URIUtil.newURI(secureScheme, baseRequest.getServerName(), securePort, baseRequest.getRequestURI(), baseRequest.getQueryString());
             response.setContentLength(0);
-            response.sendRedirect(url);
+            baseRequest.getResponse().sendRedirect(HttpServletResponse.SC_MOVED_TEMPORARILY, url, true);
         }
         else
         {

jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpInputInterceptor.java

@@ -56,6 +56,12 @@ public class GzipHttpInputInterceptor implements HttpInput.Interceptor, Destroya
             {
                 _decoder.release(chunk);
             }
+
+            @Override
+            public void failed(Throwable x)
+            {
+                _decoder.release(chunk);
+            }
         };
     }
 

jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java

@@ -260,7 +260,9 @@ public class FileSessionDataStore extends AbstractSessionDataStore
             //files with 0 expiry never expire
             if (expiry > 0 && expiry <= time)
             {
-                Files.deleteIfExists(p);
+                if (!Files.deleteIfExists(p))
+                    LOG.warn("Failed to delete {}", p.getFileName());
+                
                 if (LOG.isDebugEnabled())
                     LOG.debug("Sweep deleted {}", p.getFileName());
             }

jetty-server/src/test/java/org/eclipse/jetty/server/AsyncRequestReadTest.java

@@ -293,8 +293,8 @@ public class AsyncRequestReadTest
 
             BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
             assertThat(in.readLine(), containsString("HTTP/1.1 200 OK"));
-            assertThat(in.readLine(), containsString("Content-Length:"));
             assertThat(in.readLine(), containsString("Connection: close"));
+            assertThat(in.readLine(), containsString("Content-Length:"));
             assertThat(in.readLine(), containsString("Server:"));
             in.readLine();
             assertThat(in.readLine(), containsString("XXXXXXX"));
@@ -328,6 +328,7 @@ public class AsyncRequestReadTest
 
             BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
             assertThat(in.readLine(), containsString("HTTP/1.1 200 OK"));
+            assertThat(in.readLine(), containsString("Connection: close"));
             assertThat(in.readLine(), containsString("Content-Length:"));
             assertThat(in.readLine(), containsString("Server:"));
             in.readLine();

jetty-server/src/test/java/org/eclipse/jetty/server/ErrorHandlerTest.java

@@ -232,6 +232,90 @@ public class ErrorHandlerTest
         assertContent(response);
     }
 
+    @Test
+    public void test404PostHttp10() throws Exception
+    {
+        String rawResponse = connector.getResponse(
+            "POST / HTTP/1.0\r\n" +
+                "Host: Localhost\r\n" +
+                "Accept: text/html\r\n" +
+                "Content-Length: 10\r\n" +
+                "Connection: keep-alive\r\n" +
+                "\r\n" +
+                "0123456789");
+
+        HttpTester.Response response = HttpTester.parseResponse(rawResponse);
+
+        assertThat(response.getStatus(), is(404));
+        assertThat(response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0));
+        assertThat(response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=ISO-8859-1"));
+        assertThat(response.get(HttpHeader.CONNECTION), is("keep-alive"));
+        assertContent(response);
+    }
+
+    @Test
+    public void test404PostHttp11() throws Exception
+    {
+        String rawResponse = connector.getResponse(
+            "POST / HTTP/1.1\r\n" +
+                "Host: Localhost\r\n" +
+                "Accept: text/html\r\n" +
+                "Content-Length: 10\r\n" +
+                "Connection: keep-alive\r\n" + // This is not need by HTTP/1.1 but sometimes sent anyway
+                "\r\n" +
+                "0123456789");
+
+        HttpTester.Response response = HttpTester.parseResponse(rawResponse);
+
+        assertThat(response.getStatus(), is(404));
+        assertThat(response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0));
+        assertThat(response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=ISO-8859-1"));
+        assertThat(response.getField(HttpHeader.CONNECTION), nullValue());
+        assertContent(response);
+    }
+
+    @Test
+    public void test404PostCantConsumeHttp10() throws Exception
+    {
+        String rawResponse = connector.getResponse(
+            "POST / HTTP/1.0\r\n" +
+                "Host: Localhost\r\n" +
+                "Accept: text/html\r\n" +
+                "Content-Length: 100\r\n" +
+                "Connection: keep-alive\r\n" +
+                "\r\n" +
+                "0123456789");
+
+        HttpTester.Response response = HttpTester.parseResponse(rawResponse);
+
+        assertThat(response.getStatus(), is(404));
+        assertThat(response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0));
+        assertThat(response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=ISO-8859-1"));
+        assertThat(response.getField(HttpHeader.CONNECTION), nullValue());
+        assertContent(response);
+    }
+
+    @Test
+    public void test404PostCantConsumeHttp11() throws Exception
+    {
+        String rawResponse = connector.getResponse(
+            "POST / HTTP/1.1\r\n" +
+                "Host: Localhost\r\n" +
+                "Accept: text/html\r\n" +
+                "Content-Length: 100\r\n" +
+                "Connection: keep-alive\r\n" + // This is not need by HTTP/1.1 but sometimes sent anyway
+                "\r\n" +
+                "0123456789");
+
+        HttpTester.Response response = HttpTester.parseResponse(rawResponse);
+
+        assertThat(response.getStatus(), is(404));
+        assertThat(response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0));
+        assertThat(response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=ISO-8859-1"));
+        assertThat(response.getField(HttpHeader.CONNECTION).getValue(), is("close"));
+        assertContent(response);
+    }
+
     @Test
     public void testMoreSpecificAccept() throws Exception
     {

jetty-server/src/test/java/org/eclipse/jetty/server/GracefulStopTest.java

@@ -22,6 +22,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.net.Socket;
 import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
@@ -147,8 +148,7 @@ public class GracefulStopTest
         handler.latch = new CountDownLatch(1);
         final int port = connector.getLocalPort();
         Socket client = new Socket("127.0.0.1", port);
-        client.getOutputStream().write(post);
-        client.getOutputStream().write(BODY_67890);
+        client.getOutputStream().write(concat(post, BODY_67890));
         client.getOutputStream().flush();
         assertTrue(handler.latch.await(5, TimeUnit.SECONDS));
 
@@ -163,8 +163,7 @@ public class GracefulStopTest
     void assertAvailable(Socket client, byte[] post, TestHandler handler) throws Exception
     {
         handler.latch = new CountDownLatch(1);
-        client.getOutputStream().write(post);
-        client.getOutputStream().write(BODY_67890);
+        client.getOutputStream().write(concat(post, BODY_67890));
         client.getOutputStream().flush();
         assertTrue(handler.latch.await(5, TimeUnit.SECONDS));
 
@@ -188,8 +187,7 @@ public class GracefulStopTest
                     Thread.sleep(100);
                 }
 
-                client.getOutputStream().write(post);
-                client.getOutputStream().write(BODY_67890);
+                client.getOutputStream().write(concat(post, BODY_67890));
                 client.getOutputStream().flush();
                 HttpTester.Response response = HttpTester.parseResponse(client.getInputStream());
 
@@ -281,6 +279,13 @@ public class GracefulStopTest
         }).start();
     }
 
+    private byte[] concat(byte[] bytes1, byte[] bytes2)
+    {
+        byte[] bytes = Arrays.copyOf(bytes1, bytes1.length + bytes2.length);
+        System.arraycopy(bytes2, 0, bytes, bytes1.length, bytes2.length);
+        return bytes;
+    }
+
     @Test
     public void testNotGraceful() throws Exception
     {

jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java

@@ -1471,10 +1471,77 @@ public class ResponseTest
         output.flush();
     }
 
+    @Test
+    public void testEnsureConsumeAllOrNotPersistentHttp10() throws Exception
+    {
+        Response response = getResponse(HttpVersion.HTTP_1_0);
+        response.getHttpChannel().ensureConsumeAllOrNotPersistent();
+        assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), nullValue());
+
+        response = getResponse(HttpVersion.HTTP_1_0);
+        response.setHeader(HttpHeader.CONNECTION, "keep-alive");
+        response.getHttpChannel().ensureConsumeAllOrNotPersistent();
+        assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), nullValue());
+
+        response = getResponse(HttpVersion.HTTP_1_0);
+        response.setHeader(HttpHeader.CONNECTION, "before");
+        response.getHttpFields().add(HttpHeader.CONNECTION, "foo, keep-alive, bar");
+        response.getHttpFields().add(HttpHeader.CONNECTION, "after");
+        response.getHttpChannel().ensureConsumeAllOrNotPersistent();
+        assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), is("before, foo, bar, after"));
+
+        response = getResponse(HttpVersion.HTTP_1_0);
+        response.setHeader(HttpHeader.CONNECTION, "close");
+        response.getHttpChannel().ensureConsumeAllOrNotPersistent();
+        assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), is("close"));
+    }
+
+    @Test
+    public void testEnsureConsumeAllOrNotPersistentHttp11() throws Exception
+    {
+        Response response = getResponse(HttpVersion.HTTP_1_1);
+        response.getHttpChannel().ensureConsumeAllOrNotPersistent();
+        assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), is("close"));
+
+        response = getResponse(HttpVersion.HTTP_1_1);
+        response.setHeader(HttpHeader.CONNECTION, "keep-alive");
+        response.getHttpChannel().ensureConsumeAllOrNotPersistent();
+        assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), is("close"));
+
+        response = getResponse(HttpVersion.HTTP_1_1);
+        response.setHeader(HttpHeader.CONNECTION, "close");
+        response.getHttpChannel().ensureConsumeAllOrNotPersistent();
+        assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), is("close"));
+
+        response = getResponse(HttpVersion.HTTP_1_1);
+        response.setHeader(HttpHeader.CONNECTION, "before, close, after");
+        response.getHttpChannel().ensureConsumeAllOrNotPersistent();
+        assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), is("before, close, after"));
+
+        response = getResponse(HttpVersion.HTTP_1_1);
+        response.setHeader(HttpHeader.CONNECTION, "before");
+        response.getHttpFields().add(HttpHeader.CONNECTION, "middle, close");
+        response.getHttpFields().add(HttpHeader.CONNECTION, "after");
+        response.getHttpChannel().ensureConsumeAllOrNotPersistent();
+        assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), is("before, middle, close, after"));
+
+        response = getResponse(HttpVersion.HTTP_1_1);
+        response.setHeader(HttpHeader.CONNECTION, "one");
+        response.getHttpFields().add(HttpHeader.CONNECTION, "two");
+        response.getHttpFields().add(HttpHeader.CONNECTION, "three");
+        response.getHttpChannel().ensureConsumeAllOrNotPersistent();
+        assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), is("one, two, three, close"));
+    }
+
     private Response getResponse()
+    {
+        return getResponse(HttpVersion.HTTP_1_0);
+    }
+
+    private Response getResponse(HttpVersion version)
     {
         _channel.recycle();
-        _channel.getRequest().setMetaData(new MetaData.Request("GET", HttpURI.from("/path/info"), HttpVersion.HTTP_1_0, HttpFields.EMPTY));
+        _channel.getRequest().setMetaData(new MetaData.Request("GET", HttpURI.from("/path/info"), version, HttpFields.EMPTY));
         BufferUtil.clear(_content);
         return _channel.getResponse();
     }

jetty-servlet/pom.xml

@@ -17,6 +17,15 @@
 
   <build>
     <plugins>
+      <plugin>
+        <artifactId>maven-surefire-plugin</artifactId>
+        <configuration>
+          <argLine>
+            @{argLine} ${jetty.surefire.argLine}
+            --add-modules org.eclipse.jetty.util.ajax
+          </argLine>
+        </configuration>
+      </plugin>
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-jar-plugin</artifactId>
@@ -49,6 +58,12 @@
       <groupId>org.slf4j</groupId>
       <artifactId>slf4j-api</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty</groupId>
+      <artifactId>jetty-util-ajax</artifactId>
+      <version>${project.version}</version>
+      <optional>true</optional>
+    </dependency>
     <dependency>
       <groupId>org.eclipse.jetty</groupId>
       <artifactId>jetty-jmx</artifactId>

jetty-servlet/src/main/java/module-info.java

@@ -27,6 +27,7 @@ module org.eclipse.jetty.servlet
 
     // Only required if using StatisticsServlet.
     requires static java.management;
+    requires static org.eclipse.jetty.util.ajax;
     // Only required if using IntrospectorCleaner.
     requires static java.desktop;
     // Only required if using JMX.

jetty-servlet/src/main/java/org/eclipse/jetty/servlet/StatisticsServlet.java

@@ -19,28 +19,53 @@
 package org.eclipse.jetty.servlet;
 
 import java.io.IOException;
-import java.io.PrintWriter;
+import java.io.OutputStreamWriter;
+import java.io.Writer;
 import java.lang.management.ManagementFactory;
 import java.lang.management.MemoryMXBean;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
 import javax.servlet.ServletContext;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.http.QuotedQualityCSV;
 import org.eclipse.jetty.io.ConnectionStatistics;
-import org.eclipse.jetty.server.AbstractConnector;
 import org.eclipse.jetty.server.Connector;
-import org.eclipse.jetty.server.Handler;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.handler.ContextHandler;
 import org.eclipse.jetty.server.handler.StatisticsHandler;
-import org.eclipse.jetty.util.component.Container;
+import org.eclipse.jetty.util.StringUtil;
+import org.eclipse.jetty.util.ajax.JSON;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * Collect and report statistics about requests / responses / connections and more.
+ * <p>
+ * You can use normal HTTP content negotiation to ask for the statistics.
+ * Specify a request <code>Accept</code> header for one of the following formats:
+ * </p>
+ * <ul>
+ *     <li><code>application/json</code></li>
+ *     <li><code>text/xml</code></li>
+ *     <li><code>text/html</code></li>
+ *     <li><code>text/plain</code> - default if no <code>Accept</code> header specified</li>
+ * </ul>
+ */
 public class StatisticsServlet extends HttpServlet
 {
     private static final Logger LOG = LoggerFactory.getLogger(StatisticsServlet.class);
@@ -48,7 +73,7 @@ public class StatisticsServlet extends HttpServlet
     boolean _restrictToLocalhost = true; // defaults to true
     private StatisticsHandler _statsHandler;
     private MemoryMXBean _memoryBean;
-    private Connector[] _connectors;
+    private List<Connector> _connectors;
 
     @Override
     public void init() throws ServletException
@@ -57,20 +82,16 @@ public class StatisticsServlet extends HttpServlet
         ContextHandler.Context scontext = (ContextHandler.Context)context;
         Server server = scontext.getContextHandler().getServer();
 
-        Handler handler = server.getChildHandlerByClass(StatisticsHandler.class);
+        _statsHandler = server.getChildHandlerByClass(StatisticsHandler.class);
 
-        if (handler != null)
-        {
-            _statsHandler = (StatisticsHandler)handler;
-        }
-        else
+        if (_statsHandler == null)
         {
             LOG.warn("Statistics Handler not installed!");
             return;
         }
 
         _memoryBean = ManagementFactory.getMemoryMXBean();
-        _connectors = server.getConnectors();
+        _connectors = Arrays.asList(server.getConnectors());
 
         if (getInitParameter("restrictToLocalhost") != null)
         {
@@ -79,47 +100,147 @@ public class StatisticsServlet extends HttpServlet
     }
 
     @Override
-    public void doPost(HttpServletRequest sreq, HttpServletResponse sres) throws ServletException, IOException
+    public void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
     {
-        doGet(sreq, sres);
+        doGet(request, response);
     }
 
     @Override
-    protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
+    protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
     {
         if (_statsHandler == null)
         {
             LOG.warn("Statistics Handler not installed!");
-            resp.sendError(HttpServletResponse.SC_SERVICE_UNAVAILABLE);
+            response.sendError(HttpServletResponse.SC_SERVICE_UNAVAILABLE);
             return;
         }
         if (_restrictToLocalhost)
         {
-            if (!isLoopbackAddress(req.getRemoteAddr()))
+            if (!isLoopbackAddress(request.getRemoteAddr()))
             {
-                resp.sendError(HttpServletResponse.SC_SERVICE_UNAVAILABLE);
+                response.sendError(HttpServletResponse.SC_FORBIDDEN);
                 return;
             }
         }
 
-        if (Boolean.parseBoolean(req.getParameter("statsReset")))
+        if (Boolean.parseBoolean(request.getParameter("statsReset")))
         {
+            response.setStatus(HttpServletResponse.SC_OK);
             _statsHandler.statsReset();
             return;
         }
 
-        String wantXml = req.getParameter("xml");
-        if (wantXml == null)
-            wantXml = req.getParameter("XML");
+        if (request.getParameter("xml") != null)
+        {
+            LOG.warn("'xml' parameter is deprecated, use 'Accept' request header instead");
+        }
 
-        if (Boolean.parseBoolean(wantXml))
+        List<String> acceptable = getOrderedAcceptableMimeTypes(request);
+
+        for (String mimeType : acceptable)
         {
-            sendXmlResponse(resp);
+            switch (mimeType)
+            {
+                case "application/json":
+                    writeJsonResponse(response);
+                    return;
+                case "text/xml":
+                    writeXmlResponse(response);
+                    return;
+                case "text/html":
+                    writeHtmlResponse(response);
+                    return;
+                case "text/plain":
+                case "*/*":
+                    writeTextResponse(response);
+                    return;
+                default:
+                    if (LOG.isDebugEnabled())
+                    {
+                        LOG.debug("Ignoring unrecognized mime-type {}", mimeType);
+                    }
+                    break;
+            }
         }
-        else
+        // None of the listed `Accept` mime-types were found.
+        response.sendError(HttpServletResponse.SC_NOT_ACCEPTABLE);
+    }
+
+    private void writeTextResponse(HttpServletResponse response) throws IOException
+    {
+        response.setCharacterEncoding("utf-8");
+        response.setContentType("text/plain");
+        CharSequence text = generateResponse(new TextProducer());
+        response.getWriter().print(text.toString());
+    }
+
+    private void writeHtmlResponse(HttpServletResponse response) throws IOException
+    {
+        response.setCharacterEncoding("utf-8");
+        response.setContentType("text/html");
+        Writer htmlWriter = new OutputStreamWriter(response.getOutputStream(), UTF_8);
+        htmlWriter.append("<html><head><title>");
+        htmlWriter.append(this.getClass().getSimpleName());
+        htmlWriter.append("</title></head><body>\n");
+        CharSequence html = generateResponse(new HtmlProducer());
+        htmlWriter.append(html.toString());
+        htmlWriter.append("\n</body></html>\n");
+        htmlWriter.flush();
+    }
+
+    private void writeXmlResponse(HttpServletResponse response) throws IOException
+    {
+        response.setCharacterEncoding("utf-8");
+        response.setContentType("text/xml");
+        CharSequence xml = generateResponse(new XmlProducer());
+        response.getWriter().print(xml.toString());
+    }
+
+    private void writeJsonResponse(HttpServletResponse response) throws IOException
+    {
+        // We intentionally don't put "UTF-8" into the response headers
+        // as the rules for application/json state that it should never be
+        // present on the HTTP Content-Type header.
+        // It is also true that the application/json mime-type is always UTF-8.
+        response.setContentType("application/json");
+        CharSequence json = generateResponse(new JsonProducer());
+        Writer jsonWriter = new OutputStreamWriter(response.getOutputStream(), UTF_8);
+        jsonWriter.append(json);
+        jsonWriter.flush();
+    }
+
+    private List<String> getOrderedAcceptableMimeTypes(HttpServletRequest request)
+    {
+        QuotedQualityCSV values = new QuotedQualityCSV(QuotedQualityCSV.MOST_SPECIFIC_MIME_ORDERING);
+
+        // No accept header specified, try 'accept' parameter (for those clients that are
+        // so ancient that they cannot set the standard HTTP `Accept` header)
+        String acceptParameter = request.getParameter("accept");
+        if (acceptParameter != null)
         {
-            sendTextResponse(resp);
+            values.addValue(acceptParameter);
         }
+
+        Enumeration<String> enumAccept = request.getHeaders(HttpHeader.ACCEPT.toString());
+        if (enumAccept != null)
+        {
+            while (enumAccept.hasMoreElements())
+            {
+                String value = enumAccept.nextElement();
+                if (StringUtil.isNotBlank(value))
+                {
+                    values.addValue(value);
+                }
+            }
+        }
+
+        if (values.isEmpty())
+        {
+            // return that we allow ALL mime types
+            return Collections.singletonList("*/*");
+        }
+
+        return values.getValues();
     }
 
     private boolean isLoopbackAddress(String address)
@@ -136,140 +257,336 @@ public class StatisticsServlet extends HttpServlet
         }
     }
 
-    private void sendXmlResponse(HttpServletResponse response) throws IOException
+    private CharSequence generateResponse(OutputProducer outputProducer)
     {
-        StringBuilder sb = new StringBuilder();
+        Map<String, Object> top = new HashMap<>();
 
-        sb.append("<statistics>\n");
+        // requests
+        Map<String, Number> requests = new HashMap<>();
+        requests.put("statsOnMs", _statsHandler.getStatsOnMs());
 
-        sb.append("  <requests>\n");
-        sb.append("    <statsOnMs>").append(_statsHandler.getStatsOnMs()).append("</statsOnMs>\n");
+        requests.put("requests", _statsHandler.getRequests());
 
-        sb.append("    <requests>").append(_statsHandler.getRequests()).append("</requests>\n");
-        sb.append("    <requestsActive>").append(_statsHandler.getRequestsActive()).append("</requestsActive>\n");
-        sb.append("    <requestsActiveMax>").append(_statsHandler.getRequestsActiveMax()).append("</requestsActiveMax>\n");
-        sb.append("    <requestsTimeTotal>").append(_statsHandler.getRequestTimeTotal()).append("</requestsTimeTotal>\n");
-        sb.append("    <requestsTimeMean>").append(_statsHandler.getRequestTimeMean()).append("</requestsTimeMean>\n");
-        sb.append("    <requestsTimeMax>").append(_statsHandler.getRequestTimeMax()).append("</requestsTimeMax>\n");
-        sb.append("    <requestsTimeStdDev>").append(_statsHandler.getRequestTimeStdDev()).append("</requestsTimeStdDev>\n");
+        requests.put("requestsActive", _statsHandler.getRequestsActive());
+        requests.put("requestsActiveMax", _statsHandler.getRequestsActiveMax());
+        requests.put("requestsTimeTotal", _statsHandler.getRequestTimeTotal());
+        requests.put("requestsTimeMean", _statsHandler.getRequestTimeMean());
+        requests.put("requestsTimeMax", _statsHandler.getRequestTimeMax());
+        requests.put("requestsTimeStdDev", _statsHandler.getRequestTimeStdDev());
 
-        sb.append("    <dispatched>").append(_statsHandler.getDispatched()).append("</dispatched>\n");
-        sb.append("    <dispatchedActive>").append(_statsHandler.getDispatchedActive()).append("</dispatchedActive>\n");
-        sb.append("    <dispatchedActiveMax>").append(_statsHandler.getDispatchedActiveMax()).append("</dispatchedActiveMax>\n");
-        sb.append("    <dispatchedTimeTotalMs>").append(_statsHandler.getDispatchedTimeTotal()).append("</dispatchedTimeTotalMs>\n");
-        sb.append("    <dispatchedTimeMeanMs>").append(_statsHandler.getDispatchedTimeMean()).append("</dispatchedTimeMeanMs>\n");
-        sb.append("    <dispatchedTimeMaxMs>").append(_statsHandler.getDispatchedTimeMax()).append("</dispatchedTimeMaxMs>\n");
-        sb.append("    <dispatchedTimeStdDevMs>").append(_statsHandler.getDispatchedTimeStdDev()).append("</dispatchedTimeStdDevMs>\n");
+        requests.put("dispatched", _statsHandler.getDispatched());
+        requests.put("dispatchedActive", _statsHandler.getDispatchedActive());
+        requests.put("dispatchedActiveMax", _statsHandler.getDispatchedActiveMax());
+        requests.put("dispatchedTimeTotal", _statsHandler.getDispatchedTimeTotal());
+        requests.put("dispatchedTimeMean", _statsHandler.getDispatchedTimeMean());
+        requests.put("dispatchedTimeMax", _statsHandler.getDispatchedTimeMax());
+        requests.put("dispatchedTimeStdDev", _statsHandler.getDispatchedTimeStdDev());
 
-        sb.append("    <asyncRequests>").append(_statsHandler.getAsyncRequests()).append("</asyncRequests>\n");
-        sb.append("    <requestsSuspended>").append(_statsHandler.getAsyncRequestsWaiting()).append("</requestsSuspended>\n");
-        sb.append("    <requestsSuspendedMax>").append(_statsHandler.getAsyncRequestsWaitingMax()).append("</requestsSuspendedMax>\n");
-        sb.append("    <requestsResumed>").append(_statsHandler.getAsyncDispatches()).append("</requestsResumed>\n");
-        sb.append("    <requestsExpired>").append(_statsHandler.getExpires()).append("</requestsExpired>\n");
-        sb.append("  </requests>\n");
+        requests.put("asyncRequests", _statsHandler.getAsyncRequests());
+        requests.put("requestsSuspended", _statsHandler.getAsyncDispatches());
+        requests.put("requestsSuspendedMax", _statsHandler.getAsyncRequestsWaiting());
+        requests.put("requestsResumed", _statsHandler.getAsyncRequestsWaitingMax());
+        requests.put("requestsExpired", _statsHandler.getExpires());
 
-        sb.append("  <responses>\n");
-        sb.append("    <responses1xx>").append(_statsHandler.getResponses1xx()).append("</responses1xx>\n");
-        sb.append("    <responses2xx>").append(_statsHandler.getResponses2xx()).append("</responses2xx>\n");
-        sb.append("    <responses3xx>").append(_statsHandler.getResponses3xx()).append("</responses3xx>\n");
-        sb.append("    <responses4xx>").append(_statsHandler.getResponses4xx()).append("</responses4xx>\n");
-        sb.append("    <responses5xx>").append(_statsHandler.getResponses5xx()).append("</responses5xx>\n");
-        sb.append("    <responsesBytesTotal>").append(_statsHandler.getResponsesBytesTotal()).append("</responsesBytesTotal>\n");
-        sb.append("  </responses>\n");
+        requests.put("errors", _statsHandler.getErrors());
 
-        sb.append("  <connections>\n");
-        for (Connector connector : _connectors)
+        top.put("requests", requests);
+
+        // responses
+        Map<String, Number> responses = new HashMap<>();
+        responses.put("responses1xx", _statsHandler.getResponses1xx());
+        responses.put("responses2xx", _statsHandler.getResponses2xx());
+        responses.put("responses3xx", _statsHandler.getResponses3xx());
+        responses.put("responses4xx", _statsHandler.getResponses4xx());
+        responses.put("responses5xx", _statsHandler.getResponses5xx());
+        responses.put("responsesBytesTotal", _statsHandler.getResponsesBytesTotal());
+        top.put("responses", responses);
+
+        // connections
+        List<Object> connections = new ArrayList<>();
+        _connectors.forEach((connector) ->
         {
-            sb.append("    <connector>\n");
-            sb.append("      <name>").append(connector.getClass().getName()).append("@").append(connector.hashCode()).append("</name>\n");
-            sb.append("      <protocols>\n");
-            for (String protocol : connector.getProtocols())
-            {
-                sb.append("      <protocol>").append(protocol).append("</protocol>\n");
-            }
-            sb.append("      </protocols>\n");
+            Map<String, Object> connectorDetail = new HashMap<>();
+            connectorDetail.put("name", String.format("%s@%X", connector.getClass().getName(), connector.hashCode()));
+            connectorDetail.put("protocols", connector.getProtocols());
 
-            ConnectionStatistics connectionStats = null;
-            if (connector instanceof AbstractConnector)
-                connectionStats = ((AbstractConnector)connector).getBean(ConnectionStatistics.class);
+            ConnectionStatistics connectionStats = connector.getBean(ConnectionStatistics.class);
             if (connectionStats != null)
             {
-                sb.append("      <statsOn>true</statsOn>\n");
-                sb.append("      <connections>").append(connectionStats.getConnectionsTotal()).append("</connections>\n");
-                sb.append("      <connectionsOpen>").append(connectionStats.getConnections()).append("</connectionsOpen>\n");
-                sb.append("      <connectionsOpenMax>").append(connectionStats.getConnectionsMax()).append("</connectionsOpenMax>\n");
-                sb.append("      <connectionsDurationMean>").append(connectionStats.getConnectionDurationMean()).append("</connectionsDurationMean>\n");
-                sb.append("      <connectionsDurationMax>").append(connectionStats.getConnectionDurationMax()).append("</connectionsDurationMax>\n");
-                sb.append("      <connectionsDurationStdDev>").append(connectionStats.getConnectionDurationStdDev()).append("</connectionsDurationStdDev>\n");
-                sb.append("      <bytesIn>").append(connectionStats.getReceivedBytes()).append("</bytesIn>\n");
-                sb.append("      <bytesOut>").append(connectionStats.getSentBytes()).append("</bytesOut>\n");
-                sb.append("      <messagesIn>").append(connectionStats.getReceivedMessages()).append("</messagesIn>\n");
-                sb.append("      <messagesOut>").append(connectionStats.getSentMessages()).append("</messagesOut>\n");
+                connectorDetail.put("statsOn", true);
+                connectorDetail.put("connections", connectionStats.getConnectionsTotal());
+                connectorDetail.put("connectionsOpen", connectionStats.getConnections());
+                connectorDetail.put("connectionsOpenMax", connectionStats.getConnectionsMax());
+                connectorDetail.put("connectionsDurationMean", connectionStats.getConnectionDurationMean());
+                connectorDetail.put("connectionsDurationMax", connectionStats.getConnectionDurationMax());
+                connectorDetail.put("connectionsDurationStdDev", connectionStats.getConnectionDurationStdDev());
+                connectorDetail.put("bytesIn", connectionStats.getReceivedBytes());
+                connectorDetail.put("bytesOut", connectionStats.getSentBytes());
+                connectorDetail.put("messagesIn", connectionStats.getReceivedMessages());
+                connectorDetail.put("messagesOut", connectionStats.getSentMessages());
             }
-            else
-            {
-                sb.append("      <statsOn>false</statsOn>\n");
-            }
-            sb.append("    </connector>\n");
-        }
-        sb.append("  </connections>\n");
+            connections.add(connectorDetail);
+        });
+        top.put("connections", connections);
 
-        sb.append("  <memory>\n");
-        sb.append("    <heapMemoryUsage>").append(_memoryBean.getHeapMemoryUsage().getUsed()).append("</heapMemoryUsage>\n");
-        sb.append("    <nonHeapMemoryUsage>").append(_memoryBean.getNonHeapMemoryUsage().getUsed()).append("</nonHeapMemoryUsage>\n");
-        sb.append("  </memory>\n");
+        // memory
+        Map<String, Number> memoryMap = new HashMap<>();
+        memoryMap.put("heapMemoryUsage", _memoryBean.getHeapMemoryUsage().getUsed());
+        memoryMap.put("nonHeapMemoryUsage", _memoryBean.getNonHeapMemoryUsage().getUsed());
+        top.put("memory", memoryMap);
 
-        sb.append("</statistics>\n");
-
-        response.setContentType("text/xml");
-        PrintWriter pout = response.getWriter();
-        pout.write(sb.toString());
+        // the top level object
+        return outputProducer.generate("statistics", top);
     }
 
-    private void sendTextResponse(HttpServletResponse response) throws IOException
+    private interface OutputProducer
     {
-        StringBuilder sb = new StringBuilder();
-        sb.append(_statsHandler.toStatsHTML());
+        CharSequence generate(String id, Map<String, Object> map);
+    }
 
-        sb.append("<h2>Connections:</h2>\n");
-        for (Connector connector : _connectors)
+    private static class JsonProducer implements OutputProducer
+    {
+        @Override
+        public CharSequence generate(String id, Map<String, Object> map)
         {
-            sb.append("<h3>").append(connector.getClass().getName()).append("@").append(connector.hashCode()).append("</h3>");
-            sb.append("Protocols:");
-            for (String protocol : connector.getProtocols())
-            {
-                sb.append(protocol).append("&nbsp;");
-            }
-            sb.append("    <br />\n");
+            return new JSON().toJSON(map);
+        }
+    }
 
-            ConnectionStatistics connectionStats = null;
-            if (connector instanceof Container)
-                connectionStats = ((Container)connector).getBean(ConnectionStatistics.class);
-            if (connectionStats != null)
+    private static class XmlProducer implements OutputProducer
+    {
+        private final StringBuilder sb;
+        private int indent = 0;
+
+        public XmlProducer()
+        {
+            this.sb = new StringBuilder();
+        }
+
+        @Override
+        public CharSequence generate(String id, Map<String, Object> map)
+        {
+            add(id, map);
+            return sb;
+        }
+
+        private void indent()
+        {
+            sb.append("\n");
+            for (int i = 0; i < indent; i++)
             {
-                sb.append("Total connections: ").append(connectionStats.getConnectionsTotal()).append("<br />\n");
-                sb.append("Current connections open: ").append(connectionStats.getConnections()).append("<br />\n");
-                sb.append("Max concurrent connections open: ").append(connectionStats.getConnectionsMax()).append("<br />\n");
-                sb.append("Mean connection duration: ").append(connectionStats.getConnectionDurationMean()).append("<br />\n");
-                sb.append("Max connection duration: ").append(connectionStats.getConnectionDurationMax()).append("<br />\n");
-                sb.append("Connection duration standard deviation: ").append(connectionStats.getConnectionDurationStdDev()).append("<br />\n");
-                sb.append("Total bytes received: ").append(connectionStats.getReceivedBytes()).append("<br />\n");
-                sb.append("Total bytes sent: ").append(connectionStats.getSentBytes()).append("<br />\n");
-                sb.append("Total messages received: ").append(connectionStats.getReceivedMessages()).append("<br />\n");
-                sb.append("Total messages sent: ").append(connectionStats.getSentMessages()).append("<br />\n");
-            }
-            else
-            {
-                sb.append("Statistics gathering off.\n");
+                sb.append(' ').append(' ');
             }
         }
 
-        sb.append("<h2>Memory:</h2>\n");
-        sb.append("Heap memory usage: ").append(_memoryBean.getHeapMemoryUsage().getUsed()).append(" bytes").append("<br />\n");
-        sb.append("Non-heap memory usage: ").append(_memoryBean.getNonHeapMemoryUsage().getUsed()).append(" bytes").append("<br />\n");
+        private void add(String id, Object obj)
+        {
+            sb.append('<').append(StringUtil.sanitizeXmlString(id)).append('>');
+            indent++;
 
-        response.setContentType("text/html");
-        PrintWriter pout = response.getWriter();
-        pout.write(sb.toString());
+            boolean wasIndented = false;
+
+            if (obj instanceof Map)
+            {
+                //noinspection unchecked
+                addMap((Map<String, ?>)obj);
+                wasIndented = true;
+            }
+            else if (obj instanceof List)
+            {
+                addList(id, (List<?>)obj);
+                wasIndented = true;
+            }
+            else
+            {
+                addObject(obj);
+            }
+
+            indent--;
+            if (wasIndented)
+                indent();
+            sb.append("</").append(id).append('>');
+        }
+
+        private void addMap(Map<String, ?> map)
+        {
+            map.keySet().stream().sorted()
+                .forEach((key) ->
+                {
+                    indent();
+                    add(key, map.get(key));
+                });
+        }
+
+        private void addList(String parentId, List<?> list)
+        {
+            // drop the 's' at the end.
+            String childName = parentId.replaceFirst("s$", "");
+            list.forEach((entry) ->
+            {
+                indent();
+                add(childName, entry);
+            });
+        }
+
+        private void addObject(Object obj)
+        {
+            sb.append(StringUtil.sanitizeXmlString(Objects.toString(obj)));
+        }
+    }
+
+    private static class TextProducer implements OutputProducer
+    {
+        private final StringBuilder sb;
+        private int indent = 0;
+
+        public TextProducer()
+        {
+            this.sb = new StringBuilder();
+        }
+
+        @Override
+        public CharSequence generate(String id, Map<String, Object> map)
+        {
+            add(id, map);
+            return sb;
+        }
+
+        private void indent()
+        {
+            for (int i = 0; i < indent; i++)
+            {
+                sb.append(' ').append(' ');
+            }
+        }
+
+        private void add(String id, Object obj)
+        {
+            indent();
+            sb.append(id).append(": ");
+            indent++;
+
+            if (obj instanceof Map)
+            {
+                sb.append('\n');
+                //noinspection unchecked
+                addMap((Map<String, ?>)obj);
+            }
+            else if (obj instanceof List)
+            {
+                sb.append('\n');
+                addList(id, (List<?>)obj);
+            }
+            else
+            {
+                addObject(obj);
+                sb.append('\n');
+            }
+
+            indent--;
+        }
+
+        private void addMap(Map<String, ?> map)
+        {
+            map.keySet().stream().sorted()
+                .forEach((key) -> add(key, map.get(key)));
+        }
+
+        private void addList(String parentId, List<?> list)
+        {
+            // drop the 's' at the end.
+            String childName = parentId.replaceFirst("s$", "");
+            list.forEach((entry) -> add(childName, entry));
+        }
+
+        private void addObject(Object obj)
+        {
+            sb.append(obj);
+        }
+    }
+
+    private static class HtmlProducer implements OutputProducer
+    {
+        private final StringBuilder sb;
+        private int indent = 0;
+
+        public HtmlProducer()
+        {
+            this.sb = new StringBuilder();
+        }
+
+        @Override
+        public CharSequence generate(String id, Map<String, Object> map)
+        {
+            sb.append("<ul>\n");
+            add(id, map);
+            sb.append("</ul>\n");
+            return sb;
+        }
+
+        private void indent()
+        {
+            for (int i = 0; i < indent; i++)
+            {
+                sb.append(' ').append(' ');
+            }
+        }
+
+        private void add(String id, Object obj)
+        {
+            indent();
+            indent++;
+            sb.append("<li><em>").append(StringUtil.sanitizeXmlString(id)).append("</em>: ");
+            if (obj instanceof Map)
+            {
+                //noinspection unchecked
+                addMap((Map<String, ?>)obj);
+                indent();
+            }
+            else if (obj instanceof List)
+            {
+                addList(id, (List<?>)obj);
+                indent();
+            }
+            else
+            {
+                addObject(obj);
+            }
+            sb.append("</li>\n");
+
+            indent--;
+        }
+
+        private void addMap(Map<String, ?> map)
+        {
+            sb.append("\n");
+            indent();
+            sb.append("<ul>\n");
+            indent++;
+            map.keySet().stream().sorted(String::compareToIgnoreCase)
+                .forEach((key) -> add(key, map.get(key)));
+            indent--;
+            indent();
+            sb.append("</ul>\n");
+        }
+
+        private void addList(String parentId, List<?> list)
+        {
+            sb.append("\n");
+            indent();
+            sb.append("<ul>\n");
+            indent++;
+            // drop the 's' at the end.
+            String childName = parentId.replaceFirst("s$", "");
+            list.forEach((entry) -> add(childName, entry));
+            indent--;
+            indent();
+            sb.append("</ul>\n");
+        }
+
+        private void addObject(Object obj)
+        {
+            sb.append(StringUtil.sanitizeXmlString(Objects.toString(obj)));
+        }
     }
 }

jetty-servlet/src/test/java/org/eclipse/jetty/servlet/StatisticsServletTest.java

@@ -18,31 +18,47 @@
 
 package org.eclipse.jetty.servlet;
 
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.io.StringReader;
 import java.nio.ByteBuffer;
+import java.util.Map;
+import java.util.function.Consumer;
+import java.util.stream.Stream;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.xpath.XPath;
 import javax.xml.xpath.XPathFactory;
 
+import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpTester;
 import org.eclipse.jetty.http.HttpVersion;
 import org.eclipse.jetty.server.LocalConnector;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.handler.StatisticsHandler;
 import org.eclipse.jetty.server.session.SessionHandler;
+import org.eclipse.jetty.util.ajax.JSON;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.w3c.dom.Document;
 import org.xml.sax.InputSource;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 public class StatisticsServletTest
 {
@@ -66,9 +82,7 @@ public class StatisticsServletTest
         _server.join();
     }
 
-    @Test
-    public void getStats()
-        throws Exception
+    private void addStatisticsHandler()
     {
         StatisticsHandler statsHandler = new StatisticsHandler();
         _server.setHandler(statsHandler);
@@ -78,40 +92,267 @@ public class StatisticsServletTest
         servletHolder.setInitParameter("restrictToLocalhost", "false");
         statsContext.addServlet(servletHolder, "/stats");
         statsContext.setSessionHandler(new SessionHandler());
+    }
+
+    @Test
+    public void testGetStats()
+        throws Exception
+    {
+        addStatisticsHandler();
         _server.start();
 
-        getResponse("/test1");
-        String response = getResponse("/stats?xml=true");
-        Stats stats = parseStats(response);
+        HttpTester.Response response;
+
+        // Trigger 2xx response
+        response = getResponse("/test1");
+        assertEquals(response.getStatus(), 200);
+
+        // Look for 200 response that was tracked
+        response = getResponse("/stats");
+        assertEquals(response.getStatus(), 200);
+        Stats stats = parseStats(response.getContent());
 
         assertEquals(1, stats.responses2xx);
 
-        getResponse("/stats?statsReset=true");
-        response = getResponse("/stats?xml=true");
-        stats = parseStats(response);
+        // Reset stats
+        response = getResponse("/stats?statsReset=true");
+        assertEquals(response.getStatus(), 200);
+
+        // Request stats again
+        response = getResponse("/stats");
+        assertEquals(response.getStatus(), 200);
+        stats = parseStats(response.getContent());
 
         assertEquals(1, stats.responses2xx);
 
-        getResponse("/test1");
-        getResponse("/nothing");
-        response = getResponse("/stats?xml=true");
-        stats = parseStats(response);
+        // Trigger 2xx response
+        response = getResponse("/test1");
+        assertEquals(response.getStatus(), 200);
+        // Trigger 4xx response
+        response = getResponse("/nothing");
+        assertEquals(response.getStatus(), 404);
 
+        // Request stats again
+        response = getResponse("/stats");
+        assertEquals(response.getStatus(), 200);
+        stats = parseStats(response.getContent());
+
+        // Verify we see (from last reset)
+        // 1) request for /stats?statsReset=true [2xx]
+        // 2) request for /stats?xml=true [2xx]
+        // 3) request for /test1 [2xx]
+        // 4) request for /nothing [4xx]
         assertThat("2XX Response Count" + response, stats.responses2xx, is(3));
         assertThat("4XX Response Count" + response, stats.responses4xx, is(1));
     }
 
-    public String getResponse(String path)
+    public static Stream<Arguments> typeVariations(String mimeType)
+    {
+        return Stream.of(
+            Arguments.of(
+                new Consumer<HttpTester.Request>()
+                {
+                    @Override
+                    public void accept(HttpTester.Request request)
+                    {
+                        request.setURI("/stats");
+                        request.setHeader("Accept", mimeType);
+                    }
+
+                    @Override
+                    public String toString()
+                    {
+                        return "Header[Accept: " + mimeType + "]";
+                    }
+                }
+            ),
+            Arguments.of(
+                new Consumer<HttpTester.Request>()
+                {
+                    @Override
+                    public void accept(HttpTester.Request request)
+                    {
+                        request.setURI("/stats?accept=" + mimeType);
+                    }
+
+                    @Override
+                    public String toString()
+                    {
+                        return "query[accept=" + mimeType + "]";
+                    }
+                }
+            )
+        );
+    }
+
+    public static Stream<Arguments> xmlVariations()
+    {
+        return typeVariations("text/xml");
+    }
+
+    @ParameterizedTest(name = "[{index}] {0}")
+    @MethodSource("xmlVariations")
+    public void testGetXmlResponse(Consumer<HttpTester.Request> requestCustomizer)
+        throws Exception
+    {
+        addStatisticsHandler();
+        _server.start();
+
+        HttpTester.Response response;
+        HttpTester.Request request = new HttpTester.Request();
+
+        request.setMethod("GET");
+        request.setVersion(HttpVersion.HTTP_1_1);
+        request.setHeader("Host", "test");
+        requestCustomizer.accept(request);
+
+        ByteBuffer responseBuffer = _connector.getResponse(request.generate());
+        response = HttpTester.parseResponse(responseBuffer);
+
+        assertThat("Response.contentType", response.get(HttpHeader.CONTENT_TYPE), containsString("text/xml"));
+
+        // System.out.println(response.getContent());
+
+        // Parse it, make sure it's well formed.
+        DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance();
+        docBuilderFactory.setValidating(false);
+        DocumentBuilder docBuilder = docBuilderFactory.newDocumentBuilder();
+        try (ByteArrayInputStream input = new ByteArrayInputStream(response.getContentBytes()))
+        {
+            Document doc = docBuilder.parse(input);
+            assertNotNull(doc);
+            assertEquals("statistics", doc.getDocumentElement().getNodeName());
+        }
+    }
+
+    public static Stream<Arguments> jsonVariations()
+    {
+        return typeVariations("application/json");
+    }
+
+    @ParameterizedTest(name = "[{index}] {0}")
+    @MethodSource("jsonVariations")
+    public void testGetJsonResponse(Consumer<HttpTester.Request> requestCustomizer)
+        throws Exception
+    {
+        addStatisticsHandler();
+        _server.start();
+
+        HttpTester.Response response;
+        HttpTester.Request request = new HttpTester.Request();
+
+        request.setMethod("GET");
+        requestCustomizer.accept(request);
+        request.setVersion(HttpVersion.HTTP_1_1);
+        request.setHeader("Host", "test");
+
+        ByteBuffer responseBuffer = _connector.getResponse(request.generate());
+        response = HttpTester.parseResponse(responseBuffer);
+
+        assertThat("Response.contentType", response.get(HttpHeader.CONTENT_TYPE), is("application/json"));
+        assertThat("Response.contentType for json should never contain a charset",
+            response.get(HttpHeader.CONTENT_TYPE), not(containsString("charset")));
+
+        // System.out.println(response.getContent());
+
+        // Parse it, make sure it's well formed.
+        Object doc = new JSON().parse(new JSON.StringSource(response.getContent()));
+        assertNotNull(doc);
+        assertThat(doc, instanceOf(Map.class));
+        Map<?, ?> docMap = (Map<?, ?>)doc;
+        assertEquals(4, docMap.size());
+        assertNotNull(docMap.get("requests"));
+        assertNotNull(docMap.get("responses"));
+        assertNotNull(docMap.get("connections"));
+        assertNotNull(docMap.get("memory"));
+    }
+
+    public static Stream<Arguments> plaintextVariations()
+    {
+        return typeVariations("text/plain");
+    }
+
+    @ParameterizedTest(name = "[{index}] {0}")
+    @MethodSource("plaintextVariations")
+    public void testGetTextResponse(Consumer<HttpTester.Request> requestCustomizer)
+        throws Exception
+    {
+        addStatisticsHandler();
+        _server.start();
+
+        HttpTester.Response response;
+        HttpTester.Request request = new HttpTester.Request();
+
+        request.setMethod("GET");
+        requestCustomizer.accept(request);
+        request.setVersion(HttpVersion.HTTP_1_1);
+        request.setHeader("Host", "test");
+
+        ByteBuffer responseBuffer = _connector.getResponse(request.generate());
+        response = HttpTester.parseResponse(responseBuffer);
+
+        assertThat("Response.contentType", response.get(HttpHeader.CONTENT_TYPE), containsString("text/plain"));
+
+        // System.out.println(response.getContent());
+
+        // Look for expected content
+        assertThat(response.getContent(), containsString("requests: "));
+        assertThat(response.getContent(), containsString("responses: "));
+        assertThat(response.getContent(), containsString("connections: "));
+        assertThat(response.getContent(), containsString("memory: "));
+    }
+
+    public static Stream<Arguments> htmlVariations()
+    {
+        return typeVariations("text/html");
+    }
+
+    @ParameterizedTest(name = "[{index}] {0}")
+    @MethodSource("htmlVariations")
+    public void testGetHtmlResponse(Consumer<HttpTester.Request> requestCustomizer)
+        throws Exception
+    {
+        addStatisticsHandler();
+        _server.start();
+
+        HttpTester.Response response;
+        HttpTester.Request request = new HttpTester.Request();
+
+        request.setMethod("GET");
+        requestCustomizer.accept(request);
+        request.setVersion(HttpVersion.HTTP_1_1);
+        request.setHeader("Host", "test");
+
+        ByteBuffer responseBuffer = _connector.getResponse(request.generate());
+        response = HttpTester.parseResponse(responseBuffer);
+
+        assertThat("Response.contentType", response.get(HttpHeader.CONTENT_TYPE), containsString("text/html"));
+
+        // System.out.println(response.getContent());
+
+        // Look for things that indicate it's a well formed HTML output
+        assertThat(response.getContent(), containsString("<html>"));
+        assertThat(response.getContent(), containsString("<body>"));
+        assertThat(response.getContent(), containsString("<em>requests</em>: "));
+        assertThat(response.getContent(), containsString("<em>responses</em>: "));
+        assertThat(response.getContent(), containsString("<em>connections</em>: "));
+        assertThat(response.getContent(), containsString("<em>memory</em>: "));
+        assertThat(response.getContent(), containsString("</body>"));
+        assertThat(response.getContent(), containsString("</html>"));
+    }
+
+    public HttpTester.Response getResponse(String path)
         throws Exception
     {
         HttpTester.Request request = new HttpTester.Request();
         request.setMethod("GET");
+        request.setHeader("Accept", "text/xml");
         request.setURI(path);
         request.setVersion(HttpVersion.HTTP_1_1);
         request.setHeader("Host", "test");
 
         ByteBuffer responseBuffer = _connector.getResponse(request.generate());
-        return HttpTester.parseResponse(responseBuffer).getContent();
+        return HttpTester.parseResponse(responseBuffer);
     }
 
     public Stats parseStats(String xml)
@@ -120,7 +361,6 @@ public class StatisticsServletTest
         XPath xPath = XPathFactory.newInstance().newXPath();
 
         String responses4xx = xPath.evaluate("//responses4xx", new InputSource(new StringReader(xml)));
-
         String responses2xx = xPath.evaluate("//responses2xx", new InputSource(new StringReader(xml)));
 
         return new Stats(Integer.parseInt(responses2xx), Integer.parseInt(responses4xx));

jetty-servlets/pom.xml

@@ -21,7 +21,7 @@
         <artifactId>maven-surefire-plugin</artifactId>
         <configuration>
           <argLine>
-            @{argLine} ${jetty.surefire.argLine} --add-modules jetty.servlet.api --add-modules org.eclipse.jetty.util --add-modules org.eclipse.jetty.io --add-modules org.eclipse.jetty.http --add-modules org.eclipse.jetty.server --add-modules org.eclipse.jetty.jmx --add-reads org.eclipse.jetty.servlets=java.management --add-reads org.eclipse.jetty.servlets=org.eclipse.jetty.jmx
+            @{argLine} ${jetty.surefire.argLine} --add-modules jetty.servlet.api --add-modules org.eclipse.jetty.util --add-modules org.eclipse.jetty.io --add-modules org.eclipse.jetty.http --add-modules org.eclipse.jetty.server --add-reads org.eclipse.jetty.servlets=java.management --add-reads org.eclipse.jetty.servlets=org.eclipse.jetty.jmx
           </argLine>
         </configuration>
       </plugin>

jetty-start/src/main/java/org/eclipse/jetty/start/Main.java

@@ -266,6 +266,10 @@ public class Main
         System.out.printf("%nModules %s:%n", t);
         System.out.printf("=========%s%n", "=".repeat(t.length()));
         args.getAllModules().listModules(tags);
+
+        // for default module listings, also show enabled modules
+        if ("[-internal]".equals(t) || "[*]".equals(t))
+            args.getAllModules().listEnabled();
     }
 
     public void showModules(StartArgs args)

jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java

@@ -167,6 +167,8 @@ public class Modules implements Iterable<Module>
         if (tags.contains("-*"))
             return;
 
+        tags = new ArrayList<>(tags);
+
         boolean wild = tags.contains("*");
         Set<String> included = new HashSet<>();
         if (wild)

jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java

@@ -347,8 +347,6 @@ public class StartArgs
             }
             System.out.println();
         }
-
-        System.out.println();
     }
 
     public void dumpJvmArgs()

jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt

@@ -19,12 +19,15 @@ Command Line Options:
   --list-config    List the resolved configuration that will be used to
                    start Jetty.
                    Output includes:
+                     o  Enabled jetty modules
                      o  Java Environment
                      o  Jetty Environment
+                     o  Config file search order
                      o  JVM Arguments
+                     o  System Properties
                      o  Properties
-                     o  Server Classpath
-                     o  Server XML Configuration
+                     o  Java Classpath
+                     o  XML Configuration files
 
   --dry-run        Print the command line that the start.jar generates,
                    then exit. This may be used to generate command lines

jetty-util/src/main/java/org/eclipse/jetty/util/ArrayTernaryTrie.java

@@ -368,6 +368,12 @@ public class ArrayTernaryTrie<V> extends AbstractTrie<V>
         return getBest(0, b, offset, len);
     }
 
+    @Override
+    public V getBest(byte[] b, int offset, int len)
+    {
+        return getBest(0, b, offset, len);
+    }
+
     private V getBest(int t, byte[] b, int offset, int len)
     {
         int node = t;

jetty-util/src/main/java/org/eclipse/jetty/util/Trie.java

@@ -19,6 +19,7 @@
 package org.eclipse.jetty.util;
 
 import java.nio.ByteBuffer;
+import java.util.Collections;
 import java.util.Set;
 
 /**
@@ -131,4 +132,99 @@ public interface Trie<V>
     public boolean isCaseInsensitive();
 
     public void clear();
+
+    static <T> Trie<T> empty(final boolean caseInsensitive)
+    {
+        return new Trie<T>()
+        {
+            @Override
+            public boolean put(String s, Object o)
+            {
+                return false;
+            }
+
+            @Override
+            public boolean put(Object o)
+            {
+                return false;
+            }
+
+            @Override
+            public T remove(String s)
+            {
+                return null;
+            }
+
+            @Override
+            public T get(String s)
+            {
+                return null;
+            }
+
+            @Override
+            public T get(String s, int offset, int len)
+            {
+                return null;
+            }
+
+            @Override
+            public T get(ByteBuffer b)
+            {
+                return null;
+            }
+
+            @Override
+            public T get(ByteBuffer b, int offset, int len)
+            {
+                return null;
+            }
+
+            @Override
+            public T getBest(String s)
+            {
+                return null;
+            }
+
+            @Override
+            public T getBest(String s, int offset, int len)
+            {
+                return null;
+            }
+
+            @Override
+            public T getBest(byte[] b, int offset, int len)
+            {
+                return null;
+            }
+
+            @Override
+            public T getBest(ByteBuffer b, int offset, int len)
+            {
+                return null;
+            }
+
+            @Override
+            public Set<String> keySet()
+            {
+                return Collections.emptySet();
+            }
+
+            @Override
+            public boolean isFull()
+            {
+                return true;
+            }
+
+            @Override
+            public boolean isCaseInsensitive()
+            {
+                return caseInsensitive;
+            }
+
+            @Override
+            public void clear()
+            {
+            }
+        };
+    }
 }

jetty-webapp/pom.xml

@@ -42,7 +42,7 @@
         <artifactId>maven-surefire-plugin</artifactId>
         <configuration>
           <argLine>
-            @{argLine} ${jetty.surefire.argLine} --add-modules org.eclipse.jetty.jmx
+            @{argLine} ${jetty.surefire.argLine}
           </argLine>
           <useManifestOnlyJar>false</useManifestOnlyJar>
           <additionalClasspathElements>

jetty-websocket/websocket-core-client/src/main/config/modules/websocket.mod

@@ -1,11 +0,0 @@
-# DO NOT EDIT - See: https://www.eclipse.org/jetty/documentation/current/startup-modules.html
-
-[description]
-Enables both Jetty and javax websocket modules for deployed web applications.
-
-[tags]
-websocket
-
-[depend]
-websocket-jetty
-websocket-javax

jetty-websocket/websocket-core-client/src/main/java/org/eclipse/jetty/websocket/core/client/HttpClientProvider.java

@@ -20,23 +20,15 @@ package org.eclipse.jetty.websocket.core.client;
 
 import org.eclipse.jetty.client.HttpClient;
 import org.eclipse.jetty.util.thread.QueuedThreadPool;
-import org.slf4j.LoggerFactory;
 
 public interface HttpClientProvider
 {
     static HttpClient get()
     {
-        try
-        {
-            HttpClientProvider xmlProvider = new XmlHttpClientProvider();
-            HttpClient client = xmlProvider.newHttpClient();
-            if (client != null)
-                return client;
-        }
-        catch (Throwable x)
-        {
-            LoggerFactory.getLogger(HttpClientProvider.class).trace("IGNORED", x);
-        }
+        HttpClientProvider xmlProvider = new XmlHttpClientProvider();
+        HttpClient client = xmlProvider.newHttpClient();
+        if (client != null)
+            return client;
 
         return HttpClientProvider.newDefaultHttpClient();
     }

jetty-websocket/websocket-core-client/src/main/java/org/eclipse/jetty/websocket/core/client/XmlHttpClientProvider.java

@@ -33,12 +33,27 @@ class XmlHttpClientProvider implements HttpClientProvider
     @Override
     public HttpClient newHttpClient()
     {
-        URL resource = Thread.currentThread().getContextClassLoader().getResource("jetty-websocket-httpclient.xml");
-        if (resource == null)
-        {
+        ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader();
+        if (contextClassLoader == null)
             return null;
-        }
 
+        URL resource = contextClassLoader.getResource("jetty-websocket-httpclient.xml");
+        if (resource == null)
+            return null;
+
+        try
+        {
+            Thread.currentThread().setContextClassLoader(HttpClient.class.getClassLoader());
+            return newHttpClient(resource);
+        }
+        finally
+        {
+            Thread.currentThread().setContextClassLoader(contextClassLoader);
+        }
+    }
+
+    private static HttpClient newHttpClient(URL resource)
+    {
         try
         {
             XmlConfiguration configuration = new XmlConfiguration(Resource.newResource(resource));
@@ -46,7 +61,7 @@ class XmlHttpClientProvider implements HttpClientProvider
         }
         catch (Throwable t)
         {
-            LOG.warn("Unable to load: {}", resource, t);
+            LOG.warn("Failure to load HttpClient from XML {}", resource, t);
         }
 
         return null;

jetty-websocket/websocket-core-common/src/main/config/modules/websocket.mod

@@ -1,11 +0,0 @@
-# DO NOT EDIT - See: https://www.eclipse.org/jetty/documentation/current/startup-modules.html
-
-[description]
-Enable both Jetty and javax websocket modules for deployed web applications.
-
-[tags]
-websocket
-
-[depend]
-websocket-jetty
-websocket-javax

jetty-websocket/websocket-core-tests/fuzzingclient.json

@@ -5,8 +5,8 @@
   "outdir": "./target/reports/servers",
   "servers": [
     {
-      "agent": "Jetty-10.0.0-SNAPSHOT",
-      "url": "ws://127.0.0.1:9001",
+      "agent": "jetty-autobahn-test",
+      "url": "ws://host.testcontainers.internal:9001",
       "options": {
         "version": 18
       }

jetty-websocket/websocket-core-tests/pom.xml

@@ -35,9 +35,44 @@
       <artifactId>jetty-slf4j-impl</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.testcontainers</groupId>
+      <artifactId>testcontainers</artifactId>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.testcontainers</groupId>
+      <artifactId>junit-jupiter</artifactId>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>com.googlecode.json-simple</groupId>
+      <artifactId>json-simple</artifactId>
+      <version>1.1.1</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.codehaus.plexus</groupId>
+      <artifactId>plexus-utils</artifactId>
+      <version>3.3.0</version>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <build>
+    <pluginManagement>
+      <plugins>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-surefire-plugin</artifactId>
+          <configuration>
+            <excludes>
+              <exclude>**/AutobahnTests**</exclude>
+            </excludes>
+          </configuration>
+        </plugin>
+      </plugins>
+    </pluginManagement>
     <plugins>
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
@@ -67,41 +102,19 @@
         </property>
       </activation>
       <build>
-        <plugins>
-          <plugin>
-            <groupId>me.normanmaurer.maven.autobahntestsuite</groupId>
-            <artifactId>autobahntestsuite-maven-plugin</artifactId>
-            <version>0.1.6</version>
-            <configuration>
-
-              <!-- Optional configuration -->
-              <!-- The port to bind the server on. Default is to choose a random free port. -->
-              <!--port>9090</port-->
-
-              <!-- The number of milliseconds to wait for the server to startup -->
-              <waitTime>20000</waitTime>
-              <generateJUnitXml>true</generateJUnitXml>
-              <cases>
-                <case>*</case>
-              </cases>
-              <testFailureIgnore>true</testFailureIgnore>
-              <excludeCases></excludeCases>
-              <failOnNonStrict>false</failOnNonStrict>
-            </configuration>
-            <executions>
-              <execution>
-                <phase>test</phase>
-                <goals>
-                  <goal>fuzzingclient</goal>
-                </goals>
-                <configuration>
-                  <!-- The class that contains a main method which accepts the port as a parameter to start the server. -->
-                  <mainClass>org.eclipse.jetty.websocket.core.autobahn.CoreAutobahnServer</mainClass>
-                </configuration>
-              </execution>
-            </executions>
-          </plugin>
-        </plugins>
+        <pluginManagement>
+          <plugins>
+            <plugin>
+              <groupId>org.apache.maven.plugins</groupId>
+              <artifactId>maven-surefire-plugin</artifactId>
+              <configuration>
+                <excludes>
+                  <exclude>none</exclude>
+                </excludes>
+              </configuration>
+            </plugin>
+          </plugins>
+        </pluginManagement>
       </build>
     </profile>
   </profiles>

jetty-websocket/websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/autobahn/AutobahnTests.java

@@ -0,0 +1,436 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under
+// the terms of the Eclipse Public License 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0
+//
+// This Source Code may also be made available under the following
+// Secondary Licenses when the conditions for such availability set
+// forth in the Eclipse Public License, v. 2.0 are satisfied:
+// the Apache License v2.0 which is available at
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.websocket.core.autobahn;
+
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.Reader;
+import java.io.Writer;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.List;
+
+import com.github.dockerjava.api.DockerClient;
+import com.github.dockerjava.api.exception.NotFoundException;
+import org.apache.commons.compress.archivers.ArchiveEntry;
+import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
+import org.codehaus.plexus.util.xml.Xpp3Dom;
+import org.codehaus.plexus.util.xml.Xpp3DomWriter;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.util.IO;
+import org.json.simple.JSONObject;
+import org.json.simple.parser.JSONParser;
+import org.json.simple.parser.ParseException;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testcontainers.containers.GenericContainer;
+import org.testcontainers.containers.output.Slf4jLogConsumer;
+import org.testcontainers.containers.startupcheck.StartupCheckStrategy;
+import org.testcontainers.junit.jupiter.Testcontainers;
+import org.testcontainers.utility.DockerImageName;
+import org.testcontainers.utility.DockerStatus;
+import org.testcontainers.utility.MountableFile;
+import org.testcontainers.utility.TestcontainersConfiguration;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+@Testcontainers
+public class AutobahnTests
+{
+    
+    private static final Logger LOG = LoggerFactory.getLogger(AutobahnTests.class);
+    private static final Path USER_DIR = Paths.get(System.getProperty("user.dir"));
+
+    private static Path reportDir;
+    private static Path fuzzingServer;
+    private static Path fuzzingClient;
+
+    @BeforeAll
+    public static void before() throws Exception
+    {
+        fuzzingServer = USER_DIR.resolve("fuzzingserver.json");
+        assertTrue(Files.exists(fuzzingServer), fuzzingServer + " not exists");
+
+        fuzzingClient = USER_DIR.resolve("fuzzingclient.json");
+        assertTrue(Files.exists(fuzzingClient), fuzzingClient + " not exists");
+
+        reportDir = USER_DIR.resolve("target/reports");
+        IO.delete(reportDir.toFile());
+        Files.createDirectory(reportDir);
+    }
+
+    @Test
+    public void testClient() throws Exception
+    {
+        try (GenericContainer<?> container = new GenericContainer<>(DockerImageName.parse("jettyproject/autobahn-testsuite:latest"))
+            .withCommand("/bin/bash", "-c", "wstest -m fuzzingserver -s /config/fuzzingserver.json")
+            .withExposedPorts(9001)
+            .withCopyFileToContainer(MountableFile.forHostPath(fuzzingServer),"/config/fuzzingserver.json")
+            .withLogConsumer(new Slf4jLogConsumer(LOG))
+            .withStartupTimeout(Duration.ofHours(2)))
+        {
+            container.start();
+            Integer mappedPort = container.getMappedPort(9001);
+            CoreAutobahnClient.main(new String[]{container.getContainerIpAddress(), mappedPort.toString()});
+
+            DockerClient dockerClient = container.getDockerClient();
+            String containerId = container.getContainerId();
+            copyFromContainer(dockerClient, containerId, reportDir, Paths.get("/target/reports/clients"));
+        }
+
+        LOG.info("Test Result Overview {}", reportDir.resolve("clients/index.html").toUri());
+
+        List<AutobahnCaseResult> results = parseResults(Paths.get("target/reports/clients/index.json"));
+        String className = getClass().getName();
+        writeJUnitXmlReport(results, "autobahn-client", className + ".client");
+        throwIfFailed(results);
+    }
+
+    @Test
+    public void testServer() throws Exception
+    {
+        // We need to expose the host port of the server to the Autobahn Client in docker container.
+        final int port = 9001;
+        org.testcontainers.Testcontainers.exposeHostPorts(port);
+        Server server = CoreAutobahnServer.startAutobahnServer(port);
+
+        FileSignalWaitStrategy strategy = new FileSignalWaitStrategy(reportDir, Paths.get("/target/reports/servers"));
+        try (GenericContainer<?> container = new GenericContainer<>(DockerImageName.parse("jettyproject/autobahn-testsuite:latest"))
+            .withCommand("/bin/bash", "-c", "wstest -m fuzzingclient -s /config/fuzzingclient.json" + FileSignalWaitStrategy.END_COMMAND)
+            .withLogConsumer(new Slf4jLogConsumer(LOG))
+            .withCopyFileToContainer(MountableFile.forHostPath(fuzzingClient),"/config/fuzzingclient.json")
+            .withStartupCheckStrategy(strategy)
+            .withStartupTimeout(Duration.ofHours(2)))
+        {
+            container.start();
+        }
+        finally
+        {
+            server.stop();
+        }
+
+        LOG.info("Test Result Overview {}", reportDir.resolve("servers/index.html").toUri());
+
+        List<AutobahnCaseResult> results = parseResults(Paths.get("target/reports/servers/index.json"));
+        String className = getClass().getName();
+        writeJUnitXmlReport(results, "autobahn-server", className + ".server");
+        throwIfFailed(results);
+    }
+
+    private void throwIfFailed(List<AutobahnCaseResult> results) throws Exception
+    {
+        StringBuilder message = new StringBuilder();
+        for (AutobahnCaseResult result : results)
+        {
+            if (result.failed())
+                message.append(result.caseName).append(", ");
+        }
+
+        if (message.length() > 0)
+            throw new Exception("Failed Test Cases: " + message);
+    }
+
+    private static class FileSignalWaitStrategy extends StartupCheckStrategy
+    {
+        public static final String SIGNAL_FILE = "/signalComplete";
+        public static final String END_COMMAND = " && touch " + SIGNAL_FILE + " && sleep infinity";
+
+        Path _localDir;
+        Path _containerDir;
+
+        public FileSignalWaitStrategy(Path localDir, Path containerDir)
+        {
+            _localDir = localDir;
+            _containerDir = containerDir;
+            withTimeout(Duration.ofHours(2));
+        }
+
+        @Override
+        public StartupCheckStrategy.StartupStatus checkStartupState(DockerClient dockerClient, String containerId)
+        {
+            // If the container was stopped then we have failed to copy out the file.
+            if (DockerStatus.isContainerStopped(getCurrentState(dockerClient, containerId)))
+                return StartupStatus.FAILED;
+
+            try
+            {
+                dockerClient.copyArchiveFromContainerCmd(containerId, SIGNAL_FILE).exec().close();
+            }
+            catch (FileNotFoundException | NotFoundException e)
+            {
+                return StartupStatus.NOT_YET_KNOWN;
+            }
+            catch (Throwable t)
+            {
+                LOG.warn("Unknown Error", t);
+                return StartupStatus.FAILED;
+            }
+
+            try
+            {
+                copyFromContainer(dockerClient, containerId, _localDir, _containerDir);
+                return StartupStatus.SUCCESSFUL;
+            }
+            catch (Throwable t)
+            {
+                LOG.warn("Error copying reports", t);
+                return StartupStatus.FAILED;
+            }
+        }
+    }
+
+    private static void copyFromContainer(DockerClient dockerClient, String containerId, Path target, Path source) throws Exception
+    {
+        try (TarArchiveInputStream tarArchiveInputStream = new TarArchiveInputStream(dockerClient
+            .copyArchiveFromContainerCmd(containerId, source.toString())
+            .exec()))
+        {
+            ArchiveEntry archiveEntry;
+            while ((archiveEntry = tarArchiveInputStream.getNextEntry()) != null)
+            {
+                Path filePath = target.resolve(archiveEntry.getName());
+                if (archiveEntry.isDirectory())
+                {
+                    if (!Files.exists(filePath))
+                        Files.createDirectory(filePath);
+                    continue;
+                }
+                Files.copy(tarArchiveInputStream, filePath);
+            }
+        }
+    }
+
+    private void writeJUnitXmlReport(List<AutobahnCaseResult> results, String surefireFileName, String testName)
+        throws Exception
+    {
+        int failures = 0;
+        long suiteDuration = 0;
+        Xpp3Dom root = new Xpp3Dom("testsuite");
+        root.setAttribute("name", testName);
+        root.setAttribute("tests", Integer.toString(results.size()));
+        root.setAttribute("errors", Integer.toString(0));
+        root.setAttribute("skipped", Integer.toString(0));
+
+        for (AutobahnCaseResult r: results)
+        {
+            Xpp3Dom testcase = new Xpp3Dom("testcase");
+            testcase.setAttribute("classname", testName);
+            testcase.setAttribute("name", r.caseName());
+
+            long duration = r.duration();
+            suiteDuration += duration;
+            testcase.setAttribute("time", Double.toString(duration / 1000.0));
+
+            if (r.failed())
+            {
+                addFailure(testcase,r);
+                failures++;
+            }
+            root.addChild(testcase);
+        }
+        root.setAttribute("failures", Integer.toString(failures));
+        root.setAttribute("time", Double.toString(suiteDuration / 1000.0));
+
+        Path surefireReportsDir = Paths.get("target/surefire-reports");
+        if (!Files.exists(surefireReportsDir))
+            Files.createDirectories(surefireReportsDir);
+
+        String filename = "TEST-" + surefireFileName + ".xml";
+        try (Writer writer = Files.newBufferedWriter(surefireReportsDir.resolve(filename)))
+        {
+            Xpp3DomWriter.write(writer, root);
+        }
+    }
+
+    private void addFailure(Xpp3Dom testCase, AutobahnCaseResult result) throws IOException,
+        ParseException
+    {
+
+        JSONParser parser = new JSONParser();
+
+        try (Reader reader = Files.newBufferedReader(Paths.get(result.reportFile())))
+        {
+            JSONObject object = (JSONObject)parser.parse(reader);
+
+            Xpp3Dom sysout = new Xpp3Dom("system-out");
+            sysout.setValue(object.toJSONString());
+            testCase.addChild(sysout);
+
+            String description = object.get("description").toString();
+            String resultText = object.get("result").toString();
+            String expected = object.get("expected").toString();
+            String received = object.get("received").toString();
+
+            StringBuilder fail = new StringBuilder();
+            fail.append(description).append("\n\n");
+            fail.append("Case outcome").append("\n\n");
+            fail.append(resultText).append("\n\n");
+            fail.append("Expected").append("\n").append(expected).append("\n\n");
+            fail.append("Received").append("\n").append(received).append("\n\n");
+
+            Xpp3Dom failure = new Xpp3Dom("failure");
+            failure.setAttribute("type", "behaviorMissmatch");
+            failure.setValue(fail.toString());
+            testCase.addChild(failure);
+        }
+    }
+
+    private static List<AutobahnCaseResult> parseResults(Path jsonPath) throws Exception
+    {
+        List<AutobahnCaseResult> results = new ArrayList<>();
+        JSONParser parser = new JSONParser();
+
+        try (Reader reader = Files.newBufferedReader(jsonPath))
+        {
+            JSONObject object = (JSONObject)parser.parse(reader);
+            JSONObject agent = (JSONObject)object.values().iterator().next();
+            if (agent == null)
+                throw new Exception("no agent");
+
+            for (Object cases : agent.keySet())
+            {
+                JSONObject c = (JSONObject)agent.get(cases);
+                String behavior = (String)c.get("behavior");
+                String behaviorClose = (String)c.get("behaviorClose");
+                Number duration = (Number)c.get("duration");
+                Number remoteCloseCode = (Number)c.get("remoteCloseCode");
+
+                Long code = (remoteCloseCode == null) ? null : remoteCloseCode.longValue();
+                String reportfile = (String)c.get("reportfile");
+                AutobahnCaseResult result = new AutobahnCaseResult(cases.toString(),
+                    AutobahnCaseResult.Behavior.parse(behavior),
+                    AutobahnCaseResult.Behavior.parse(behaviorClose),
+                    duration.longValue(), code,
+                    jsonPath.toFile().getParent() + File.separator + reportfile);
+
+                results.add(result);
+            }
+        }
+        catch (Exception e)
+        {
+            throw new Exception("Could not parse results", e);
+        }
+        return results;
+    }
+
+    public static class AutobahnCaseResult
+    {
+        enum Behavior
+        {
+            FAILED,
+            OK,
+            NON_STRICT,
+            WRONG_CODE,
+            UNCLEAN,
+            FAILED_BY_CLIENT,
+            INFORMATIONAL,
+            UNIMPLEMENTED;
+
+            static Behavior parse(String value)
+            {
+                switch (value)
+                {
+                    case "NON-STRICT":
+                        return NON_STRICT;
+                    case "WRONG CODE":
+                        return WRONG_CODE;
+                    case "FAILED BY CLIENT":
+                        return FAILED_BY_CLIENT;
+                    default:
+                        return valueOf(value);
+                }
+            }
+        }
+
+        private final String caseName;
+        private final Behavior behavior;
+        private final Behavior behaviorClose;
+        private final long duration;
+        private final Long remoteCloseCode;
+        private final String reportFile;
+
+        AutobahnCaseResult(String caseName, Behavior behavior, Behavior behaviorClose, long duration, Long remoteCloseCode, String reportFile)
+        {
+            this.caseName = caseName;
+            this.behavior = behavior;
+            this.behaviorClose = behaviorClose;
+            this.duration = duration;
+            this.remoteCloseCode = remoteCloseCode;
+            this.reportFile = reportFile;
+        }
+
+        public String caseName()
+        {
+            return caseName;
+        }
+
+        public Behavior behavior()
+        {
+            return behavior;
+        }
+
+        public boolean failed()
+        {
+            switch (behavior)
+            {
+                case OK:
+                case INFORMATIONAL:
+                case UNIMPLEMENTED:
+                    return false;
+
+                case NON_STRICT:
+                default:
+                    return true;
+            }
+        }
+
+        public Behavior behaviorClose()
+        {
+            return behaviorClose;
+        }
+
+        public long duration()
+        {
+            return duration;
+        }
+
+        public Long remoteCloseCode()
+        {
+            return remoteCloseCode;
+        }
+
+        public String reportFile()
+        {
+            return reportFile;
+        }
+
+        @Override
+        public String toString()
+        {
+            return "[" + caseName + "] behavior: " + behavior.name() + ", behaviorClose: " + behaviorClose.name() +
+                ", duration: " + duration + "ms, remoteCloseCode: " + remoteCloseCode + ", reportFile: " + reportFile;
+        }
+    }
+}

jetty-websocket/websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/autobahn/CoreAutobahnClient.java

@@ -18,18 +18,19 @@
 
 package org.eclipse.jetty.websocket.core.autobahn;
 
-import java.io.IOException;
 import java.net.URI;
-import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
 
+import org.eclipse.jetty.http.HttpField;
+import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.Jetty;
 import org.eclipse.jetty.util.UrlEncoded;
 import org.eclipse.jetty.websocket.core.CoreSession;
+import org.eclipse.jetty.websocket.core.MessageHandler;
 import org.eclipse.jetty.websocket.core.TestMessageHandler;
+import org.eclipse.jetty.websocket.core.client.CoreClientUpgradeRequest;
 import org.eclipse.jetty.websocket.core.client.WebSocketCoreClient;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -73,7 +74,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
  */
 public class CoreAutobahnClient
 {
-    public static void main(String[] args)
+    public static void main(String[] args) throws Exception
     {
         String hostname = "localhost";
         int port = 9001;
@@ -130,6 +131,7 @@ public class CoreAutobahnClient
         catch (Throwable t)
         {
             LOG.warn("Test Failed", t);
+            throw t;
         }
         finally
         {
@@ -155,7 +157,7 @@ public class CoreAutobahnClient
     {
         URI wsUri = baseWebsocketUri.resolve("/getCaseCount");
         TestMessageHandler onCaseCount = new TestMessageHandler();
-        CoreSession session = client.connect(onCaseCount, wsUri).get(5, TimeUnit.SECONDS);
+        CoreSession session = upgrade(onCaseCount, wsUri).get(5, TimeUnit.SECONDS);
         assertTrue(onCaseCount.openLatch.await(5, TimeUnit.SECONDS));
         String msg = onCaseCount.textMessages.poll(5, TimeUnit.SECONDS);
 
@@ -167,13 +169,13 @@ public class CoreAutobahnClient
         return Integer.decode(msg);
     }
 
-    public void runCaseByNumber(int caseNumber) throws IOException, InterruptedException
+    public void runCaseByNumber(int caseNumber) throws Exception
     {
         URI wsUri = baseWebsocketUri.resolve("/runCase?case=" + caseNumber + "&agent=" + UrlEncoded.encodeString(userAgent));
         LOG.info("test uri: {}", wsUri);
 
         AutobahnFrameHandler echoHandler = new AutobahnFrameHandler();
-        Future<CoreSession> response = client.connect(echoHandler, wsUri);
+        Future<CoreSession> response = upgrade(echoHandler, wsUri);
         if (waitForUpgrade(wsUri, response))
         {
             // Wait up to 5 min as some of the tests can take a while
@@ -197,11 +199,19 @@ public class CoreAutobahnClient
         }
     }
 
-    public void updateReports() throws IOException, InterruptedException, ExecutionException, TimeoutException
+    public Future<CoreSession> upgrade(MessageHandler handler, URI uri) throws Exception
+    {
+        // We manually set the port as we run the server in docker container.
+        CoreClientUpgradeRequest upgradeRequest = CoreClientUpgradeRequest.from(client, uri, handler);
+        upgradeRequest.addHeader(new HttpField(HttpHeader.HOST, "localhost:9001"));
+        return client.connect(upgradeRequest);
+    }
+
+    public void updateReports() throws Exception
     {
         URI wsUri = baseWebsocketUri.resolve("/updateReports?agent=" + UrlEncoded.encodeString(userAgent));
         TestMessageHandler onUpdateReports = new TestMessageHandler();
-        Future<CoreSession> response = client.connect(onUpdateReports, wsUri);
+        Future<CoreSession> response = upgrade(onUpdateReports, wsUri);
         response.get(5, TimeUnit.SECONDS);
         assertTrue(onUpdateReports.closeLatch.await(15, TimeUnit.SECONDS));
         LOG.info("Reports updated.");

jetty-websocket/websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/autobahn/CoreAutobahnServer.java

@@ -65,6 +65,12 @@ public class CoreAutobahnServer
         if (args != null && args.length > 0)
             port = Integer.parseInt(args[0]);
 
+        Server server = startAutobahnServer(port);
+        server.join();
+    }
+
+    public static Server startAutobahnServer(int port) throws Exception
+    {
         Server server = new Server(port);
         ServerConnector connector = new ServerConnector(server);
         connector.setIdleTimeout(10000);
@@ -76,6 +82,6 @@ public class CoreAutobahnServer
         context.setHandler(handler);
 
         server.start();
-        server.join();
+        return server;
     }
 }

jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/config/JavaxWebSocketConfiguration.java

@@ -28,8 +28,7 @@ import org.eclipse.jetty.webapp.WebXmlConfiguration;
 /**
  * <p>Websocket Configuration</p>
  * <p>This configuration configures the WebAppContext server/system classes to
- * be able to see the org.eclipse.jetty.websocket package.
- * </p>
+ * be able to see the {@code org.eclipse.jetty.websocket.javax} packages.</p>
  */
 public class JavaxWebSocketConfiguration extends AbstractConfiguration
 {
@@ -37,6 +36,7 @@ public class JavaxWebSocketConfiguration extends AbstractConfiguration
     {
         addDependencies(WebXmlConfiguration.class, MetaInfConfiguration.class, WebInfConfiguration.class, FragmentConfiguration.class);
         addDependents("org.eclipse.jetty.annotations.AnnotationConfiguration", WebAppConfiguration.class.getName());
+
         protectAndExpose("org.eclipse.jetty.websocket.util.server."); // For WebSocketUpgradeFilter
         protectAndExpose("org.eclipse.jetty.websocket.javax.server.config.");
         protectAndExpose("org.eclipse.jetty.websocket.javax.client.JavaxWebSocketClientContainerProvider");

jetty-websocket/websocket-jetty-client/pom.xml

@@ -35,6 +35,12 @@
       <artifactId>jetty-client</artifactId>
       <version>${project.version}</version>
     </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty</groupId>
+      <artifactId>jetty-webapp</artifactId>
+      <version>${project.version}</version>
+      <optional>true</optional>
+    </dependency>
     <dependency>
       <groupId>org.slf4j</groupId>
       <artifactId>slf4j-api</artifactId>

jetty-websocket/websocket-jetty-client/src/main/config/modules/websocket-jetty-client.mod

@@ -0,0 +1,24 @@
+# DO NOT EDIT - See: https://www.eclipse.org/jetty/documentation/current/startup-modules.html
+
+[description]
+Expose the Jetty WebSocket Client classes to deployed web applications.
+
+[tags]
+websocket
+
+[depend]
+client
+annotations
+
+[lib]
+lib/websocket/websocket-core-common-${jetty.version}.jar
+lib/websocket/websocket-core-client-${jetty.version}.jar
+lib/websocket/websocket-util-${jetty.version}.jar
+lib/websocket/websocket-jetty-api-${jetty.version}.jar
+lib/websocket/websocket-jetty-common-${jetty.version}.jar
+lib/websocket/websocket-jetty-client-${jetty.version}.jar
+
+[jpms]
+# The implementation needs to access method handles in
+# classes that are in the web application classloader.
+add-reads: org.eclipse.jetty.websocket.jetty.common=ALL-UNNAMED

jetty-websocket/websocket-jetty-client/src/main/java/module-info.java

@@ -20,6 +20,7 @@ module org.eclipse.jetty.websocket.jetty.client
 {
     exports org.eclipse.jetty.websocket.client;
 
+    requires static org.eclipse.jetty.webapp;
     requires org.eclipse.jetty.websocket.core.client;
     requires org.eclipse.jetty.websocket.jetty.common;
     requires org.slf4j;

jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/config/JettyWebSocketClientConfiguration.java

@@ -0,0 +1,49 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under
+// the terms of the Eclipse Public License 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0
+//
+// This Source Code may also be made available under the following
+// Secondary Licenses when the conditions for such availability set
+// forth in the Eclipse Public License, v. 2.0 are satisfied:
+// the Apache License v2.0 which is available at
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.websocket.client.config;
+
+import org.eclipse.jetty.webapp.AbstractConfiguration;
+import org.eclipse.jetty.webapp.FragmentConfiguration;
+import org.eclipse.jetty.webapp.MetaInfConfiguration;
+import org.eclipse.jetty.webapp.WebAppConfiguration;
+import org.eclipse.jetty.webapp.WebInfConfiguration;
+import org.eclipse.jetty.webapp.WebXmlConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * <p>Websocket Configuration</p>
+ * <p>This configuration configures the WebAppContext server/system classes to
+ * be able to see the {@code org.eclipse.jetty.websocket.client} package.</p>
+ */
+public class JettyWebSocketClientConfiguration extends AbstractConfiguration
+{
+    private static final Logger LOG = LoggerFactory.getLogger(JettyWebSocketClientConfiguration.class);
+
+    public JettyWebSocketClientConfiguration()
+    {
+        addDependencies(WebXmlConfiguration.class, MetaInfConfiguration.class, WebInfConfiguration.class, FragmentConfiguration.class);
+        addDependents("org.eclipse.jetty.annotations.AnnotationConfiguration", WebAppConfiguration.class.getName());
+
+        protectAndExpose("org.eclipse.jetty.websocket.api.");
+        protectAndExpose("org.eclipse.jetty.websocket.client.");
+        hide("org.eclipse.jetty.client.impl.");
+        hide("org.eclipse.jetty.client.config.");
+    }
+}

jetty-websocket/websocket-jetty-client/src/main/resources/META-INF/services/org.eclipse.jetty.webapp.Configuration

@@ -0,0 +1 @@
+org.eclipse.jetty.websocket.client.config.JettyWebSocketClientConfiguration

jetty-websocket/websocket-jetty-server/src/main/config/modules/websocket-jetty.mod

@@ -7,12 +7,10 @@ Enable the Jetty WebSocket API for deployed web applications.
 websocket
 
 [depend]
-client
 annotations
 
 [lib]
 lib/websocket/websocket-core-common-${jetty.version}.jar
-lib/websocket/websocket-core-client-${jetty.version}.jar
 lib/websocket/websocket-core-server-${jetty.version}.jar
 lib/websocket/websocket-util-${jetty.version}.jar
 lib/websocket/websocket-util-server-${jetty.version}.jar

jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/config/JettyWebSocketConfiguration.java

@@ -18,11 +18,7 @@
 
 package org.eclipse.jetty.websocket.server.config;
 
-import java.util.ServiceLoader;
-
-import org.eclipse.jetty.util.Loader;
 import org.eclipse.jetty.webapp.AbstractConfiguration;
-import org.eclipse.jetty.webapp.Configuration;
 import org.eclipse.jetty.webapp.FragmentConfiguration;
 import org.eclipse.jetty.webapp.MetaInfConfiguration;
 import org.eclipse.jetty.webapp.WebAppConfiguration;
@@ -34,12 +30,8 @@ import org.slf4j.LoggerFactory;
 /**
  * <p>Websocket Configuration</p>
  * <p>This configuration configures the WebAppContext server/system classes to
- * be able to see the org.eclipse.jetty.websocket package.
- * This class is defined in the webapp package, as it implements the {@link Configuration} interface,
- * which is unknown to the websocket package.  However, the corresponding {@link ServiceLoader}
- * resource is defined in the websocket package, so that this configuration only be
- * loaded if the jetty-websocket jars are on the classpath.
- * </p>
+ * be able to see the {@code org.eclipse.jetty.websocket.api}, {@code org.eclipse.jetty.websocket.server} and
+ * {@code org.eclipse.jetty.websocket.util.server} packages.</p>
  */
 public class JettyWebSocketConfiguration extends AbstractConfiguration
 {
@@ -48,39 +40,12 @@ public class JettyWebSocketConfiguration extends AbstractConfiguration
     public JettyWebSocketConfiguration()
     {
         addDependencies(WebXmlConfiguration.class, MetaInfConfiguration.class, WebInfConfiguration.class, FragmentConfiguration.class);
+        addDependents("org.eclipse.jetty.annotations.AnnotationConfiguration", WebAppConfiguration.class.getName());
 
-        if (isAvailable("org.eclipse.jetty.osgi.annotations.AnnotationConfiguration"))
-            addDependents("org.eclipse.jetty.osgi.annotations.AnnotationConfiguration", WebAppConfiguration.class.getName());
-        else if (isAvailable("org.eclipse.jetty.annotations.AnnotationConfiguration"))
-            addDependents("org.eclipse.jetty.annotations.AnnotationConfiguration", WebAppConfiguration.class.getName());
-        else
-            throw new RuntimeException("Unable to add AnnotationConfiguration dependent (not present in classpath)");
-
-        protectAndExpose(
-            "org.eclipse.jetty.websocket.api.",
-            "org.eclipse.jetty.websocket.server.",
-            "org.eclipse.jetty.websocket.util.server."); // For WebSocketUpgradeFilter
-
-        hide("org.eclipse.jetty.server.internal.",
-            "org.eclipse.jetty.server.config.");
-    }
-
-    @Override
-    public boolean isAvailable()
-    {
-        return isAvailable("org.eclipse.jetty.websocket.common.JettyWebSocketFrame");
-    }
-
-    private boolean isAvailable(String classname)
-    {
-        try
-        {
-            return Loader.loadClass(classname) != null;
-        }
-        catch (Throwable e)
-        {
-            LOG.trace("IGNORED", e);
-            return false;
-        }
+        protectAndExpose("org.eclipse.jetty.websocket.api.");
+        protectAndExpose("org.eclipse.jetty.websocket.server.");
+        protectAndExpose("org.eclipse.jetty.websocket.util.server."); // For WebSocketUpgradeFilter
+        hide("org.eclipse.jetty.server.internal.");
+        hide("org.eclipse.jetty.server.config.");
     }
 }

jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/WebSocketUpgradeFilter.java

@@ -94,7 +94,6 @@ public class WebSocketUpgradeFilter implements Filter, Dumpable
     /**
      * Ensure a {@link WebSocketUpgradeFilter} is available on the provided {@link ServletContext},
      * a new filter will added if one does not already exist.
-     * </p>
      * <p>
      * The default {@link WebSocketUpgradeFilter} is also available via
      * the {@link ServletContext} attribute named {@code org.eclipse.jetty.websocket.server.WebSocketUpgradeFilter}

pom.xml

@@ -47,10 +47,10 @@
     <unix.socket.tmp></unix.socket.tmp>
     <!-- enable or not TestTracker junit5 extension i.e log message when test method is starting -->
     <jetty.testtracker.log>false</jetty.testtracker.log>
-    <jetty.surefire.argLine>-Dfile.encoding=UTF-8 -Duser.language=en -Duser.region=US -showversion -Xmx2g -Xms2g -Xlog:gc:stderr:time,level,tags</jetty.surefire.argLine>
+    <jetty.surefire.argLine>-Dfile.encoding=UTF-8 -Duser.language=en -Duser.region=US -showversion -Xmx4g -Xms2g -Xlog:gc:stderr:time,level,tags</jetty.surefire.argLine>
 
     <!-- some maven plugins versions -->
-    <maven.surefire.version>3.0.0-M4</maven.surefire.version>
+    <maven.surefire.version>3.0.0-M5</maven.surefire.version>
     <maven.compiler.plugin.version>3.8.1</maven.compiler.plugin.version>
     <maven.dependency.plugin.version>3.1.2</maven.dependency.plugin.version>
     <maven.resources.plugin.version>3.2.0</maven.resources.plugin.version>
@@ -650,6 +650,7 @@
           <artifactId>maven-surefire-plugin</artifactId>
           <version>${maven.surefire.version}</version>
           <configuration>
+            <trimStackTrace>false</trimStackTrace>
             <rerunFailingTestsCount>${surefire.rerunFailingTestsCount}</rerunFailingTestsCount>
             <forkedProcessTimeoutInSeconds>3600</forkedProcessTimeoutInSeconds>
             <argLine>
@@ -1176,6 +1177,16 @@
         <artifactId>ant-launcher</artifactId>
         <version>${ant.version}</version>
       </dependency>
+      <dependency>
+        <groupId>commons-codec</groupId>
+        <artifactId>commons-codec</artifactId>
+        <version>1.13</version>
+      </dependency>
+      <dependency>
+        <groupId>org.apache.commons</groupId>
+        <artifactId>commons-lang3</artifactId>
+        <version>3.9</version>
+      </dependency>
     </dependencies>
   </dependencyManagement>
 
@@ -1354,9 +1365,7 @@
     <profile>
       <id>ci</id>
       <properties>
-        <settingsPath>${env.GLOBAL_MVN_SETTINGS}</settingsPath>
-        <invoker.mergeUserSettings>true</invoker.mergeUserSettings>
-        <surefire.rerunFailingTestsCount>3</surefire.rerunFailingTestsCount>
+        <surefire.rerunFailingTestsCount>0</surefire.rerunFailingTestsCount>
       </properties>
       <build>
         <pluginManagement>

tests/test-distribution/pom.xml

@@ -134,6 +134,12 @@
       <version>${project.version}</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty</groupId>
+      <artifactId>jetty-util-ajax</artifactId>
+      <version>${project.version}</version>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.eclipse.jetty.websocket</groupId>
       <artifactId>websocket-jetty-api</artifactId>

tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java

@@ -47,7 +47,6 @@ import org.eclipse.jetty.websocket.api.Session;
 import org.eclipse.jetty.websocket.api.StatusCode;
 import org.eclipse.jetty.websocket.api.WebSocketListener;
 import org.eclipse.jetty.websocket.client.WebSocketClient;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.condition.DisabledOnJre;
 import org.junit.jupiter.api.condition.DisabledOnOs;
@@ -129,7 +128,7 @@ public class DistributionTests extends AbstractJettyHomeTest
                 Path quickstartWebXml = webInf.resolve("quickstart-web.xml");
                 assertTrue(Files.exists(quickstartWebXml));
                 assertNotEquals(0, Files.size(quickstartWebXml));
-                
+
                 int port = distribution.freePort();
 
                 try (JettyHomeTester.Run run3 = distribution.start("jetty.http.port=" + port, "jetty.quickstart.mode=QUICKSTART"))
@@ -145,7 +144,7 @@ public class DistributionTests extends AbstractJettyHomeTest
             }
         }
     }
-    
+
     @Test
     public void testSimpleWebAppWithJSP() throws Exception
     {
@@ -376,7 +375,6 @@ public class DistributionTests extends AbstractJettyHomeTest
         }
     }
 
-    @Disabled
     @ParameterizedTest
     @ValueSource(strings = {"http", "https"})
     public void testWebsocketClientInWebappProvidedByServer(String scheme) throws Exception
@@ -389,11 +387,12 @@ public class DistributionTests extends AbstractJettyHomeTest
             .mavenLocalRepository(System.getProperty("mavenRepoPath"))
             .build();
 
+        String module = "https".equals(scheme) ? "test-keystore," + scheme : scheme;
         String[] args1 = {
             "--create-startd",
             "--approve-all-licenses",
-            "--add-to-start=resources,server,webapp,deploy,jsp,jmx,servlet,servlets,websocket,test-keystore," + scheme
-        };
+            "--add-to-start=resources,server,webapp,deploy,jsp,jmx,servlet,servlets,websocket,websocket-jetty-client," + module,
+            };
         try (JettyHomeTester.Run run1 = distribution.start(args1))
         {
             assertTrue(run1.awaitFor(5, TimeUnit.SECONDS));
@@ -425,7 +424,6 @@ public class DistributionTests extends AbstractJettyHomeTest
         }
     }
 
-    @Disabled
     @ParameterizedTest
     @ValueSource(strings = {"http", "https"})
     public void testWebsocketClientInWebapp(String scheme) throws Exception
@@ -457,7 +455,7 @@ public class DistributionTests extends AbstractJettyHomeTest
                 "jetty.http.port=" + port,
                 "jetty.ssl.port=" + port,
                 // "jetty.server.dumpAfterStart=true",
-                };
+            };
 
             try (JettyHomeTester.Run run2 = distribution.start(args2))
             {
@@ -515,8 +513,8 @@ public class DistributionTests extends AbstractJettyHomeTest
 
     /**
      * This reproduces some classloading issue with MethodHandles in JDK14-15, this has been fixed in JDK16.
-     * @see <a href="https://bugs.openjdk.java.net/browse/JDK-8244090">JDK-8244090</a>
      * @throws Exception if there is an error during the test.
+     * @see <a href="https://bugs.openjdk.java.net/browse/JDK-8244090">JDK-8244090</a>
      */
     @ParameterizedTest
     @ValueSource(strings = {"", "--jpms"})
@@ -641,5 +639,4 @@ public class DistributionTests extends AbstractJettyHomeTest
             }
         }
     }
-
 }