2011-06-02 Yuta Kitamura <yutak@chromium.org>
authoryutak@chromium.org <yutak@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 2 Jun 2011 08:12:04 +0000 (08:12 +0000)
committeryutak@chromium.org <yutak@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 2 Jun 2011 08:12:04 +0000 (08:12 +0000)
        Reviewed by Kent Tamura.

        WebSocket: Call WebSocketChannel::fail() when WebSocketHandshake has failed
        https://bugs.webkit.org/show_bug.cgi?id=61841

        * http/tests/websocket/tests/bad-handshake-crash-expected.txt:
        Now the error message starts with a capital letter.
2011-06-02  Yuta Kitamura  <yutak@chromium.org>

        Reviewed by Kent Tamura.

        WebSocket: Call WebSocketChannel::fail() when WebSocketHandshake has failed
        https://bugs.webkit.org/show_bug.cgi?id=61841

        There is no change in behavior except that capitalization of a few error messages
        has been changed. No new tests are added.

        * websockets/WebSocketChannel.cpp:
        (WebCore::WebSocketChannel::processBuffer):
        Pass m_handshake.failureReason() to fail() if the handshake has failed.
        * websockets/WebSocketHandshake.cpp:
        Replace occurrences of m_handle->addMessage() with assignments to m_failureReason.
        Change capitalization of a few messages so that all messages start with a capital letter.
        (WebCore::WebSocketHandshake::readServerHandshake):
        (WebCore::WebSocketHandshake::failureReason):
        (WebCore::WebSocketHandshake::readStatusLine):
        (WebCore::WebSocketHandshake::readHTTPHeaders):
        (WebCore::WebSocketHandshake::checkResponseHeaders):
        * websockets/WebSocketHandshake.h:
        Add failureReason(), which returns a string that describes why WebSocket handshake
        has failed.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@87883 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/websocket/tests/bad-handshake-crash-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/websockets/WebSocketChannel.cpp
Source/WebCore/websockets/WebSocketHandshake.cpp
Source/WebCore/websockets/WebSocketHandshake.h

index 370b72f1a633a035db0746fa40d9d55e1e47da44..58331da7549de9ffe156f48c9dc52499367c78dd 100644 (file)
@@ -1,3 +1,13 @@
+2011-06-02  Yuta Kitamura  <yutak@chromium.org>
+
+        Reviewed by Kent Tamura.
+
+        WebSocket: Call WebSocketChannel::fail() when WebSocketHandshake has failed
+        https://bugs.webkit.org/show_bug.cgi?id=61841
+
+        * http/tests/websocket/tests/bad-handshake-crash-expected.txt:
+        Now the error message starts with a capital letter.
+
 2011-06-01  Kent Tamura  <tkent@chromium.org>
 
         Reviewed by Dimitri Glazkov.
index 3e7a886f8c5b2344e38b971aba0ff5bb720bebce..a5f20f4c94098f4caa151db028839ba3fa69e12a 100644 (file)
@@ -1,4 +1,4 @@
-CONSOLE MESSAGE: line 0: invalid UTF-8 sequence in header name
+CONSOLE MESSAGE: line 0: Invalid UTF-8 sequence in header name
 Make sure WebSocket doesn't crash with bad handshake message.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
