Modified: trunk/Source/WebKit2/ChangeLog (155641 => 155642)
--- trunk/Source/WebKit2/ChangeLog 2013-09-12 19:52:04 UTC (rev 155641)
+++ trunk/Source/WebKit2/ChangeLog 2013-09-12 20:00:17 UTC (rev 155642)
@@ -1,3 +1,34 @@
+2013-09-12 Andre Moreira Magalhaes <andre.magalh...@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 <ander...@apple.com>
SharedBuffer::createNSData should return a RetainPtr<NSData>
Modified: trunk/Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp (155641 => 155642)
--- trunk/Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp 2013-09-12 19:52:04 UTC (rev 155641)
+++ trunk/Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp 2013-09-12 20:00:17 UTC (rev 155642)
@@ -240,7 +240,34 @@
g_assert_cmpstr(webkit_web_view_get_title(test->m_webView), ==, "Web Inspector - http://127.0.0.1:2999/");
}
+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()
{
// Overwrite WEBKIT_INSPECTOR_SERVER variable with default IP address but different port to avoid conflict with the test inspector server page.
@@ -250,6 +277,7 @@
InspectorServerTest::add("WebKitWebInspectorServer", "test-page-list", testInspectorServerPageList);
InspectorServerTest::add("WebKitWebInspectorServer", "test-remote-debugging-message", testRemoteDebuggingMessage);
InspectorServerTest::add("WebKitWebInspectorServer", "test-open-debugging-session", openRemoteDebuggingSession);
+ InspectorServerTest::add("WebKitWebInspectorServer", "test-incomplete-request", sendIncompleteRequest);
}
Modified: trunk/Source/WebKit2/UIProcess/InspectorServer/HTTPRequest.cpp (155641 => 155642)
--- trunk/Source/WebKit2/UIProcess/InspectorServer/HTTPRequest.cpp 2013-09-12 19:52:04 UTC (rev 155641)
+++ trunk/Source/WebKit2/UIProcess/InspectorServer/HTTPRequest.cpp 2013-09-12 20:00:17 UTC (rev 155642)
@@ -93,6 +93,13 @@
break;
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;
}
