cutelyst / simple-mail

An SMTP library written in C++ for Qt. Allows applications to send emails (MIME with text, html, attachments, inline files, etc.) via SMTP. Supports SSL and SMTP authentication.
GNU Lesser General Public License v2.1
213 stars 64 forks source link

SimpleMail gets stuck in somewhere and don't report a timeout #73

Closed MuratUrsavas closed 1 year ago

MuratUrsavas commented 2 years ago

Hi,

I'm trying to use SimpleMail in my app and would like to thank you for your efforts. I'm using it asynchronously and it does not respond back in my current condition. Let me explain what's going on:

Note: I'm using C++17 and trying to use it like in the example, with little differences.

Here's my variable definitions in the header:

    SimpleMail::Server server;
    SimpleMail::ServerReply *serverReply = nullptr;
    SimpleMail::MimeText *mimeText = nullptr;

Here's the SimpleMail server configuration in the constructor:

    server.setHost(smtpHost);
    server.setPort(smtpPort);
    server.setConnectionType(SimpleMail::Server::SslConnection);
    server.setUsername(senderAddr);
    server.setPassword(senderPw);

The parameters are all defined as static constexpr char[] for test purposes.

Here's the actual send sequence:

        SimpleMail::MimeMessage message;
        message.setSender(SimpleMail::EmailAddress(senderAddr, senderName));
        message.addTo(SimpleMail::EmailAddress(recipient));
        message.setSubject(subject);

        // First we create a MimeText object.
        // This must be created with new otherwise it will be deleted once we leave the scope.
        mimeText = new SimpleMail::MimeText;

        // Now add some text to the email.
        mimeText->setText(body);

        // Now add it to the mail
        message.addPart(mimeText);

        // Now we can send the mail
        sendOperationCompleted = false;
        serverReply = server.sendMail(message);
        QObject::connect(serverReply, &SimpleMail::ServerReply::finished, [this] {
            this->sendOperationCompleted = true;
            if(this->serverReply->error())
            {
                this->threadResult = Result::Code::netSmtpError;
                this->smtpErrorText = this->serverReply->responseText();
            }
            else
            {
                this->threadResult = Result::Code::success;
            }
            this->triggerThread();
        });

I've removed most of the irrelevant code to simplify the case.

Here's the log send by SimpleMail:

simplemail.server: Connecting to host encrypted "smtp.gmail.com" 465
simplemail.server: stateChanged QAbstractSocket::HostLookupState ""
simplemail.server: stateChanged QAbstractSocket::ConnectingState ""
simplemail.server: stateChanged QAbstractSocket::ConnectedState ""
simplemail.server: connected 2 ""
simplemail.server: readyRead 53
simplemail.server: Got response "220 smtp.gmail.com ESMTP kz3sm2422235ejc.71 - gsmtp" expected 220
simplemail.server: readyRead 0
simplemail.server: readyRead 222
simplemail.server: Got response "250-smtp.gmail.com at your service, [xxx.xxx.xxx.xxx]"
simplemail.server: Got response "250-SIZE 35882577"
simplemail.server: Got response "250-8BITMIME"
simplemail.server: Got response "250-AUTH LOGIN PLAIN XOAUTH2 PLAIN-CLIENTTOKEN OAUTHBEARER XOAUTH"
simplemail.server: Got response "250-ENHANCEDSTATUSCODES"
simplemail.server: Got response "250-PIPELINING"
simplemail.server: Got response "250-CHUNKING"
simplemail.server: Got response "250 SMTPUTF8"
simplemail.server: CAPS ("250-smtp.gmail.com at your service, [xxx.xxx.xxx.xxx]", "250-SIZE 35882577", "250-8BITMIME", "250-AUTH LOGIN PLAIN XOAUTH2 PLAIN-CLIENTTOKEN OAUTHBEARER XOAUTH", "250-ENHANCEDSTATUSCODES", "250-PIPELINING", "250-CHUNKING", "250 SMTPUTF8")
simplemail.server: LOGIN SimpleMail::Server::AuthPlain
simplemail.server: Sending authentication plain 4
simplemail.server: readyRead 0
simplemail.server: readyRead 20
simplemail.server: Got response "235 2.7.0 Accepted" expected 235
simplemail.server: Sending MAIL command true 3 ("MAIL FROM:<xxxxxx@xxxxxx.com.tr>\r\n", "RCPT TO:<xxxxxxx@gmail.com>\r\n", "DATA\r\n") (250, 250, 354)
simplemail.server: readyRead 0
simplemail.server: readyRead 41
simplemail.server: Got response "250 2.1.0 OK kz3sm2422235ejc.71 - gsmtp"
simplemail.server: readyRead 0
simplemail.server: readyRead 41
simplemail.server: Got response "250 2.1.5 OK kz3sm2422235ejc.71 - gsmtp"
simplemail.server: readyRead 0
simplemail.server: readyRead 42
simplemail.server: Got response "354  Go ahead kz3sm2422235ejc.71 - gsmtp"
simplemail.server: Mail sent
simplemail.server: readyRead 0

