Sending email using libcurl

Question

I developed a function to send email based on this. Does this code have problems? Also, I'm not sure if this code is thread-safe because of the global payloadText variable.

#include <string>
#include <curl/curl.h>

static std::string payloadText[11];

std::string dateTimeNow();
std::string generateMessageId();

void setPayloadText(const std::string &to,
                    const std::string &from,
                    const std::string &cc,
                    const std::string &nameFrom,
                    const std::string &subject,
                    const std::string &body)
{
    payloadText[ 0] = "Date: "  + dateTimeNow();
    payloadText[ 1] = "To: <"   + to   + ">\r\n";
    payloadText[ 2] = "From: <" + from + "> (" + nameFrom + ")\r\n";
    payloadText[ 3] = "Cc: <"   + cc   + "> (" + nameFrom + ")\r\n";
    payloadText[ 4] = "Message-ID: <" + generateMessageId() + "@" + from.substr(from.find('@') + 1) + ">\r\n";
    payloadText[ 5] = "Subject: " + subject + "\r\n";
    payloadText[ 6] = "\r\n";
    payloadText[ 7] = body + "\r\n";
    payloadText[ 8] = "\r\n";
    payloadText[ 9] = "\r\n"; // "It could be a lot of lines, could be MIME encoded, whatever.\r\n";
    payloadText[10] = "\r\n"; // "Check RFC5322.\r\n";
}

std::string dateTimeNow()
{
    static const char *DAY_NAMES  [] = { "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat" };
    static const char *MONTH_NAMES[] = { "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec" };

    const int RFC1123_TIME_LEN = 29;
    time_t t;
    struct tm tm;

    std::string ret;
    ret.resize(RFC1123_TIME_LEN);

    time(&t);
    gmtime_s(&tm, &t);

    strftime(&ret[0], RFC1123_TIME_LEN + 1, "---, %d --- %Y %H:%M:%S GMT", &tm);
    memcpy(&ret[0], DAY_NAMES  [tm.tm_wday], 3);
    memcpy(&ret[8], MONTH_NAMES[tm.tm_mon],  3);

    return ret;
}

std::string generateMessageId()
{
    const int MESSAGE_ID_LEN = 37;
    time_t t;
    struct tm tm;

    std::string ret;
    ret.resize(15);

    time(&t);
    gmtime_s(&tm, &t);

    strftime(const_cast<char *>(ret.c_str()),
             MESSAGE_ID_LEN,
             "%Y%m%d%H%M%S.",
             &tm);

    ret.reserve(MESSAGE_ID_LEN);

    static const char alphanum[] =
            "0123456789"
            "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
            "abcdefghijklmnopqrstuvwxyz";

    while (ret.size() < MESSAGE_ID_LEN) {
        ret += alphanum[rand() % (sizeof(alphanum) - 1)];
    }

    return ret;
}

struct upload_status { int lines_read; };

static size_t payload_source(void *ptr, size_t size, size_t nmemb, void *userp)
{
    std::string s = generateMessageId();

    static const char *pt[12] = {};

    for (int i = 0; i < 11; ++i) {
        pt[i] = payloadText[i].c_str();
    }

    pt[11] = NULL;

    struct upload_status *upload_ctx = (struct upload_status *)userp;
    const char *data;

    if ((size == 0) || (nmemb == 0) || ((size*nmemb) < 1)) {
        return 0;
    }

    data = pt[upload_ctx->lines_read];

    if (data) {
        size_t len = strlen(data);
        memcpy(ptr, data, len);
        upload_ctx->lines_read++;

        return len;
    }

    return 0;
}

CURLcode sendEmail(const std::string &to,
                   const std::string &from,
                   const std::string &cc,
                   const std::string &nameFrom,
                   const std::string &subject,
                   const std::string &body,
                   const std::string &url,
                   const std::string &password)
{
    CURLcode ret = CURLE_OK;

    struct curl_slist *recipients = NULL;
    struct upload_status upload_ctx;

    upload_ctx.lines_read = 0;

    CURL *curl = curl_easy_init();

    setPayloadText(to, from, cc, nameFrom, subject, body);

    if (curl) {
        curl_easy_setopt(curl, CURLOPT_USERNAME,     from    .c_str());
        curl_easy_setopt(curl, CURLOPT_PASSWORD,     password.c_str());
        curl_easy_setopt(curl, CURLOPT_URL,          url     .c_str());

        curl_easy_setopt(curl, CURLOPT_USE_SSL,      (long)CURLUSESSL_ALL);
        //curl_easy_setopt(curl, CURLOPT_CAINFO, "/path/to/certificate.pem");

        curl_easy_setopt(curl, CURLOPT_MAIL_FROM,    ("<" + from + ">").c_str());
        recipients = curl_slist_append(recipients,   ("<" + to   + ">").c_str());
        recipients = curl_slist_append(recipients,   ("<" + cc   + ">").c_str());

        curl_easy_setopt(curl, CURLOPT_MAIL_RCPT,    recipients);
        curl_easy_setopt(curl, CURLOPT_READFUNCTION, payload_source);
        curl_easy_setopt(curl, CURLOPT_READDATA,     &upload_ctx);
        curl_easy_setopt(curl, CURLOPT_UPLOAD,       1L);
        curl_easy_setopt(curl, CURLOPT_VERBOSE,      1L);

        ret = curl_easy_perform(curl);

        if (ret != CURLE_OK) {
            fprintf(stderr, "curl_easy_perform() failed: %s\n", curl_easy_strerror(ret));
        }

        curl_slist_free_all(recipients);
        curl_easy_cleanup(curl);
    }

    return ret;
}

