diff --git a/dlls/wininet/ftp.c b/dlls/wininet/ftp.c index 56fade1e8b2..e8a31b46885 100644 --- a/dlls/wininet/ftp.c +++ b/dlls/wininet/ftp.c @@ -1193,6 +1193,8 @@ static void AsyncFtpGetFileProc(WORKREQUEST *workRequest) HeapFree(GetProcessHeap(), 0, req->lpszNewFile); } +#define FTP_CONDITION_MASK 0x0007 + BOOL WINAPI FtpGetFileW(HINTERNET hInternet, LPCWSTR lpszRemoteFile, LPCWSTR lpszNewFile, BOOL fFailIfExists, DWORD dwLocalFlagsAttribute, DWORD dwInternetFlags, DWORD dwContext) @@ -1201,13 +1203,35 @@ BOOL WINAPI FtpGetFileW(HINTERNET hInternet, LPCWSTR lpszRemoteFile, LPCWSTR lps LPWININETAPPINFOW hIC = NULL; BOOL r = FALSE; + if (!lpszRemoteFile || !lpszNewFile) + { + INTERNET_SetLastError(ERROR_INVALID_PARAMETER); + return FALSE; + } + lpwfs = (LPWININETFTPSESSIONW) WININET_GetObject( hInternet ); - if (NULL == lpwfs || WH_HFTPSESSION != lpwfs->hdr.htype) + if (!lpwfs) + { + INTERNET_SetLastError(ERROR_INVALID_HANDLE); + return FALSE; + } + + if (WH_HFTPSESSION != lpwfs->hdr.htype) { INTERNET_SetLastError(ERROR_INTERNET_INCORRECT_HANDLE_TYPE); goto lend; } + /* Testing shows that Windows only accepts dwInternetFlags where the last + * 3 (yes 3) bits define FTP_TRANSFER_TYPE_UNKNOWN, FTP_TRANSFER_TYPE_ASCII or FTP_TRANSFER_TYPE_BINARY. + */ + + if ((dwInternetFlags & FTP_CONDITION_MASK) > FTP_TRANSFER_TYPE_BINARY) + { + INTERNET_SetLastError(ERROR_INVALID_PARAMETER); + goto lend; + } + if (lpwfs->download_in_progress != NULL) { INTERNET_SetLastError(ERROR_FTP_TRANSFER_IN_PROGRESS); goto lend; @@ -1238,8 +1262,7 @@ BOOL WINAPI FtpGetFileW(HINTERNET hInternet, LPCWSTR lpszRemoteFile, LPCWSTR lps } lend: - if( lpwfs ) - WININET_Release( &lpwfs->hdr ); + WININET_Release( &lpwfs->hdr ); return r; } diff --git a/dlls/wininet/tests/ftp.c b/dlls/wininet/tests/ftp.c index ae33560a48e..43be9428719 100644 --- a/dlls/wininet/tests/ftp.c +++ b/dlls/wininet/tests/ftp.c @@ -249,7 +249,6 @@ static void test_getfile(void) SetLastError(0xdeadbeef); bRet = FtpGetFileA(NULL, NULL, "should_be_non_existing_deadbeef", FALSE, FILE_ATTRIBUTE_NORMAL, FTP_TRANSFER_TYPE_UNKNOWN, 0); ok ( bRet == FALSE, "Expected FtpGetFileA to fail\n"); - todo_wine ok ( GetLastError() == ERROR_INVALID_PARAMETER, "Expected ERROR_INVALID_PARAMETER, got %d\n", GetLastError()); @@ -257,7 +256,6 @@ static void test_getfile(void) SetLastError(0xdeadbeef); bRet = FtpGetFileA(NULL, "welcome.msg", "should_be_non_existing_deadbeef", FALSE, FILE_ATTRIBUTE_NORMAL, 5, 0); ok ( bRet == FALSE, "Expected FtpGetFileA to fail\n"); - todo_wine ok ( GetLastError() == ERROR_INVALID_HANDLE, "Expected ERROR_INVALID_HANDLE, got %d\n", GetLastError()); @@ -281,21 +279,16 @@ static void test_getfile(void) SetLastError(0xdeadbeef); bRet = FtpGetFileA(hFtp, NULL, "should_be_non_existing_deadbeef", FALSE, FILE_ATTRIBUTE_NORMAL, FTP_TRANSFER_TYPE_UNKNOWN, 0); ok ( bRet == FALSE, "Expected FtpGetFileA to fail\n"); - todo_wine - { ok ( GetLastError() == ERROR_INVALID_PARAMETER, "Expected ERROR_INVALID_PARAMETER, got %d\n", GetLastError()); - /* Currently Wine always creates the local file (even on failure) which is not correct, hence the test */ ok (GetFileAttributesA("should_be_non_existing_deadbeef") == INVALID_FILE_ATTRIBUTES, "Local file should not have been created\n"); - } DeleteFileA("should_be_non_existing_deadbeef"); /* No local file */ SetLastError(0xdeadbeef); bRet = FtpGetFileA(hFtp, "welcome.msg", NULL, FALSE, FILE_ATTRIBUTE_NORMAL, FTP_TRANSFER_TYPE_UNKNOWN, 0); ok ( bRet == FALSE, "Expected FtpGetFileA to fail\n"); - todo_wine ok ( GetLastError() == ERROR_INVALID_PARAMETER, "Expected ERROR_INVALID_PARAMETER, got %d\n", GetLastError()); @@ -317,13 +310,10 @@ static void test_getfile(void) SetLastError(0xdeadbeef); bRet = FtpGetFileA(hFtp, "welcome.msg", "should_be_non_existing_deadbeef", FALSE, FILE_ATTRIBUTE_NORMAL, 5, 0); ok ( bRet == FALSE, "Expected FtpGetFileA to fail\n"); - todo_wine - { ok ( GetLastError() == ERROR_INVALID_PARAMETER, "Expected ERROR_INVALID_PARAMETER, got %d\n", GetLastError()); ok (GetFileAttributesA("should_be_non_existing_deadbeef") == INVALID_FILE_ATTRIBUTES, "Local file should not have been created\n"); - } DeleteFileA("should_be_non_existing_deadbeef"); /* Remote file doesn't exist */ @@ -334,6 +324,7 @@ static void test_getfile(void) { ok ( GetLastError() == ERROR_INTERNET_EXTENDED_ERROR, "Expected ERROR_INTERNET_EXTENDED_ERROR, got %d\n", GetLastError()); + /* Currently Wine always creates the local file (even on failure) which is not correct, hence the test */ ok (GetFileAttributesA("should_also_be_non_existing_deadbeef") == INVALID_FILE_ATTRIBUTES, "Local file should not have been created\n"); } @@ -390,7 +381,6 @@ static void test_getfile(void) SetLastError(0xdeadbeef); bRet = FtpGetFileA(hConnect, NULL, "should_be_non_existing_deadbeef", FALSE, FILE_ATTRIBUTE_NORMAL, FTP_TRANSFER_TYPE_UNKNOWN, 0); ok ( bRet == FALSE, "Expected FtpGetFileA to fail\n"); - todo_wine ok ( GetLastError() == ERROR_INVALID_PARAMETER, "Expected ERROR_INVALID_PARAMETER, got %d\n", GetLastError());