index ae8aae17b1a985a99aa4cbfceda376ba72bb26b5..218c0935651ca37fc28102bbc271066242397e32 100644 (file)
@@ -1,3 +1,28 @@
+2011-06-02  Yuta Kitamura  <yutak@chromium.org>
+
+        Reviewed by Kent Tamura.
+
+        WebSocket: Call WebSocketChannel::fail() when WebSocketHandshake has failed
+        https://bugs.webkit.org/show_bug.cgi?id=61841
+
+        There is no change in behavior except that capitalization of a few error messages
+        has been changed. No new tests are added.
+
+        * websockets/WebSocketChannel.cpp:
+        (WebCore::WebSocketChannel::processBuffer):
+        Pass m_handshake.failureReason() to fail() if the handshake has failed.
+        * websockets/WebSocketHandshake.cpp:
+        Replace occurrences of m_handle->addMessage() with assignments to m_failureReason.
+        Change capitalization of a few messages so that all messages start with a capital letter.
+        (WebCore::WebSocketHandshake::readServerHandshake):
+        (WebCore::WebSocketHandshake::failureReason):
+        (WebCore::WebSocketHandshake::readStatusLine):
+        (WebCore::WebSocketHandshake::readHTTPHeaders):
+        (WebCore::WebSocketHandshake::checkResponseHeaders):
+        * websockets/WebSocketHandshake.h:
+        Add failureReason(), which returns a string that describes why WebSocket handshake
+        has failed.
+
 2011-06-01  Dan Bernstein  <mitz@apple.com>
 
         Reviewed by Anders Carlsson.
index 6d49a563a0db98a94a3c54cbc1f21513f83a3efa..4ca29d772674b23da913111945c3e76f0575f626 100644 (file)
@@ -330,11 +330,11 @@ bool WebSocketChannel::processBuffer()
             LOG(Network, "remaining in read buf %lu", static_cast<unsigned long>(m_bufferSize));
             return m_buffer;
         }
+        ASSERT(m_handshake.mode() == WebSocketHandshake::Failed);
         LOG(Network, "WebSocketChannel %p connection failed", this);
         skipBuffer(headerLength);
         m_shouldDiscardReceivedData = true;
-        if (!m_closed)
-            m_handle->disconnect();
+        fail(m_handshake.failureReason());
         return false;
     }
     if (m_handshake.mode() != WebSocketHandshake::Connected)
index 7edecdbf46f12521007839cab605582833320677..74e436174603633e025306a604fcd0d3c4322a20 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009 Google Inc.  All rights reserved.
+ * Copyright (C) 2011 Google Inc.  All rights reserved.
  * Copyright (C) Research In Motion Limited 2011. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -319,7 +319,7 @@ int WebSocketHandshake::readServerHandshake(const char* header, size_t len)
     if (lineLength == -1)
         return -1;
     if (statusCode == -1) {
-        m_mode = Failed;
+        m_mode = Failed; // m_failureReason is set inside readStatusLine().
         return len;
     }
     LOG(Network, "response code: %d", statusCode);
@@ -327,7 +327,7 @@ int WebSocketHandshake::readServerHandshake(const char* header, size_t len)
     m_response.setStatusText(statusText);
     if (statusCode != 101) {
         m_mode = Failed;
-        m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Unexpected response code: " + String::number(statusCode), 0, clientOrigin(), 0);
+        m_failureReason = "Unexpected response code: " + String::number(statusCode);
         return len;
     }
     m_mode = Normal;
@@ -339,7 +339,7 @@ int WebSocketHandshake::readServerHandshake(const char* header, size_t len)
     const char* p = readHTTPHeaders(header + lineLength, header + len);
     if (!p) {
         LOG(Network, "readHTTPHeaders failed");
-        m_mode = Failed;
+        m_mode = Failed; // m_failureReason is set inside readHTTPHeaders().
         return len;
     }
     if (!checkResponseHeaders()) {
@@ -366,6 +366,11 @@ WebSocketHandshake::Mode WebSocketHandshake::mode() const
     return m_mode;
 }
 
