[PATCH v4 0/3] MR9369: wininet: Return correct errors and put handle into invalid state when seeking beyond end of file.

[Torge Matthies (＠tmatthies)]

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

[PATCH v4 1/3] wininet: Move dwError to http_request_t.

[Jacek Caban]

From: Jacek Caban

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;

[PATCH v4 2/3] wininet/tests: Add tests for seeking beyond end of file.

[Torge Matthies]

From: Torge Matthies

---
 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);

[PATCH v4 3/3] wininet: Return correct errors and put handle into invalid state when seeking beyond end of file.

[Torge Matthies]

From: Torge Matthies

---
 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);