The state machine of SimpleMail gets stuck in this phase and there is nothing to save it from this state. So I'm planning to add a timeout logic with a QTimer. But first, I need to understand what's wrong right now.

This is the Wireshark capture of the sequence:

image

From what I see, mail body is sent and ACK is received from the SMTP server, but it does not respond back with some feedback and SimpleMail just waits for it forever.

How can I solve the issue? What am I doing wrong?

Edit:

Took a log from Thunderbird for the same operation and seen that this response is missing indeed:

Response: 250 2.0.0 OK  1640334423 y11sm2476362ejw.147 - gsmtp

Therefore the following code is never called and SimpleMail gets stuck in the ServerReplyContainer::SendingData state forever (at least till to the TCP close from the remote party).

                    } else if (cont.state == ServerReplyContainer::SendingData) {
                        QByteArray responseText;
                        const int code = parseResponseCode(&responseText);
                        if (!cont.reply.isNull()) {
                            ServerReply *reply = cont.reply;
                            queue.removeFirst();
                            reply->finish(code != 250, code, QString::fromLatin1(responseText));
                        } else {
                            queue.removeFirst();
                        }
                        qCDebug(SIMPLEMAIL_SERVER) << "MAIL FINISHED" << code << queue.size() << socket->canReadLine();

                        processNextMail();
                    }
dantti commented 2 years ago

have you tried to use the demo application? it used to work with gmail servers tho there's been some time that I don't use it

MuratUrsavas commented 2 years ago

Nope, not tried. Jumped right into actual code instead.

There's been a recent OAuth2 change happened in GMail but I've explicitly disabled it from the settings and tested the same settings with Thunderbird.

I'm suspecting from a missing expected part in the mail body but interestingly GMail server is also not responding with error or disconnecting in a timely manner.

I'll try the demo and get back with the results.

dantti commented 2 years ago

there was some commit to change the ending \r\n sent to the server, the code looked fine but I didn't test so maybe it's related as it seems the server is still waiting for the message, you might try with an older version.

MuratUrsavas commented 2 years ago

I suspected it and tried the same via adding those lines into mail body first, but it wasn't a success. Then reverted back to HEAD-1, and voila. Just like you said, removing double \r\n's wasn't a good idea in the first place. Reverting the last commit would fix the issue.

Would you like me to do that and send a PR or simply revert it back? I'll add the timeout logic after this fix. GMail server somehow could get lost in somewhere, so SimpleMail should be able to recover without any intervention from server side.

dantti commented 2 years ago

Hi @michaelernst can you spot what went wrong with your path, how to fix it without reverting?

michaelernst commented 2 years ago

I will have a look in monday or thuesday

adam-seychell commented 2 years ago

I am seeing the same issue. All the asynchronous versions of the demos do not respond and the email is never sent. I have tested with with smtp.gmail.com and my ISP's server. The blocking mode of the demos all work as expected. I tried revert to the above mentioned commit, but did not fix the problem.

mariusheise commented 2 years ago

Was having the same issue with async, TLS and AuthLogin. Can confirm, that reverting 82f877c455380a899186854a91b4534ac5050d8c fixes this.

dantti commented 1 year ago

Mind trying again with current master (to be v3) ?

dantti commented 1 year ago

actually yes, this is due https://github.com/cutelyst/simple-mail/commit/82f877c455380a899186854a91b4534ac5050d8c which was fixed in v2 branch and master