wininet: Don't assume that end of chunk means end of stream. (try 2)

Jacek Caban jacek at codeweavers.com
Wed Sep 25 08:41:20 CDT 2013


Hi Hans,

Sorry for the delay, I wanted to test it a bit more before answering.

On 09/23/13 10:50, Hans Leidekker wrote:
> On Thu, 2013-09-19 at 17:38 +0200, Jacek Caban wrote:
>> I was hoping for a test like the attached one, which shows that there is
>> still more work to do. But it's a step in the right direction, so I'm
>> fine with your patch.
> The issue here is that HTTP_ReceiveRequestData calls refill_read_buffer, which
> calls generic read_http_stream with a read size of 8192 (the read buffer
> is still empty). In non-chunked mode this should read the minimum of that size
> and the content length. In chunked mode we don't know the content length,
> and we should read the minimum of chunk size and read buffer size, as shown by
> your test.
> Maybe we should add a refill_read_buffer method to the backends and call that
> instead of read_http_stream?

We should be able to do that with read semantics that is intended for
READMODE_ASYNC: read anything, not more than buf size and, once any data
is read, don't block. The attached extended tests show the problem with
chunked reads in this case. We'd need to maintain full state of the
stream to be able to do non-blocking chunk headers/tails reads.

Jacek
-------------- next part --------------
diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c
index 12eab57..889707c 100644
--- a/dlls/wininet/tests/http.c
+++ b/dlls/wininet/tests/http.c
@@ -103,7 +103,7 @@ static int expect[MAX_INTERNET_STATUS], optional[MAX_INTERNET_STATUS],
     wine_allow[MAX_INTERNET_STATUS], notified[MAX_INTERNET_STATUS];
 static const char *status_string[MAX_INTERNET_STATUS];
 
-static HANDLE hCompleteEvent, conn_close_event;
+static HANDLE hCompleteEvent, conn_close_event, server_event;
 static DWORD req_error;
 
 #define TESTF_REDIRECT      0x01
@@ -2114,6 +2114,36 @@ static DWORD CALLBACK server_thread(LPVOID param)
         }
         if (strstr(buffer, "GET /test_premature_disconnect"))
             trace("closing connection\n");
+        if (strstr(buffer, "GET /test_chunked")) {
+            static const char chunked_header[] = "HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\n\r\n";
+            static const char chunk1[] = "20\r\nxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\r\n";
+            static const char chunk2[] = "18\r\nxxxxxxxxxxxxxxxxxxxxxxxx\r\n";
+            static const char chunk3[] = "28\r\nxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\r\n";
+            static const char last_chunk[] = "0\r\n\r\n";
+
+            send(c, chunked_header, sizeof(chunked_header)-1, 0);
+            send(c, chunk1, sizeof(chunk1)-1, 0);
+            WaitForSingleObject(server_event, INFINITE);
+
+            send(c, chunk2, sizeof(chunk2)-1, 0);
+            WaitForSingleObject(server_event, INFINITE);
+
+            send(c, chunk3, sizeof(chunk3)-1, 0);
+            WaitForSingleObject(server_event, INFINITE);
+
+            send(c, chunk1, sizeof(chunk1)-1, 0);
+            send(c, chunk2, sizeof(chunk2)-1, 0);
+            WaitForSingleObject(server_event, INFINITE);
+
+            send(c, chunk1, sizeof(chunk1)-1, 0);
+            send(c, chunk2, 3, 0);
+            WaitForSingleObject(server_event, INFINITE);
+
+            send(c, chunk2+3, sizeof(chunk2)-4, 0);
+            WaitForSingleObject(server_event, INFINITE);
+
+            send(c, last_chunk, sizeof(last_chunk)-1, 0);
+        }
 
         shutdown(c, 2);
         closesocket(c);
@@ -2917,6 +2947,181 @@ static void test_no_content(int port)
     CloseHandle(hCompleteEvent);
 }
 
