From 1eca1e7c3cbd403bd0f85a283f3f5ae2860e1408 Mon Sep 17 00:00:00 2001
From: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Date: Thu, 10 Oct 2013 09:04:45 -0700
Subject: [PATCH] Making subprotocol negotiation behavior report warnings on
 bad configuration

---
 .../BasicServerEndpointConfigurator.java       | 18 ++++++++++++++++++
 .../server/browser/JsrBrowserSocket.java       |  2 +-
 .../resources/jsr-browser-debug-tool/main.css  |  4 ++++
 .../jsr-browser-debug-tool/websocket.js        | 11 +++++++++++
 4 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/BasicServerEndpointConfigurator.java b/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/BasicServerEndpointConfigurator.java
index 5137303f745..11ad5686f76 100644
--- a/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/BasicServerEndpointConfigurator.java
+++ b/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/BasicServerEndpointConfigurator.java
@@ -28,6 +28,7 @@ import javax.websocket.server.ServerEndpointConfig.Configurator;
 
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
+import org.eclipse.jetty.websocket.api.util.QuoteUtil;
 
 public class BasicServerEndpointConfigurator extends Configurator
 {
@@ -63,6 +64,21 @@ public class BasicServerEndpointConfigurator extends Configurator
     @Override
     public String getNegotiatedSubprotocol(List<String> supported, List<String> requested)
     {
+        if ((requested == null) || (requested.size() == 0))
+        {
+            // nothing requested, don't return anything
+            return null;
+        }
+
+        // Nothing specifically called out as being supported by the endpoint
+        if ((supported == null) || (supported.isEmpty()))
+        {
+            // Just return the first hit in this case
+            LOG.warn("Client requested Subprotocols on endpoint with none supported: {}", QuoteUtil.join(requested,","));
+            return null;
+        }
+
+        // Return the first matching hit from the list of supported protocols.
         for (String possible : requested)
         {
             if (supported.contains(possible))
@@ -70,6 +86,8 @@ public class BasicServerEndpointConfigurator extends Configurator
                 return possible;
             }
         }
+
+        LOG.warn("Client requested subprotocols {} do not match any endpoint supported subprotocols {}", QuoteUtil.join(requested,","), QuoteUtil.join(supported,","));
         return null;
     }
 
diff --git a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/browser/JsrBrowserSocket.java b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/browser/JsrBrowserSocket.java
index 0ea19c99fb1..611a2478b66 100644
--- a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/browser/JsrBrowserSocket.java
+++ b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/browser/JsrBrowserSocket.java
@@ -36,7 +36,7 @@ import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
 
-@ServerEndpoint(value = "/", configurator = JsrBrowserConfigurator.class)
+@ServerEndpoint(value = "/", subprotocols = { "tool" }, configurator = JsrBrowserConfigurator.class)
 public class JsrBrowserSocket
 {
     private static class WriteMany implements Runnable
diff --git a/jetty-websocket/javax-websocket-server-impl/src/test/resources/jsr-browser-debug-tool/main.css b/jetty-websocket/javax-websocket-server-impl/src/test/resources/jsr-browser-debug-tool/main.css
index 9eebead468d..7a808dcfce0 100644
--- a/jetty-websocket/javax-websocket-server-impl/src/test/resources/jsr-browser-debug-tool/main.css
+++ b/jetty-websocket/javax-websocket-server-impl/src/test/resources/jsr-browser-debug-tool/main.css
@@ -20,6 +20,10 @@ div#console .info {
 	color: black;
 }
 
+div#console .error {
+    color: red;
+}
+
 div#console .client {
 	color: blue;
 }
diff --git a/jetty-websocket/javax-websocket-server-impl/src/test/resources/jsr-browser-debug-tool/websocket.js b/jetty-websocket/javax-websocket-server-impl/src/test/resources/jsr-browser-debug-tool/websocket.js
index 1bc5fe78947..3e1d5a993f0 100644
--- a/jetty-websocket/javax-websocket-server-impl/src/test/resources/jsr-browser-debug-tool/websocket.js
+++ b/jetty-websocket/javax-websocket-server-impl/src/test/resources/jsr-browser-debug-tool/websocket.js
@@ -34,6 +34,7 @@ var wstool = {
             this._ws.onopen = this._onopen;
             this._ws.onmessage = this._onmessage;
             this._ws.onclose = this._onclose;
+            this._ws.onerror = this._onerror;
         } catch (exception) {
             wstool.info("Connect Error: " + exception);
         }
@@ -58,6 +59,10 @@ var wstool = {
         wstool._out("info", message);
     },
 
+    error : function(message) {
+        wstool._out("error", message);
+    },
+
     infoc : function(message) {
         wstool._out("client", "[c] " + message);
     },
@@ -83,6 +88,12 @@ var wstool = {
         wstool.setState(true);
         wstool.info("Websocket Connected");
     },
+    
+    _onerror : function(evt) {
+        wstool.setState(false);
+        wstool.error("Websocket Error: " + evt.data);
+        wstool.error("See Javascript Console for possible detailed error message");
+    },
 
     _send : function(message) {
         if (this._ws) {