+String WebSocketHandshake::failureReason() const
+{
+    return m_failureReason;
+}
+
 String WebSocketHandshake::serverWebSocketOrigin() const
 {
     return m_response.headerFields().get("sec-websocket-origin");
@@ -440,8 +445,8 @@ int WebSocketHandshake::readStatusLine(const char* header, size_t headerLength,
         } else if (*p == '\0') {
             // The caller isn't prepared to deal with null bytes in status
             // line. WebSockets specification doesn't prohibit this, but HTTP
-            // does, so we'll just treat this as an error. 
-            m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Status line contains embedded null", 0, clientOrigin(), 0);
+            // does, so we'll just treat this as an error.
+            m_failureReason = "Status line contains embedded null";
             return p + 1 - header;
         } else if (*p == '\n')
             break;
@@ -452,18 +457,18 @@ int WebSocketHandshake::readStatusLine(const char* header, size_t headerLength,
     const char* end = p + 1;
     int lineLength = end - header;
     if (lineLength > maximumLength) {
-        m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Status line is too long", 0, clientOrigin(), 0);
+        m_failureReason = "Status line is too long";
         return maximumLength;
     }
 
     // The line must end with "\r\n".
     if (lineLength < 2 || *(end - 2) != '\r') {
-        m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Status line does not end with CRLF", 0, clientOrigin(), 0);
+        m_failureReason = "Status line does not end with CRLF";
         return lineLength;
     }
 
     if (!space1 || !space2) {
-        m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "No response code found: " + trimConsoleMessage(header, lineLength - 2), 0, clientOrigin(), 0);
+        m_failureReason = "No response code found: " + trimConsoleMessage(header, lineLength - 2);
         return lineLength;
     }
 
@@ -472,7 +477,7 @@ int WebSocketHandshake::readStatusLine(const char* header, size_t headerLength,
         return lineLength;
     for (int i = 0; i < 3; ++i)
         if (statusCodeString[i] < '0' || statusCodeString[i] > '9') {
-            m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Invalid status code: " + statusCodeString, 0, clientOrigin(), 0);
+            m_failureReason = "Invalid status code: " + statusCodeString;
             return lineLength;
         }
 
@@ -500,13 +505,13 @@ const char* WebSocketHandshake::readHTTPHeaders(const char* start, const char* e
                 if (name.isEmpty()) {
                     if (p + 1 < end && *(p + 1) == '\n')
                         return p + 2;
-                    m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "CR doesn't follow LF at " + trimConsoleMessage(p, end - p), 0, clientOrigin(), 0);
+                    m_failureReason = "CR doesn't follow LF at " + trimConsoleMessage(p, end - p);
                     return 0;
                 }
-                m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Unexpected CR in name at " + trimConsoleMessage(name.data(), name.size()), 0, clientOrigin(), 0);
+                m_failureReason = "Unexpected CR in name at " + trimConsoleMessage(name.data(), name.size());
                 return 0;
             case '\n':
-                m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Unexpected LF in name at " + trimConsoleMessage(name.data(), name.size()), 0, clientOrigin(), 0);
+                m_failureReason = "Unexpected LF in name at " + trimConsoleMessage(name.data(), name.size());
                 return 0;
             case ':':
                 break;
@@ -527,7 +532,7 @@ const char* WebSocketHandshake::readHTTPHeaders(const char* start, const char* e
             case '\r':
                 break;
             case '\n':
-                m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Unexpected LF in value at " + trimConsoleMessage(value.data(), value.size()), 0, clientOrigin(), 0);
+                m_failureReason = "Unexpected LF in value at " + trimConsoleMessage(value.data(), value.size());
                 return 0;
             default:
                 value.append(*p);
@@ -538,17 +543,17 @@ const char* WebSocketHandshake::readHTTPHeaders(const char* start, const char* e
             }
         }
         if (p >= end || *p != '\n') {
-            m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "CR doesn't follow LF after value at " + trimConsoleMessage(p, end - p), 0, clientOrigin(), 0);
+            m_failureReason = "CR doesn't follow LF after value at " + trimConsoleMessage(p, end - p);
             return 0;
         }
         AtomicString nameStr = AtomicString::fromUTF8(name.data(), name.size());
         String valueStr = String::fromUTF8(value.data(), value.size());
         if (nameStr.isNull()) {
-            m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "invalid UTF-8 sequence in header name", 0, clientOrigin(), 0);
+            m_failureReason = "Invalid UTF-8 sequence in header name";
             return 0;
         }
         if (valueStr.isNull()) {
-            m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "invalid UTF-8 sequence in header value", 0, clientOrigin(), 0);
+            m_failureReason = "Invalid UTF-8 sequence in header value";
             return 0;
         }
         LOG(Network, "name=%s value=%s", nameStr.string().utf8().data(), valueStr.utf8().data());