+static void test_chunked_stream(int port)
+{
+    HINTERNET session, connection, req;
+    DWORD res, avail, size;
+    BYTE buf[256];
+
+    trace("Testing chunked stream...\n");
+
+    hCompleteEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
+    server_event = CreateEvent(NULL, FALSE, FALSE, NULL);
+
+    session = InternetOpenA("", INTERNET_OPEN_TYPE_PRECONFIG, NULL, NULL, INTERNET_FLAG_ASYNC);
+    ok(session != NULL,"InternetOpen failed with error %u\n", GetLastError());
+
+    pInternetSetStatusCallbackA(session, callback);
+
+    SET_EXPECT(INTERNET_STATUS_HANDLE_CREATED);
+    connection = InternetConnectA(session, "localhost", port,
+            NULL, NULL, INTERNET_SERVICE_HTTP, 0x0, 0xdeadbeef);
+    ok(connection != NULL,"InternetConnect failed with error %u\n", GetLastError());
+    CHECK_NOTIFIED(INTERNET_STATUS_HANDLE_CREATED);
+
+    SET_EXPECT(INTERNET_STATUS_HANDLE_CREATED);
+    req = HttpOpenRequestA(connection, "GET", "/test_chunked", NULL, NULL, NULL,
+            INTERNET_FLAG_KEEP_CONNECTION | INTERNET_FLAG_RESYNCHRONIZE, 0xdeadbead);
+    ok(req != NULL, "HttpOpenRequest failed: %u\n", GetLastError());
+    CHECK_NOTIFIED(INTERNET_STATUS_HANDLE_CREATED);
+
+    trace("sending request...\n");
+
+    SET_OPTIONAL(INTERNET_STATUS_COOKIE_SENT);
+    SET_EXPECT(INTERNET_STATUS_CONNECTING_TO_SERVER);
+    SET_EXPECT(INTERNET_STATUS_CONNECTED_TO_SERVER);
+    SET_EXPECT(INTERNET_STATUS_SENDING_REQUEST);
+    SET_EXPECT(INTERNET_STATUS_REQUEST_SENT);
+    SET_EXPECT(INTERNET_STATUS_RECEIVING_RESPONSE);
+    SET_EXPECT(INTERNET_STATUS_RESPONSE_RECEIVED);
+    SET_EXPECT(INTERNET_STATUS_REQUEST_COMPLETE);
+
+    res = HttpSendRequestA(req, NULL, -1, NULL, 0);
+    ok(!res && (GetLastError() == ERROR_IO_PENDING),
+       "Asynchronous HttpSendRequest NOT returning 0 with error ERROR_IO_PENDING\n");
+    WaitForSingleObject(hCompleteEvent, INFINITE);
+    ok(req_error == ERROR_SUCCESS, "req_error = %u\n", req_error);
+
+    CLEAR_NOTIFIED(INTERNET_STATUS_COOKIE_SENT);
+    CHECK_NOTIFIED(INTERNET_STATUS_CONNECTING_TO_SERVER);
+    CHECK_NOTIFIED(INTERNET_STATUS_CONNECTED_TO_SERVER);
+    CHECK_NOTIFIED(INTERNET_STATUS_SENDING_REQUEST);
+    CHECK_NOTIFIED(INTERNET_STATUS_REQUEST_SENT);
+    CHECK_NOTIFIED(INTERNET_STATUS_RECEIVING_RESPONSE);
+    CHECK_NOTIFIED(INTERNET_STATUS_RESPONSE_RECEIVED);
+    CHECK_NOTIFIED(INTERNET_STATUS_REQUEST_COMPLETE);
+
+    avail = 0;
+    res = InternetQueryDataAvailable(req, &avail, 0, 0);
+    ok(res, "InternetQueryDataAvailable failed: %u\n", GetLastError());
+    ok(avail == 32, "avail = %d\n", avail);
+
+    size = 0;
+    res = InternetReadFile(req, buf, avail, &size);
+    ok(res, "InternetReadFile failed: %u\n", GetLastError());
+    ok(size == 32, "size = %d\n", size);
+
+    trace("waiting for second chunk (24 bytes)...\n");
+
+    avail = 0;
+    res = InternetQueryDataAvailable(req, &avail, 0, 0);
+    ok(!res && GetLastError() == ERROR_IO_PENDING, "InternetQueryDataAvailable returned: %x(%u)\n", res, GetLastError());
+    ok(!avail, "avail = %d\n", avail);
+    SET_EXPECT(INTERNET_STATUS_REQUEST_COMPLETE);
+    SetEvent(server_event);
+    WaitForSingleObject(hCompleteEvent, INFINITE);
+    CHECK_NOTIFIED(INTERNET_STATUS_REQUEST_COMPLETE);
+    ok(avail == 24, "avail = %d\n", avail);
+
+    avail = 0;
+    res = InternetQueryDataAvailable(req, &avail, 0, 0);
+    ok(res, "InternetQueryDataAvailable failed: %u\n", GetLastError());
+    ok(avail == 24, "avail = %d\n", avail);
+
+    size = 0;
+    res = InternetReadFile(req, buf, avail, &size);
+    ok(res, "InternetReadFile failed: %u\n", GetLastError());
+    ok(size == 24, "size = %d\n", size);
+
+    trace("waiting for third chunk (40 bytes)...\n");
+
+    avail = 0;
+    res = InternetQueryDataAvailable(req, &avail, 0, 0);
+    ok(!res && GetLastError() == ERROR_IO_PENDING, "InternetQueryDataAvailable returned: %x(%u)\n", res, GetLastError());
+    ok(!avail, "avail = %d\n", avail);
+    SET_EXPECT(INTERNET_STATUS_REQUEST_COMPLETE);
+    SetEvent(server_event);
+    WaitForSingleObject(hCompleteEvent, INFINITE);
+    CHECK_NOTIFIED(INTERNET_STATUS_REQUEST_COMPLETE);
+    ok(avail == 40, "avail = %d\n", avail);
+
+    size = 0;
+    res = InternetReadFile(req, buf, avail, &size);
+    ok(res, "InternetReadFile failed: %u\n", GetLastError());
+    ok(size == 40, "size = %d\n", size);
+
+    trace("waiting for chunk 1 and 2 (32 and 24 bytes)...\n");
+
+    avail = 0;
+    res = InternetQueryDataAvailable(req, &avail, 0, 0);
+    ok(!res && GetLastError() == ERROR_IO_PENDING, "InternetQueryDataAvailable returned: %x(%u)\n", res, GetLastError());
+    ok(!avail, "avail = %d\n", avail);
+    SET_EXPECT(INTERNET_STATUS_REQUEST_COMPLETE);
+    SetEvent(server_event);
+    WaitForSingleObject(hCompleteEvent, INFINITE);
+    CHECK_NOTIFIED(INTERNET_STATUS_REQUEST_COMPLETE);
+    ok(avail == 56, "avail = %d\n", avail);
+
+    size = 0;
+    res = InternetReadFile(req, buf, avail, &size);
+    ok(res, "InternetReadFile failed: %u\n", GetLastError());
+    ok(size == 56, "size = %d\n", size);
+
+    trace("waiting for chunk 1 and chunk 2 header...\n");
+
+    avail = 0;
+    res = InternetQueryDataAvailable(req, &avail, 0, 0);
+    ok(!res && GetLastError() == ERROR_IO_PENDING, "InternetQueryDataAvailable returned: %x(%u)\n", res, GetLastError());
+    ok(!avail, "avail = %d\n", avail);
+    SET_EXPECT(INTERNET_STATUS_REQUEST_COMPLETE);
+    SetEvent(server_event);
+    WaitForSingleObject(hCompleteEvent, INFINITE);
+    CHECK_NOTIFIED(INTERNET_STATUS_REQUEST_COMPLETE);
+    ok(avail == 32, "avail = %d\n", avail);
+
+    size = 0;
+    res = InternetReadFile(req, buf, avail, &size);
+    ok(res, "InternetReadFile failed: %u\n", GetLastError());
+    ok(size == 32, "size = %d\n", size);
+
+    trace("waiting for the rest of chunk 2 header...\n");
+
+    avail = 0;
+    res = InternetQueryDataAvailable(req, &avail, 0, 0);
+    ok(!res && GetLastError() == ERROR_IO_PENDING, "InternetQueryDataAvailable returned: %x(%u)\n", res, GetLastError());
+    ok(!avail, "avail = %d\n", avail);
+    SET_EXPECT(INTERNET_STATUS_REQUEST_COMPLETE);
+    SetEvent(server_event);
+    WaitForSingleObject(hCompleteEvent, INFINITE);
+    CHECK_NOTIFIED(INTERNET_STATUS_REQUEST_COMPLETE);
+    ok(avail == 24, "avail = %d\n", avail);
+
+    size = 0;
+    res = InternetReadFile(req, buf, avail, &size);
+    ok(res, "InternetReadFile failed: %u\n", GetLastError());
+    ok(size == 24, "size = %d\n", size);
+
+    trace("waiting for the last chunk (0 bytes)...\n");
+
+    avail = 0;
+    res = InternetQueryDataAvailable(req, &avail, 0, 0);
+    ok(!res && GetLastError() == ERROR_IO_PENDING, "InternetQueryDataAvailable returned: %x(%u)\n", res, GetLastError());
+    ok(!avail, "avail = %d\n", avail);
+    SET_EXPECT(INTERNET_STATUS_REQUEST_COMPLETE);
+    SetEvent(server_event);
+    WaitForSingleObject(hCompleteEvent, INFINITE);
+    CHECK_NOTIFIED(INTERNET_STATUS_REQUEST_COMPLETE);
+    ok(!avail, "avail = %d\n", avail);
+
+    avail = 0xdeadbeef;
+    res = InternetQueryDataAvailable(req, &avail, 0, 0);
+    ok(res, "InternetQueryDataAvailable failed: %u\n", GetLastError());
+    ok(!avail, "avail = %d\n", avail);
+
+    close_async_handle(session, hCompleteEvent, 2);
+    CloseHandle(hCompleteEvent);
+}
+
 static void test_conn_close(int port)
 {
     HINTERNET session, connection, req;
@@ -3833,6 +4038,7 @@ static void test_http_connection(void)
     test_premature_disconnect(si.port);
     test_connection_closing(si.port);
     test_cache_control_verb(si.port);
+    test_chunked_stream(si.port);
 
     /* send the basic request again to shutdown the server thread */
     test_basic_request(si.port, "GET", "/quit");


More information about the wine-devel mailing list