Reputation: 53
Can someone tell me what is wrong with my code?
I am trying to download a file from the internet using WinInet. The function connects to the target site just fine, I don't understand why this code isn't working. Can anyone help me out?
Here is my code:
HANDLE hFile = CreateFileW(FilePath, GENERIC_WRITE, NULL, NULL, CREATE_ALWAYS, NULL, NULL);
if (hFile != INVALID_HANDLE_VALUE || GetLastError() == ERROR_ALREADY_EXISTS)
{
CHAR Buffer[2048];
DWORD BytesRead=0, BytesToRead=0;
DWORD BytesWritten=0, BytesToWrite=0;
SetFilePointer(hFile, 0, 0, FILE_BEGIN);
do
{
if (BytesRead)
{
WriteFile(hFile, Buffer, BytesWritten, &BytesToWrite, FALSE);
}
}
while
(InternetReadFile(hRequest, (LPVOID)Buffer, BytesToRead, &BytesRead) != FALSE);
}
CloseHandle(hFile);
}
hRequest is passed to the function, it is the HINTERNET handle from HttpOpenRequestA.
Upvotes: 2
Views: 1863
Reputation: 596352
Your code has some logic problems.
you are misusing GetLastError()
when calling CreateFileW()
. Regardless of whether the file already exists or not, CreateFileW()
will not return INVALID_HANDLE
if it successfully creates/opens the file. That is all you need to check for (call GetLastError()
only if CreateFileW()
fails and you want to find out why). also, there is no need to call SetFilePointer()
at all, as CREATE_ALWAYS
ensures the opened file is empty, truncating the file if it already exists and has data in it.
your do..while
loop should be a while
loop instead, so that InternetReadFile()
is called first. There is no point in skipping WriteFile()
on the first loop iteration. If you use a do..while
loop, InternetReadFile()
should not be used as the loop condition.
more importantly, you are breaking the loop only if InternetReadFile()
fails with an error. You are expecting it to fail when it reaches the end of the response, but it actually returns TRUE and sets BytesRead
to 0. This is documented behavior, which you are not handling at all:
InternetReadFile operates much like the base ReadFile function, with a few exceptions. Typically, InternetReadFile retrieves data from an HINTERNET handle as a sequential stream of bytes. The amount of data to be read for each call to InternetReadFile is specified by the dwNumberOfBytesToRead parameter and the data is returned in the lpBuffer parameter. A normal read retrieves the specified dwNumberOfBytesToRead for each call to InternetReadFile until the end of the file is reached. To ensure all data is retrieved, an application must continue to call the InternetReadFile function until the function returns TRUE and the lpdwNumberOfBytesRead parameter equals zero. This is especially important if the requested data is written to the cache, because otherwise the cache will not be properly updated and the file downloaded will not be committed to the cache. Note that caching happens automatically unless the original request to open the data stream set the INTERNET_FLAG_NO_CACHE_WRITE flag.
When a synchronous read operation reaches the end of a file, ReadFile returns TRUE and sets *lpNumberOfBytesRead to zero.
when calling WriteFile()
, you are passing BytesWritten
to the nNumberOfBytesToWrite
parameter, but BytesWritten
is never set to anything other than 0, so nothing gets written to the file. You need to pass BytesRead
instead.
With that said, use something more like this:
HANDLE hFile = CreateFileW(FilePath, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, FILE_FLAG_SEQUENTIAL_SCAN, NULL);
if (hFile == INVALID_HANDLE_VALUE)
{
// handle error as needed...
}
else
{
BYTE Buffer[2048];
DWORD BytesRead, BytesWritten;
do
{
if (!InternetReadFile(hRequest, Buffer, sizeof(Buffer), &BytesRead))
{
// handle error as needed...
break;
}
if (!BytesRead)
break;
if (!WriteFile(hFile, Buffer, BytesRead, &BytesWritten, FALSE))
{
// handle error as needed...
break;
}
}
while (true);
CloseHandle(hFile);
}
MSDN even has a full example of how to use InternetReadFile()
:
HOWTO: Using InternetReadFile To Get File
BOOL GetFile (HINTERNET IN hOpen, // Handle from InternetOpen()
CHAR *szUrl, // Full URL
CHAR *szFileName) // Local file name
{
DWORD dwSize;
CHAR szHead[] = "Accept: */*\r\n\r\n";
VOID * szTemp[25];
HINTERNET hConnect;
FILE * pFile;
if ( !(hConnect = InternetOpenUrl ( hOpen, szUrl, szHead,
lstrlen (szHead), INTERNET_FLAG_DONT_CACHE, 0)))
{
cerr << "Error !" << endl;
return 0;
}
if ( !(pFile = fopen (szFileName, "wb" ) ) )
{
cerr << "Error !" << endl;
return FALSE;
}
do
{
// Keep coping in 25 bytes chunks, while file has any data left.
// Note: bigger buffer will greatly improve performance.
if (!InternetReadFile (hConnect, szTemp, 50, &dwSize) )
{
fclose (pFile);
cerr << "Error !" << endl;
return FALSE;
}
if (!dwSize)
break; // Condition of dwSize=0 indicate EOF. Stop.
else
fwrite(szTemp, sizeof (char), dwSize , pFile);
} // do
while (TRUE);
fflush (pFile);
fclose (pFile);
return TRUE;
}
Upvotes: 3