[PATCH v4 0/3] MR9369: wininet: Return correct errors and put handle into invalid state when seeking beyond end of file.
When `InternetSetFilePointer` is used to seek and then read beyond the size of the remote file, the initial read will fail with `ERROR_NOACCESS` and put the handle into an invalid state such that `InternetSetFilePointer`, `InternetReadFile`, and `InternetQueryDataAvailable` will return errors from then on. Based on !9644. -- v4: wininet: Return correct errors and put handle into invalid state when seeking beyond end of file. wininet/tests: Add tests for seeking beyond end of file. wininet: Move dwError to http_request_t. https://gitlab.winehq.org/wine/wine/-/merge_requests/9369
From: Jacek Caban <jacek(a)codeweavers.com> And rename it to state. --- dlls/wininet/http.c | 28 ++++++++++++++-------------- dlls/wininet/internet.h | 2 +- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c index 497a582ff89..c1bba8a6594 100644 --- a/dlls/wininet/http.c +++ b/dlls/wininet/http.c @@ -3265,10 +3265,10 @@ static DWORD HTTPREQ_ReadFile(object_header_t *hdr, void *buf, DWORD size, DWORD if(allow_blocking || TryEnterCriticalSection(&req->read_section)) { if(allow_blocking) EnterCriticalSection(&req->read_section); - if(hdr->dwError == ERROR_SUCCESS) - hdr->dwError = INTERNET_HANDLE_IN_USE; - else if(hdr->dwError == INTERNET_HANDLE_IN_USE) - hdr->dwError = ERROR_INTERNET_INTERNAL_ERROR; + if(!req->state) + req->state = INTERNET_HANDLE_IN_USE; + else if(req->state == INTERNET_HANDLE_IN_USE) + req->state = ERROR_INTERNET_INTERNAL_ERROR; if(req->read_size) { read = min(size, req->read_size); @@ -3300,10 +3300,10 @@ static DWORD HTTPREQ_ReadFile(object_header_t *hdr, void *buf, DWORD size, DWORD } } - if(hdr->dwError == INTERNET_HANDLE_IN_USE) - hdr->dwError = ERROR_SUCCESS; + if(req->state == INTERNET_HANDLE_IN_USE) + req->state = 0; else - error = hdr->dwError; + error = req->state; LeaveCriticalSection( &req->read_section ); }else { @@ -3361,10 +3361,10 @@ static DWORD HTTPREQ_QueryDataAvailable(object_header_t *hdr, DWORD *available, if(allow_blocking || TryEnterCriticalSection(&req->read_section)) { if(allow_blocking) EnterCriticalSection(&req->read_section); - if(hdr->dwError == ERROR_SUCCESS) - hdr->dwError = INTERNET_HANDLE_IN_USE; - else if(hdr->dwError == INTERNET_HANDLE_IN_USE) - hdr->dwError = ERROR_INTERNET_INTERNAL_ERROR; + if(!req->state) + req->state = INTERNET_HANDLE_IN_USE; + else if(req->state == INTERNET_HANDLE_IN_USE) + req->state = ERROR_INTERNET_INTERNAL_ERROR; avail = req->read_size; if(req->cache_size > req->content_pos) @@ -3379,10 +3379,10 @@ static DWORD HTTPREQ_QueryDataAvailable(object_header_t *hdr, DWORD *available, res = refill_read_buffer(req, allow_blocking, &avail); } - if(hdr->dwError == INTERNET_HANDLE_IN_USE) - hdr->dwError = ERROR_SUCCESS; + if(req->state == INTERNET_HANDLE_IN_USE) + req->state = 0; else - error = hdr->dwError; + error = req->state; LeaveCriticalSection( &req->read_section ); }else { diff --git a/dlls/wininet/internet.h b/dlls/wininet/internet.h index f77cadc730f..0e867529232 100644 --- a/dlls/wininet/internet.h +++ b/dlls/wininet/internet.h @@ -248,7 +248,6 @@ struct _object_header_t BOOL valid_handle; DWORD dwFlags; DWORD_PTR dwContext; - DWORD dwError; ULONG ErrorMask; DWORD dwInternalFlags; LONG refs; @@ -311,6 +310,7 @@ typedef struct { typedef struct { object_header_t hdr; + DWORD state; http_session_t *session; server_t *server; server_t *proxy; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9369
From: Torge Matthies <openglfreak(a)googlemail.com> --- dlls/wininet/tests/http.c | 93 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c index d63f42e27d7..3fa48ba8996 100644 --- a/dlls/wininet/tests/http.c +++ b/dlls/wininet/tests/http.c @@ -587,6 +587,7 @@ static void InternetSetFilePointer_test(const char *host, const char *path) { BYTE expect_response[8192], buf[8192]; HINTERNET hi = 0, hic = 0, hor = 0; + HANDLE file = INVALID_HANDLE_VALUE; BOOL res, expected; DWORD count, size, i, pos, err; @@ -706,6 +707,8 @@ static void InternetSetFilePointer_test(const char *host, const char *path) expected = pos == INVALID_SET_FILE_POINTER && err == ERROR_INTERNET_INVALID_OPERATION; ok(expected, "Expected ERROR_INTERNET_INVALID_OPERATION. Got %lu\n", err); + SET_EXPECT(INTERNET_STATUS_CLOSING_CONNECTION); + SET_EXPECT(INTERNET_STATUS_CONNECTION_CLOSED); SET_EXPECT(INTERNET_STATUS_HANDLE_CLOSING); InternetCloseHandle(hor); @@ -844,6 +847,96 @@ static void InternetSetFilePointer_test(const char *host, const char *path) CLEAR_NOTIFIED(INTERNET_STATUS_RECEIVING_RESPONSE); CLEAR_NOTIFIED(INTERNET_STATUS_RESPONSE_RECEIVED); + SET_EXPECT(INTERNET_STATUS_CLOSING_CONNECTION); + SET_EXPECT(INTERNET_STATUS_CONNECTION_CLOSED); + SET_EXPECT(INTERNET_STATUS_HANDLE_CLOSING); + InternetCloseHandle(hor); + + SET_EXPECT(INTERNET_STATUS_HANDLE_CREATED); + hor = HttpOpenRequestA(hic, NULL, path, NULL, NULL, NULL, + INTERNET_FLAG_RELOAD, + 0xdeadbead); + ok(hor != 0x0, "HttpOpenRequest failed: %lu\n", GetLastError()); + if(hor == 0x0) goto abort; + CHECK_NOTIFIED(INTERNET_STATUS_HANDLE_CREATED); + + SET_OPTIONAL(INTERNET_STATUS_CONNECTING_TO_SERVER); + SET_OPTIONAL(INTERNET_STATUS_CONNECTED_TO_SERVER); + + SET_EXPECT(INTERNET_STATUS_SENDING_REQUEST); + SET_EXPECT(INTERNET_STATUS_REQUEST_SENT); + SET_EXPECT(INTERNET_STATUS_RECEIVING_RESPONSE); + SET_EXPECT(INTERNET_STATUS_RESPONSE_RECEIVED); + + res = HttpSendRequestA(hor, NULL, 0, NULL, 0); + err = !res ? GetLastError() : NO_ERROR; + expected = res && err == NO_ERROR; + ok(expected, "HttpSendRequest failed: %lu\n", err); + + CHECK_NOTIFIED(INTERNET_STATUS_SENDING_REQUEST); + CHECK_NOTIFIED(INTERNET_STATUS_REQUEST_SENT); + CHECK_NOTIFIED(INTERNET_STATUS_RECEIVING_RESPONSE); + CHECK_NOTIFIED(INTERNET_STATUS_RESPONSE_RECEIVED); + + SetLastError(0xdeadbeef); + res = InternetReadFile(hor, buf, 10, &count); + err = !res ? GetLastError() : NO_ERROR; + ok(res, "InternetReadFile failed: %lu\n", err); + ok(count, "InternetReadFile returned no content\n"); + + SetLastError(0xdeadbeef); + pos = InternetSetFilePointer(hor, INT_MAX, NULL, FILE_BEGIN, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == INT_MAX && err == NO_ERROR; + ok(expected, "Expected position %#x. Got %#lx. GetLastError() %lu\n", INT_MAX, pos, err); + + SetLastError(0xdeadbeef); + res = InternetReadFile(hor, buf, 1024, &count); + err = !res ? GetLastError() : NO_ERROR; + todo_wine + ok(!res, "InternetReadFile succeeded unexpectedly\n"); + todo_wine + ok(err == ERROR_NOACCESS, "InternetReadFile unexpected error %lu\n", err); + + SetLastError(0xdeadbeef); + res = InternetReadFile(hor, buf, 1024, &count); + err = !res ? GetLastError() : NO_ERROR; + todo_wine + ok(!res, "InternetReadFile succeeded unexpectedly\n"); + todo_wine + ok(err == ERROR_INTERNET_INCORRECT_HANDLE_STATE, "InternetReadFile unexpected error %lu\n", err); + + SetLastError(0xdeadbeef); + pos = InternetSetFilePointer(hor, 0, NULL, FILE_BEGIN, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == INVALID_SET_FILE_POINTER && err == ERROR_INTERNET_INVALID_OPERATION; + todo_wine + ok(expected, "Expected position %#x. Got %#lx. GetLastError() %lu\n", INVALID_SET_FILE_POINTER, pos, err); + + count = 0; + SetLastError(0xdeadbeef); + res = InternetQueryDataAvailable(hor, &count, 0x0, 0x0); + err = !res ? GetLastError() : NO_ERROR; + todo_wine + ok(!res, "InternetQueryDataAvailable succeeded unexpectedly\n"); + todo_wine + ok(err == ERROR_INTERNET_INCORRECT_HANDLE_STATE, "InternetQueryDataAvailable unexpected error %lu\n", err); + todo_wine + ok(count == 0, "InternetQueryDataAvailable unexpected count: %lu\n", count); + + size = ARRAY_SIZE(buf); + res = InternetQueryOptionA(hor, INTERNET_OPTION_URL, buf, &size); + ok(res, "InternetQueryOptionA(INTERNET_OPTION_URL) failed with error %ld\n", GetLastError()); + + res = TRUE; + res = InternetSetOptionA(hor, INTERNET_OPTION_HTTP_DECODING, &res, sizeof(res)); + ok(res, "InternetSetOption(INTERNET_OPTION_HTTP_DECODING) failed: %ld\n", GetLastError()); + + res = InternetLockRequestFile(hor, &file); + if (res) + InternetUnlockRequestFile(file); + ok(res, "InternetLockRequestFile failed: %ld\n", GetLastError()); + abort: SET_OPTIONAL(INTERNET_STATUS_CLOSING_CONNECTION); SET_OPTIONAL(INTERNET_STATUS_CONNECTION_CLOSED); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9369
From: Torge Matthies <openglfreak(a)googlemail.com> --- dlls/wininet/http.c | 39 +++++++++++++++++++++++++++++++++++++++ dlls/wininet/tests/http.c | 8 -------- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c index c1bba8a6594..2ebdc0a77f4 100644 --- a/dlls/wininet/http.c +++ b/dlls/wininet/http.c @@ -3048,11 +3048,21 @@ static DWORD read_req_file(http_request_t *req, BYTE *buffer, DWORD size, DWORD LARGE_INTEGER off; BYTE buf[1024]; + if (req->state == ERROR_INTERNET_INCORRECT_HANDLE_STATE) { + *read = 0; + return ERROR_INTERNET_INCORRECT_HANDLE_STATE; + } + while (req->content_pos > req->cache_size) { res = read_http_stream(req, (BYTE*)buf, min(sizeof(buf), req->content_pos - req->cache_size), &ret_read, allow_blocking); if (res != ERROR_SUCCESS) return res; + if (!ret_read && req->content_pos > req->cache_size) { + req->state = ERROR_INTERNET_INCORRECT_HANDLE_STATE; + *read = 0; + return ERROR_NOACCESS; + } if (!ret_read) { *read = 0; return ERROR_SUCCESS; @@ -3065,6 +3075,10 @@ static DWORD read_req_file(http_request_t *req, BYTE *buffer, DWORD size, DWORD return GetLastError(); if (!ReadFile(req->req_file->file_handle, buffer, size, &ret_read, NULL)) return GetLastError(); + } else if (req->content_pos > req->cache_size) { + req->state = ERROR_INTERNET_INCORRECT_HANDLE_STATE; + *read = 0; + return ERROR_NOACCESS; } *read = ret_read; @@ -3147,12 +3161,26 @@ static void async_read_file_proc(task_header_t *hdr) TRACE("req %p buf %p size %lu read_pos %lu ret_read %p\n", req, task->buf, task->size, task->read_pos, task->ret_read); + if (req->state == ERROR_INTERNET_INCORRECT_HANDLE_STATE) { + if(task->ret_read) + *task->ret_read = 0; + send_request_complete(req, FALSE, ERROR_INTERNET_INCORRECT_HANDLE_STATE); + return; + } + if(req->req_file && req->req_file->file_handle) { DWORD ret, ret_read; BYTE buf[1024]; while (req->content_pos > req->cache_size) { ret = read_http_stream(req, (BYTE*)buf, min(sizeof(buf), req->content_pos - req->cache_size), &ret_read, TRUE); + if (!ret_read && req->content_pos > req->cache_size) { + req->state = ERROR_INTERNET_INCORRECT_HANDLE_STATE; + if(task->ret_read) + *task->ret_read = 0; + send_request_complete(req, FALSE, ERROR_NOACCESS); + return; + } if(ret != ERROR_SUCCESS || !ret_read) break; } @@ -3219,6 +3247,12 @@ static DWORD HTTPREQ_SetFilePointer(object_header_t *hdr, LONG lDistanceToMove, EnterCriticalSection(&req->read_section); + if (req->state == ERROR_INTERNET_INCORRECT_HANDLE_STATE) { + LeaveCriticalSection(&req->read_section); + SetLastError(ERROR_INTERNET_INVALID_OPERATION); + return INVALID_SET_FILE_POINTER; + } + switch (dwMoveContext) { case FILE_BEGIN: res = lDistanceToMove; @@ -3366,6 +3400,11 @@ static DWORD HTTPREQ_QueryDataAvailable(object_header_t *hdr, DWORD *available, else if(req->state == INTERNET_HANDLE_IN_USE) req->state = ERROR_INTERNET_INTERNAL_ERROR; + if (req->state == ERROR_INTERNET_INCORRECT_HANDLE_STATE) { + LeaveCriticalSection( &req->read_section ); + return ERROR_INTERNET_INCORRECT_HANDLE_STATE; + } + avail = req->read_size; if(req->cache_size > req->content_pos) avail = max(avail, req->cache_size - req->content_pos); diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c index 3fa48ba8996..42d40dc26e3 100644 --- a/dlls/wininet/tests/http.c +++ b/dlls/wininet/tests/http.c @@ -893,35 +893,27 @@ static void InternetSetFilePointer_test(const char *host, const char *path) SetLastError(0xdeadbeef); res = InternetReadFile(hor, buf, 1024, &count); err = !res ? GetLastError() : NO_ERROR; - todo_wine ok(!res, "InternetReadFile succeeded unexpectedly\n"); - todo_wine ok(err == ERROR_NOACCESS, "InternetReadFile unexpected error %lu\n", err); SetLastError(0xdeadbeef); res = InternetReadFile(hor, buf, 1024, &count); err = !res ? GetLastError() : NO_ERROR; - todo_wine ok(!res, "InternetReadFile succeeded unexpectedly\n"); - todo_wine ok(err == ERROR_INTERNET_INCORRECT_HANDLE_STATE, "InternetReadFile unexpected error %lu\n", err); SetLastError(0xdeadbeef); pos = InternetSetFilePointer(hor, 0, NULL, FILE_BEGIN, 0); err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; expected = pos == INVALID_SET_FILE_POINTER && err == ERROR_INTERNET_INVALID_OPERATION; - todo_wine ok(expected, "Expected position %#x. Got %#lx. GetLastError() %lu\n", INVALID_SET_FILE_POINTER, pos, err); count = 0; SetLastError(0xdeadbeef); res = InternetQueryDataAvailable(hor, &count, 0x0, 0x0); err = !res ? GetLastError() : NO_ERROR; - todo_wine ok(!res, "InternetQueryDataAvailable succeeded unexpectedly\n"); - todo_wine ok(err == ERROR_INTERNET_INCORRECT_HANDLE_STATE, "InternetQueryDataAvailable unexpected error %lu\n", err); - todo_wine ok(count == 0, "InternetQueryDataAvailable unexpected count: %lu\n", count); size = ARRAY_SIZE(buf); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9369
participants (3)
-
Jacek Caban -
Torge Matthies
-
Torge Matthies (＠tmatthies)