401317 - Make Safari 5.x websocket support minVersion level error more clear

 + Making error message about minVersion configurable more clear on both
   the Logging on the server side, and the HTTP/1.1 400 error response
   line

3 files changed, 150 insertions, 3 deletions

diff --git a/jetty-websocket/src/main/java/org/eclipse/jetty/websocket/WebSocketFactory.java b/jetty-websocket/src/main/java/org/eclipse/jetty/websocket/WebSocketFactory.java
index 1378956342..7459887bb7 100644
--- a/jetty-websocket/src/main/java/org/eclipse/jetty/websocket/WebSocketFactory.java
+++ b/jetty-websocket/src/main/java/org/eclipse/jetty/websocket/WebSocketFactory.java
@@ -230,6 +230,8 @@ public class WebSocketFactory extends AbstractLifeCycle
             // Old pre-RFC version specifications (header not present in RFC-6455)
             draft = request.getIntHeader("Sec-WebSocket-Draft");
         }
+        // Remember requested version for possible error message later
+        int requestedVersion = draft;
         AbstractHttpConnection http = AbstractHttpConnection.getCurrentConnection();
         if (http instanceof BlockingHttpConnection)
             throw new IllegalStateException("Websockets not supported on blocking connectors");
@@ -252,7 +254,7 @@ public class WebSocketFactory extends AbstractLifeCycle
             draft=Integer.MAX_VALUE;
         switch (draft)
         {
-            case -1: // unspecified draft/version
+            case -1: // unspecified draft/version (such as early OSX Safari 5.1 and iOS 5.x)
             case 0: // Old school draft/version
             {
                 connection = new WebSocketServletConnectionD00(this, websocket, endp, _buffers, http.getTimeStamp(), _maxIdleTime, protocol);
@@ -283,7 +285,6 @@ public class WebSocketFactory extends AbstractLifeCycle
             }
             default:
             {
-                LOG.warn("Unsupported Websocket version: " + draft);
                 // Per RFC 6455 - 4.4 - Supporting Multiple Versions of WebSocket Protocol
                 // Using the examples as outlined
                 String versions="13";
@@ -295,7 +296,20 @@ public class WebSocketFactory extends AbstractLifeCycle
                     versions+=", 0";
                     
                 response.setHeader("Sec-WebSocket-Version", versions);
-                throw new HttpException(400, "Unsupported websocket version specification: " + draft);
+
+                // Make error clear for developer / end-user
+                StringBuilder err = new StringBuilder();
+                err.append("Unsupported websocket client version specification ");
+                if(requestedVersion >= 0) {
+                    err.append("[").append(requestedVersion).append("]");
+                } else {
+                    err.append("<Unspecified, likely a pre-draft version of websocket>");
+                }
+                err.append(", configured minVersion [").append(_minVersion).append("]");
+                err.append(", reported supported versions [").append(versions).append("]");
+                LOG.warn(err.toString()); // Log it
+                // use spec language for unsupported versions
+                throw new HttpException(400, "Unsupported websocket version specification"); // Tell client
             }
         }
 
