Hans Leidekker : wininet: Always close the data connection before receiving a server response.
Alexandre Julliard
julliard at winehq.org
Mon Oct 29 08:34:46 CDT 2007
Module: wine
Branch: master
Commit: e8eaee4f6a4c76c7ebdda07d61064e923982e717
URL: http://source.winehq.org/git/wine.git/?a=commit;h=e8eaee4f6a4c76c7ebdda07d61064e923982e717
Author: Hans Leidekker <hans at it.vu.nl>
Date: Fri Oct 26 23:19:38 2007 +0200
wininet: Always close the data connection before receiving a server response.
---
dlls/wininet/ftp.c | 19 ++++++--------
dlls/wininet/tests/ftp.c | 59 +++++++++++++++++++--------------------------
2 files changed, 33 insertions(+), 45 deletions(-)
diff --git a/dlls/wininet/ftp.c b/dlls/wininet/ftp.c
index 1fce6ea..88d97ff 100644
--- a/dlls/wininet/ftp.c
+++ b/dlls/wininet/ftp.c
@@ -1365,15 +1365,16 @@ BOOL WINAPI FTP_FtpGetFileW(LPWININETFTPSESSIONW lpwfs, LPCWSTR lpszRemoteFile,
/* Receive data */
FTP_RetrieveFileData(lpwfs, nDataSocket, hFile);
+ closesocket(nDataSocket);
+
nResCode = FTP_ReceiveResponse(lpwfs, dwContext);
if (nResCode)
{
if (nResCode == 226)
bSuccess = TRUE;
- else
+ else
FTP_SetResponseError(nResCode);
}
- closesocket(nDataSocket);
}
}
@@ -3008,7 +3009,7 @@ static void FTP_CloseFindNextHandle(LPWININETHANDLEHEADER hdr)
* FTP_CloseFileTransferHandle (internal)
*
* Closes the file transfer handle. This also 'cleans' the data queue of
- * the 'transfer conplete' message (this is a bit of a hack though :-/ )
+ * the 'transfer complete' message (this is a bit of a hack though :-/ )
*
*/
static void FTP_CloseFileTransferHandle(LPWININETHANDLEHEADER hdr)
@@ -3022,18 +3023,14 @@ static void FTP_CloseFileTransferHandle(LPWININETHANDLEHEADER hdr)
WININET_Release(&lpwh->lpFtpSession->hdr);
if (!lpwh->session_deleted)
- lpwfs->download_in_progress = NULL;
-
- /* This just serves to flush the control socket of any spurrious lines written
- to it (like '226 Transfer complete.').
+ lpwfs->download_in_progress = NULL;
- Wonder what to do if the server sends us an error code though...
- */
- nResCode = FTP_ReceiveResponse(lpwfs, lpwfs->hdr.dwContext);
-
if (lpwh->nDataSocket != -1)
closesocket(lpwh->nDataSocket);
+ nResCode = FTP_ReceiveResponse(lpwfs, lpwfs->hdr.dwContext);
+ if (nResCode > 0 && nResCode != 226) WARN("server reports failed transfer\n");
+
HeapFree(GetProcessHeap(), 0, lpwh);
}
diff --git a/dlls/wininet/tests/ftp.c b/dlls/wininet/tests/ftp.c
index 25367c1..9eab05d 100644
--- a/dlls/wininet/tests/ftp.c
+++ b/dlls/wininet/tests/ftp.c
@@ -301,16 +301,12 @@ static void test_getfile(void)
/* Zero attributes */
SetLastError(0xdeadbeef);
- bRet = FtpGetFileA(hFtp, "welcome.msg", "should_be_non_existing_deadbeef", FALSE, 0, FTP_TRANSFER_TYPE_UNKNOWN, 0);
- ok ( bRet == FALSE, "Expected FtpGetFileA to fail\n");
- todo_wine
- {
- ok ( GetLastError() == ERROR_INTERNET_EXTENDED_ERROR,
- "Expected ERROR_INTERNET_EXTENDED_ERROR, 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");
+ bRet = FtpGetFileA(hFtp, "welcome.msg", "should_be_existing_non_deadbeef", FALSE, 0, FTP_TRANSFER_TYPE_UNKNOWN, 0);
+ ok ( bRet == TRUE, "Expected FtpGetFileA to succeed\n");
+ ok (GetLastError() == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", GetLastError());
+ ok (GetFileAttributesA("should_be_existing_non_deadbeef") != INVALID_FILE_ATTRIBUTES,
+ "Local file should have been created\n");
+ DeleteFileA("should_be_existing_non_deadbeef");
/* Illegal condition flags */
SetLastError(0xdeadbeef);
@@ -326,12 +322,14 @@ static void test_getfile(void)
SetLastError(0xdeadbeef);
bRet = FtpGetFileA(hFtp, "should_be_non_existing_deadbeef", "should_also_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_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 */
- todo_wine
ok (GetFileAttributesA("should_also_be_non_existing_deadbeef") == INVALID_FILE_ATTRIBUTES,
"Local file should not have been created\n");
+ }
DeleteFileA("should_also_be_non_existing_deadbeef");
/* Same call as the previous but now the local file does exists. Windows just removes the file if the call fails
@@ -346,32 +344,30 @@ static void test_getfile(void)
SetLastError(0xdeadbeef);
bRet = FtpGetFileA(hFtp, "should_be_non_existing_deadbeef", "should_also_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_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 */
- todo_wine
ok (GetFileAttributesA("should_also_be_non_existing_deadbeef") == INVALID_FILE_ATTRIBUTES,
"Local file should not have been created\n");
+ }
DeleteFileA("should_also_be_non_existing_deadbeef");
- /* This one should fail */
+ /* This one should succeed */
SetLastError(0xdeadbeef);
- bRet = FtpGetFileA(hFtp, "welcome.msg", "should_be_non_existing_deadbeef", FALSE, FILE_ATTRIBUTE_NORMAL, FTP_TRANSFER_TYPE_UNKNOWN, 0);
- ok ( bRet == FALSE, "Expected FtpGetFileA to fail\n");
- ok ( GetLastError() == ERROR_INTERNET_EXTENDED_ERROR,
- "Expected ERROR_INTERNET_EXTENDED_ERROR, got %d\n", GetLastError());
+ bRet = FtpGetFileA(hFtp, "welcome.msg", "should_be_existing_non_deadbeef", FALSE, FILE_ATTRIBUTE_NORMAL, FTP_TRANSFER_TYPE_UNKNOWN, 0);
+ ok ( bRet == TRUE, "Expected FtpGetFileA to fail\n");
+ ok ( GetLastError() == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %d\n", GetLastError());
- if (GetFileAttributesA("should_be_non_existing_deadbeef") != INVALID_FILE_ATTRIBUTES)
+ if (GetFileAttributesA("should_be_existing_non_deadbeef") != INVALID_FILE_ATTRIBUTES)
{
/* Should succeed as fFailIfExists is set to FALSE (meaning don't fail if local file exists) */
SetLastError(0xdeadbeef);
bRet = FtpGetFileA(hFtp, "welcome.msg", "should_be_non_existing_deadbeef", FALSE, FILE_ATTRIBUTE_NORMAL, FTP_TRANSFER_TYPE_UNKNOWN, 0);
- todo_wine
- {
ok ( bRet == TRUE, "Expected FtpGetFileA to succeed\n");
ok ( GetLastError() == ERROR_SUCCESS,
"Expected ERROR_SUCCESS, got %d\n", GetLastError());
- }
/* Should fail as fFailIfExists is set to TRUE */
SetLastError(0xdeadbeef);
@@ -387,7 +383,7 @@ static void test_getfile(void)
ok ( GetLastError() == ERROR_FILE_EXISTS,
"Expected ERROR_FILE_EXISTS, got %d\n", GetLastError());
- DeleteFileA("should_be_non_existing_deadbeef");
+ DeleteFileA("should_be_existing_non_deadbeef");
}
InternetCloseHandle(hFtp);
@@ -478,13 +474,8 @@ static void test_openfile(void)
SetLastError(0xdeadbeef);
hOpenFile = FtpOpenFileA(hFtp, "welcome.msg", GENERIC_READ, FTP_TRANSFER_TYPE_ASCII, 0);
- todo_wine
- {
- ok ( hOpenFile == NULL, "Expected FtpOpenFileA to fail\n");
- /* For some strange/unknown reason, win98 returns ERROR_FILE_NOT_FOUND */
- ok ( GetLastError() == ERROR_INTERNET_EXTENDED_ERROR || GetLastError() == ERROR_FILE_NOT_FOUND,
- "Expected ERROR_INTERNET_EXTENDED_ERROR or ERROR_FILE_NOT_FOUND (win98), got %d\n", GetLastError());
- }
+ ok ( hOpenFile != NULL, "Expected FtpOpenFileA to succeed\n");
+ ok ( GetLastError() == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", GetLastError());
if (hOpenFile)
{
@@ -840,14 +831,14 @@ static void test_multiple(void)
}
/* A correct call */
- bRet = FtpGetFileA(hFtp, "welcome.msg", "should_be_non_existing_deadbeef", FALSE, FILE_ATTRIBUTE_NORMAL, FTP_TRANSFER_TYPE_UNKNOWN, 0);
- DeleteFileA("should_be_non_existing_deadbeef");
+ bRet = FtpGetFileA(hFtp, "welcome.msg", "should_be_existing_non_deadbeef", FALSE, FILE_ATTRIBUTE_NORMAL, FTP_TRANSFER_TYPE_UNKNOWN, 0);
+ ok(bRet, "Expected FtpGetFileA to succeed\n");
+ DeleteFileA("should_be_existing_non_deadbeef");
SetLastError(0xdeadbeef);
hOpenFile = FtpOpenFileA(hFtp, "welcome.msg", GENERIC_READ, FTP_TRANSFER_TYPE_ASCII, 0);
- ok ( hOpenFile == NULL, "Expected FtpOpenFileA to fail\n");
- ok ( GetLastError() == ERROR_INTERNET_EXTENDED_ERROR || GetLastError() == ERROR_FILE_NOT_FOUND,
- "Expected ERROR_INTERNET_EXTENDED_ERROR or ERROR_FILE_NOT_FOUND (win98), got %d\n", GetLastError());
+ ok(hOpenFile != NULL, "Expected FtpOpenFileA to succeed\n");
+ ok(GetLastError() == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", GetLastError());
InternetCloseHandle(hOpenFile);
InternetCloseHandle(hFtp);
More information about the wine-cvs
mailing list