2011-05-25 Yuta Kitamura <yutak@chromium.org>
authoryutak@chromium.org <yutak@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 May 2011 09:52:36 +0000 (09:52 +0000)
committeryutak@chromium.org <yutak@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 May 2011 09:52:36 +0000 (09:52 +0000)
        Reviewed by Kent Tamura.

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

        * http/tests/websocket/tests/frame-length-overflow-expected.txt:
        Added a new console message.
2011-05-25  Yuta Kitamura  <yutak@chromium.org>

        Reviewed by Kent Tamura.

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

        An existing error message has been modified, but it is impossible
        to test this message in LayoutTests because it is only shown when
        memory allocation has failed, which is hard to reproduce reliably.

        One new message has been added. It is covered by an existing test
        http/tests/websocket/tests/frame-length-overflow.html.

        There is no other change in behavior. No new tests are added.

        * websockets/WebSocketChannel.cpp:
        (WebCore::WebSocketChannel::fail):
        Do not close if we know the socket stream is already closed. This does not
        change the behavior, because SocketStreamBase does nothing if it is already
        closed.
        (WebCore::WebSocketChannel::didOpen):
        (WebCore::WebSocketChannel::didReceiveData):
        We need to set m_shouldDiscardReceivedData to true before calling fail(),
        so I moved the error message from appendToBuffer() to here.
        The error message was rephrased in order to improve readability.
        (WebCore::WebSocketChannel::appendToBuffer):
        Unnested the code.
        (WebCore::WebSocketChannel::processBuffer):

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

LayoutTests/ChangeLog
LayoutTests/http/tests/websocket/tests/frame-length-overflow-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/websockets/WebSocketChannel.cpp

index 95adc7276f86f783e09a9e312dcfa8fae021fba9..b8d1ed416b65e9bd5a2eaec39e8d5859e52c432d 100644 (file)
@@ -1,3 +1,13 @@
+2011-05-25  Yuta Kitamura  <yutak@chromium.org>
+
+        Reviewed by Kent Tamura.
+
+        WebSocket: Use fail() when WebSocketChannel has failed
+        https://bugs.webkit.org/show_bug.cgi?id=61353
+
+        * http/tests/websocket/tests/frame-length-overflow-expected.txt:
+        Added a new console message.
+
 2011-05-24  Pavel Podivilov  <podivilov@chromium.org>
 
         Reviewed by Yury Semikhatsky.
index 1a15207c4afb95e641b8821f90d2dd066d2081f3..dc216f4e8246049d170aa666178c8be9a1acb398 100644 (file)
@@ -1,3 +1,4 @@
+CONSOLE MESSAGE: line 0: WebSocket frame length too large
 Make sure WebSocket does not crash and report error when it sees length overflow
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
index bc768ae614d230f130d8d8eb3ce29d22580bc299..70ef6b2ba96655c7637d52b2c41ebe0cc0d05f18 100644 (file)
@@ -1,3 +1,33 @@
+2011-05-25  Yuta Kitamura  <yutak@chromium.org>
+
+        Reviewed by Kent Tamura.
+
+        WebSocket: Use fail() when WebSocketChannel has failed
+        https://bugs.webkit.org/show_bug.cgi?id=61353
+
+        An existing error message has been modified, but it is impossible
+        to test this message in LayoutTests because it is only shown when
+        memory allocation has failed, which is hard to reproduce reliably.
+
+        One new message has been added. It is covered by an existing test
+        http/tests/websocket/tests/frame-length-overflow.html.
+
+        There is no other change in behavior. No new tests are added.
+
+        * websockets/WebSocketChannel.cpp:
+        (WebCore::WebSocketChannel::fail):
+        Do not close if we know the socket stream is already closed. This does not
+        change the behavior, because SocketStreamBase does nothing if it is already
+        closed.
+        (WebCore::WebSocketChannel::didOpen):
+        (WebCore::WebSocketChannel::didReceiveData):
+        We need to set m_shouldDiscardReceivedData to true before calling fail(),
+        so I moved the error message from appendToBuffer() to here.
+        The error message was rephrased in order to improve readability.
+        (WebCore::WebSocketChannel::appendToBuffer):
+        Unnested the code.
+        (WebCore::WebSocketChannel::processBuffer):
+
 2011-05-16  Alexander Pavlov  <apavlov@chromium.org>
 
         Reviewed by David Levin.
