Changeset 155642 in webkit

Timestamp:

Sep 12, 2013 1:00:17 PM (11 years ago)

Author:

commit-queue@webkit.org

Message:

Web Inspector: Do not try to parse incomplete HTTP requests
​https://bugs.webkit.org/show_bug.cgi?id=121123

Patch by Andre Moreira Magalhaes <Andre Moreira Magalhaes> on 2013-09-12
Reviewed by Darin Adler.

When working on a patch for bug #121121 I found an issue with the InspectorServer where it would try
to parse an HTTP message before receiving the full message and thus fail connecting with the
chromedevtools plugin.

What happens is that the WebSocketServerConnection receives buffers on
WebSocketServerConnection::didReceiveSocketStreamData and calls
WebSocketServerConnection::readHTTPMessage which then checks if we have a valid request by calling
HTTPRequest::parseHTTPRequestFromBuffer. If the request is valid it tries to parse the message and
clears the buffer, otherwise it continues adding data to the internal buffer until we have a valid
request.

The problem is that currently HTTPRequest::parseHTTPRequestFromBuffer considers the request as valid
before receiving the full message. To solve this we should make the method check if the request
headers end with a blank line otherwise we consider the request as invalid (see also
​http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html).

• UIProcess/API/gtk/tests/TestInspectorServer.cpp:

(sendIncompleteRequest):
(beforeAll):
Add GTK specific test to check if the inspector server replies to incomplete requests.

• UIProcess/InspectorServer/HTTPRequest.cpp:

(WebKit::HTTPRequest::parseHeaders):
Do not consider request valid if headers didn't end with a blank line.

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/API/gtk/tests/TestInspectorServer.cpp (modified) (2 diffs)
• UIProcess/InspectorServer/HTTPRequest.cpp (modified) (1 diff)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2013-09-12  Andre Moreira Magalhaes   <andre.magalhaes@collabora.co.uk>
+
+        Web Inspector: Do not try to parse incomplete HTTP requests
+        https://bugs.webkit.org/show_bug.cgi?id=121123
+
+        Reviewed by Darin Adler.
+
+        When working on a patch for bug #121121 I found an issue with the InspectorServer where it would try
+        to parse an HTTP message before receiving the full message and thus fail connecting with the
+        chromedevtools plugin.
+
+        What happens is that the WebSocketServerConnection receives buffers on
+        WebSocketServerConnection::didReceiveSocketStreamData and calls
+        WebSocketServerConnection::readHTTPMessage which then checks if we have a valid request by calling
+        HTTPRequest::parseHTTPRequestFromBuffer. If the request is valid it tries to parse the message and
+        clears the buffer, otherwise it continues adding data to the internal buffer until we have a valid
+        request.
+
+        The problem is that currently HTTPRequest::parseHTTPRequestFromBuffer considers the request as valid
+        before receiving the full message. To solve this we should make the method check if the request
+        headers end with a blank line otherwise we consider the request as invalid (see also
+        http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html).
+
+        * UIProcess/API/gtk/tests/TestInspectorServer.cpp:
+        (sendIncompleteRequest):
+        (beforeAll):
+        Add GTK specific test to check if the inspector server replies to incomplete requests.
+        * UIProcess/InspectorServer/HTTPRequest.cpp:
+        (WebKit::HTTPRequest::parseHeaders):
+        Do not consider request valid if headers didn't end with a blank line.
+
 2013-09-12  Anders Carlsson  <andersca@apple.com>
 

trunk/Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp

@@ -241 +241 @@
 }
 
+static void sendIncompleteRequest(InspectorServerTest* test, gconstpointer)
+{
+    GOwnPtr<GError> error;
+
+    // Connect to the inspector server.
+    GSocketClient* client = g_socket_client_new();
+    GSocketConnection* connection = g_socket_client_connect_to_host(client, "127.0.0.1", 2999, NULL, &error.outPtr());
+    g_assert(!error.get());
+
+    // Send incomplete request (missing blank line after headers) and check if inspector server
+    // replies. The server should not reply to an incomplete request and the test should timeout
+    // on read.
+    GOutputStream* ostream = g_io_stream_get_output_stream(G_IO_STREAM(connection));
+    // Request missing blank line after headers.
+    const gchar* incompleteRequest = "GET /devtools/page/1 HTTP/1.1\r\nHost: Localhost\r\n";
+    g_output_stream_write(ostream, incompleteRequest, strlen(incompleteRequest), NULL, &error.outPtr());
+    g_assert(!error.get());
+
+    GInputStream* istream = g_io_stream_get_input_stream(G_IO_STREAM(connection));
+    char response[16];
+    memset(response, 0, sizeof(response));
+    g_input_stream_read_async(istream, response, sizeof(response) - 1, G_PRIORITY_DEFAULT, NULL, NULL, NULL);
+    // Give a chance for the server to reply.
+    test->wait(2);
+    // If we got any answer it means the server replied to an incomplete request, lets fail.
+    g_assert(String(response).isEmpty());
+}
 
 void beforeAll()
@@ -251 +278 @@
     InspectorServerTest::add("WebKitWebInspectorServer", "test-remote-debugging-message", testRemoteDebuggingMessage);
     InspectorServerTest::add("WebKitWebInspectorServer", "test-open-debugging-session", openRemoteDebuggingSession);
+    InspectorServerTest::add("WebKitWebInspectorServer", "test-incomplete-request", sendIncompleteRequest);
 
 }

trunk/Source/WebKit2/UIProcess/InspectorServer/HTTPRequest.cpp

@@ -94 +94 @@
         m_headerFields.add(name, value);
     }
+
+    // If we got here and "name" is empty, it means the headers are valid and ended with a
+    // blank line (parseHTTPHeader returns "name" as empty if parsing a blank line), otherwise
+    // the headers didn't end with a blank line and we have an invalid request.
+    // See also http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html
+    if (!name.isEmpty())
+        return 0;
     return p - data;
 }