|
|
Comments and Discussions
|
|
 |
|
If you attempt to send an attachment that is greater than the default limit of 5MB (say 6MB, for example), the program waits indefinitely until the server times out and closes the connection and then proceeds to crash.
Here's the sequence:
1. Start sending attachment.
2. Notice that the file is too large, throw exception MSG_TOO_BIG
3. Catch exception and call DisconnectRemoteServer.
4. Send QUIT command.
5. Waits for response.
6. Server times out waiting for message data.
7. My server responded with: 421 SMTP incoming data timeout - closing connection.
8. Server closes connection abruptly.
9. Select returns with response received in step 7 (421...)
10. Checks response.
11. 421 is not a valid response for QUIT.
12. Throws exception
13. Program aborts because it is a new exception is thrown that is unhandled (at least in the example program).
The underlying problem here (at least in this case) is that the QUIT command is not valid because it is currently inside the message data portion. The message data must be ended first before the server can start accepting commands again. I suggest sending the end of message line before throwing for the failed attachment. Also perhaps it's better to check the file's validity before even putting out the MIME header data for it.
The secondary problem is the unhandled exception. I suggest wrapping the call to DisconnectRemoteServer (from inside the Send catch handler) with its own try/catch block. These exceptions are then trapped and ignored. Unfortunately I'm not sure what to do about the original exception. I've tried different methods, but in each one I've tried the program aborts when rethrowing the original exception.
Hopefully this makes sense. There may be other failure points inside the message data section as well that have this issue, but this is what I found so far.
|
|
|
|
|
GKarRacer,
Which version of the project are you using? This is one of the primary issues resolved in version 2.2 of the code - at least I think. I just want to make sure if you are seeing an issue with it that you are on the latest version of the project.
Thanks,
David
|
|
|
|
|
I have version 2.1. I had downloaded it fairly recently and didn't realize there was an update out already. I will check it out. Thanks.
|
|
|
|
|
I took a quick look at 2.2. I haven't had a chance to run it yet, but the attachment handling does look better. Thanks.
There is still an issue, however, with exceptions occurring within DisconnectRemoteServer. If an exception occurs while calling this, the program most certainly will crash due to nested exceptions or worse by throwing from within the destructor. I think for safety all calls to DisconnectRemoteServer need to have its own try/catch block wrapper and these exceptions should not be forwarded onto the caller. The destructor for CSmtp especially needs this.
To get around the double exception problem in the Send function I mentioned earlier, here's a suggestion: Don't call DisconnectRemoteServer from within Send's catch handler. Let the destructor handle it during the normal stack unwind from the original exception. Make sure, of course, to wrap DisconnectRemoteServer in the destructor within its own try/catch block. If you do this Send doesn't even need its own try/catch block.
Come to think of it instead of always wrapping DisconnectRemoteServer, you could simply have the DisconnectRemoteServer function have its own internal try/catch block that eats all exceptions.
I think ConnectRemoteServer also may have the same issue as Send.
What do you think?
|
|
|
|
|
When sending attachments you read the entire file to calculate the file size. This seems highly inefficient. There are, of course, various operating system specific ways to get this information, but even with straight C++ you can do the following:
fseek(hFile, 0, SEEK_END);
TotalSize = ftell(hFile);
fseek(hFile, 0, SEEK_SET);
The only problem with this is if the total size of the file is greater than 4GB (clearly too big for an attachment, but you know those crazy users...). With VS, at least, you can use _ftelli64, but I don't know if there's a cross platform equivalent. You're current code also will fail with excessively sized files. Admittedly though, this is unexpected input.
|
|
|
|
|
Sorry, the second line should be:
FileSize = ftell(hFile);
TotalSize += FileSize;
or simply:
TotalSize += ftell(hFile);
|
|
|
|
|
GKarRacer,
Great point. Also, in the process of putting this in, I realized a couple of things that aren't good about rev 2.2. Now it reads the attachments one time just to check for total size and throws an error there if it is too large. However, in doing so:
1. Like you mentioned, it reads the whole file in the process just to find out how large it is.
2. It doesn't close the files so they will be left locked.
3. It still throws an error in the middle of sending so that if the file size changes between when it checks the first time and when it sends them, it could still end up in limbo.
To fix these problems I've made the following changes.
Starting on line 533:
TotalSize = 0;
for(FileId=0;FileId<Attachments.size();FileId++)
{
snprintf(FileName, 255, Attachments[FileId].c_str());
hFile = fopen(FileName,"rb");
if(hFile == NULL)
throw ECSmtp(ECSmtp::FILE_NOT_EXIST);
fseek(hFile, 0, SEEK_END);
FileSize = ftell(hFile);
TotalSize += FileSize;
if(TotalSize/1024 > MSG_SIZE_IN_MB*1024)
throw ECSmtp(ECSmtp::MSG_TOO_BIG);
fclose(hFile);
}
and then after making that change, starting on line 647:
fseek(hFile, 0, SEEK_END);
FileSize = ftell(hFile);
fseek (hFile,0,SEEK_SET);
MsgPart = 0;
for(i=0;i<FileSize/54+1;i++)
{
res = fread(FileBuf,sizeof(char),54,hFile);
MsgPart ? strcat(SendBuf,base64_encode(reinterpret_cast<const unsigned char*>(FileBuf),res).c_str())
: strcpy(SendBuf,base64_encode(reinterpret_cast<const unsigned char*>(FileBuf),res).c_str());
strcat(SendBuf,"\r\n");
MsgPart += res + 2;
if(MsgPart >= BUFFER_SIZE/2)
{ MsgPart = 0;
SendData(pEntry); }
}
if(MsgPart)
{
SendData(pEntry); }
fclose(hFile);
|
|
|
|
|
how to send email through a proxy?
does it have the setproxy function?
|
|
|
|
|
ASERERTA@#@s,
Sorry, this capability is not currently supported. If you want to develop it and post back we could look into adding it.
Thanks,
David
|
|
|
|
|
|
|
|
 General  News  Suggestion  Question  Bug  Answer  Joke  Rant  Admin
|
C++ SMTP client, support SSL and TLS encrypted connection to SMTP server
| Type | Article |
| Licence | CPOL |
| First Posted | 3 Aug 2010 |
| Views | 227,227 |
| Downloads | 11,937 |
| Bookmarked | 134 times |
|
|
Submit an article or tip
Post your Blog
