[Webkit-unassigned] [Bug 121123] Web Inspector: Do not try to parse incomplete HTTP requests

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=121123





--- Comment #6 from Carlos Garcia Campos <cgarcia at igalia.com>  2013-09-13 00:59:23 PST ---
(From update of attachment 211453)
View in context: https://bugs.webkit.org/attachment.cgi?id=211453&action=review

I'm late again, sorry, I have some comments, though, please consider fixing them in a follow up commit.

> Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp:248
> +    GSocketClient* client = g_socket_client_new();

You are leaking this, please use GRefPtr

> Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp:249
> +    GSocketConnection* connection = g_socket_client_connect_to_host(client, "127.0.0.1", 2999, NULL, &error.outPtr());

Use 0 instead of NULL for the cancellable. You are leaking the connection too, please use GRefPtr.

> Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp:250
> +    g_assert(!error.get());

Use g_assert_no_error instead, that way if there's an error you will see a message with the error description.

> Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp:258
> +    g_output_stream_write(ostream, incompleteRequest, strlen(incompleteRequest), NULL, &error.outPtr());

Use 0 instead of NULL.

> Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp:259
> +    g_assert(!error.get());

Use g_assert_no_error instead, that way if there's an error you will see a message with the error description.

> Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp:264
> +    g_input_stream_read_async(istream, response, sizeof(response) - 1, G_PRIORITY_DEFAULT, NULL, NULL, NULL);

Use 0 instead of NULL.

> Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp:268
> +    // 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());

I don't think this is the proper way to test this. You should use a GAsyncReadyCallback and spin a run loop to get the bytes read with g_input_stream_read_finish(). You can also start a timeout idle to assert when reached. With this code we are always waiting for 2 seconds, making running the tests slower even with it runs correctly. Also you don't need to initialize the buffer, and then convert it to a String to check if it's empty, just check the bytes actually read.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.