[PATCH v11 0/2] MR1268: wininet: Partially implement InternetSetFilePointer
-- v11: wininet: Partially implement InternetSetFilePointer wininet/tests: Add InternetSetFilePointer tests. https://gitlab.winehq.org/wine/wine/-/merge_requests/1268
From: Jason Kuo <j20001970(a)gmail.com> --- dlls/wininet/tests/http.c | 243 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 243 insertions(+) diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c index 8fcb9df293a..667ee74fadb 100644 --- a/dlls/wininet/tests/http.c +++ b/dlls/wininet/tests/http.c @@ -581,6 +581,247 @@ static void close_async_handle(HINTERNET handle, int handle_cnt) CHECK_NOTIFIED2(INTERNET_STATUS_HANDLE_CLOSING, handle_cnt); } +static void InternetSetFilePointer_test(const char *host, const char *path) +{ +#ifndef BUFFER_SIZE +#define BUFFER_SIZE 8192 +#endif + + CHAR expect_response[BUFFER_SIZE], buf[BUFFER_SIZE]; + HINTERNET hi = 0, hic = 0, hor = 0; + BOOL res, expected; + DWORD count, size, i, pos, err; + + hi = InternetOpenA("Winetest", INTERNET_OPEN_TYPE_DIRECT, NULL, NULL, 0); + ok((hi != 0x0), "InternetOpen failed: %lu\n", GetLastError()); + if(hi == 0x0) goto abort; + + hic = InternetConnectA(hi, host, INTERNET_DEFAULT_HTTP_PORT, + NULL, NULL, INTERNET_SERVICE_HTTP, 0, 0); + ok((hic != 0x0), "InternetConnect failed: %lu\n", GetLastError()); + if(hic == 0x0) goto abort; + + hor = HttpOpenRequestA(hic, NULL, path, NULL, NULL, NULL, + INTERNET_FLAG_RELOAD, + 0x0); + ok((hor != 0x0), "HttpOpenRequest failed: %lu\n", GetLastError()); + if(hor == 0x0) goto abort; + + res = HttpSendRequestA(hor, NULL, 0, NULL, 0); + ok(res, "HttpSendRequest failed: %lu\n", GetLastError()); + if(!res) goto abort; + + size = 0; + while(size < BUFFER_SIZE) { + res = InternetReadFile(hor, expect_response+size, BUFFER_SIZE-size, &count); + if(!res || !count) + break; + size += count; + } + ok(size, "InternetReadFile returned no content\n"); + if(!size) goto abort; + + if(hor) InternetCloseHandle(hor); + if(hic) InternetCloseHandle(hic); + if(hi) InternetCloseHandle(hi); + + reset_events(); + + hi = InternetOpenA("Winetest", INTERNET_OPEN_TYPE_DIRECT, NULL, NULL, 0); + ok((hi != 0x0), "InternetOpen failed: %lu\n", GetLastError()); + if(hi == 0x0) goto abort; + + pInternetSetStatusCallbackA(hi, &callback); + + SET_EXPECT(INTERNET_STATUS_HANDLE_CREATED); + hic = InternetConnectA(hi, host, INTERNET_DEFAULT_HTTP_PORT, + NULL, NULL, INTERNET_SERVICE_HTTP, 0, 0xdeadbeef); + ok((hic != 0x0), "InternetConnect failed: %lu\n", GetLastError()); + if(hic == 0x0) goto abort; + + SET_EXPECT(INTERNET_STATUS_HANDLE_CREATED); + hor = HttpOpenRequestA(hic, NULL, path, NULL, NULL, NULL, + INTERNET_FLAG_RELOAD|INTERNET_FLAG_DONT_CACHE, + 0xdeadbead); + ok((hor != 0x0), "HttpOpenRequest failed: %lu\n", GetLastError()); + if(hor == 0x0) goto abort; + + /* NULL handle tests */ + pos = InternetSetFilePointer(NULL, 0, NULL, FILE_BEGIN, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == INVALID_SET_FILE_POINTER && err == ERROR_INVALID_HANDLE; + ok(expected, "Expected ERROR_INVALID_HANDLE. Got %lu\n", err); + pos = InternetSetFilePointer(NULL, 0, NULL, FILE_CURRENT, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == INVALID_SET_FILE_POINTER && err == ERROR_INVALID_HANDLE; + ok(expected, "Expected ERROR_INVALID_HANDLE. Got %lu\n", err); + pos = InternetSetFilePointer(NULL, 0, NULL, FILE_END, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == INVALID_SET_FILE_POINTER && err == ERROR_INVALID_HANDLE; + ok(expected, "Expected ERROR_INVALID_HANDLE. Got %lu\n", err); + + /* INTERNET_FLAG_DONT_CACHE tests */ + 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; + ok(expected, "Expected ERROR_INTERNET_INVALID_OPERATION. Got %lu\n", err); + pos = InternetSetFilePointer(hor, 0, NULL, FILE_CURRENT, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == INVALID_SET_FILE_POINTER && err == ERROR_INTERNET_INVALID_OPERATION; + ok(expected, "Expected ERROR_INTERNET_INVALID_OPERATION. Got %lu\n", err); + pos = InternetSetFilePointer(hor, 0, NULL, FILE_END, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == INVALID_SET_FILE_POINTER && err == ERROR_INTERNET_INVALID_OPERATION; + ok(expected, "Expected ERROR_INTERNET_INVALID_OPERATION. Got %lu\n", err); + if(hor) { + 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_NOT_NOTIFIED(INTERNET_STATUS_RESOLVING_NAME); + CHECK_NOT_NOTIFIED(INTERNET_STATUS_NAME_RESOLVED); + if (first_connection_to_test_url) + { + SET_EXPECT(INTERNET_STATUS_RESOLVING_NAME); + SET_EXPECT(INTERNET_STATUS_NAME_RESOLVED); + } + SET_EXPECT(INTERNET_STATUS_CONNECTING_TO_SERVER); + SET_EXPECT(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); + + /* FILE_BEGIN tests */ + i = 0; + while(i < min(size, 4)) { + SET_OPTIONAL(INTERNET_STATUS_RECEIVING_RESPONSE); + SET_OPTIONAL(INTERNET_STATUS_RESPONSE_RECEIVED); + pos = InternetSetFilePointer(hor, i, NULL, FILE_BEGIN, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == i && err == NO_ERROR; + ok(expected, "Expected position %#lx. Got %#lx. GetLastError() %lu\n", i, pos, err); + res = InternetReadFile(hor, buf, BUFFER_SIZE, &count); + err = !res ? GetLastError() : NO_ERROR; + ok(res, "InternetReadFile failed: %lu\n", err); + ok(count, "InternetReadFile returned no content\n"); + ok(!memcmp(expect_response+i, buf, min(BUFFER_SIZE-i, count)), + "Unexpected result from InternetReadFile\n"); + i = i + 1; + } + while(i > 0) { + SET_OPTIONAL(INTERNET_STATUS_RECEIVING_RESPONSE); + SET_OPTIONAL(INTERNET_STATUS_RESPONSE_RECEIVED); + pos = InternetSetFilePointer(hor, i, NULL, FILE_BEGIN, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == i && err == NO_ERROR; + ok(expected, "Expected position %#lx. Got %#lx. GetLastError() %lu\n", i, pos, err); + res = InternetReadFile(hor, buf, BUFFER_SIZE, &count); + err = !res ? GetLastError() : NO_ERROR; + ok(res, "InternetReadFile failed: %lu\n", err); + ok(!memcmp(expect_response+i, buf, min(BUFFER_SIZE-i, count)), + "Unexpected result from InternetReadFile\n"); + i = i - 1; + } + 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); + pos = InternetSetFilePointer(hor, -1, NULL, FILE_BEGIN, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == -1 && err == NO_ERROR; + ok(expected, "Expected position %#x. Got %#lx. GetLastError() %lu\n", -1, pos, err); + + /* FILE_CURRENT tests */ + i = 0; + while(i < min(size, 4)) { + SET_OPTIONAL(INTERNET_STATUS_RECEIVING_RESPONSE); + SET_OPTIONAL(INTERNET_STATUS_RESPONSE_RECEIVED); + i = i + 1; + pos = InternetSetFilePointer(hor, 0, NULL, FILE_BEGIN, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == 0 && err == NO_ERROR; + ok(expected, "Expected position %#lx. Got %#lx. GetLastError() %lu\n", i, pos, err); + pos = InternetSetFilePointer(hor, i, NULL, FILE_CURRENT, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == i && err == NO_ERROR; + ok(expected, "Expected position %#lx. Got %#lx. GetLastError() %lu\n", i, pos, err); + res = InternetReadFile(hor, buf, 1024, &count); + err = !res ? GetLastError() : NO_ERROR; + ok(res, "InternetReadFile failed: %lu\n", err); + ok(!memcmp(expect_response+i, buf, min(BUFFER_SIZE-i, count)), + "Unexpected result from InternetReadFile\n"); + } + pos = InternetSetFilePointer(hor, -1, NULL, FILE_CURRENT, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == INVALID_SET_FILE_POINTER && err == ERROR_NEGATIVE_SEEK; + ok(expected, "Expected ERROR_NEGATIVE_SEEK. Got %lu\n", err); + pos = InternetSetFilePointer(hor, 0, NULL, FILE_BEGIN, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == 0 && err == NO_ERROR; + ok(expected, "Expected position %#x. Got %#lx. GetLastError() %lu\n", 0, pos, err); + pos = InternetSetFilePointer(hor, -1, NULL, FILE_CURRENT, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == -1 && err == NO_ERROR; + ok(expected, "Expected position %#x. Got %#lx. GetLastError() %lu\n", -1, pos, err); + pos = InternetSetFilePointer(hor, -1, NULL, FILE_CURRENT, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == INVALID_SET_FILE_POINTER && err == ERROR_NEGATIVE_SEEK; + ok(expected, "Expected ERROR_NEGATIVE_SEEK. Got %lu\n", err); + + /* FILE_END tests */ + pos = InternetSetFilePointer(hor, 0, NULL, FILE_END, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == size && err == NO_ERROR; + todo_wine ok(expected, "Expected position %#lx. Got %#lx. GetLastError() %lu\n", size, pos, err); + i = 0; + while(i < min(size, 4)) { + SET_OPTIONAL(INTERNET_STATUS_RECEIVING_RESPONSE); + SET_OPTIONAL(INTERNET_STATUS_RESPONSE_RECEIVED); + i = i + 1; + pos = InternetSetFilePointer(hor, i, NULL, FILE_END, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == size+i && err == NO_ERROR; + todo_wine ok(expected, "Expected position %#lx. Got %#lx. GetLastError() %lu\n", + size+i, pos, err); + } + i = 0; + while(i < min(size, 4)) { + SET_OPTIONAL(INTERNET_STATUS_RECEIVING_RESPONSE); + SET_OPTIONAL(INTERNET_STATUS_RESPONSE_RECEIVED); + i = i + 1; + pos = InternetSetFilePointer(hor, -i, NULL, FILE_END, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == INVALID_SET_FILE_POINTER && err == ERROR_NEGATIVE_SEEK; + todo_wine ok(expected, "Expected ERROR_NEGATIVE_SEEK. Got %lu\n", err); + } + +abort: + SET_OPTIONAL(INTERNET_STATUS_CLOSING_CONNECTION); + SET_OPTIONAL(INTERNET_STATUS_CONNECTION_CLOSED); + SET_OPTIONAL(INTERNET_STATUS_HANDLE_CLOSING); + if(hor) InternetCloseHandle(hor); + SET_OPTIONAL(INTERNET_STATUS_HANDLE_CLOSING); + if(hic) InternetCloseHandle(hic); + if(hi) InternetCloseHandle(hi); + +#ifdef BUFFER_SIZE +#undef BUFFER_SIZE +#endif +} + static void InternetReadFile_test(int flags, const test_data_t *test) { char *post_data = NULL; @@ -7946,6 +8187,8 @@ START_TEST(http) InternetReadFile_chunked_test(); HttpSendRequestEx_test(); InternetReadFile_test(INTERNET_FLAG_ASYNC, &test_data[3]); + InternetSetFilePointer_test("test.winehq.org", "/tests/hello.html"); + InternetSetFilePointer_test("test.winehq.org", "/favicon.ico"); test_connection_failure(); test_default_service_port(); test_concurrent_header_access(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1268
From: Jason Kuo <j20001970(a)gmail.com> The function is implemented by setting the newly added contentPos that keep track of current read position in the content, when InternetReadFile related function is called, data will be read from req_file->file_handle before continuing reading from http stream. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=26570 --- dlls/wininet/ftp.c | 3 ++ dlls/wininet/http.c | 93 ++++++++++++++++++++++++++++++++++++++++- dlls/wininet/internet.c | 29 +++++++++++-- dlls/wininet/internet.h | 2 + 4 files changed, 123 insertions(+), 4 deletions(-) diff --git a/dlls/wininet/ftp.c b/dlls/wininet/ftp.c index cf6555c63e2..f3f586cd6e1 100644 --- a/dlls/wininet/ftp.c +++ b/dlls/wininet/ftp.c @@ -1293,6 +1293,7 @@ static const object_vtbl_t FTPFILEVtbl = { NULL, FTPFILE_QueryOption, INET_SetOption, + NULL, FTPFILE_ReadFile, FTPFILE_WriteFile, FTPFILE_QueryDataAvailable, @@ -2397,6 +2398,7 @@ static const object_vtbl_t FTPSESSIONVtbl = { NULL, NULL, NULL, + NULL, NULL }; @@ -3525,6 +3527,7 @@ static const object_vtbl_t FTPFINDNEXTVtbl = { NULL, NULL, NULL, + NULL, FTPFINDNEXT_FindNextFileW }; diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c index 18f61f108fc..3a21da42afa 100644 --- a/dlls/wininet/http.c +++ b/dlls/wininet/http.c @@ -329,6 +329,7 @@ static void reset_data_stream(http_request_t *req) destroy_data_stream(req->data_stream); req->data_stream = &req->netconn_stream.data_stream; req->read_pos = req->read_size = req->netconn_stream.content_read = 0; + req->contentPos = 0; req->read_gzip = FALSE; } @@ -2499,6 +2500,7 @@ static void create_cache_entry(http_request_t *req) create_req_file(file_name, &req->req_file); req->req_file->url = url; + req->contentPos = 0; req->hCacheFile = CreateFileW(file_name, GENERIC_WRITE, FILE_SHARE_READ|FILE_SHARE_WRITE, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); @@ -2549,6 +2551,7 @@ static void remove_data( http_request_t *req, int count ) { if (!(req->read_size -= count)) req->read_pos = 0; else req->read_pos += count; + req->contentPos += count; } static DWORD read_line( http_request_t *req, LPSTR buffer, DWORD *len ) @@ -3033,6 +3036,37 @@ static void HTTP_ReceiveRequestData(http_request_t *req) send_request_complete(req, req->session->hdr.dwInternalFlags & INET_OPENURL ? (DWORD_PTR)req->hdr.hInternet : 1, 0); } +static DWORD read_req_file(http_request_t *req, BYTE *buffer, DWORD size, DWORD *read, BOOL allow_blocking) +{ + DWORD file_size, ret_read, res; + BYTE buf[1024]; + + file_size = GetFileSize(req->req_file->file_handle, NULL); + if(file_size == INVALID_FILE_SIZE) + return GetLastError(); + + if(!allow_blocking && req->contentPos > file_size) + return WSAEWOULDBLOCK; + + while (req->contentPos > file_size) { + res = read_http_stream(req, (BYTE*)buf, min(1024, req->contentPos-file_size), &ret_read, TRUE); + if(res != ERROR_SUCCESS || !ret_read) + break; + file_size = GetFileSize(req->req_file->file_handle, NULL); + } + + res = SetFilePointer(req->req_file->file_handle, req->contentPos, NULL, FILE_BEGIN); + if(res == INVALID_SET_FILE_POINTER) + return GetLastError(); + if(ReadFile(req->req_file->file_handle, buffer, size, &ret_read, NULL)) { + *read = ret_read; + req->contentPos += ret_read; + return ERROR_SUCCESS; + } + else + return GetLastError(); +} + /* read data from the http connection (the read section must be held) */ static DWORD HTTPREQ_Read(http_request_t *req, void *buffer, DWORD size, DWORD *read, BOOL allow_blocking) { @@ -3046,13 +3080,22 @@ static DWORD HTTPREQ_Read(http_request_t *req, void *buffer, DWORD size, DWORD * memcpy(buffer, req->read_buf+req->read_pos, ret_read); req->read_size -= ret_read; req->read_pos += ret_read; + req->contentPos += ret_read; allow_blocking = FALSE; } + if(ret_read < size && req->req_file && req->req_file->file_handle) { + res = read_req_file(req, (BYTE*)buffer+ret_read, size-ret_read, ¤t_read, allow_blocking); + if(res == ERROR_SUCCESS) + ret_read += current_read; + } + if(ret_read < size) { res = read_http_stream(req, (BYTE*)buffer+ret_read, size-ret_read, ¤t_read, allow_blocking); - if(res == ERROR_SUCCESS) + if(res == ERROR_SUCCESS) { ret_read += current_read; + req->contentPos += current_read; + } else if(res == WSAEWOULDBLOCK && ret_read) res = ERROR_SUCCESS; } @@ -3155,6 +3198,45 @@ static DWORD async_read(http_request_t *req, void *buf, DWORD size, DWORD read_p return ERROR_IO_PENDING; } +static DWORD HTTPREQ_SetFilePointer(object_header_t *hdr, LONG lDistanceToMove, DWORD dwMoveContext) +{ + http_request_t *req = (http_request_t*)hdr; + DWORD err = ERROR_INTERNET_INVALID_OPERATION, res = INVALID_SET_FILE_POINTER; + + if(req->hdr.dwFlags & (INTERNET_FLAG_DONT_CACHE|INTERNET_FLAG_NO_CACHE_WRITE)) + goto lend; + + switch (dwMoveContext) { + case FILE_BEGIN: + res = lDistanceToMove; + break; + case FILE_CURRENT: + if(req->contentPos && lDistanceToMove < 0) { + err = ERROR_NEGATIVE_SEEK; + goto lend; + } + res = req->contentPos + lDistanceToMove; + break; + case FILE_END: + FIXME("dwMoveContext FILE_END not implemented\n"); + goto lend; + default: + goto lend; + } + + EnterCriticalSection(&req->read_section); + req->contentPos = res; + req->read_pos = req->read_size = 0; + LeaveCriticalSection(&req->read_section); + + err = NO_ERROR; + +lend: + if(res == INVALID_SET_FILE_POINTER) + SetLastError(err); + return res; +} + static DWORD HTTPREQ_ReadFile(object_header_t *hdr, void *buf, DWORD size, DWORD *ret_read, DWORD flags, DWORD_PTR context) { @@ -3182,6 +3264,13 @@ static DWORD HTTPREQ_ReadFile(object_header_t *hdr, void *buf, DWORD size, DWORD memcpy(buf, req->read_buf + req->read_pos, read); req->read_size -= read; req->read_pos += read; + req->contentPos += read; + } + + if(read < size && req->req_file && req->req_file->file_handle) { + res = read_req_file(req, (BYTE*)buf+read, size-read, &cread, allow_blocking); + if(res == ERROR_SUCCESS) + read += cread; } if(read < size && (!read || !(flags & IRF_NO_WAIT)) && !end_of_read_data(req)) { @@ -3318,6 +3407,7 @@ static const object_vtbl_t HTTPREQVtbl = { HTTPREQ_CloseConnection, HTTPREQ_QueryOption, HTTPREQ_SetOption, + HTTPREQ_SetFilePointer, HTTPREQ_ReadFile, HTTPREQ_WriteFile, HTTPREQ_QueryDataAvailable, @@ -5807,6 +5897,7 @@ static const object_vtbl_t HTTPSESSIONVtbl = { NULL, NULL, NULL, + NULL, NULL }; diff --git a/dlls/wininet/internet.c b/dlls/wininet/internet.c index 26d262b9665..57be3922d63 100644 --- a/dlls/wininet/internet.c +++ b/dlls/wininet/internet.c @@ -957,6 +957,7 @@ static const object_vtbl_t APPINFOVtbl = { APPINFO_SetOption, NULL, NULL, + NULL, NULL }; @@ -2155,14 +2156,36 @@ INTERNET_STATUS_CALLBACK WINAPI InternetSetStatusCallbackW( /*********************************************************************** * InternetSetFilePointer (WININET.@) + * Sets read position for an open internet file. + * + * RETURNS + * Current position of the file on success + * INVALID_SET_FILE_POINTER on failure */ DWORD WINAPI InternetSetFilePointer(HINTERNET hFile, LONG lDistanceToMove, PVOID pReserved, DWORD dwMoveContext, DWORD_PTR dwContext) { - FIXME("(%p %ld %p %ld %Ix): stub\n", hFile, lDistanceToMove, pReserved, dwMoveContext, dwContext); + DWORD err = ERROR_INVALID_HANDLE, res = INVALID_SET_FILE_POINTER; + object_header_t *hdr; + + TRACE("(%p %ld %p %ld %Ix)\n", hFile, lDistanceToMove, pReserved, dwMoveContext, dwContext); + + hdr = get_handle_object(hFile); + if(!hdr) + goto lend; + + if(hdr->vtbl->SetFilePointer) { + res = hdr->vtbl->SetFilePointer(hdr, lDistanceToMove, dwMoveContext); + if(res == INVALID_SET_FILE_POINTER) + err = GetLastError(); + } - SetLastError(ERROR_INTERNET_INVALID_OPERATION); - return INVALID_SET_FILE_POINTER; + WININET_Release(hdr); + +lend: + if(res == INVALID_SET_FILE_POINTER) + SetLastError(err); + return res; } /*********************************************************************** diff --git a/dlls/wininet/internet.h b/dlls/wininet/internet.h index 6756488d5a5..a1ad1e0153e 100644 --- a/dlls/wininet/internet.h +++ b/dlls/wininet/internet.h @@ -225,6 +225,7 @@ typedef struct { void (*CloseConnection)(object_header_t*); DWORD (*QueryOption)(object_header_t*,DWORD,void*,DWORD*,BOOL); DWORD (*SetOption)(object_header_t*,DWORD,void*,DWORD); + DWORD (*SetFilePointer)(object_header_t*,LONG,DWORD); DWORD (*ReadFile)(object_header_t*,void*,DWORD,DWORD*,DWORD,DWORD_PTR); DWORD (*WriteFile)(object_header_t*,const void*,DWORD,DWORD*); DWORD (*QueryDataAvailable)(object_header_t*,DWORD*,DWORD,DWORD_PTR); @@ -339,6 +340,7 @@ typedef struct CRITICAL_SECTION read_section; /* section to protect the following fields */ ULONGLONG contentLength; /* total number of bytes to be read */ + ULONGLONG contentPos; BOOL read_gzip; /* are we reading in gzip mode? */ DWORD read_pos; /* current read position in read_buf */ DWORD read_size; /* valid data size in read_buf */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1268
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=126828 Your paranoid android. === debian11 (32 bit report) === Report validation errors: advapi32:security has no test summary line (early exit of the main process?) advapi32:security has unaccounted for todo messages
Jacek Caban (@jacek) commented about dlls/wininet/internet.c:
+ * RETURNS + * Current position of the file on success + * INVALID_SET_FILE_POINTER on failure */ DWORD WINAPI InternetSetFilePointer(HINTERNET hFile, LONG lDistanceToMove, PVOID pReserved, DWORD dwMoveContext, DWORD_PTR dwContext) { - FIXME("(%p %ld %p %ld %Ix): stub\n", hFile, lDistanceToMove, pReserved, dwMoveContext, dwContext); + DWORD err = ERROR_INVALID_HANDLE, res = INVALID_SET_FILE_POINTER; + object_header_t *hdr; + + TRACE("(%p %ld %p %ld %Ix)\n", hFile, lDistanceToMove, pReserved, dwMoveContext, dwContext); + + hdr = get_handle_object(hFile); + if(!hdr) + goto lend; Error handling is more complicated than it needs to be, you could just SetLastError() and return here and avoid GetLastError()+SetLastError() in the other error path.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1268#note_17649
Jacek Caban (@jacek) commented about dlls/wininet/http.c:
memcpy(buffer, req->read_buf+req->read_pos, ret_read); req->read_size -= ret_read; req->read_pos += ret_read; + req->contentPos += ret_read; allow_blocking = FALSE; }
+ if(ret_read < size && req->req_file && req->req_file->file_handle) { + res = read_req_file(req, (BYTE*)buffer+ret_read, size-ret_read, ¤t_read, allow_blocking); + if(res == ERROR_SUCCESS) + ret_read += current_read; + }
Implementing it like that means that we will send `INTERNET_STATUS_RECEIVING_RESPONSE` on each `InternetReadFile` call that reads from cache, is that intended? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1268#note_17650
Jacek Caban (@jacek) commented about dlls/wininet/http.c:
{ if (!(req->read_size -= count)) req->read_pos = 0; else req->read_pos += count; + req->contentPos += count; This doesn't look right, `remove_data()` is used for reading headers that are not part of content.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1268#note_17651
Jacek Caban (@jacek) commented about dlls/wininet/tests/http.c:
CHECK_NOTIFIED2(INTERNET_STATUS_HANDLE_CLOSING, handle_cnt); }
+static void InternetSetFilePointer_test(const char *host, const char *path) +{ +#ifndef BUFFER_SIZE +#define BUFFER_SIZE 8192 +#endif
#ifndef part is redundant. I think it would be cleaner just to hardcode it in buffer declaration and use sizeof() later. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1268#note_17652
Jacek Caban (@jacek) commented about dlls/wininet/tests/http.c:
CHECK_NOTIFIED2(INTERNET_STATUS_HANDLE_CLOSING, handle_cnt); }
+static void InternetSetFilePointer_test(const char *host, const char *path) +{ +#ifndef BUFFER_SIZE +#define BUFFER_SIZE 8192 +#endif + + CHAR expect_response[BUFFER_SIZE], buf[BUFFER_SIZE]; + HINTERNET hi = 0, hic = 0, hor = 0; + BOOL res, expected; + DWORD count, size, i, pos, err; + + hi = InternetOpenA("Winetest", INTERNET_OPEN_TYPE_DIRECT, NULL, NULL, 0); + ok((hi != 0x0), "InternetOpen failed: %lu\n", GetLastError());
Brackets in the expressions are redundant (same in a few other places). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1268#note_17653
Jacek Caban (@jacek) commented about dlls/wininet/tests/http.c:
+ ok(res, "HttpSendRequest failed: %lu\n", GetLastError()); + if(!res) goto abort; + + size = 0; + while(size < BUFFER_SIZE) { + res = InternetReadFile(hor, expect_response+size, BUFFER_SIZE-size, &count); + if(!res || !count) + break; + size += count; + } + ok(size, "InternetReadFile returned no content\n"); + if(!size) goto abort; + + if(hor) InternetCloseHandle(hor); + if(hic) InternetCloseHandle(hic); + if(hi) InternetCloseHandle(hi); Those if()s are redundant.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1268#note_17654
Jacek Caban (@jacek) commented about dlls/wininet/tests/http.c:
+ ok(size, "InternetReadFile returned no content\n"); + if(!size) goto abort; + + if(hor) InternetCloseHandle(hor); + if(hic) InternetCloseHandle(hic); + if(hi) InternetCloseHandle(hi); + + reset_events(); + + hi = InternetOpenA("Winetest", INTERNET_OPEN_TYPE_DIRECT, NULL, NULL, 0); + ok((hi != 0x0), "InternetOpen failed: %lu\n", GetLastError()); + if(hi == 0x0) goto abort; + + pInternetSetStatusCallbackA(hi, &callback); + + SET_EXPECT(INTERNET_STATUS_HANDLE_CREATED); SET_EXPECT() should be paired with CHECK_NOTIFIED() (same in other places).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1268#note_17655
Jacek Caban (@jacek) commented about dlls/wininet/tests/http.c:
+ + /* NULL handle tests */ + pos = InternetSetFilePointer(NULL, 0, NULL, FILE_BEGIN, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == INVALID_SET_FILE_POINTER && err == ERROR_INVALID_HANDLE; + ok(expected, "Expected ERROR_INVALID_HANDLE. Got %lu\n", err); + pos = InternetSetFilePointer(NULL, 0, NULL, FILE_CURRENT, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == INVALID_SET_FILE_POINTER && err == ERROR_INVALID_HANDLE; + ok(expected, "Expected ERROR_INVALID_HANDLE. Got %lu\n", err); + pos = InternetSetFilePointer(NULL, 0, NULL, FILE_END, 0); + err = pos == INVALID_SET_FILE_POINTER ? GetLastError() : NO_ERROR; + expected = pos == INVALID_SET_FILE_POINTER && err == ERROR_INVALID_HANDLE; + ok(expected, "Expected ERROR_INVALID_HANDLE. Got %lu\n", err); + + /* INTERNET_FLAG_DONT_CACHE tests */ INTERNET_FLAG_DONT_CACHE will have no effect until request is actually sent. Those tests are interesting as well, but actual INTERNET_FLAG_DONT_CACHE tests would be still interesting.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1268#note_17656
Sorry for the delay. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1268#note_17657
On Mon Nov 28 12:28:46 2022 +0000, Jacek Caban wrote:
Implementing it like that means that we will send `INTERNET_STATUS_RECEIVING_RESPONSE` on each `InternetReadFile` call that reads from cache, is that intended? No, but `read_req_file` is placed here in case of asynchronous requests when contentPos is greater than current cache size.
And `read_req_file` is called first in `HTTPREQ_ReadFile` before sending `INTERNET_STATUS_RECEIVING_RESPONSE` and calling `HTTPREQ_Read`, so it is unlikely that we are receiving `INTERNET_STATUS_RECEIVING_RESPONSE` and getting data entirely from cache without reading HTTP stream if I understand it right. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1268#note_17843
On Mon Nov 28 12:28:51 2022 +0000, Jacek Caban wrote:
INTERNET_FLAG_DONT_CACHE will have no effect until request is actually sent. Those tests are interesting as well, but actual INTERNET_FLAG_DONT_CACHE tests would be still interesting. Does that mean we should test INTERNET_FLAG_DONT_CACHE flags for both before and after sending request?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1268#note_17863
participants (4)
-
Jacek Caban (＠jacek) -
Jason Kuo
-
Jason Kuo (＠j20001970)
-
Marvin