Changeset 87282 in webkit

Timestamp:

May 25, 2011 2:52:36 AM (13 years ago)

Author:

yutak@chromium.org

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

• 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):

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/websocket/tests/frame-length-overflow-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/websockets/WebSocketChannel.cpp (modified) (6 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/http/tests/websocket/tests/frame-length-overflow-expected.txt

@@ +1 @@
+CONSOLE MESSAGE: line 0: WebSocket frame length too large
 Make sure WebSocket does not crash and report error when it sees length overflow
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/websockets/WebSocketChannel.cpp

@@ -1 +1 @@
 /*
- * 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
@@ -128 +128 @@
     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().
 }
@@ -165 +165 @@
         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.");
 }
 
@@ -209 +207 @@
     if (!appendToBuffer(data, len)) {
         m_shouldDiscardReceivedData = true;
-        handle->close();
+        fail("Ran out of memory while receiving WebSocket data.");
         return;
     }
@@ -255 +253 @@
     }
     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;
 }
 
@@ -362 +359 @@
             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;
         }