|
I just find out the issue.
The initialization and release process for OpenSSL can only be done once.
With the initialization you can do it many times and will not affect but you could be leaking some memory.
With the release/free process you should do it just at the end of the use you are doing of OpenSSL.
hope it help some one, it took me some time to refactor so to avoid this.
|
|
|
|
|
i have noticed that Bcc recipients are added to header, i think this will cause all other recipients can see other mails.
This code is in "FormatHeader"
if(BCCRecipients.size())
{
for (i=0;i<BCCRecipients.size();i++)
{
if(i > 0)
bcc.append(",");
bcc += BCCRecipients[i].Name;
bcc.append("<");
bcc += BCCRecipients[i].Mail;
bcc.append(">");
}
}
|
|
|
|
|
BCC recipients should be added as RCPT TO: but not as part of the DATA in the headers.
this way it will receive the message but will not be included in the email info.
If you add it to the headers in the DATA, the recipients will be able to see then.
Maybe this should work as there is a header bcc: but I've tried it and it didn't work.
What I manage to get working as BCC should, is to included tehn as RCPT TO: smtp command and avoid
to included then as bcc: header in DATA.
hope it helps.
|
|
|
|
|
Great point! Kind of ruins the purpose of the Bcc. I've removed it for the next release.
|
|
|
|
|
In CSmtp::Send() function,
snprintf(FileName, 255, Attachments[FileId].c_str());
Why use snprintf to copy Attachments[FileId].c_str() to FileName?
If Attachments[FileId].c_str() contains characters like "%s" or other format specifier, the code would be erroneous.
|
|
|
|
|
uni_gauldoth,
Excellent point. I changed this in the last release of the code after finding that if you use strcpy in MSVC 2012 it gives warnings about wanting you to use strcpy_s instead. Unfortunately strcpy_s is not only not portable, but for whatever reason, MS switched the order of the arguments relative to strncpy so that you can't just use a #define to toggle between different platforms. You have to create a whole separate line - one that MSVC 2012 likes and another that everything else likes. I was trying to be clever about copying the string in a portable manner, but it seems I missed out on an important caveat. Any ideas other than making it like this:
#if _MSC_VER >= 1400
strcpy_s(FileName, 255, Attachments[FileId].c_str());
#else
strncpy(FileName, Attachments[FileId].c_str(), 255);
#endif
|
|
|
|
|
Thanks for reply, the lib is great and easy to use.
Why do we need to copy Attachments[FileId].c_str() into FileName buf?
Can we just use Attachments[FileId] directly?
Maybe we could use std::string instead of c string functions.
I changed the code a little like below,
hFile = fopen(Attachments[FileId].c_str(),"rb");
#ifndef LINUX
string::size_type pos = Attachments[FileId].find_last_of("\\");
#else
string::size_type pos = Attachments[FileId].find_last_of("/");
#endif
string fileName;
if(pos != string::npos)
{
fileName = Attachments[FileId].substr(pos+1);
encodedFileName += "=?UTF-8?B?";
encodedFileName += base64_encode((unsigned char *)fileName.c_str(),fileName.size());
encodedFileName += "?=";
}
strcat(SendBuf,"Content-Type: application/x-msdownload; name=\"");
strcat(SendBuf,encodedFileName.c_str());
strcat(SendBuf,"\"\r\n");
strcat(SendBuf,"Content-Transfer-Encoding: base64\r\n");
strcat(SendBuf,"Content-Disposition: attachment; filename=\"");
strcat(SendBuf,encodedFileName.c_str());
strcat(SendBuf,"\"\r\n");
strcat(SendBuf,"\r\n");
After all, we only need Attachment's name when,
1)opening the file,
2)filling info into "Content-Type: application/x-msdownload; name=" and "Content-Disposition: attachment; filename=".
In the above code,I have encoded the file name so unicode file names could be displayed correctly in email clients.
The same could be done with subject to make subject support unicode.
The above code works but may be Problematic due to my poor knowledge with email.
Could you consider making subject and attachment names support unicode in the next release?
|
|
|
|
|
uni_gauldoth,
Great idea! I changed your code just a little to streamline it. Do you see any problems with this implementation:
unsigned int i,rcpt_count,res,FileId;
char *FileBuf = NULL;
FILE* hFile = NULL;
unsigned long int FileSize,TotalSize,MsgPart;
string FileName,EncodedFileName;
string::size_type pos;
...
hFile = fopen(Attachments[FileId].c_str(), "rb");
...
for(FileId=0;FileId<Attachments.size();FileId++)
{
#ifndef LINUX
pos = Attachments[FileId].find_last_of("\\");
#else
pos = Attachments[FileId].find_last_of("/");
#endif
if(pos == string::npos) continue;
FileName = Attachments[FileId].substr(pos+1);
EncodedFileName = "=?UTF-8?B?";
EncodedFileName += base64_encode((unsigned char *) FileName.c_str(), FileName.size());
EncodedFileName += "?=";
snprintf(SendBuf, BUFFER_SIZE, "--%s\r\n", BOUNDARY_TEXT);
strcat(SendBuf, "Content-Type: application/x-msdownload; name=\"");
strcat(SendBuf, EncodedFileName.c_str());
strcat(SendBuf, "\"\r\n");
strcat(SendBuf, "Content-Transfer-Encoding: base64\r\n");
strcat(SendBuf, "Content-Disposition: attachment; filename=\"");
strcat(SendBuf, EncodedFileName.c_str());
strcat(SendBuf, "\"\r\n");
strcat(SendBuf, "\r\n");
SendData(pEntry);
...
To encode the subject do you just surround it with the same "=?UTF-8?B?" ..."?=" block and then base64_encode it?
Thanks,
David
|
|
|
|
|
Yes, I just put the utf-8 subject encoded in base64 in one "=?UTF-8?B?" ... "?=" block, and it does displays correctly in thunderbird, but other email clients may not display it correctly.
But acording to RFC 2047, if the subject is too long, multiple encoded-word should be used.
An 'encoded-word' may not be more than 75 characters long, including
'charset', 'encoding', 'encoded-text', and delimiters. If it is
desirable to encode more text than will fit in an 'encoded-word' of
75 characters, multiple 'encoded-word's (separated by CRLF SPACE) may
be used.
While there is no limit to the length of a multiple-line header
field, each line of a header field that contains one or more
'encoded-word's is limited to 76 characters.
The length restrictions are included both to ease interoperability
through internetwork mail gateways, and to impose a limit on the
amount of lookahead a header parser must employ (while looking for a
final ?= delimiter) before it can decide whether a token is an
"encoded-word" or something else.
Also RFC 2047 says,
Each 'encoded-word' MUST represent an integral number of characters.
A multi-octet character may not be split across adjacent 'encoded-
word's.
If we decide to split the subject into multiple encoded-words, we should not split it in the middle of an utf-8 character.
If an utf-8 character has three bytes, the three bytes should all be put into one encoded-word.
I don't known how to split utf-8 strings without breaking characters in the middle.
Below is a subject header composed by outlook 2013,
Subject: =?utf-8?B?UGxlYXNlIGpvaW4gdGhlIHJldmlldyBvZiDgpKTgpLLgpJXgpYsg4KS4?=
=?utf-8?B?4KS+4KSo4KWLIOCkrOCkvuCkleCkuCDgpK3gpL/gpKTgpY3gpLAg4KSw4KWL?=
=?utf-8?B?4KSu4KSo4KSu4KS+IOCkn+CkvuCkiOCkqiDgpJfgpLDgpY3gpKjgpYHgpLngpYs=?=
=?utf-8?B?4KS44KWN4LaG4Lav4La74LeK4LeBIOC3g+C3kuC2guC3hOC2vSDgtr0=?=
=?utf-8?B?4Lea4Lab4LarLiDgt4Pgt5Lgtrrgtrbgt4Pgt4og4LeA4LeZ4La24LeKIOC2hQ==?=
=?utf-8?B?4Lap4LeA4LeS4La6IOC3g+C3kuC2guC3hOC2vSDgtrrgt5TgtrHgt5Lgtprgt50=?=
=?utf-8?B?4Lap4LeKIOC3hOC2s+C3lOC2seC3jyDgtpzgt5DgtrHgt5Pgtrjgtqcg4La44LeZ?=
=?utf-8?B?4LeE4LeSIOC2tOC3kOC2uOC3kuC2q+C3meC2sSDgtpTgtrYg4LeD4LeP4Lav4La7?=
=?utf-8?B?4La64LeZ4Lax4LeKIOC2tOC3kuC3heC3kiDgtpzgtrHgt5MhIOC2uOC3meC2uA==?=
=?utf-8?B?IOC3gOC3meC2tuC3iiDtlqXssLBfcmV2aWV3LnBkZi4=?=
|
|
|
|
|
This sounds like something that would be good to incorporate. Do you want to come up with some code and post it and I'll pull it into the main distribution?
|
|
|
|
|
Yes, it's my pleasure.
I just got a little busy those days.
When I have time I will try to make it support utf-8 in subject, attachment name and attachment path.
The code is already done, but I need some time to make sure that they are compliant with those RFCs.
Does fopen support unicode in other system except windows?
I also think about making attachment path(used to locate the file, not the attachment path displayed in email) to use unicode, because attachment path contains characters not in the local code page is very common under windows.
|
|
|
|
|
That would be great and much appreciated. Just by way of note, however, version 2.3 of the library does support UFT-8 filenames for the attachments. It would be good for you to check over the implementation and make sure it is compliant, but it is already there.
Thanks,
David
|
|
|
|
|
In
int CSmtp::ConnectRemoteServer(...):
if(connect(hSocket,(LPSOCKADDR)&sockAddr,sizeof(sockAddr)) == SOCKET_ERROR)
{
...
}
else
{
return true; <======================= (??) ==============================
}
...
if(securityType!=DO_NOT_SET) SetSecurityType(securityType);
if(GetSecurityType() == USE_TLS || GetSecurityType() == USE_SSL)
{
InitOpenSSL();
if(GetSecurityType() == USE_SSL)
{
OpenSSLConnect();
}
}
modified 25-Jun-13 3:42am.
|
|
|
|
|
This part of the code probably doesn't work like you would expect based on typical syntax. The connect call is actually supposed to return a SOCKET_ERROR with the error code being WSAWOULDBLOCK so the else branch is avoided unless there is another error. See the explanation of WSAWOULDBLOCK here for more information:
http://msdn.microsoft.com/en-us/library/windows/desktop/ms740668(v=vs.85).aspx[^]
|
|
|
|
|
|
|
Tadkie,
Thanks for the suggestion. If you want to implement this and post back your proposed changes to the code to make it work I'd be happy to look at the result and add it to the main branch of the code.
Thanks,
David
|
|
|
|
|
Great article. This has helped me a lot. I hope I haven't sounded overly critical before because that certainly wasn't my intent.
Anyway here's a couple thoughts from looking at 2.2:
1. In CSmtp::Send (lines 535-553) you're missing an fclose(hFile) when opening attachments for the size check.
2. It'd be nice to have a ClearMessage function or some similar name that clears all of the "message specific" data. In other words, anything that refers to the actual email (recipients, attachments, body, etc.) and not anything referring to the server connection. This way it will be easy to send multiple emails during the same connection. This, of course, can done manually in the calling code, but a member function that handles this will make sure the caller doesn't miss anything when resetting for the next message.
An even better solution (although this is definitely more radical) is to separate out the connection info and the message info into separate classes. Then you'd have something like:
CSmtp connection;
CSmtpMessage m1, m2, m3;
// Configure messages here... e.g. m1.AddRecipient, etc.
connection.Send(m1);
connection.Send(m2);
connection.Send(m3);
3. What did you think of my idea to move the calls to DisconnectRemoteServer to only in the destructor for better exception safety?
|
|
|
|
|
GKarRacer,
Thanks for your input to the project. Your input has only been taken as intended to help the project get better for everyone's benefit. No offense taken whatsoever. Item 1 has been addressed and a patch of code to fix it and improve efficiency of adding attachments has been posted here:
http://www.codeproject.com/Messages/4562782/Re-retreiving-file-size-for-attachments.aspx
I like your idea on item 2. I've put that into release 2.3. As for the point about separating the message and the connection into two classes - actually I have done that in how I use the code. I came to this project from my own home-baked SMTP library to get the ability to use SSL. I just patched my old library into this one, which included a separate class for the message object. Maybe I should add that to the library.
I guess I thought it was a bit friendlier on the server to disconnect right away when a problem is encountered. Currently it's only called two other places: ConnectRemoteServer() and Send(), in both cases only when a known SMTP error is encountered. I don't think that is unsafe. I certainly value your further comments on it, though. Please post back your thoughts.
|
|
|
|
|
The reason its unsafe is that DisconnectRemoteServer can throw an exception that potentially won't be caught. This is especially a big no-no when inside a destructor. This causes the program to immediately terminate. There is also the potential of a double exception happening when Send (and maybe ConnectRemoteServer as well) throws an exception. Since DisconnectRemoteServer is called inside the catch handler for Send/ConnectRemoteServer, it's possible for another exception to get thrown. Unfortunately I'm a little unclear on the standard of what's supposed to happen in this case. I can tell you what Visual Studio 2010's implentation does: program terminates.
This exact scenario was happening with the original attachment code problem. I tried wrapping the DisconnectRemoteServer call (inside Send) with its own catch handler. The nested handler was able to catch the exception, but when Send's handler tried to rethrow the original exception, the program aborted. I tried many different variations, but every time I tried to throw from inside Send's catch handler, it aborted.
Now since the attachment code has been fixed, this scenario is less likely, but the potential still exists. The only solution I can think of at the moment that still lets the original exception propagate back up to the caller is to NOT call DisconnectRemoteServer from the Send/ConnectRemoteServer function and only call it from the destructor. But, absolutely ensuring that it is wrapped with its own try/catch block that does NOT propagate the exception.
Try it for yourself. Put in a deliberate throw ECSmtp from somewhere inside DisconnectRemoteServer and then make sure to throw while inside Send. See what happens.
|
|
|
|
|
Sorry to take so long thinking about this and looking it over multiple times. I wonder if we could just put a
try{
...
}catch(...){
}
handler surrounding all the code inside DisonnectRemoteServer and leave it as-is. I know that a throw; statement inside a catch block rethrows the existing exception. What I don't know for sure is if you have another exception thrown in the middle of that catch block, which exception does it re-throw? The original one or the one that ended up getting thrown inside the block? I would think as long as the second one was thrown and caught in a different function it would throw the original one. From what you described, I got the impression that you had tested putting another nested try{}catch{} block around the call the DisconnectRemoteServer, but had not tried putting it inside DisconnectRemoteServer. Is that correct? Do you have a way to test this alternative?
Thanks,
David
|
|
|
|
|
Sorry, just saw this. I haven't looked at it in a while, but if I remember correctly I believe I had tried it with nested catch blocks and it was still crashing as soon as I tried to rethrow the exception. I had even tried creating a new exception and throwing that instead, but alas it didn't work. Although I don't think I tried a generic catch handler. Honestly I'm not sure if the crash isn't just a bug in Visual Studio. I'm not sure exactly what the standard says about nested exceptions.
I'll have to pull up my test program and give it a whirl again. Hopefully I'll get a chance soon.
Edit:
OK, reread my original posts. I forgot I was talking about exceptions within the destructor. That part's very clear in the standard. That can't be allowed.
|
|
|
|
|
Hi David,
Can I used this code in commercial applications,
Thanks.
|
|
|
|
|
Yes, you may. It is subject to the CPOL:
http://www.codeproject.com/info/cpol10.aspx
It also carries with it certain obligations from OpenSSL, which you could find on their website, and obligations for including the MD5.cpp and MD5.h files, which you can read in their headers.
|
|
|
|
|
Hi,
i know we got this Topic already here but this is 3 years ago and the solution doesn't work at me.
So my question:
I want to compile the Project files in Dev-C++.
What i did:
-Download your package.
-Put the necessary files (*.h and *.cpp) into a new Project file.
-Add "openssl-0.9.8l\inc32" to Additional Include Directories with clicking on: Project -> Project Options -> Directories -> Include Directories
-Deleted the following lines in CSmtp.cpp:
#ifndef LINUX
#endif
Downloaded the openssl package for Dev-C++ from here: http://sourceforge.net/projects/devpaks/files/openssl/[^] and installed it with the Package Manager.
Then I clicked: Project -> Project Options -> Parameters -> then in the Linker Box:
-Ws2_32.lib
openssl-0.9.8l/out32/libeay32.lib
openssl-0.9.8l/out32/ssleay32.lib
But now i get 500 Linker Errors:
https://dl.dropboxusercontent.com/u/22986507/Linker%20Errors.txt[^]
The openssl Folder is in the Project Folder. So the structure is like that:
https://dl.dropboxusercontent.com/u/22986507/Data%20Structure.pdf[^]
Thanks for your help and i hope you understand me
|
|
|
|
|