[wininet/ftp.c] #6 Fix some returned error codes
Paul Vriens
paul.vriens.wine at gmail.com
Mon Feb 12 04:33:26 CST 2007
Hi,
Fixes for FtpGetFile[A|W].
I did some testing (in a loop) how windows checks the dwInternetFlags. It turns
out that the last 3 bits are used for this. They have to define the transfer
type. The rest of the bits are not checked for validity. I've add a '#define'
for this. For later fixes this define will be moved to the top.
The fixes should also please Coverity CID-173 and CID-174.
Changelog
Fix some returned error codes
Cheers,
Paul.
-------------- next part --------------
diff --git a/dlls/wininet/ftp.c b/dlls/wininet/ftp.c
index 56fade1..e8a31b4 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 ae33560..43be942 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());
--
1.4.4.4
More information about the wine-patches
mailing list