diff --git a/jetty-websocket/src/test/java/org/eclipse/jetty/websocket/WebSocketMinVersionTest.java b/jetty-websocket/src/test/java/org/eclipse/jetty/websocket/WebSocketMinVersionTest.java
new file mode 100644
index 0000000000..5fb4713c0f
--- /dev/null
+++ b/jetty-websocket/src/test/java/org/eclipse/jetty/websocket/WebSocketMinVersionTest.java
@@ -0,0 +1,119 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2013 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.websocket;
+
+import static org.hamcrest.Matchers.*;
+
+import java.net.URI;
+import java.util.concurrent.TimeUnit;
+
+import org.eclipse.jetty.server.Connector;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.servlet.ServletHolder;
+import org.eclipse.jetty.websocket.helper.CaptureSocket;
+import org.eclipse.jetty.websocket.helper.SafariD00;
+import org.eclipse.jetty.websocket.helper.WebSocketCaptureServlet;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class WebSocketMinVersionTest
+{
+    private Server server;
+    private WebSocketCaptureServlet servlet;
+    private URI serverUri;
+
+    @BeforeClass
+    public static void initLogging()
+    {
+        // Configure Logging
+        // System.setProperty("org.eclipse.jetty.util.log.class",StdErrLog.class.getName());
+        System.setProperty("org.eclipse.jetty.websocket.helper.LEVEL","DEBUG");
+    }
+
+    @Before
+    public void startServer() throws Exception
+    {
+        // Configure Server
+        server = new Server(0);
+
+        ServletContextHandler context = new ServletContextHandler();
+        context.setContextPath("/");
+        server.setHandler(context);
+
+        // Serve capture servlet
+        servlet = new WebSocketCaptureServlet();
+        ServletHolder holder = new ServletHolder(servlet);
+        holder.setInitParameter("minVersion","8");
+        context.addServlet(holder,"/");
+
+        // Start Server
+        server.start();
+
+        Connector conn = server.getConnectors()[0];
+        String host = conn.getHost();
+        if (host == null)
+        {
+            host = "localhost";
+        }
+        int port = conn.getLocalPort();
+        serverUri = new URI(String.format("ws://%s:%d/",host,port));
+        // System.out.printf("Server URI: %s%n",serverUri);
+    }
+
+    @Test
+    public void testAttemptUpgrade() throws Exception
+    {
+        SafariD00 safari = new SafariD00(serverUri);
+
+        try
+        {
+            safari.connect();
+            safari.issueHandshake();
+            Assert.fail("Expected upgrade failure");
+        }
+        catch(IllegalStateException e) {
+            String respHeader = e.getMessage();
+            Assert.assertThat("Response Header", respHeader, allOf(
+                    containsString("HTTP/1.1 400 Unsupported"),
+                    containsString("minVersion [8]"),
+                    containsString("[13, 8]")));
+        }
+        finally
+        {
+            // System.out.println("Closing client socket");
+            safari.disconnect();
+        }
+    }
+
+    public static void threadSleep(int dur, TimeUnit unit) throws InterruptedException
+    {
+        long ms = TimeUnit.MILLISECONDS.convert(dur,unit);
+        Thread.sleep(ms);
+    }
+
+    @After
+    public void stopServer() throws Exception
+    {
+        server.stop();
+    }
+}
diff --git a/jetty-websocket/src/test/java/org/eclipse/jetty/websocket/helper/SafariD00.java b/jetty-websocket/src/test/java/org/eclipse/jetty/websocket/helper/SafariD00.java
index d1a91e83ca..ac3c4757d0 100644
--- a/jetty-websocket/src/test/java/org/eclipse/jetty/websocket/helper/SafariD00.java
+++ b/jetty-websocket/src/test/java/org/eclipse/jetty/websocket/helper/SafariD00.java
@@ -23,6 +23,7 @@ import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.OutputStream;
 import java.io.UnsupportedEncodingException;
+import java.net.ConnectException;
 import java.net.InetSocketAddress;
 import java.net.Socket;
 import java.net.SocketAddress;
@@ -35,6 +36,7 @@ import org.eclipse.jetty.util.log.Logger;
 import org.eclipse.jetty.util.log.StdErrLog;
 import org.junit.Assert;
 
+import static org.hamcrest.Matchers.*;
 import static org.hamcrest.Matchers.is;
 
 public class SafariD00
@@ -109,12 +111,14 @@ public class SafariD00
 
         // Read HTTP 101 Upgrade / Handshake Response
         InputStreamReader reader = new InputStreamReader(in);
+        StringBuilder respHeaders = new StringBuilder();
 
         LOG.debug("Reading http headers");
         int crlfs = 0;
         while (true)
         {
             int read = in.read();
+            respHeaders.append((char)read);
             if (read == '\r' || read == '\n')
                 ++crlfs;
             else
@@ -122,6 +126,16 @@ public class SafariD00
             if (crlfs == 4)
                 break;
         }
+        
+        if(respHeaders.toString().startsWith("HTTP/1.1 101 ") == false) {
+            String respLine = respHeaders.toString();
+            int idx = respLine.indexOf('\r');
+            if(idx > 0) {
+                respLine = respLine.substring(0,idx);
+            }
+            LOG.debug("Response Headers: {}",respHeaders.toString());
+            throw new IllegalStateException(respLine);
+        }
 
         // Read expected handshake hixie bytes
         byte hixieHandshakeExpected[] = TypeUtil.fromHexString("c7438d956cf611a6af70603e6fa54809");