[PATCH v3 0/2] 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.
-- v3: 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.
From: Torge Matthies openglfreak@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);
From: Torge Matthies openglfreak@googlemail.com
--- dlls/wininet/http.c | 39 +++++++++++++++++++++++++++++++++++++++ dlls/wininet/internet.h | 1 + dlls/wininet/tests/http.c | 8 -------- 3 files changed, 40 insertions(+), 8 deletions(-)
diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c index 497a582ff89..588dd3a742b 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->invalid_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->invalid_state = TRUE; + *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->invalid_state = TRUE; + *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->invalid_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->invalid_state = TRUE; + 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->invalid_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(hdr->dwError == INTERNET_HANDLE_IN_USE) hdr->dwError = ERROR_INTERNET_INTERNAL_ERROR;
+ if (req->invalid_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/internet.h b/dlls/wininet/internet.h index f77cadc730f..6db8823b995 100644 --- a/dlls/wininet/internet.h +++ b/dlls/wininet/internet.h @@ -324,6 +324,7 @@ typedef struct DWORD bytesToWrite; DWORD bytesWritten; BOOL clear_auth; /* Flag to clear the password field on the authorization dialog */ + BOOL invalid_state;
CRITICAL_SECTION headers_section; /* section to protect the headers array */ HTTPHEADERW *custHeaders; 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);
On Wed Nov 12 11:21:56 2025 +0000, Torge Matthies wrote:
You mean determining that we are in this invalid state by looking at the last returned error? Since there are multiple different error values, and there may be other calls that return these errors for different conditions, we shouldn't really determine whether we're in this invalid handle state by looking at the last returned error, I'd say.
I didn't mean "last error". I was referring to the `dwError` value, which basically works as a state variable. It's in a generic header for some reason and the naming is confusing. I created !9644 to clean that up, but it still seems appropriate to use the same mechanism here if possible. I'm not sure how much the existing code was tested, so it might need some tweaks, but I think it's worth a try.
On Mon Dec 1 20:06:13 2025 +0000, Jacek Caban wrote:
I didn't mean "last error". I was referring to the `dwError` value, which basically works as a state variable. It's in a generic header for some reason and the naming is confusing. I created !9644 to clean that up, but it still seems appropriate to use the same mechanism here if possible. I'm not sure how much the existing code was tested, so it might need some tweaks, but I think it's worth a try.
Oh, that makes it clearer. It looks like that variable is also only used in `QueryDataAvailable` and `ReadFile` in the http code, so that works for this. I'll change the code to just set that variable to `ERROR_INTERNET_INCORRECT_HANDLE_STATE`, and detect this handle state from that condition.
-
Jacek Caban (＠jacek) -
Torge Matthies
-
Torge Matthies (＠tmatthies)