SMTP Client improvements. #2802
Conversation
| ~MailMessage() | ||
| { | ||
| delete stream; | ||
| for(auto attachment : attachments) { | ||
| delete attachment.headers; | ||
| delete attachment.stream; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Looks like stream should be unique_ptr. Also not sure it's necessary to destroy attachments, should happen automatically when vector is destroyed.
There was a problem hiding this comment.
Just done some checking and of course attachment has no destructor...
| if(hostName) { | ||
| ssl->hostName = hostName; | ||
| } |
There was a problem hiding this comment.
Is check necessary? Or is that in case there's an sslInit callback registered which might do it. Maybe document to clarify? Or Just assign ssl->hostName = hostName
| useSsl = true; | ||
| useSsl = (internalOnConnected(ERR_OK) == ERR_OK); |
There was a problem hiding this comment.
Remove redundant useSsl = true;
There was a problem hiding this comment.
I know that the code looks a bit wierd but in internalOnConnected() useSsl has to be true to kick start the SSL connection.
err_t TcpConnection::internalOnConnected(err_t err)
{
debug_tcp_d("connected: useSSL: %d, Error: %d", useSsl, err);
if(useSsl && err == ERR_OK) {There was a problem hiding this comment.
May I suggest a comment and/or slight rewrite:
useSsl = true;
if (internalOnConnected(ERR_OK) != ERR_OK) {
useSsl = false;
}
There was a problem hiding this comment.
May I suggest a comment and/or slight rewrite:
Sure. I will apply your change in a minute.
…ction. If SSL is not compiled don't bail out and try to communicate via plain-text tcp.
slaff
force-pushed
the
feature/tcp-connection-enablessl
branch
from
d0f2185 to
6228d34
Compare
slaff
force-pushed
the
feature/tcp-connection-enablessl
branch
from
f8a2858 to
30c0838
Compare
|
@mikee47 shall I merge this PR with the applied review changes? |
| @@ -288,7 +289,7 @@ void SmtpClient::sendMailHeaders(MailMessage* mail) | |||
|
|
|||
| if(!mail->headers.contains(HTTP_HEADER_CONTENT_TRANSFER_ENCODING)) { | |||
| mail->headers[HTTP_HEADER_CONTENT_TRANSFER_ENCODING] = _F("quoted-printable"); | |||
| mail->stream = new QuotedPrintableOutputStream(mail->stream); | |||
| mail->stream.reset(new QuotedPrintableOutputStream(mail->stream.release())); | |||
There was a problem hiding this comment.
mail->stream = std::make_unique<QuotedPrintableOutputStream>(mail->stream.release());
TcpConnection::enableSsl(hostName)which allows a plain-text TCP connection to be changed to SSL one after the tcp connection is established.