improve javadoc to strengthen contract for websocket upgrade

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

jetty-core/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/Handshaker.java

@@ -31,5 +31,20 @@ public interface Handshaker
 
     boolean isWebSocketUpgradeRequest(Request request);
 
+    /**
+     * This method returns true the WebSocket upgrade was accepted. A return value of true means this method has taken the
+     * responsibility for completing the callback, the request will be upgraded to WebSocket or a response will be
+     * sent. If this method returns false the WebSocket upgrade was not accepted and the caller is still responsible for completing
+     * the callback.
+     *
+     * @param negotiator the negotiator
+     * @param request the request
+     * @param response the response
+     * @param callback the callback
+     * @param components the WebSocket components
+     * @param defaultCustomizer the customizer
+     * @return true if the WebSocket upgrade was accepted
+     * @throws IOException there is an error during the upgrade
+     */
     boolean upgradeRequest(WebSocketNegotiator negotiator, Request request, Response response, Callback callback, WebSocketComponents components, Configuration.Customizer defaultCustomizer) throws IOException;
 }

jetty-core/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketCreator.java

@@ -15,12 +15,11 @@ package org.eclipse.jetty.websocket.core.server;
 
 import org.eclipse.jetty.util.Callback;
 
-// TODO: improve javadoc.
 /**
  * Abstract WebSocket creator interface.
  * <p>
- * Should you desire filtering of the WebSocket object creation due to criteria such as origin or sub-protocol,
+ * This can be used for filtering of the WebSocket object creation due to criteria such as origin or sub-protocol,
- * then you will be required to implement a custom WebSocketCreator implementation.
+ * or for choosing a specific WebSocket object based on the upgrade request.
  * </p>
  */
 public interface WebSocketCreator
@@ -28,10 +27,14 @@ public interface WebSocketCreator
     /**
      * Create a websocket from the incoming request.
      *
+     * <p>If the creator returns null it is responsible for completing the {@link Callback} and sending a response.
+     * If the creator intends to return non-null WebSocket object, it MUST NOT write content to the response or
+     * complete the {@link Callback}.</p>
+     *
      * @param req the request details
      * @param resp the response details
-     * @param callback callback
+     * @param callback the callback, should only be completed by the creator if a null WebSocket object is returned.
-     * @return a websocket object to use, or null if no websocket should be created from this request.
+     * @return the WebSocket object, or null to take responsibility to send error response if no WebSocket is to be created.
      */
     Object createWebSocket(ServerUpgradeRequest req, ServerUpgradeResponse resp, Callback callback);
 }

jetty-core/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketMappings.java

@@ -229,6 +229,19 @@ public class WebSocketMappings implements Dumpable, LifeCycle.Listener
         return negotiator;
     }
 
+    /**
+     * This method returns true the WebSocket upgrade was accepted. A return value of true means this method has taken the
+     * responsibility for completing the callback, the request will be upgraded to WebSocket or a response will be
+     * sent. If this method returns false the WebSocket upgrade was not accepted and the caller is still responsible for completing
+     * the callback.
+     *
+     * @param request the request
+     * @param response the response
+     * @param callback the callback
+     * @param defaultCustomizer the customizer
+     * @return true if the WebSocket upgrade was accepted
+     * @throws IOException there is an error during the upgrade
+     */
     public boolean upgrade(Request request, Response response, Callback callback, Configuration.Customizer defaultCustomizer) throws IOException
     {
         String target = request.getPathInContext();
@@ -239,13 +252,23 @@ public class WebSocketMappings implements Dumpable, LifeCycle.Listener
             request.setAttribute(PathSpec.class.getName(), pathSpec);
         });
 
-        if (negotiator == null)
+        return upgrade(negotiator, request, response, callback, defaultCustomizer);
-            return false;
-
-        // We have an upgrade request
-        return handshaker.upgradeRequest(negotiator, request, response, callback, components, defaultCustomizer);
     }
 
+    /**
+     * This method returns true the WebSocket upgrade was accepted. A return value of true means this method has taken the
+     * responsibility for completing the callback, the request will be upgraded to WebSocket or a response will be
+     * sent. If this method returns false the WebSocket upgrade was not accepted and the caller is still responsible for completing
+     * the callback.
+     *
+     * @param negotiator the negotiator
+     * @param request the request
+     * @param response the response
+     * @param callback the callback
+     * @param defaultCustomizer the customizer
+     * @return true if the WebSocket upgrade was accepted
+     * @throws IOException there is an error during the upgrade
+     */
     public boolean upgrade(WebSocketNegotiator negotiator, Request request, Response response, Callback callback, Configuration.Customizer defaultCustomizer) throws IOException
     {
         if (negotiator == null)

jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee10/websocket/server/JettyWebSocketCreator.java

@@ -25,6 +25,10 @@ public interface JettyWebSocketCreator
     /**
      * Create a websocket from the incoming request.
      *
+     * <p>If no websocket is to be created (return value of null), the {@link JettyWebSocketCreator}
+     * is responsible for sending a response with {@link JettyServerUpgradeResponse#sendError(int, String)},
+     * {@link JettyServerUpgradeResponse#sendForbidden(String)} or {@link JettyServerUpgradeResponse#setStatusCode(int)}.</p>
+     *
      * @param req the request details
      * @param resp the response details
      * @return a websocket object to use, or null if no websocket should be created from this request.

jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee10/websocket/server/JettyWebSocketServerContainer.java

@@ -152,7 +152,8 @@ public class JettyWebSocketServerContainer extends ContainerLifeCycle implements
             try
             {
                 Object webSocket = creator.createWebSocket(new DelegatedServerUpgradeRequest(req), new DelegatedServerUpgradeResponse(resp));
-                cb.succeeded();
+                if (webSocket == null)
+                    cb.succeeded();
                 return webSocket;
             }
             catch (Throwable t)