int main()
{
    sendEmail("[email protected]",
              "[email protected]",
              "[email protected]",
              "FromName",
              "Subject",
              "Body",
              "smtp://smtp.email.x:25",
              "password");
}

Answer 1

Here are some things that may help you improve your code.

Use required #includes

The code invokes gmtime_s, memcpy and strlen but doesn't have the required headers that define those. The code should include these lines:

#include <ctime>
#include <cstring>

First, as noted in http://en.cppreference.com/w/c/chrono/gmtime :

As with all bounds-checked functions, gmtime_s is only guaranteed to be available if __STDC_LIB_EXT1__ is defined by the implementation and if the user defines __STDC_WANT_LIB_EXT1__ to the integer constant 1 before including time.h.

It's also not guaranteed be thread safe, so you might want to use the POSIX gmtime_r instead.

Don't write C code in C++

This is basically C code written with a little bit of C++. It could be greatly improved by using more of a C++ approach, as by using a class to represent an email and having most of the functions implemented as member functions. For example, I did a fairly simple refactoring of this code and came up with this class interface:

class email 
{
public:
    email(const std::string &to,
          const std::string &from,
          const std::string &nameFrom,
          const std::string &subject,
          const std::string &body,
          const std::string &cc = ""
         );
    std::string dateTimeNow() const;
    CURLcode send(const std::string &url, 
                  const std::string &username, 
                  const std::string &password);
private:
    // data
    std::string to_, from_, cc_, nameFrom_, subject_, body_;
    // functions
    std::string setPayloadText();
    std::string generateMessageId() const;
    // static functions
    static size_t payload_source(void *ptr, size_t size, size_t nmemb, void *userp);
};

Fix the bugs

The code does not currently format a message correctly because there is no "\r\n" after the date string. It also does not work if there is no "CC" address. Please read the relevant RFCs (e.g. RFC 5322) to understand how an email is properly formatted. For example, the time string format is old. Instead of using the time zone abbreviation "GMT", the preferred format is to use the timezone offset. For that reason, you could just as easily use localtime rather than gmtime.

Use C++11 version of strftime

If you set the locale appropriately, you can eliminate all of the day and month parts from dateTimeNow() and just use this:

std::string dateTimeNow()
{
    const int RFC5322_TIME_LEN = 32;
    time_t t;
    struct tm *tm;

    std::string ret;
    ret.resize(RFC5322_TIME_LEN);

    time(&t);
    tm = localtime(&t);

    strftime(&ret[0], RFC5322_TIME_LEN, "%a, %d %b %Y %H:%M:%S %z", tm);

    return ret;
}

Eliminate global variables

There is absolutely no reason that payloadText needs to be 11 strings and no reason that it needs to be global. The last parameter of the payload_source is intended to be used for user data. One way to do that would be to define your own class like this:

class stringdata {
public:
    std::string msg;
    size_t bytesleft;

    stringdata(std::string &&m) 
        : msg{m}, bytesleft{msg.size()}
    {}
    stringdata(std::string &m) = delete;
};

The reworked payload_source would be this:

static size_t payload_source(void *ptr, size_t size, size_t nmemb, void *userp)
{
    stringdata *text = reinterpret_cast<stringdata *>(userp);

    if ((size == 0) || (nmemb == 0) || ((size*nmemb) < 1) || (text->bytesleft == 0)) {
        return 0;
    }

    if ((nmemb * size) >= text->msg.size()) {
        text->bytesleft = 0;
        return text->msg.copy(reinterpret_cast<char *>(ptr), text->msg.size());
    }

    return 0;
}

And finally, to use it within sendEmail we can initialize the data like this:

stringdata textdata{setPayloadText(to, from, cc, nameFrom, subject, body)};

(Note that this assumes that setPayloadText is rewritten to return a single std::string -- it's a fairly trivial rewrite.)

Then the options to be able to use these together would look like this:

curl_easy_setopt(curl, CURLOPT_READFUNCTION, payload_source);
curl_easy_setopt(curl, CURLOPT_READDATA,     &textdata);

Use iostreams rather than printf

Instead of this:

if (ret != CURLE_OK) {
    fprintf(stderr, "curl_easy_perform() failed: %s\n", curl_easy_strerror(ret));
}

You could use this:

if (ret != CURLE_OK) {
    std::cerr << "curl_easy_perform() failed: " 
        << curl_easy_strerror(ret) 
        << "\n";
}

Answer 2

C VS C++

Your question is taged with both C++ and C++14, yet the only C++ part in your code seems to the use of std::string.

Use RAII

As you are using a C library, creating a small wrapper around it allows you to benefit from C++'s features and thus handle the task of freeing up resources much easier/safer.

Global variables

You're right in expressing concerns regarding the global variable. It can introduce problems, concurrency issues among them. However it is very easy to fix. Approaches include:

• Return the value from the function that creates it (preferred)
• Encapsulate your code inside a class

CURLOPT_READFUNCTION

Your implementation of the callback provided for CURLOPT_READFUNCTION looks wrong. That function is handing you a buffer that you have to fill. It also provides you with the size of that buffer so you don't overflow it. Your code isn't using the provided buffer size for anything except checking that it's a positive value.

Secondly, this function deals with binary data. It doesn't care about lines, so you seems to have over-complicated the design by keeping separate lines of your output. My suggestion:

• Compose your output as a simple string
• Use the upload status structure to store how many bytes you have already transferred in the callback (0 at first)
• Whenever the callback is called, copy as much data as possible (without overflowing the buffer) and update what's left. You may end up sending the entire buffer in one call if the buffer is large enough.