index a31c155e764515766352d7db5f9d2263051ab657..e3164f15e43ba083d03cfd977f3f0c9b2c0f09c1 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009 Google Inc.  All rights reserved.
+ * Copyright (C) 2011 Google Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -127,7 +127,7 @@ void WebSocketChannel::fail(const String& reason)
     ASSERT(!m_suspended);
     if (m_context)
         m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, reason, 0, m_handshake.clientOrigin(), 0);
-    if (m_handle)
+    if (m_handle && !m_closed)
         m_handle->close(); // Will call didClose().
 }
 
@@ -164,10 +164,8 @@ void WebSocketChannel::didOpen(SocketStreamHandle* handle)
     if (m_identifier)
         InspectorInstrumentation::willSendWebSocketHandshakeRequest(m_context, m_identifier, m_handshake.clientHandshakeRequest());
     CString handshakeMessage = m_handshake.clientHandshakeMessage();
-    if (!handle->send(handshakeMessage.data(), handshakeMessage.length())) {
-        m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "Error sending handshake message.", 0, m_handshake.clientOrigin(), 0);
-        handle->close();
-    }
+    if (!handle->send(handshakeMessage.data(), handshakeMessage.length()))
+        fail("Failed to send WebSocket handshake.");
 }
 
 void WebSocketChannel::didClose(SocketStreamHandle* handle)
@@ -208,7 +206,7 @@ void WebSocketChannel::didReceiveData(SocketStreamHandle* handle, const char* da
         return;
     if (!appendToBuffer(data, len)) {
         m_shouldDiscardReceivedData = true;
-        handle->close();
+        fail("Ran out of memory while receiving WebSocket data.");
         return;
     }
     while (!m_suspended && m_client && m_buffer)
@@ -254,17 +252,16 @@ bool WebSocketChannel::appendToBuffer(const char* data, size_t len)
         return false;
     }
     char* newBuffer = 0;
-    if (tryFastMalloc(newBufferSize).getValue(newBuffer)) {
-        if (m_buffer)
-            memcpy(newBuffer, m_buffer, m_bufferSize);
-        memcpy(newBuffer + m_bufferSize, data, len);
-        fastFree(m_buffer);
-        m_buffer = newBuffer;
-        m_bufferSize = newBufferSize;
-        return true;
-    }
-    m_context->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, "WebSocket frame (at " + String::number(static_cast<unsigned long>(newBufferSize)) + " bytes) is too long.", 0, m_handshake.clientOrigin(), 0);
-    return false;
+    if (!tryFastMalloc(newBufferSize).getValue(newBuffer))
+        return false;
+
+    if (m_buffer)
+        memcpy(newBuffer, m_buffer, m_bufferSize);
+    memcpy(newBuffer + m_bufferSize, data, len);
+    fastFree(m_buffer);
+    m_buffer = newBuffer;
+    m_bufferSize = newBufferSize;
+    return true;
 }
 
 void WebSocketChannel::skipBuffer(size_t len)
@@ -361,10 +358,7 @@ bool WebSocketChannel::processBuffer()
             skipBuffer(m_bufferSize); // Save memory.
             m_shouldDiscardReceivedData = true;
             m_client->didReceiveMessageError();
-            if (!m_client)
-                return false;
-            if (!m_closed)
-                m_handle->close();
+            fail("WebSocket frame length too large");
             return false;
         }
         ASSERT(p + length >= p);