@@ -567,41 +572,41 @@ bool WebSocketHandshake::checkResponseHeaders()
     const String& serverConnection = this->serverConnection();
 
     if (serverUpgrade.isNull()) {
-        m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Error during WebSocket handshake: 'Upgrade' header is missing", 0, clientOrigin(), 0);
+        m_failureReason = "Error during WebSocket handshake: 'Upgrade' header is missing";
         return false;
     }
     if (serverConnection.isNull()) {
-        m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Error during WebSocket handshake: 'Connection' header is missing", 0, clientOrigin(), 0);
+        m_failureReason = "Error during WebSocket handshake: 'Connection' header is missing";
         return false;
     }
     if (serverWebSocketOrigin.isNull()) {
-        m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Error during WebSocket handshake: 'Sec-WebSocket-Origin' header is missing", 0, clientOrigin(), 0);
+        m_failureReason = "Error during WebSocket handshake: 'Sec-WebSocket-Origin' header is missing";
         return false;
     }
     if (serverWebSocketLocation.isNull()) {
-        m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Error during WebSocket handshake: 'Sec-WebSocket-Location' header is missing", 0, clientOrigin(), 0);
+        m_failureReason = "Error during WebSocket handshake: 'Sec-WebSocket-Location' header is missing";
         return false;
     }
 
     if (!equalIgnoringCase(serverUpgrade, "websocket")) {
-        m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Error during WebSocket handshake: 'Upgrade' header value is not 'WebSocket'", 0, clientOrigin(), 0);
+        m_failureReason = "Error during WebSocket handshake: 'Upgrade' header value is not 'WebSocket'";
         return false;
     }
     if (!equalIgnoringCase(serverConnection, "upgrade")) {
-        m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Error during WebSocket handshake: 'Connection' header value is not 'Upgrade'", 0, clientOrigin(), 0);
+        m_failureReason = "Error during WebSocket handshake: 'Connection' header value is not 'Upgrade'";
         return false;
     }
 
     if (clientOrigin() != serverWebSocketOrigin) {
-        m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Error during WebSocket handshake: origin mismatch: " + clientOrigin() + " != " + serverWebSocketOrigin, 0, clientOrigin(), 0);
+        m_failureReason = "Error during WebSocket handshake: origin mismatch: " + clientOrigin() + " != " + serverWebSocketOrigin;
         return false;
     }
     if (clientLocation() != serverWebSocketLocation) {
-        m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Error during WebSocket handshake: location mismatch: " + clientLocation() + " != " + serverWebSocketLocation, 0, clientOrigin(), 0);
+        m_failureReason = "Error during WebSocket handshake: location mismatch: " + clientLocation() + " != " + serverWebSocketLocation;
         return false;
     }
     if (!m_clientProtocol.isEmpty() && m_clientProtocol != serverWebSocketProtocol) {
-        m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Error during WebSocket handshake: protocol mismatch: " + m_clientProtocol + " != " + serverWebSocketProtocol, 0, clientOrigin(), 0);
+        m_failureReason = "Error during WebSocket handshake: protocol mismatch: " + m_clientProtocol + " != " + serverWebSocketProtocol;
         return false;
     }
     return true;
index 90916a7df0a78a1dee2ff7cfeb04b7f3c8249b94..5556eef51471274bd4185993cd86f6330bbe2656 100644 (file)
@@ -71,6 +71,7 @@ namespace WebCore {
 
         int readServerHandshake(const char* header, size_t len);
         Mode mode() const;
+        String failureReason() const; // Returns a string indicating the reason of failure if mode() == Failed.
 
         String serverWebSocketOrigin() const;
         String serverWebSocketLocation() const;
@@ -105,6 +106,8 @@ namespace WebCore {
         unsigned char m_expectedChallengeResponse[16];
 
         WebSocketHandshakeResponse m_response;
+
+        String m_failureReason;
     };
 
 } // namespace WebCore