[PATCH 0/8] MR1582: winhttp: Read server reply in send_request().
It turns out that WinHttpReceiveResponse() completes synchronously in async mode (unless recursive request for handling authorization or redirect is involved). Some apps depend on that and do not wait for WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, calling WinHttpQueryHeaders() or WinHttpWebSocketCompleteUpgrade() right after calling WinHttpReceiveResponse, relying on that to finish synchronously. My initial out of tree testing shows that no network communication is performed during WinHttpReceiveResponse() call (when recursive request is not involved). I tested that by inserting a wait between WinHttpSendRequest and WinHttpReceiveResponse and disabling network connection during the wait. WinHttpReceiveResponse still succeeds on Windows. I think the above means that the actual response receiving from server is performed during WinHttpSendRequest. WinHttpReceiveResponse is not a complete no-op however. As shown by the existing tests the notifications related to receiving response are still delivered during WinHttpReceiveResponse (albeit in the same thread). Also WinHttpReceiveResponse affects request state: querying headers or upgrading to websocket without calling WinHttpReceiveResponse does not succeed. When redirect is involved, all the WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED and WINHTTP_CALLBACK_STATUS_REDIRECT notifications are delivered synchronously from the calling thread. Then, the new request send notifications and response receiving notifications are delivered from the new thread. An interesting case is when WinHttpReceiveResponse is called from SendRequest callbacks in async mode. If WinHttpReceiveResponse is called from WINHTTP_CALLBACK_STATUS_SENDING_REQUEST or WINHTTP_CALLBACK_STATUS_REQUEST_SENT (i. e., when request is not complete yet), calling WinHttpReceiveResponse() suddenly succeeds an shows the following message sequence (that is partially reflected in the tests I am adding): - calling WinHttpReceiveResponse from WINHTTP_CALLBACK_STATUS_SENDING_REQUEST (which is already called on the async thread on Win10, thread A). Win8 queues WINHTTP_CALLBACK_STATUS_SENDING_REQUEST synchronously and goes async a bit later. - WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, thread A; - WinHttpReceiveResponse() returns to the caller WINHTTP_CALLBACK_STATUS_REQUEST_SENT callback; returning from user callback; - WINHTTP_CALLBACK_STATUS_REQUEST_SENT, thread A; - WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE (another thread, although the sequence is probably synced; I am not implementing this part and calling this callback from the same thread A); - WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED in thread A; - WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE in thread A. So the receive_response() state RECEIVE_RESPONSE_SEND_INCOMPLETE is primarily needed to handle this case. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1582
From: Paul Gofman <pgofman(a)codeweavers.com> Let send_request() do it. --- dlls/winhttp/request.c | 1 - 1 file changed, 1 deletion(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index 1a9b4e14ee8..e04d5a04e9d 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -2733,7 +2733,6 @@ static DWORD handle_redirect( struct request *request, DWORD status ) else free( hostname ); if ((ret = add_host_header( request, WINHTTP_ADDREQ_FLAG_ADD | WINHTTP_ADDREQ_FLAG_REPLACE ))) goto end; - if ((ret = open_connection( request ))) goto end; free( request->path ); request->path = NULL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1582
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 64 +++++++++++++++++----------------- dlls/winhttp/session.c | 1 + dlls/winhttp/winhttp_private.h | 3 ++ 3 files changed, 36 insertions(+), 32 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index e04d5a04e9d..f878fd18dc0 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -1772,6 +1772,7 @@ static DWORD read_more_data( struct request *request, int maxlen, BOOL notify ) maxlen - request->read_size, 0, &len ); if (notify) send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, &len, sizeof(len) ); + request->read_reply_len += len; request->read_size += len; return ret; @@ -2174,6 +2175,10 @@ static DWORD send_request( struct request *request, const WCHAR *headers, DWORD int bytes_sent; DWORD ret, len; + request->read_reply_len = 0; + free( request->redirect_location ); + request->redirect_location = NULL; + if (request->flags & REQUEST_FLAG_WEBSOCKET_UPGRADE && request->websocket_set_send_buffer_size < MIN_WEBSOCKET_SEND_BUFFER_SIZE) { @@ -2492,7 +2497,7 @@ static DWORD read_line( struct request *request, char *buffer, DWORD *len ) remove_data( request, bytes_read ); if (eol) break; - if ((ret = read_more_data( request, -1, TRUE ))) return ret; + if ((ret = read_more_data( request, -1, FALSE ))) return ret; if (!request->read_size) { *len = 0; @@ -2629,6 +2634,12 @@ static DWORD get_redirect_url( struct request *request, WCHAR **ret_url, DWORD * DWORD size, ret; WCHAR *url; + if (*ret_url) + { + free( *ret_url ); + *ret_url = NULL; + } + ret = query_headers( request, WINHTTP_QUERY_LOCATION, NULL, NULL, &size, NULL ); if (ret != ERROR_INSUFFICIENT_BUFFER) return ret; if (!(url = malloc( size ))) return ERROR_OUTOFMEMORY; @@ -2644,13 +2655,15 @@ static DWORD get_redirect_url( struct request *request, WCHAR **ret_url, DWORD * static DWORD handle_redirect( struct request *request, DWORD status ) { - DWORD ret, len, len_loc = 0; + DWORD ret, len, len_loc; URL_COMPONENTS uc; struct connect *connect = request->connect; INTERNET_PORT port; - WCHAR *hostname = NULL, *location = NULL; + WCHAR *hostname = NULL, *location; - if ((ret = get_redirect_url( request, &location, &len_loc ))) return ret; + if ((ret = get_redirect_url( request, &request->redirect_location, &request->redirect_location_len ))) return ret; + location = request->redirect_location; + len_loc = request->redirect_location_len; memset( &uc, 0, sizeof(uc) ); uc.dwStructSize = sizeof(uc); @@ -2660,10 +2673,9 @@ static DWORD handle_redirect( struct request *request, DWORD status ) { WCHAR *path, *p; - ret = ERROR_OUTOFMEMORY; if (location[0] == '/') { - if (!(path = malloc( (len_loc + 1) * sizeof(WCHAR) ))) goto end; + if (!(path = malloc( (len_loc + 1) * sizeof(WCHAR) ))) return ERROR_OUTOFMEMORY; memcpy( path, location, len_loc * sizeof(WCHAR) ); path[len_loc] = 0; } @@ -2671,7 +2683,7 @@ static DWORD handle_redirect( struct request *request, DWORD status ) { if ((p = wcsrchr( request->path, '/' ))) *p = 0; len = lstrlenW( request->path ) + 1 + len_loc; - if (!(path = malloc( (len + 1) * sizeof(WCHAR) ))) goto end; + if (!(path = malloc( (len + 1) * sizeof(WCHAR) ))) return ERROR_OUTOFMEMORY; lstrcpyW( path, request->path ); lstrcatW( path, L"/" ); memcpy( path + lstrlenW(path), location, len_loc * sizeof(WCHAR) ); @@ -2679,19 +2691,14 @@ static DWORD handle_redirect( struct request *request, DWORD status ) } free( request->path ); request->path = path; - ret = ERROR_SUCCESS; - - send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_REDIRECT, location, len_loc + 1 ); } else { if (uc.nScheme == INTERNET_SCHEME_HTTP && request->hdr.flags & WINHTTP_FLAG_SECURE) { if (request->hdr.redirect_policy == WINHTTP_OPTION_REDIRECT_POLICY_DISALLOW_HTTPS_TO_HTTP) - { - ret = ERROR_WINHTTP_REDIRECT_FAILED; - goto end; - } + return ERROR_WINHTTP_REDIRECT_FAILED; + TRACE("redirect from secure page to non-secure page\n"); request->hdr.flags &= ~WINHTTP_FLAG_SECURE; } @@ -2700,15 +2707,8 @@ static DWORD handle_redirect( struct request *request, DWORD status ) TRACE("redirect from non-secure page to secure page\n"); request->hdr.flags |= WINHTTP_FLAG_SECURE; } - - send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_REDIRECT, location, len_loc + 1 ); - len = uc.dwHostNameLength; - if (!(hostname = malloc( (len + 1) * sizeof(WCHAR) ))) - { - ret = ERROR_OUTOFMEMORY; - goto end; - } + if (!(hostname = malloc( (len + 1) * sizeof(WCHAR) ))) return ERROR_OUTOFMEMORY; memcpy( hostname, uc.lpszHostName, len * sizeof(WCHAR) ); hostname[len] = 0; @@ -2718,11 +2718,7 @@ static DWORD handle_redirect( struct request *request, DWORD status ) free( connect->hostname ); connect->hostname = hostname; connect->hostport = port; - if (!set_server_for_hostname( connect, hostname, port )) - { - ret = ERROR_OUTOFMEMORY; - goto end; - } + if (!set_server_for_hostname( connect, hostname, port )) return ERROR_OUTOFMEMORY; netconn_release( request->netconn ); request->netconn = NULL; @@ -2732,14 +2728,14 @@ static DWORD handle_redirect( struct request *request, DWORD status ) } else free( hostname ); - if ((ret = add_host_header( request, WINHTTP_ADDREQ_FLAG_ADD | WINHTTP_ADDREQ_FLAG_REPLACE ))) goto end; + if ((ret = add_host_header( request, WINHTTP_ADDREQ_FLAG_ADD | WINHTTP_ADDREQ_FLAG_REPLACE ))) return ret; free( request->path ); request->path = NULL; if (uc.dwUrlPathLength) { len = uc.dwUrlPathLength + uc.dwExtraInfoLength; - if (!(request->path = malloc( (len + 1) * sizeof(WCHAR) ))) goto end; + if (!(request->path = malloc( (len + 1) * sizeof(WCHAR) ))) return ERROR_OUTOFMEMORY; memcpy( request->path, uc.lpszUrlPath, (len + 1) * sizeof(WCHAR) ); request->path[len] = 0; } @@ -2754,9 +2750,7 @@ static DWORD handle_redirect( struct request *request, DWORD status ) request->optional_len = 0; } -end: - free( location ); - return ret; + return ERROR_SUCCESS; } static BOOL is_passport_request( struct request *request ) @@ -2803,6 +2797,9 @@ static DWORD receive_response( struct request *request, BOOL async ) netconn_set_timeout( request->netconn, FALSE, request->receive_response_timeout ); for (;;) { + send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NULL, 0 ); + send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, + &request->read_reply_len, sizeof(request->read_reply_len) ); if ((ret = read_reply( request ))) break; size = sizeof(DWORD); @@ -2826,6 +2823,9 @@ static DWORD receive_response( struct request *request, BOOL async ) if ((ret = handle_redirect( request, status ))) break; + send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_REDIRECT, + request->redirect_location, request->redirect_location_len + 1 ); + /* recurse synchronously */ if (!(ret = send_request( request, NULL, 0, request->optional, request->optional_len, 0, 0, FALSE ))) continue; } diff --git a/dlls/winhttp/session.c b/dlls/winhttp/session.c index 8aa99f0ece7..b0b50ef06af 100644 --- a/dlls/winhttp/session.c +++ b/dlls/winhttp/session.c @@ -678,6 +678,7 @@ static void request_destroy( struct object_header *hdr ) } } + free( request->redirect_location ); free( request ); } diff --git a/dlls/winhttp/winhttp_private.h b/dlls/winhttp/winhttp_private.h index 0326a8fc1e3..ed0f6e590ab 100644 --- a/dlls/winhttp/winhttp_private.h +++ b/dlls/winhttp/winhttp_private.h @@ -220,6 +220,9 @@ struct request } creds[TARGET_MAX][SCHEME_MAX]; unsigned int websocket_receive_buffer_size; unsigned int websocket_send_buffer_size, websocket_set_send_buffer_size; + int read_reply_len; + WCHAR *redirect_location; + DWORD redirect_location_len; }; enum socket_state -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1582
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 98 +++++++++++++++++++++------------- dlls/winhttp/winhttp_private.h | 1 + 2 files changed, 63 insertions(+), 36 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index f878fd18dc0..3f84443af0d 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -2178,6 +2178,7 @@ static DWORD send_request( struct request *request, const WCHAR *headers, DWORD request->read_reply_len = 0; free( request->redirect_location ); request->redirect_location = NULL; + request->recursive_request = FALSE; if (request->flags & REQUEST_FLAG_WEBSOCKET_UPGRADE && request->websocket_set_send_buffer_size < MIN_WEBSOCKET_SEND_BUFFER_SIZE) @@ -2788,62 +2789,87 @@ static DWORD handle_passport_redirect( struct request *request ) return ERROR_SUCCESS; } -static DWORD receive_response( struct request *request, BOOL async ) +static DWORD receive_server_response( struct request *request ) { DWORD ret, size, query, status; - if (!request->netconn) return ERROR_WINHTTP_INCORRECT_HANDLE_STATE; + TRACE( "request %p.\n", request ); + + request->recursive_request = FALSE; netconn_set_timeout( request->netconn, FALSE, request->receive_response_timeout ); - for (;;) + + if ((ret = read_reply( request ))) { - send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NULL, 0 ); - send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, - &request->read_reply_len, sizeof(request->read_reply_len) ); - if ((ret = read_reply( request ))) break; + TRACE( "read_reply ret %lu.\n", ret); + return ret; + } - size = sizeof(DWORD); - query = WINHTTP_QUERY_STATUS_CODE | WINHTTP_QUERY_FLAG_NUMBER; - if ((ret = query_headers( request, query, NULL, &status, &size, NULL ))) break; + size = sizeof(DWORD); + query = WINHTTP_QUERY_STATUS_CODE | WINHTTP_QUERY_FLAG_NUMBER; + if ((ret = query_headers( request, query, NULL, &status, &size, NULL ))) + { + TRACE( "query_headers ret %lu.\n", ret); + return ret; + } - set_content_length( request, status ); + set_content_length( request, status ); - if (!(request->hdr.disable_flags & WINHTTP_DISABLE_COOKIES)) record_cookies( request ); + if (!(request->hdr.disable_flags & WINHTTP_DISABLE_COOKIES)) record_cookies( request ); - if (status == HTTP_STATUS_REDIRECT && is_passport_request( request )) + if (status == HTTP_STATUS_REDIRECT && is_passport_request( request )) + { + ret = handle_passport_redirect( request ); + } + else if (status == HTTP_STATUS_MOVED || status == HTTP_STATUS_REDIRECT || status == HTTP_STATUS_REDIRECT_KEEP_VERB) + { + if (!(request->hdr.disable_flags & WINHTTP_DISABLE_REDIRECTS || + request->hdr.redirect_policy == WINHTTP_OPTION_REDIRECT_POLICY_NEVER)) { - ret = handle_passport_redirect( request ); + if (++request->redirect_count > request->max_redirects) + { + TRACE("redirect_count %lu, max_redirects %lu.\n", request->redirect_count, request->max_redirects); + return ERROR_WINHTTP_REDIRECT_FAILED; + } + request->recursive_request = !(ret = handle_redirect( request, status )); } - else if (status == HTTP_STATUS_MOVED || status == HTTP_STATUS_REDIRECT || status == HTTP_STATUS_REDIRECT_KEEP_VERB) - { - if (request->hdr.disable_flags & WINHTTP_DISABLE_REDIRECTS || - request->hdr.redirect_policy == WINHTTP_OPTION_REDIRECT_POLICY_NEVER) break; + } + else if (status == HTTP_STATUS_DENIED || status == HTTP_STATUS_PROXY_AUTH_REQ) + { + if (!(request->hdr.disable_flags & WINHTTP_DISABLE_AUTHENTICATION)) + request->recursive_request = !handle_authorization( request, status ); + } - if (++request->redirect_count > request->max_redirects) return ERROR_WINHTTP_REDIRECT_FAILED; + if (request->netconn) netconn_set_timeout( request->netconn, FALSE, request->receive_timeout ); + if (request->content_length && !ret && !request->recursive_request) + { + ret = refill_buffer( request, FALSE ); + TRACE("refill_buffer ret %ld.\n", ret); + } + return ret; +} + +static DWORD receive_response( struct request *request, BOOL async ) +{ + DWORD ret; - if ((ret = handle_redirect( request, status ))) break; + if (!request->netconn) return ERROR_WINHTTP_INCORRECT_HANDLE_STATE; + for (;;) + { + send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NULL, 0 ); + send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, + &request->read_reply_len, sizeof(request->read_reply_len) ); + if ((ret = receive_server_response( request ))) break; + if (!request->recursive_request) break; + if (request->redirect_location) send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_REDIRECT, request->redirect_location, request->redirect_location_len + 1 ); - - /* recurse synchronously */ - if (!(ret = send_request( request, NULL, 0, request->optional, request->optional_len, 0, 0, FALSE ))) continue; - } - else if (status == HTTP_STATUS_DENIED || status == HTTP_STATUS_PROXY_AUTH_REQ) - { - if (request->hdr.disable_flags & WINHTTP_DISABLE_AUTHENTICATION) break; - - if (handle_authorization( request, status )) break; - - /* recurse synchronously */ - if (!(ret = send_request( request, NULL, 0, request->optional, request->optional_len, 0, 0, FALSE ))) continue; - } + /* recurse synchronously */ + if (!(ret = send_request( request, NULL, 0, request->optional, request->optional_len, 0, 0, FALSE ))) continue; break; } - if (request->netconn) netconn_set_timeout( request->netconn, FALSE, request->receive_timeout ); - if (request->content_length) ret = refill_buffer( request, FALSE ); - if (async) { if (!ret) send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, NULL, 0 ); diff --git a/dlls/winhttp/winhttp_private.h b/dlls/winhttp/winhttp_private.h index ed0f6e590ab..cffad0e0c74 100644 --- a/dlls/winhttp/winhttp_private.h +++ b/dlls/winhttp/winhttp_private.h @@ -223,6 +223,7 @@ struct request int read_reply_len; WCHAR *redirect_location; DWORD redirect_location_len; + BOOL recursive_request; }; enum socket_state -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1582
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index 3f84443af0d..91febf40e97 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -2849,6 +2849,19 @@ static DWORD receive_server_response( struct request *request ) return ret; } +static void task_receive_response( void *ctx, BOOL abort ); + +static DWORD queue_receive_response( struct request *request ) +{ + struct receive_response *r; + DWORD ret; + + if (!(r = malloc( sizeof(*r) ))) return ERROR_OUTOFMEMORY; + if ((ret = queue_task( &request->queue, task_receive_response, &r->task_hdr, &request->hdr ))) + free( r ); + return ret; +} + static DWORD receive_response( struct request *request, BOOL async ) { DWORD ret; @@ -2917,20 +2930,8 @@ BOOL WINAPI WinHttpReceiveResponse( HINTERNET hrequest, LPVOID reserved ) return FALSE; } - if (request->connect->hdr.flags & WINHTTP_FLAG_ASYNC) - { - struct receive_response *r; - - if (!(r = malloc( sizeof(*r) ))) - { - release_object( &request->hdr ); - SetLastError( ERROR_OUTOFMEMORY ); - return FALSE; - } - if ((ret = queue_task( &request->queue, task_receive_response, &r->task_hdr, &request->hdr ))) - free( r ); - } - else ret = receive_response( request, FALSE ); + if (request->connect->hdr.flags & WINHTTP_FLAG_ASYNC) ret = queue_receive_response( request ); + else ret = receive_response( request, FALSE ); release_object( &request->hdr ); SetLastError( ret ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1582
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 57 ++++++++++++++++++++++------------ dlls/winhttp/winhttp_private.h | 8 +++++ 2 files changed, 45 insertions(+), 20 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index 91febf40e97..cddd9235b9e 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -2851,39 +2851,57 @@ static DWORD receive_server_response( struct request *request ) static void task_receive_response( void *ctx, BOOL abort ); -static DWORD queue_receive_response( struct request *request ) +static DWORD queue_receive_response( struct request *request, enum receive_response_queue_state queue_state ) { struct receive_response *r; DWORD ret; if (!(r = malloc( sizeof(*r) ))) return ERROR_OUTOFMEMORY; + r->queue_state = queue_state; if ((ret = queue_task( &request->queue, task_receive_response, &r->task_hdr, &request->hdr ))) free( r ); return ret; } -static DWORD receive_response( struct request *request, BOOL async ) +static DWORD receive_response( struct request *request, enum receive_response_queue_state queue_state ) { + BOOL async_mode = request->connect->hdr.flags & WINHTTP_FLAG_ASYNC; DWORD ret; - if (!request->netconn) return ERROR_WINHTTP_INCORRECT_HANDLE_STATE; + TRACE( "request %p, queue_state %d, async_mode %d.\n", request, queue_state, async_mode ); - for (;;) + if (queue_state == RECEIVE_RESPONSE_RECURSIVE_REQUEST) { - send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NULL, 0 ); - send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, - &request->read_reply_len, sizeof(request->read_reply_len) ); - if ((ret = receive_server_response( request ))) break; - if (!request->recursive_request) break; - if (request->redirect_location) - send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_REDIRECT, - request->redirect_location, request->redirect_location_len + 1 ); - /* recurse synchronously */ - if (!(ret = send_request( request, NULL, 0, request->optional, request->optional_len, 0, 0, FALSE ))) continue; - break; + TRACE( "Sending request.\n" ); + ret = send_request( request, NULL, 0, request->optional, request->optional_len, 0, 0, FALSE ); + if (ret) goto done; } - if (async) + if (async_mode && !queue_state) + return queue_receive_response( request, RECEIVE_RESPONSE_SEND_INCOMPLETE ); + + if (!request->netconn) + { + ret = ERROR_WINHTTP_INCORRECT_HANDLE_STATE; + goto done; + } + + ret = receive_server_response( request ); + + send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NULL, 0 ); + send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, + &request->read_reply_len, sizeof(request->read_reply_len) ); + + if (ret || !request->recursive_request) goto done; + + if (request->redirect_location) + send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_REDIRECT, request->redirect_location, + request->redirect_location_len + 1 ); + + return receive_response( request, RECEIVE_RESPONSE_RECURSIVE_REQUEST ); + +done: + if (async_mode) { if (!ret) send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, NULL, 0 ); else @@ -2894,7 +2912,7 @@ static DWORD receive_response( struct request *request, BOOL async ) send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, &result, sizeof(result) ); } } - return ret; + return async_mode ? ERROR_SUCCESS : ret; } static void task_receive_response( void *ctx, BOOL abort ) @@ -2905,7 +2923,7 @@ static void task_receive_response( void *ctx, BOOL abort ) if (abort) return; TRACE("running %p\n", ctx); - receive_response( request, TRUE ); + receive_response( request, r->queue_state ); } /*********************************************************************** @@ -2930,8 +2948,7 @@ BOOL WINAPI WinHttpReceiveResponse( HINTERNET hrequest, LPVOID reserved ) return FALSE; } - if (request->connect->hdr.flags & WINHTTP_FLAG_ASYNC) ret = queue_receive_response( request ); - else ret = receive_response( request, FALSE ); + ret = receive_response( request, RECEIVE_RESPONSE_NOT_QUEUED ); release_object( &request->hdr ); SetLastError( ret ); diff --git a/dlls/winhttp/winhttp_private.h b/dlls/winhttp/winhttp_private.h index cffad0e0c74..f0680c16469 100644 --- a/dlls/winhttp/winhttp_private.h +++ b/dlls/winhttp/winhttp_private.h @@ -311,9 +311,17 @@ struct send_request DWORD_PTR context; }; +enum receive_response_queue_state +{ + RECEIVE_RESPONSE_NOT_QUEUED, + RECEIVE_RESPONSE_SEND_INCOMPLETE, + RECEIVE_RESPONSE_RECURSIVE_REQUEST, +}; + struct receive_response { struct task_header task_hdr; + enum receive_response_queue_state queue_state; }; struct query_data -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1582
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 42 +++++++++++++++++++++++++++------- dlls/winhttp/tests/winhttp.c | 8 +++---- dlls/winhttp/winhttp_private.h | 4 ++++ 3 files changed, 42 insertions(+), 12 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index cddd9235b9e..24894b48188 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -800,6 +800,13 @@ BOOL WINAPI WinHttpQueryHeaders( HINTERNET hrequest, DWORD level, const WCHAR *n SetLastError( ERROR_WINHTTP_INCORRECT_HANDLE_TYPE ); return FALSE; } + if (!request->receive_response_complete && !(level & WINHTTP_QUERY_FLAG_REQUEST_HEADERS) + && ((level & ~QUERY_MODIFIER_MASK) != WINHTTP_QUERY_REQUEST_METHOD)) + { + release_object( &request->hdr ); + SetLastError( ERROR_WINHTTP_INCORRECT_HANDLE_STATE ); + return FALSE; + } ret = query_headers( request, level, name, buffer, buflen, index ); @@ -2166,6 +2173,8 @@ static DWORD add_websocket_key_header( struct request *request ) return ERROR_SUCCESS; } +static DWORD receive_server_response( struct request *request ); + static DWORD send_request( struct request *request, const WCHAR *headers, DWORD headers_len, void *optional, DWORD optional_len, DWORD total_len, DWORD_PTR context, BOOL async ) { @@ -2175,6 +2184,8 @@ static DWORD send_request( struct request *request, const WCHAR *headers, DWORD int bytes_sent; DWORD ret, len; + request->server_reply_complete = FALSE; + request->receive_response_complete = FALSE; request->read_reply_len = 0; free( request->redirect_location ); request->redirect_location = NULL; @@ -2245,6 +2256,7 @@ static DWORD send_request( struct request *request, const WCHAR *headers, DWORD } TRACE("full request: %s\n", debugstr_a(wire_req)); + request->sending_request = TRUE; send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST, NULL, 0 ); ret = netconn_send( request->netconn, wire_req, len, &bytes_sent, NULL ); @@ -2259,8 +2271,11 @@ static DWORD send_request( struct request *request, const WCHAR *headers, DWORD len += optional_len; } send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_REQUEST_SENT, &len, sizeof(len) ); - + request->server_reply_status = receive_server_response( request ); + request->server_reply_complete = TRUE; end: + request->sending_request = FALSE; + if (async) { if (!ret) send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE, NULL, 0 ); @@ -2870,6 +2885,12 @@ static DWORD receive_response( struct request *request, enum receive_response_qu TRACE( "request %p, queue_state %d, async_mode %d.\n", request, queue_state, async_mode ); + if (request->receive_response_complete) + { + ret = ERROR_WINHTTP_INCORRECT_HANDLE_STATE; + goto done; + } + if (queue_state == RECEIVE_RESPONSE_RECURSIVE_REQUEST) { TRACE( "Sending request.\n" ); @@ -2877,30 +2898,35 @@ static DWORD receive_response( struct request *request, enum receive_response_qu if (ret) goto done; } - if (async_mode && !queue_state) - return queue_receive_response( request, RECEIVE_RESPONSE_SEND_INCOMPLETE ); - - if (!request->netconn) + if (!request->server_reply_complete && !(async_mode && request->sending_request)) { ret = ERROR_WINHTTP_INCORRECT_HANDLE_STATE; goto done; } - ret = receive_server_response( request ); + if (queue_state != RECEIVE_RESPONSE_SEND_INCOMPLETE) + send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NULL, 0 ); + + if (!request->server_reply_complete && async_mode && request->sending_request) + return queue_receive_response( request, RECEIVE_RESPONSE_SEND_INCOMPLETE ); - send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NULL, 0 ); send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, &request->read_reply_len, sizeof(request->read_reply_len) ); + ret = request->server_reply_status; if (ret || !request->recursive_request) goto done; if (request->redirect_location) send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_REDIRECT, request->redirect_location, request->redirect_location_len + 1 ); - return receive_response( request, RECEIVE_RESPONSE_RECURSIVE_REQUEST ); + if (async_mode) + return queue_receive_response( request, RECEIVE_RESPONSE_RECURSIVE_REQUEST ); + else + return receive_response( request, RECEIVE_RESPONSE_RECURSIVE_REQUEST ); done: + if (!ret) request->receive_response_complete = TRUE; if (async_mode) { if (!ret) send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, NULL, 0 ); diff --git a/dlls/winhttp/tests/winhttp.c b/dlls/winhttp/tests/winhttp.c index b7deaea324a..76f12837e86 100644 --- a/dlls/winhttp/tests/winhttp.c +++ b/dlls/winhttp/tests/winhttp.c @@ -3277,14 +3277,14 @@ static void test_websocket(int port) ret = WinHttpQueryHeaders(request, WINHTTP_QUERY_UPGRADE, NULL, &header, &size, NULL); error = GetLastError(); ok(!ret, "success\n"); - todo_wine ok(error == ERROR_WINHTTP_INCORRECT_HANDLE_STATE, "got %lu\n", error); + ok(error == ERROR_WINHTTP_INCORRECT_HANDLE_STATE, "got %lu\n", error); size = sizeof(header); SetLastError(0xdeadbeef); ret = WinHttpQueryHeaders(request, WINHTTP_QUERY_CONNECTION, NULL, &header, &size, NULL); error = GetLastError(); ok(!ret, "success\n"); - todo_wine ok(error == ERROR_WINHTTP_INCORRECT_HANDLE_STATE, "got %lu\n", error); + ok(error == ERROR_WINHTTP_INCORRECT_HANDLE_STATE, "got %lu\n", error); index = 0; size = sizeof(buf); @@ -3312,14 +3312,14 @@ static void test_websocket(int port) ret = WinHttpQueryHeaders(request, WINHTTP_QUERY_UPGRADE, NULL, &header, &size, NULL); error = GetLastError(); ok(!ret, "success\n"); - todo_wine ok(error == ERROR_WINHTTP_INCORRECT_HANDLE_STATE, "got %lu\n", error); + ok(error == ERROR_WINHTTP_INCORRECT_HANDLE_STATE, "got %lu\n", error); size = sizeof(header); SetLastError(0xdeadbeef); ret = WinHttpQueryHeaders(request, WINHTTP_QUERY_CONNECTION, NULL, &header, &size, NULL); error = GetLastError(); ok(!ret, "success\n"); - todo_wine ok(error == ERROR_WINHTTP_INCORRECT_HANDLE_STATE, "got %lu\n", error); + ok(error == ERROR_WINHTTP_INCORRECT_HANDLE_STATE, "got %lu\n", error); index = 0; buf[0] = 0; diff --git a/dlls/winhttp/winhttp_private.h b/dlls/winhttp/winhttp_private.h index f0680c16469..e1519c73c4f 100644 --- a/dlls/winhttp/winhttp_private.h +++ b/dlls/winhttp/winhttp_private.h @@ -220,10 +220,14 @@ struct request } creds[TARGET_MAX][SCHEME_MAX]; unsigned int websocket_receive_buffer_size; unsigned int websocket_send_buffer_size, websocket_set_send_buffer_size; + BOOL sending_request; + BOOL server_reply_complete; + DWORD server_reply_status; int read_reply_len; WCHAR *redirect_location; DWORD redirect_location_len; BOOL recursive_request; + BOOL receive_response_complete; }; enum socket_state -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1582
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/tests/notification.c | 198 +++++++++++++++++++++++------- 1 file changed, 152 insertions(+), 46 deletions(-) diff --git a/dlls/winhttp/tests/notification.c b/dlls/winhttp/tests/notification.c index a9f782f5e3d..d029a0a9054 100644 --- a/dlls/winhttp/tests/notification.c +++ b/dlls/winhttp/tests/notification.c @@ -64,6 +64,7 @@ struct notification #define NF_SIGNAL 0x0004 /* signal wait handle when notified */ #define NF_MAIN_THREAD 0x0008 /* the operation completes synchronously and callback is called from the main thread */ #define NF_SAVE_BUFFER 0x0010 /* save buffer data when notified */ +#define NF_OTHER_THREAD 0x0020 /* the operation completes asynchronously and callback is called from the other thread */ struct info { @@ -119,8 +120,13 @@ static void CALLBACK check_notification( HINTERNET handle, DWORD_PTR context, DW if (info->test[info->index].flags & NF_MAIN_THREAD) { - ok(GetCurrentThreadId() == info->main_thread_id, "%u: expected callback to be called from the same thread\n", - info->line); + ok(GetCurrentThreadId() == info->main_thread_id, "%u: expected callback %#lx to be called from the same thread\n", + info->line, status); + } + else if (info->test[info->index].flags & NF_OTHER_THREAD) + { + ok(GetCurrentThreadId() != info->main_thread_id, "%u: expected callback %#lx to be called from the other thread\n", + info->line, status); } if (info->test[info->index].flags & NF_SAVE_BUFFER) { @@ -134,6 +140,56 @@ static void CALLBACK check_notification( HINTERNET handle, DWORD_PTR context, DW } } +static const struct notification cache_test_async[] = +{ + { winhttp_connect, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED }, + { winhttp_open_request, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_RESOLVING_NAME, NF_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_NAME_RESOLVED, NF_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTING_TO_SERVER, NF_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTED_TO_SERVER, NF_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE, NF_SIGNAL }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, NF_MAIN_THREAD | NF_SIGNAL }, + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL }, + { winhttp_open_request, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTING_TO_SERVER, NF_WINE_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTED_TO_SERVER, NF_WINE_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE, NF_SIGNAL }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, NF_MAIN_THREAD | NF_SIGNAL }, + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING }, + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL }, + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL }, + { winhttp_connect, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED }, + { winhttp_open_request, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_RESOLVING_NAME, NF_WINE_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_NAME_RESOLVED, NF_WINE_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTING_TO_SERVER, NF_WINE_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTED_TO_SERVER, NF_WINE_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, NF_MAIN_THREAD }, + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL }, + { winhttp_open_request, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTING_TO_SERVER, NF_WINE_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTED_TO_SERVER, NF_WINE_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, NF_MAIN_THREAD }, + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING }, + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL }, + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL } +}; + static const struct notification cache_test[] = { { winhttp_connect, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED }, @@ -143,17 +199,17 @@ static const struct notification cache_test[] = { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTING_TO_SERVER }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTED_TO_SERVER }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST }, - { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT, NF_SIGNAL }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, NF_MAIN_THREAD | NF_SIGNAL }, { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL }, { winhttp_open_request, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTING_TO_SERVER, NF_WINE_ALLOW }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTED_TO_SERVER, NF_WINE_ALLOW }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST }, - { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT, NF_SIGNAL }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, NF_MAIN_THREAD | NF_SIGNAL }, { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING }, { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL }, { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL }, @@ -165,16 +221,16 @@ static const struct notification cache_test[] = { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTED_TO_SERVER, NF_WINE_ALLOW }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, NF_MAIN_THREAD }, { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL }, { winhttp_open_request, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTING_TO_SERVER, NF_WINE_ALLOW }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTED_TO_SERVER, NF_WINE_ALLOW }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, NF_MAIN_THREAD }, { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING }, { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL }, { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL } @@ -202,19 +258,19 @@ static void end_test( struct info *info, unsigned int line ) info->test[info->index].status); } -static void test_connection_cache( void ) +static void test_connection_cache( BOOL async ) { HANDLE ses, con, req, event; DWORD size, status, err; BOOL ret, unload = TRUE; struct info info, *context = &info; - info.test = cache_test; - info.count = ARRAY_SIZE( cache_test ); + info.test = async ? cache_test_async : cache_test; + info.count = async ? ARRAY_SIZE( cache_test_async ) : ARRAY_SIZE ( cache_test ); info.index = 0; info.wait = CreateEventW( NULL, FALSE, FALSE, NULL ); - ses = WinHttpOpen( L"winetest", 0, NULL, NULL, 0 ); + ses = WinHttpOpen( L"winetest", 0, NULL, NULL, async ? WINHTTP_FLAG_ASYNC : 0 ); ok( ses != NULL, "failed to open session %lu\n", GetLastError() ); event = CreateEventW( NULL, FALSE, FALSE, NULL ); @@ -247,11 +303,14 @@ static void test_connection_cache( void ) goto done; } ok( ret, "failed to send request %lu\n", GetLastError() ); + WaitForSingleObject( info.wait, INFINITE ); setup_test( &info, winhttp_receive_response, __LINE__ ); ret = WinHttpReceiveResponse( req, NULL ); ok( ret, "failed to receive response %lu\n", GetLastError() ); + WaitForSingleObject( info.wait, INFINITE ); + size = sizeof(status); ret = WinHttpQueryHeaders( req, WINHTTP_QUERY_STATUS_CODE | WINHTTP_QUERY_FLAG_NUMBER, NULL, &status, &size, NULL ); ok( ret, "failed unexpectedly %lu\n", GetLastError() ); @@ -260,6 +319,7 @@ static void test_connection_cache( void ) ResetEvent( info.wait ); setup_test( &info, winhttp_close_handle, __LINE__ ); WinHttpCloseHandle( req ); + WaitForSingleObject( info.wait, INFINITE ); setup_test( &info, winhttp_open_request, __LINE__ ); @@ -279,10 +339,14 @@ static void test_connection_cache( void ) } ok( ret, "failed to send request %lu\n", GetLastError() ); + WaitForSingleObject( info.wait, INFINITE ); + setup_test( &info, winhttp_receive_response, __LINE__ ); ret = WinHttpReceiveResponse( req, NULL ); ok( ret, "failed to receive response %lu\n", GetLastError() ); + WaitForSingleObject( info.wait, INFINITE ); + size = sizeof(status); ret = WinHttpQueryHeaders( req, WINHTTP_QUERY_STATUS_CODE | WINHTTP_QUERY_FLAG_NUMBER, NULL, &status, &size, NULL ); ok( ret, "failed unexpectedly %lu\n", GetLastError() ); @@ -414,6 +478,32 @@ done: } static const struct notification redirect_test[] = +{ + { winhttp_connect, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED }, + { winhttp_open_request, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_RESOLVING_NAME, NF_WINE_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_NAME_RESOLVED, NF_WINE_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTING_TO_SERVER, NF_WINE_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTED_TO_SERVER, NF_WINE_ALLOW }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST, NF_MAIN_THREAD }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT, NF_MAIN_THREAD | NF_SIGNAL }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_REDIRECT, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESOLVING_NAME, NF_ALLOW | NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_NAME_RESOLVED, NF_ALLOW | NF_MAIN_THREAD}, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_CONNECTING_TO_SERVER, NF_ALLOW | NF_MAIN_THREAD}, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_CONNECTED_TO_SERVER, NF_ALLOW | NF_MAIN_THREAD}, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST, NF_MAIN_THREAD}, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_REQUEST_SENT, NF_MAIN_THREAD}, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NF_MAIN_THREAD}, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, NF_MAIN_THREAD | NF_SIGNAL}, + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING }, + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING }, + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL } +}; + +static const struct notification redirect_test_async[] = { { winhttp_connect, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED }, { winhttp_open_request, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED }, @@ -423,35 +513,37 @@ static const struct notification redirect_test[] = { winhttp_send_request, WINHTTP_CALLBACK_STATUS_CONNECTED_TO_SERVER, NF_WINE_ALLOW }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_REDIRECT }, + { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE, NF_SIGNAL | NF_OTHER_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_REDIRECT, NF_MAIN_THREAD }, { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESOLVING_NAME, NF_ALLOW }, { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_NAME_RESOLVED, NF_ALLOW }, { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_CONNECTING_TO_SERVER, NF_ALLOW }, { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_CONNECTED_TO_SERVER, NF_ALLOW }, { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST }, { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_REQUEST_SENT }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NF_OTHER_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, NF_OTHER_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, NF_OTHER_THREAD | NF_SIGNAL }, { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING }, { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING }, { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL } }; -static void test_redirect( void ) +static void test_redirect( BOOL async ) { HANDLE ses, con, req; DWORD size, status, err; BOOL ret; struct info info, *context = &info; - info.test = redirect_test; - info.count = ARRAY_SIZE( redirect_test ); + info.test = async ? redirect_test_async : redirect_test; + info.count = async ? ARRAY_SIZE( redirect_test_async ) : ARRAY_SIZE( redirect_test ); info.index = 0; info.wait = CreateEventW( NULL, FALSE, FALSE, NULL ); - ses = WinHttpOpen( L"winetest", 0, NULL, NULL, 0 ); + ses = WinHttpOpen( L"winetest", 0, NULL, NULL, async ? WINHTTP_FLAG_ASYNC : 0 ); ok( ses != NULL, "failed to open session %lu\n", GetLastError() ); WinHttpSetStatusCallback( ses, check_notification, WINHTTP_CALLBACK_FLAG_ALL_NOTIFICATIONS, 0 ); @@ -476,12 +568,15 @@ static void test_redirect( void ) goto done; } ok( ret, "failed to send request %lu\n", GetLastError() ); + WaitForSingleObject( info.wait, INFINITE ); setup_test( &info, winhttp_receive_response, __LINE__ ); ret = WinHttpReceiveResponse( req, NULL ); ok( ret, "failed to receive response %lu\n", GetLastError() ); + WaitForSingleObject( info.wait, INFINITE ); size = sizeof(status); + status = 0xdeadbeef; ret = WinHttpQueryHeaders( req, WINHTTP_QUERY_STATUS_CODE | WINHTTP_QUERY_FLAG_NUMBER, NULL, &status, &size, NULL ); ok( ret, "failed unexpectedly %lu\n", GetLastError() ); ok( status == 200, "request failed unexpectedly %lu\n", status ); @@ -507,9 +602,10 @@ static const struct notification async_test[] = { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE, NF_SIGNAL }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, NF_SIGNAL }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, NF_SIGNAL | NF_MAIN_THREAD }, { winhttp_query_data, WINHTTP_CALLBACK_STATUS_DATA_AVAILABLE, NF_SIGNAL }, { winhttp_read_data, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NF_ALLOW }, { winhttp_read_data, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, NF_ALLOW }, @@ -599,6 +695,12 @@ static void test_async( void ) ok( ret, "failed to receive response %lu\n", err ); ok( err == ERROR_SUCCESS, "got %lu\n", err ); + SetLastError( 0xdeadbeef ); + ret = WinHttpReceiveResponse( req, NULL ); + err = GetLastError(); + ok( ret, "failed to receive response %lu\n", err ); + ok( err == ERROR_SUCCESS, "got %lu\n", err ); + WaitForSingleObject( info.wait, INFINITE ); size = sizeof(status); @@ -668,9 +770,9 @@ static const struct notification websocket_test[] = { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE, NF_SIGNAL }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, NF_SIGNAL }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, NF_MAIN_THREAD | NF_SIGNAL }, { winhttp_websocket_complete_upgrade, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED }, { winhttp_websocket_complete_upgrade, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL }, { winhttp_websocket_send, WINHTTP_CALLBACK_STATUS_WRITE_COMPLETE, NF_MAIN_THREAD | NF_SIGNAL }, @@ -694,9 +796,9 @@ static const struct notification websocket_test2[] = { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE, NF_SIGNAL }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, NF_SIGNAL }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, NF_SIGNAL | NF_MAIN_THREAD}, { winhttp_websocket_complete_upgrade, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED, NF_SIGNAL }, { winhttp_websocket_receive, WINHTTP_CALLBACK_STATUS_READ_COMPLETE, NF_SIGNAL }, { winhttp_websocket_close, WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, NF_MAIN_THREAD | NF_SAVE_BUFFER}, @@ -715,9 +817,9 @@ static const struct notification websocket_test3[] = { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE, NF_SIGNAL }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, NF_SIGNAL }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, NF_SIGNAL | NF_MAIN_THREAD }, { winhttp_websocket_complete_upgrade, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED, NF_SIGNAL }, { winhttp_websocket_receive, WINHTTP_CALLBACK_STATUS_READ_COMPLETE, NF_SIGNAL }, @@ -738,9 +840,9 @@ static struct notification websocket_test4[] = { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE, NF_SIGNAL }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, NF_SIGNAL }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, NF_SIGNAL | NF_MAIN_THREAD }, { winhttp_websocket_complete_upgrade, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED, NF_SIGNAL }, { winhttp_websocket_receive, WINHTTP_CALLBACK_STATUS_READ_COMPLETE, NF_SIGNAL }, @@ -760,10 +862,10 @@ static const struct notification websocket_test5[] = { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDING_REQUEST }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_REQUEST_SENT }, { winhttp_send_request, WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE, NF_SIGNAL }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED }, - { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, NF_SIGNAL }, - { winhttp_websocket_complete_upgrade, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED, NF_SIGNAL }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED, NF_MAIN_THREAD }, + { winhttp_receive_response, WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE, NF_SIGNAL | NF_MAIN_THREAD }, + { winhttp_websocket_complete_upgrade, WINHTTP_CALLBACK_STATUS_HANDLE_CREATED, NF_SIGNAL | NF_MAIN_THREAD }, { winhttp_websocket_receive, WINHTTP_CALLBACK_STATUS_READ_COMPLETE, NF_SIGNAL }, { winhttp_websocket_shutdown, WINHTTP_CALLBACK_STATUS_SHUTDOWN_COMPLETE, NF_MAIN_THREAD }, @@ -1901,8 +2003,12 @@ START_TEST (notification) pWinHttpWebSocketSend = (void *)GetProcAddress( mod, "WinHttpWebSocketSend" ); pWinHttpWebSocketShutdown = (void *)GetProcAddress( mod, "WinHttpWebSocketShutdown" ); - test_connection_cache(); - test_redirect(); + test_connection_cache( FALSE ); + test_redirect( FALSE ); + winetest_push_context( "async" ); + test_connection_cache( TRUE ); + test_redirect( TRUE ); + winetest_pop_context(); test_async(); test_websocket( FALSE ); winetest_push_context( "secure" ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1582
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/tests/notification.c | 90 ++++++++++++++++++++++++++++--- 1 file changed, 83 insertions(+), 7 deletions(-) diff --git a/dlls/winhttp/tests/notification.c b/dlls/winhttp/tests/notification.c index d029a0a9054..ba23d897f06 100644 --- a/dlls/winhttp/tests/notification.c +++ b/dlls/winhttp/tests/notification.c @@ -1848,6 +1848,10 @@ struct test_recursion_context LONG recursion_count, max_recursion_query, max_recursion_read; BOOL read_from_callback; BOOL have_sync_callback; + DWORD call_receive_response_status; + DWORD main_thread_id; + DWORD receive_response_thread_id; + BOOL headers_available; }; /* The limit is 128 before Win7 and 3 on newer Windows. */ @@ -1863,8 +1867,25 @@ static void CALLBACK test_recursion_callback( HINTERNET handle, DWORD_PTR contex switch (status) { + case WINHTTP_CALLBACK_STATUS_SENDING_REQUEST: + case WINHTTP_CALLBACK_STATUS_REQUEST_SENT: case WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE: + if (status == context->call_receive_response_status) + { + context->receive_response_thread_id = GetCurrentThreadId(); + ret = WinHttpReceiveResponse( context->request, NULL ); + ok( ret, "failed to receive response, GetLastError() %lu\n", GetLastError() ); + } + break; + case WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE: + if (context->call_receive_response_status == WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE) + ok( GetCurrentThreadId() == context->receive_response_thread_id, + "expected callback to be called from the same thread, got %lx.\n", GetCurrentThreadId() ); + else + ok( GetCurrentThreadId() != context->main_thread_id, + "expected callback to be called from the other thread, got main.\n" ); + context->headers_available = TRUE; SetEvent( context->wait ); break; @@ -1911,20 +1932,39 @@ static void CALLBACK test_recursion_callback( HINTERNET handle, DWORD_PTR contex if (err == ERROR_SUCCESS) context->have_sync_callback = TRUE; InterlockedDecrement( &context->recursion_count ); break; + case WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE: + case WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED: + if (context->headers_available) + break; + if (context->headers_available) + ok( GetCurrentThreadId() == context->main_thread_id, + "expected callback to be called from the same thread, got %lx.\n", GetCurrentThreadId() ); + else if (context->call_receive_response_status == WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE) + ok( GetCurrentThreadId() == context->receive_response_thread_id, + "expected callback to be called from the same thread, got %lx.\n", GetCurrentThreadId() ); + break; } } static void test_recursion(void) { + static DWORD request_callback_status_tests[] = + { + WINHTTP_CALLBACK_STATUS_SENDING_REQUEST, + WINHTTP_CALLBACK_STATUS_REQUEST_SENT, + }; struct test_recursion_context context; HANDLE session, connection, request; DWORD size, status, err; + char buffer[1024]; + unsigned int i; BOOL ret; BYTE b; memset( &context, 0, sizeof(context) ); context.wait = CreateEventW( NULL, FALSE, FALSE, NULL ); + context.main_thread_id = GetCurrentThreadId(); session = WinHttpOpen( L"winetest", 0, NULL, NULL, WINHTTP_FLAG_ASYNC ); ok( !!session, "failed to open session, GetLastError() %lu\n", GetLastError() ); @@ -1938,6 +1978,11 @@ static void test_recursion(void) ok( !!request, "failed to open a request, GetLastError() %lu\n", GetLastError() ); context.request = request; + + ret = WinHttpReceiveResponse( request, NULL ); + ok( ret, "failed to receive response, GetLastError() %lu\n", GetLastError() ); + + context.call_receive_response_status = WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE; ret = WinHttpSendRequest( request, NULL, 0, NULL, 0, 0, (DWORD_PTR)&context ); err = GetLastError(); if (!ret && (err == ERROR_WINHTTP_CANNOT_CONNECT || err == ERROR_WINHTTP_TIMEOUT)) @@ -1954,11 +1999,6 @@ static void test_recursion(void) WaitForSingleObject( context.wait, INFINITE ); - ret = WinHttpReceiveResponse( request, NULL ); - ok( ret, "failed to receive response, GetLastError() %lu\n", GetLastError() ); - - WaitForSingleObject( context.wait, INFINITE ); - size = sizeof(status); ret = WinHttpQueryHeaders( request, WINHTTP_QUERY_STATUS_CODE | WINHTTP_QUERY_FLAG_NUMBER, NULL, &status, &size, NULL ); @@ -1981,9 +2021,45 @@ static void test_recursion(void) } else skip( "no sync callbacks\n"); - WinHttpSetStatusCallback( session, NULL, WINHTTP_CALLBACK_FLAG_ALL_NOTIFICATIONS, 0 ); - WinHttpCloseHandle( request ); + + for (i = 0; i < ARRAY_SIZE(request_callback_status_tests); ++i) + { + winetest_push_context( "i %u", i ); + + request = WinHttpOpenRequest( connection, NULL, L"/tests/hello.html", NULL, NULL, NULL, 0 ); + ok( !!request, "failed to open a request, GetLastError() %lu\n", GetLastError() ); + + context.request = request; + context.call_receive_response_status = request_callback_status_tests[i]; + context.headers_available = FALSE; + + ret = WinHttpSendRequest( request, NULL, 0, NULL, 0, 0, (DWORD_PTR)&context ); + err = GetLastError(); + if (!ret && (err == ERROR_WINHTTP_CANNOT_CONNECT || err == ERROR_WINHTTP_TIMEOUT)) + { + skip("Connection failed, skipping\n"); + WinHttpSetStatusCallback( session, NULL, WINHTTP_CALLBACK_FLAG_ALL_NOTIFICATIONS, 0 ); + WinHttpCloseHandle( request ); + WinHttpCloseHandle( connection ); + WinHttpCloseHandle( session ); + CloseHandle( context.wait ); + winetest_pop_context(); + return; + } + + WaitForSingleObject( context.wait, INFINITE ); + + ret = WinHttpReadData( request, buffer, sizeof(buffer), NULL ); + ok( ret, "failed to read data, GetLastError() %lu\n", GetLastError() ); + + WaitForSingleObject( context.wait, INFINITE ); + + WinHttpCloseHandle( request ); + winetest_pop_context(); + } + + WinHttpSetStatusCallback( session, NULL, WINHTTP_CALLBACK_FLAG_ALL_NOTIFICATIONS, 0 ); WinHttpCloseHandle( connection ); WinHttpCloseHandle( session ); CloseHandle( context.wait ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1582
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=126900 Your paranoid android. === w1064v1507 (32 bit report) === winhttp: notification.c:118: Test failed: 332: expected status 0x10 got 0x1 notification.c:118: Test failed: 332: expected status 0x10 got 0x2 notification.c:118: Test failed: 332: expected status 0x10 got 0x4 notification.c:118: Test failed: 332: expected status 0x10 got 0x8 === debian11 (32 bit report) === wldap32: parse.c:245: Test failed: ldap_get_next_page_s failed 0x51 parse.c:246: Test failed: expected res != NULL parse.c:247: Test failed: got 3735928559 Unhandled exception: page fault on read access to 0x0000001c in 32-bit code (0x6f8d7bce).
Hans Leidekker (@hans) commented about dlls/winhttp/tests/notification.c:
if (err == ERROR_SUCCESS) context->have_sync_callback = TRUE; InterlockedDecrement( &context->recursion_count ); break; + case WINHTTP_CALLBACK_STATUS_RECEIVING_RESPONSE: + case WINHTTP_CALLBACK_STATUS_RESPONSE_RECEIVED: + if (context->headers_available) + break; + if (context->headers_available) + ok( GetCurrentThreadId() == context->main_thread_id, + "expected callback to be called from the same thread, got %lx.\n", GetCurrentThreadId() );
This is probably not what you meant... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1582#note_18628
Overall I'm worried by the complexity added in this patchset. There are 8 new fields in struct request and it has become a lot harder to follow the code. It seems to me that not all of sending_request/recursive_request/receive_response_queue_state should be necessary with the right queuing model. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1582#note_18629
One way how the things could be probably made much simpler is if we could get rid of refill_buffer() in the end of reading response (that would allow simpler splitting between send and receive part). Do you happen to know why that refill_buffer() was needed?
On 5 Dec 2022, at 06:29, Hans Leidekker (@hans) <wine(a)gitlab.winehq.org> wrote:
Overall I'm worried by the complexity added in this patchset. There are 8 new fields in struct request and it has become a lot harder to follow the code. It seems to me that not all of sending_request/recursive_request/receive_response_queue_state should be necessary with the right queuing model.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1582#note_18629
On Mon Dec 5 15:20:16 2022 +0000, **** wrote:
Paul Gofman replied on the mailing list: ``` One way how the things could be probably made much simpler is if we could get rid of refill_buffer() in the end of reading response (that would allow simpler splitting between send and receive part). Do you happen to know why that refill_buffer() was needed? ``` Looks like it has to do with reporting available data in chunked mode. If you remove it you'll see get an error from WinHttpQueryDataAvailable().
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1582#note_18654
On 12/5/22 10:18, Hans Leidekker (@hans) wrote:
On Mon Dec 5 15:20:16 2022 +0000, **** wrote:
Paul Gofman replied on the mailing list: ``` One way how the things could be probably made much simpler is if we could get rid of refill_buffer() in the end of reading response (that would allow simpler splitting between send and receive part). Do you happen to know why that refill_buffer() was needed? ``` Looks like it has to do with reporting available data in chunked mode. If you remove it you'll see get an error from WinHttpQueryDataAvailable().
Thanks. I guess I will look if it is possible to remove refill buffer from receive_response and handle that in WinHttpQueryDataAvailable(). In my previous attempts to straighten up this patchset that refill_buffer() buffer in the end of receive_response() is the major source of complication. Besides, from what I saw in test_recursion() when exploring all the notifications is details, the moment when we fill it doesn't seem to match Windows anyway: there are more data receiving notifications on Windows which we don't have since we already filled the buffer when receiving response.
participants (5)
-
Hans Leidekker (＠hans) -
Marvin -
Paul Gofman -
Paul Gofman
-
Paul Gofman (＠gofman)