[PATCH 1/9] winhttp: Return ERROR_INVALID_OPERATION when websocket is in the wrong state.
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- The main idea behind this series is to address a few things regarind socket shutdown and close: - avoid directly sending shutdown frame from WinHttpWebSocketClose() which doesn't cope nicely with sync / async send duality; - address the case of closing websocket which has receive waiting. - changing the socket status right away before the close or shutdown operation is complete through the queue (which is needed to straigten up the above and the following later bit which didn't fit in this series). In theory we could maybe cancel socket I/O on the socket to abort receive instead of teaching socket receive to read the close status but that seems harder to synchronize and I suppose that would complicate the things considerably without a clear advantage. The remaining part of WinHttpWebSocketClose() changes which didn't fit in in this patch series concerns the way the callback is sent when receive operation is pending during WinHttpWebSocketClose() call. Windows always calls the callback synchronously during WinHttpWebSocketClose() with dwResult 0 and dwError ERROR_WINHTTP_OPERATION_CANCELLED, the async receive thread does not call any callback in this case. dlls/winhttp/request.c | 10 +++++----- dlls/winhttp/tests/notification.c | 15 ++++++++++++++- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index 31e65a68359..fc2c06bd11f 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -3324,7 +3324,7 @@ DWORD WINAPI WinHttpWebSocketSend( HINTERNET hsocket, WINHTTP_WEB_SOCKET_BUFFER_ if (socket->state != SOCKET_STATE_OPEN) { release_object( &socket->hdr ); - return ERROR_WINHTTP_INCORRECT_HANDLE_STATE; + return ERROR_INVALID_OPERATION; } if (socket->request->connect->hdr.flags & WINHTTP_FLAG_ASYNC) @@ -3625,7 +3625,7 @@ DWORD WINAPI WinHttpWebSocketReceive( HINTERNET hsocket, void *buf, DWORD len, D if (socket->state > SOCKET_STATE_SHUTDOWN) { release_object( &socket->hdr ); - return ERROR_WINHTTP_INCORRECT_HANDLE_STATE; + return ERROR_INVALID_OPERATION; } if (socket->request->connect->hdr.flags & WINHTTP_FLAG_ASYNC) @@ -3704,7 +3704,7 @@ DWORD WINAPI WinHttpWebSocketShutdown( HINTERNET hsocket, USHORT status, void *r if (socket->state >= SOCKET_STATE_SHUTDOWN) { release_object( &socket->hdr ); - return ERROR_WINHTTP_INCORRECT_HANDLE_STATE; + return ERROR_INVALID_OPERATION; } if (socket->request->connect->hdr.flags & WINHTTP_FLAG_ASYNC) @@ -3814,7 +3814,7 @@ DWORD WINAPI WinHttpWebSocketClose( HINTERNET hsocket, USHORT status, void *reas if (socket->state >= SOCKET_STATE_CLOSED) { release_object( &socket->hdr ); - return ERROR_WINHTTP_INCORRECT_HANDLE_STATE; + return ERROR_INVALID_OPERATION; } if (socket->request->connect->hdr.flags & WINHTTP_FLAG_ASYNC) @@ -3859,7 +3859,7 @@ DWORD WINAPI WinHttpWebSocketQueryCloseStatus( HINTERNET hsocket, USHORT *status if (socket->state < SOCKET_STATE_CLOSED) { release_object( &socket->hdr ); - return ERROR_WINHTTP_INCORRECT_HANDLE_STATE; + return ERROR_INVALID_OPERATION; } *status = socket->status; diff --git a/dlls/winhttp/tests/notification.c b/dlls/winhttp/tests/notification.c index e919159ba98..0c3ff98c342 100644 --- a/dlls/winhttp/tests/notification.c +++ b/dlls/winhttp/tests/notification.c @@ -816,6 +816,12 @@ static void test_websocket(BOOL secure) ok( err == ERROR_SUCCESS, "got %u\n", err ); WaitForSingleObject( info.wait, INFINITE ); + err = pWinHttpWebSocketShutdown( socket, 1000, (void *)"success", sizeof("success") ); + ok( err == ERROR_INVALID_OPERATION, "got %u\n", err ); + err = pWinHttpWebSocketSend( socket, WINHTTP_WEB_SOCKET_BINARY_MESSAGE_BUFFER_TYPE, + (void*)"hello", sizeof("hello") ); + ok( err == ERROR_INVALID_OPERATION, "got %u\n", err ); + setup_test( &info, winhttp_websocket_receive, __LINE__ ); buffer[0] = 0; size = 0xdeadbeef; @@ -838,8 +844,15 @@ static void test_websocket(BOOL secure) ok( type == 0xdeadbeef, "got %u\n", type ); ok( buffer[0] == 'h', "unexpected data\n" ); + close_status = 0xdead; + size = sizeof(buffer) + 1; + err = pWinHttpWebSocketQueryCloseStatus( socket, &close_status, buffer, sizeof(buffer), &size ); + ok( err == ERROR_INVALID_OPERATION, "got %u\n", err ); + ok( close_status == 0xdead, "got %u\n", close_status ); + ok( size == sizeof(buffer) + 1, "got %u\n", size ); + setup_test( &info, winhttp_websocket_close, __LINE__ ); - ret = pWinHttpWebSocketClose( socket, 1000, (void *)"success", sizeof("success") ); + err = pWinHttpWebSocketClose( socket, 1000, (void *)"success", sizeof("success") ); ok( err == ERROR_SUCCESS, "got %u\n", err ); WaitForSingleObject( info.wait, INFINITE ); -- 2.34.1
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 11 +++-------- dlls/winhttp/tests/notification.c | 5 +++++ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index fc2c06bd11f..d9c5174f152 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -3652,14 +3652,7 @@ DWORD WINAPI WinHttpWebSocketReceive( HINTERNET hsocket, void *buf, DWORD len, D static DWORD socket_shutdown( struct socket *socket, USHORT status, const void *reason, DWORD len ) { - DWORD ret; - - stop_queue( &socket->send_q ); - if (!(ret = send_frame( socket, SOCKET_OPCODE_CLOSE, status, reason, len, TRUE, NULL ))) - { - socket->state = SOCKET_STATE_SHUTDOWN; - } - return ret; + return send_frame( socket, SOCKET_OPCODE_CLOSE, status, reason, len, TRUE, NULL ); } static void CALLBACK task_socket_shutdown( TP_CALLBACK_INSTANCE *instance, void *ctx, TP_WORK *work ) @@ -3707,6 +3700,8 @@ DWORD WINAPI WinHttpWebSocketShutdown( HINTERNET hsocket, USHORT status, void *r return ERROR_INVALID_OPERATION; } + socket->state = SOCKET_STATE_SHUTDOWN; + if (socket->request->connect->hdr.flags & WINHTTP_FLAG_ASYNC) { struct socket_shutdown *s; diff --git a/dlls/winhttp/tests/notification.c b/dlls/winhttp/tests/notification.c index 0c3ff98c342..4127f9933bb 100644 --- a/dlls/winhttp/tests/notification.c +++ b/dlls/winhttp/tests/notification.c @@ -814,6 +814,11 @@ static void test_websocket(BOOL secure) setup_test( &info, winhttp_websocket_shutdown, __LINE__ ); err = pWinHttpWebSocketShutdown( socket, 1000, (void *)"success", sizeof("success") ); ok( err == ERROR_SUCCESS, "got %u\n", err ); + + err = pWinHttpWebSocketSend( socket, WINHTTP_WEB_SOCKET_BINARY_MESSAGE_BUFFER_TYPE, + (void*)"hello", sizeof("hello") ); + ok( err == ERROR_INVALID_OPERATION, "got %u\n", err ); + WaitForSingleObject( info.wait, INFINITE ); err = pWinHttpWebSocketShutdown( socket, 1000, (void *)"success", sizeof("success") ); -- 2.34.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=105875 Your paranoid android. === w8adm (32 bit report) === winhttp: notification.c:248: Test failed: failed to send request 12007 notification.c:250: Test failed: unexpected function 3, expected 4. probably some notifications were missing notification.c:252: Test failed: failed to receive response 12019 notification.c:256: Test failed: failed unexpectedly 12019 notification.c:257: Test failed: request failed unexpectedly 1 notification.c:260: Test failed: unexpected function 3, expected 13. probably some notifications were missing notification.c:113: Test failed: 260: expected status 0x00000002 got 0x00000800 notification.c:114: Test failed: 260: expected function 3 got 13 notification: Timeout
Signed-off-by: Hans Leidekker <hans(a)codeweavers.com>
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 54 +++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index d9c5174f152..51e450c5ff1 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -3650,11 +3650,6 @@ DWORD WINAPI WinHttpWebSocketReceive( HINTERNET hsocket, void *buf, DWORD len, D return ret; } -static DWORD socket_shutdown( struct socket *socket, USHORT status, const void *reason, DWORD len ) -{ - return send_frame( socket, SOCKET_OPCODE_CLOSE, status, reason, len, TRUE, NULL ); -} - static void CALLBACK task_socket_shutdown( TP_CALLBACK_INSTANCE *instance, void *ctx, TP_WORK *work ) { struct socket_shutdown *s = ctx; @@ -3662,7 +3657,7 @@ static void CALLBACK task_socket_shutdown( TP_CALLBACK_INSTANCE *instance, void TRACE("running %p\n", work); - ret = socket_shutdown( s->socket, s->status, s->reason, s->len ); + ret = send_frame( s->socket, SOCKET_OPCODE_CLOSE, s->status, s->reason, s->len, TRUE, NULL ); send_io_complete( &s->socket->hdr ); if (!ret) send_callback( &s->socket->hdr, WINHTTP_CALLBACK_STATUS_SHUTDOWN_COMPLETE, NULL, 0 ); @@ -3679,27 +3674,11 @@ static void CALLBACK task_socket_shutdown( TP_CALLBACK_INSTANCE *instance, void free( s ); } -DWORD WINAPI WinHttpWebSocketShutdown( HINTERNET hsocket, USHORT status, void *reason, DWORD len ) +static DWORD send_socket_shutdown( struct socket *socket, USHORT status, const void *reason, DWORD len, + BOOL send_callback) { - struct socket *socket; DWORD ret; - TRACE("%p, %u, %p, %u\n", hsocket, status, reason, len); - - if ((len && !reason) || len > sizeof(socket->reason)) return ERROR_INVALID_PARAMETER; - - if (!(socket = (struct socket *)grab_object( hsocket ))) return ERROR_INVALID_HANDLE; - if (socket->hdr.type != WINHTTP_HANDLE_TYPE_SOCKET) - { - release_object( &socket->hdr ); - return ERROR_WINHTTP_INCORRECT_HANDLE_TYPE; - } - if (socket->state >= SOCKET_STATE_SHUTDOWN) - { - release_object( &socket->hdr ); - return ERROR_INVALID_OPERATION; - } - socket->state = SOCKET_STATE_SHUTDOWN; if (socket->request->connect->hdr.flags & WINHTTP_FLAG_ASYNC) @@ -3721,8 +3700,33 @@ DWORD WINAPI WinHttpWebSocketShutdown( HINTERNET hsocket, USHORT status, void *r free( s ); } } - else ret = socket_shutdown( socket, status, reason, len ); + else ret = send_frame( socket, SOCKET_OPCODE_CLOSE, status, reason, len, TRUE, NULL ); + + return ret; +} + +DWORD WINAPI WinHttpWebSocketShutdown( HINTERNET hsocket, USHORT status, void *reason, DWORD len ) +{ + struct socket *socket; + DWORD ret; + + TRACE("%p, %u, %p, %u\n", hsocket, status, reason, len); + + if ((len && !reason) || len > sizeof(socket->reason)) return ERROR_INVALID_PARAMETER; + + if (!(socket = (struct socket *)grab_object( hsocket ))) return ERROR_INVALID_HANDLE; + if (socket->hdr.type != WINHTTP_HANDLE_TYPE_SOCKET) + { + release_object( &socket->hdr ); + return ERROR_WINHTTP_INCORRECT_HANDLE_TYPE; + } + if (socket->state >= SOCKET_STATE_SHUTDOWN) + { + release_object( &socket->hdr ); + return ERROR_INVALID_OPERATION; + } + ret = send_socket_shutdown( socket, status, reason, len, TRUE ); release_object( &socket->hdr ); return ret; } -- 2.34.1
Signed-off-by: Hans Leidekker <hans(a)codeweavers.com>
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 41 ++++++++++++++++------------------ dlls/winhttp/winhttp_private.h | 1 + 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index 51e450c5ff1..893afdbfb52 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -3660,16 +3660,18 @@ static void CALLBACK task_socket_shutdown( TP_CALLBACK_INSTANCE *instance, void ret = send_frame( s->socket, SOCKET_OPCODE_CLOSE, s->status, s->reason, s->len, TRUE, NULL ); send_io_complete( &s->socket->hdr ); - if (!ret) send_callback( &s->socket->hdr, WINHTTP_CALLBACK_STATUS_SHUTDOWN_COMPLETE, NULL, 0 ); - else + if (s->send_callback) { - WINHTTP_WEB_SOCKET_ASYNC_RESULT result; - result.AsyncResult.dwResult = API_WRITE_DATA; - result.AsyncResult.dwError = ret; - result.Operation = WINHTTP_WEB_SOCKET_SHUTDOWN_OPERATION; - send_callback( &s->socket->hdr, WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, &result, sizeof(result) ); + if (!ret) send_callback( &s->socket->hdr, WINHTTP_CALLBACK_STATUS_SHUTDOWN_COMPLETE, NULL, 0 ); + else + { + WINHTTP_WEB_SOCKET_ASYNC_RESULT result; + result.AsyncResult.dwResult = API_WRITE_DATA; + result.AsyncResult.dwError = ret; + result.Operation = WINHTTP_WEB_SOCKET_SHUTDOWN_OPERATION; + send_callback( &s->socket->hdr, WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, &result, sizeof(result) ); + } } - release_object( &s->socket->hdr ); free( s ); } @@ -3690,6 +3692,7 @@ static DWORD send_socket_shutdown( struct socket *socket, USHORT status, const v s->status = status; memcpy( s->reason, reason, len ); s->len = len; + s->send_callback = send_callback; addref_object( &socket->hdr ); InterlockedIncrement( &socket->hdr.pending_sends ); @@ -3731,19 +3734,12 @@ DWORD WINAPI WinHttpWebSocketShutdown( HINTERNET hsocket, USHORT status, void *r return ret; } -static DWORD socket_close( struct socket *socket, USHORT status, const void *reason, DWORD len, BOOL async ) +static DWORD socket_close( struct socket *socket, BOOL async ) { DWORD ret, count; if ((ret = socket_drain( socket ))) goto done; - if (socket->state < SOCKET_STATE_SHUTDOWN) - { - stop_queue( &socket->send_q ); - if ((ret = send_frame( socket, SOCKET_OPCODE_CLOSE, status, reason, len, TRUE, NULL ))) goto done; - socket->state = SOCKET_STATE_SHUTDOWN; - } - while (1) { if ((ret = receive_frame( socket, &count, &socket->opcode ))) goto done; @@ -3788,7 +3784,7 @@ static void CALLBACK task_socket_close( TP_CALLBACK_INSTANCE *instance, void *ct { struct socket_shutdown *s = ctx; - socket_close( s->socket, s->status, s->reason, s->len, TRUE ); + socket_close( s->socket, TRUE ); TRACE("running %p\n", work); release_object( &s->socket->hdr ); @@ -3816,15 +3812,15 @@ DWORD WINAPI WinHttpWebSocketClose( HINTERNET hsocket, USHORT status, void *reas return ERROR_INVALID_OPERATION; } + if (socket->state < SOCKET_STATE_SHUTDOWN + && (ret = send_socket_shutdown( socket, status, reason, len, FALSE ))) goto done; + if (socket->request->connect->hdr.flags & WINHTTP_FLAG_ASYNC) { struct socket_shutdown *s; - if (!(s = malloc( sizeof(*s) ))) return FALSE; + if (!(s = calloc( 1, sizeof(*s) ))) return FALSE; s->socket = socket; - s->status = status; - memcpy( s->reason, reason, len ); - s->len = len; addref_object( &socket->hdr ); if ((ret = queue_task( &socket->recv_q, task_socket_close, s ))) @@ -3833,8 +3829,9 @@ DWORD WINAPI WinHttpWebSocketClose( HINTERNET hsocket, USHORT status, void *reas free( s ); } } - else ret = socket_close( socket, status, reason, len, FALSE ); + else ret = socket_close( socket, FALSE ); +done: release_object( &socket->hdr ); return ret; } diff --git a/dlls/winhttp/winhttp_private.h b/dlls/winhttp/winhttp_private.h index 7c904071a14..ef52944e412 100644 --- a/dlls/winhttp/winhttp_private.h +++ b/dlls/winhttp/winhttp_private.h @@ -314,6 +314,7 @@ struct socket_shutdown USHORT status; char reason[123]; DWORD len; + BOOL send_callback; }; struct object_header *addref_object( struct object_header * ) DECLSPEC_HIDDEN; -- 2.34.1
Signed-off-by: Hans Leidekker <hans(a)codeweavers.com>
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 47 +++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index 893afdbfb52..b31ba2480bb 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -3734,57 +3734,52 @@ DWORD WINAPI WinHttpWebSocketShutdown( HINTERNET hsocket, USHORT status, void *r return ret; } -static DWORD socket_close( struct socket *socket, BOOL async ) +static DWORD socket_close( struct socket *socket ) { DWORD ret, count; - if ((ret = socket_drain( socket ))) goto done; + if ((ret = socket_drain( socket ))) return ret; while (1) { - if ((ret = receive_frame( socket, &count, &socket->opcode ))) goto done; + if ((ret = receive_frame( socket, &count, &socket->opcode ))) return ret; if (socket->opcode == SOCKET_OPCODE_CLOSE) break; socket->read_size = count; - if ((ret = socket_drain( socket ))) goto done; + if ((ret = socket_drain( socket ))) return ret; } if ((count && (count < sizeof(socket->status) || count > sizeof(socket->status) + sizeof(socket->reason)))) - { - ret = ERROR_WINHTTP_INVALID_SERVER_RESPONSE; - goto done; - } + return ERROR_WINHTTP_INVALID_SERVER_RESPONSE; if (count) { DWORD reason_len = count - sizeof(socket->status); - if ((ret = receive_bytes( socket, (char *)&socket->status, sizeof(socket->status), &count, TRUE ))) goto done; + if ((ret = receive_bytes( socket, (char *)&socket->status, sizeof(socket->status), &count, TRUE ))) return ret; socket->status = RtlUshortByteSwap( socket->status ); - if ((ret = receive_bytes( socket, socket->reason, reason_len, &socket->reason_len, TRUE ))) goto done; + if ((ret = receive_bytes( socket, socket->reason, reason_len, &socket->reason_len, TRUE ))) return ret; } socket->state = SOCKET_STATE_CLOSED; -done: - if (async) - { - if (!ret) send_callback( &socket->hdr, WINHTTP_CALLBACK_STATUS_CLOSE_COMPLETE, NULL, 0 ); - else - { - WINHTTP_WEB_SOCKET_ASYNC_RESULT result; - result.AsyncResult.dwResult = API_READ_DATA; /* FIXME */ - result.AsyncResult.dwError = ret; - result.Operation = WINHTTP_WEB_SOCKET_CLOSE_OPERATION; - send_callback( &socket->hdr, WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, &result, sizeof(result) ); - } - } - return ret; + return ERROR_SUCCESS; } static void CALLBACK task_socket_close( TP_CALLBACK_INSTANCE *instance, void *ctx, TP_WORK *work ) { struct socket_shutdown *s = ctx; + DWORD ret; + + ret = socket_close( s->socket ); - socket_close( s->socket, TRUE ); + if (!ret) send_callback( &s->socket->hdr, WINHTTP_CALLBACK_STATUS_CLOSE_COMPLETE, NULL, 0 ); + else + { + WINHTTP_WEB_SOCKET_ASYNC_RESULT result; + result.AsyncResult.dwResult = API_READ_DATA; /* FIXME */ + result.AsyncResult.dwError = ret; + result.Operation = WINHTTP_WEB_SOCKET_CLOSE_OPERATION; + send_callback( &s->socket->hdr, WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, &result, sizeof(result) ); + } TRACE("running %p\n", work); release_object( &s->socket->hdr ); @@ -3829,7 +3824,7 @@ DWORD WINAPI WinHttpWebSocketClose( HINTERNET hsocket, USHORT status, void *reas free( s ); } } - else ret = socket_close( socket, FALSE ); + else ret = socket_close( socket ); done: release_object( &socket->hdr ); -- 2.34.1
Signed-off-by: Hans Leidekker <hans(a)codeweavers.com>
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index b31ba2480bb..890a5a645e7 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -3505,6 +3505,22 @@ static DWORD socket_drain( struct socket *socket ) return ERROR_SUCCESS; } +static DWORD receive_close_status( struct socket *socket, unsigned int len ) +{ + DWORD reason_len, ret; + + if ((len && (len < sizeof(socket->status) || len > sizeof(socket->status) + sizeof(socket->reason)))) + return ERROR_WINHTTP_INVALID_SERVER_RESPONSE; + + if (!len) return ERROR_SUCCESS; + + reason_len = len - sizeof(socket->status); + if ((ret = receive_bytes( socket, (char *)&socket->status, sizeof(socket->status), &len, TRUE ))) + return ret; + socket->status = RtlUshortByteSwap( socket->status ); + return receive_bytes( socket, socket->reason, reason_len, &socket->reason_len, TRUE ); +} + static DWORD handle_control_frame( struct socket *socket ) { switch (socket->opcode) @@ -3749,19 +3765,8 @@ static DWORD socket_close( struct socket *socket ) if ((ret = socket_drain( socket ))) return ret; } - if ((count && (count < sizeof(socket->status) || count > sizeof(socket->status) + sizeof(socket->reason)))) - return ERROR_WINHTTP_INVALID_SERVER_RESPONSE; - - if (count) - { - DWORD reason_len = count - sizeof(socket->status); - if ((ret = receive_bytes( socket, (char *)&socket->status, sizeof(socket->status), &count, TRUE ))) return ret; - socket->status = RtlUshortByteSwap( socket->status ); - if ((ret = receive_bytes( socket, socket->reason, reason_len, &socket->reason_len, TRUE ))) return ret; - } socket->state = SOCKET_STATE_CLOSED; - - return ERROR_SUCCESS; + return receive_close_status( socket, count ); } static void CALLBACK task_socket_close( TP_CALLBACK_INSTANCE *instance, void *ctx, TP_WORK *work ) -- 2.34.1
Signed-off-by: Hans Leidekker <hans(a)codeweavers.com>
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 27 +++++++++++++++++++++++---- dlls/winhttp/winhttp_private.h | 2 ++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index 890a5a645e7..cb2db948278 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -3509,20 +3509,24 @@ static DWORD receive_close_status( struct socket *socket, unsigned int len ) { DWORD reason_len, ret; + socket->close_frame_received = TRUE; if ((len && (len < sizeof(socket->status) || len > sizeof(socket->status) + sizeof(socket->reason)))) - return ERROR_WINHTTP_INVALID_SERVER_RESPONSE; + return (socket->close_frame_receive_err = ERROR_WINHTTP_INVALID_SERVER_RESPONSE); - if (!len) return ERROR_SUCCESS; + if (!len) return (socket->close_frame_receive_err = ERROR_SUCCESS); reason_len = len - sizeof(socket->status); if ((ret = receive_bytes( socket, (char *)&socket->status, sizeof(socket->status), &len, TRUE ))) - return ret; + return (socket->close_frame_receive_err = ret); socket->status = RtlUshortByteSwap( socket->status ); - return receive_bytes( socket, socket->reason, reason_len, &socket->reason_len, TRUE ); + return (socket->close_frame_receive_err + = receive_bytes( socket, socket->reason, reason_len, &socket->reason_len, TRUE )); } static DWORD handle_control_frame( struct socket *socket ) { + TRACE( "opcode %u.\n", socket->opcode ); + switch (socket->opcode) { case SOCKET_OPCODE_PING: @@ -3531,6 +3535,19 @@ static DWORD handle_control_frame( struct socket *socket ) case SOCKET_OPCODE_PONG: return socket_drain( socket ); + case SOCKET_OPCODE_CLOSE: + if (socket->state != SOCKET_STATE_CLOSED) + WARN( "SOCKET_OPCODE_CLOSE received, socket->state %u.\n", socket->state ); + if (socket->close_frame_received) + { + FIXME( "Close frame already received.\n" ); + return ERROR_WINHTTP_INVALID_SERVER_RESPONSE; + } + + receive_close_status( socket, socket->read_size ); + socket->read_size = 0; + return ERROR_WINHTTP_INVALID_SERVER_RESPONSE; + default: ERR("unhandled control opcode %02x\n", socket->opcode); return ERROR_WINHTTP_INVALID_SERVER_RESPONSE; @@ -3754,6 +3771,8 @@ static DWORD socket_close( struct socket *socket ) { DWORD ret, count; + if (socket->close_frame_received) return socket->close_frame_receive_err; + if ((ret = socket_drain( socket ))) return ret; while (1) diff --git a/dlls/winhttp/winhttp_private.h b/dlls/winhttp/winhttp_private.h index ef52944e412..5eb3e97a916 100644 --- a/dlls/winhttp/winhttp_private.h +++ b/dlls/winhttp/winhttp_private.h @@ -246,6 +246,8 @@ struct socket struct queue recv_q; enum socket_opcode opcode; DWORD read_size; + BOOL close_frame_received; + DWORD close_frame_receive_err; USHORT status; char reason[123]; DWORD reason_len; -- 2.34.1
Signed-off-by: Hans Leidekker <hans(a)codeweavers.com>
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/tests/notification.c | 101 ++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/dlls/winhttp/tests/notification.c b/dlls/winhttp/tests/notification.c index 4127f9933bb..88c6918cd98 100644 --- a/dlls/winhttp/tests/notification.c +++ b/dlls/winhttp/tests/notification.c @@ -676,6 +676,29 @@ static const struct notification websocket_test[] = { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING }, { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_CLOSING_CONNECTION, NF_WINE_ALLOW }, { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_CONNECTION_CLOSED, NF_WINE_ALLOW }, + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING, NF_SIGNAL }, +}; + +static const struct notification websocket_test2[] = +{ + { 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 }, + { 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_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_websocket_receive, WINHTTP_CALLBACK_STATUS_READ_COMPLETE, NF_SIGNAL }, + { winhttp_websocket_close, WINHTTP_CALLBACK_STATUS_REQUEST_ERROR }, + { winhttp_websocket_close, WINHTTP_CALLBACK_STATUS_CLOSE_COMPLETE, NF_SIGNAL }, + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING }, + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_CLOSING_CONNECTION, NF_WINE_ALLOW }, + { winhttp_close_handle, WINHTTP_CALLBACK_STATUS_CONNECTION_CLOSED, NF_WINE_ALLOW }, { 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 } @@ -869,6 +892,84 @@ static void test_websocket(BOOL secure) ok( size <= sizeof(buffer), "got %u\n", size ); setup_test( &info, winhttp_close_handle, __LINE__ ); + WinHttpCloseHandle( socket ); + WinHttpCloseHandle( request ); + + WaitForSingleObject( info.wait, INFINITE ); + end_test( &info, __LINE__ ); + + /* Test socket close while receive is pending. */ + info.test = websocket_test2; + info.count = ARRAY_SIZE( websocket_test2 ); + info.index = 0; + + setup_test( &info, winhttp_open_request, __LINE__ ); + request = WinHttpOpenRequest( connection, NULL, L"/", NULL, NULL, NULL, secure ? WINHTTP_FLAG_SECURE : 0); + ok( request != NULL, "got %u\n", err ); + + if (secure) + { + flags = SECURITY_FLAG_IGNORE_UNKNOWN_CA | SECURITY_FLAG_IGNORE_CERT_DATE_INVALID | + SECURITY_FLAG_IGNORE_CERT_CN_INVALID; + ret = WinHttpSetOption(request, WINHTTP_OPTION_SECURITY_FLAGS, &flags, sizeof(flags)); + ok(ret, "failed to set security flags %u\n", GetLastError()); + } + + ret = WinHttpSetOption( request, WINHTTP_OPTION_UPGRADE_TO_WEB_SOCKET, NULL, 0 ); + ok( ret, "got %u\n", GetLastError() ); + + setup_test( &info, winhttp_send_request, __LINE__ ); + ret = WinHttpSendRequest( request, NULL, 0, NULL, 0, 0, 0 ); + ok( ret, "got %u\n", GetLastError() ); + WaitForSingleObject( info.wait, INFINITE ); + + setup_test( &info, winhttp_receive_response, __LINE__ ); + ret = WinHttpReceiveResponse( request, NULL ); + ok( ret, "got %u\n", err ); + WaitForSingleObject( info.wait, INFINITE ); + + size = sizeof(status); + ret = WinHttpQueryHeaders( request, WINHTTP_QUERY_STATUS_CODE|WINHTTP_QUERY_FLAG_NUMBER, NULL, &status, &size, NULL ); + ok( ret, "failed unexpectedly %u\n", err ); + ok( status == 101, "got %u\n", status ); + + setup_test( &info, winhttp_websocket_complete_upgrade, __LINE__ ); + socket = pWinHttpWebSocketCompleteUpgrade( request, (DWORD_PTR)context ); + ok( socket != NULL, "got %u\n", err ); + WaitForSingleObject( info.wait, INFINITE ); + + setup_test( &info, winhttp_websocket_receive, __LINE__ ); + buffer[0] = 0; + err = pWinHttpWebSocketReceive( socket, buffer, sizeof(buffer), &size, &type ); + ok( err == ERROR_SUCCESS, "got %u\n", err ); + WaitForSingleObject( info.wait, INFINITE ); + ok( buffer[0] == 'R', "unexpected data\n" ); + + err = pWinHttpWebSocketReceive( socket, buffer, sizeof(buffer), &size, &type ); + ok( err == ERROR_SUCCESS, "got %u\n", err ); + + setup_test( &info, winhttp_websocket_close, __LINE__ ); + ret = pWinHttpWebSocketClose( socket, 1000, (void *)"success", sizeof("success") ); + ok( err == ERROR_SUCCESS, "got %u\n", err ); + + close_status = 0xdead; + size = sizeof(buffer) + 1; + err = pWinHttpWebSocketQueryCloseStatus( socket, &close_status, buffer, sizeof(buffer), &size ); + ok( err == ERROR_INVALID_OPERATION, "got %u\n", err ); + ok( close_status == 0xdead, "got %u\n", close_status ); + ok( size == sizeof(buffer) + 1, "got %u\n", size ); + + WaitForSingleObject( info.wait, INFINITE ); + + close_status = 0xdead; + size = sizeof(buffer) + 1; + err = pWinHttpWebSocketQueryCloseStatus( socket, &close_status, buffer, sizeof(buffer), &size ); + todo_wine ok( err == ERROR_SUCCESS, "got %u\n", err ); + todo_wine ok( close_status == 1000, "got %u\n", close_status ); + todo_wine ok( size <= sizeof(buffer), "got %u\n", size ); + + setup_test( &info, winhttp_close_handle, __LINE__ ); + WinHttpCloseHandle( socket ); WinHttpCloseHandle( request ); WinHttpCloseHandle( connection ); -- 2.34.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=105881 Your paranoid android. === w8adm (32 bit report) === winhttp: notification.c:248: Test failed: failed to send request 12007 notification.c:250: Test failed: unexpected function 3, expected 4. probably some notifications were missing notification.c:252: Test failed: failed to receive response 12019 notification.c:256: Test failed: failed unexpectedly 12019 notification.c:257: Test failed: request failed unexpectedly 1 notification.c:260: Test failed: unexpected function 3, expected 13. probably some notifications were missing notification.c:113: Test failed: 260: expected status 0x00000002 got 0x00000800 notification.c:114: Test failed: 260: expected function 3 got 13 notification: Timeout
Signed-off-by: Hans Leidekker <hans(a)codeweavers.com>
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 2 +- dlls/winhttp/tests/notification.c | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index cb2db948278..f37c33b202c 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -3784,7 +3784,6 @@ static DWORD socket_close( struct socket *socket ) if ((ret = socket_drain( socket ))) return ret; } - socket->state = SOCKET_STATE_CLOSED; return receive_close_status( socket, count ); } @@ -3834,6 +3833,7 @@ DWORD WINAPI WinHttpWebSocketClose( HINTERNET hsocket, USHORT status, void *reas if (socket->state < SOCKET_STATE_SHUTDOWN && (ret = send_socket_shutdown( socket, status, reason, len, FALSE ))) goto done; + socket->state = SOCKET_STATE_CLOSED; if (socket->request->connect->hdr.flags & WINHTTP_FLAG_ASYNC) { struct socket_shutdown *s; diff --git a/dlls/winhttp/tests/notification.c b/dlls/winhttp/tests/notification.c index 88c6918cd98..2707151e00f 100644 --- a/dlls/winhttp/tests/notification.c +++ b/dlls/winhttp/tests/notification.c @@ -955,18 +955,18 @@ static void test_websocket(BOOL secure) close_status = 0xdead; size = sizeof(buffer) + 1; err = pWinHttpWebSocketQueryCloseStatus( socket, &close_status, buffer, sizeof(buffer), &size ); - ok( err == ERROR_INVALID_OPERATION, "got %u\n", err ); - ok( close_status == 0xdead, "got %u\n", close_status ); - ok( size == sizeof(buffer) + 1, "got %u\n", size ); + todo_wine ok( err == ERROR_INVALID_OPERATION, "got %u\n", err ); + todo_wine ok( close_status == 0xdead, "got %u\n", close_status ); + todo_wine ok( size == sizeof(buffer) + 1, "got %u\n", size ); WaitForSingleObject( info.wait, INFINITE ); close_status = 0xdead; size = sizeof(buffer) + 1; err = pWinHttpWebSocketQueryCloseStatus( socket, &close_status, buffer, sizeof(buffer), &size ); - todo_wine ok( err == ERROR_SUCCESS, "got %u\n", err ); - todo_wine ok( close_status == 1000, "got %u\n", close_status ); - todo_wine ok( size <= sizeof(buffer), "got %u\n", size ); + ok( err == ERROR_SUCCESS, "got %u\n", err ); + ok( close_status == 1000, "got %u\n", close_status ); + ok( size <= sizeof(buffer), "got %u\n", size ); setup_test( &info, winhttp_close_handle, __LINE__ ); -- 2.34.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=105882 Your paranoid android. === w8adm (32 bit report) === winhttp: notification.c:248: Test failed: failed to send request 12007 notification.c:250: Test failed: unexpected function 3, expected 4. probably some notifications were missing notification.c:252: Test failed: failed to receive response 12019 notification.c:256: Test failed: failed unexpectedly 12019 notification.c:257: Test failed: request failed unexpectedly 1 notification.c:260: Test failed: unexpected function 3, expected 13. probably some notifications were missing notification.c:113: Test failed: 260: expected status 0x00000002 got 0x00000800 notification.c:114: Test failed: 260: expected function 3 got 13 notification: Timeout
Signed-off-by: Hans Leidekker <hans(a)codeweavers.com>
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=105874 Your paranoid android. === w8adm (32 bit report) === winhttp: notification.c:248: Test failed: failed to send request 12007 notification.c:250: Test failed: unexpected function 3, expected 4. probably some notifications were missing notification.c:252: Test failed: failed to receive response 12019 notification.c:256: Test failed: failed unexpectedly 12019 notification.c:257: Test failed: request failed unexpectedly 1 notification.c:260: Test failed: unexpected function 3, expected 13. probably some notifications were missing notification.c:113: Test failed: 260: expected status 0x00000002 got 0x00000800 notification.c:114: Test failed: 260: expected function 3 got 13 notification: Timeout
participants (3)
-
Hans Leidekker -
Marvin -
Paul Gofman