libcurl smtp message sending

Question

Background

I've been building a small test program that uses libcurl to send an email. The ultimate goal is to implement this as a remote notification system for a larger piece of software. I've mostly used this and this as guides.

Problem

The program works up until the callback function. Debugger shows curl_callback is receiving the 'string' TESTMESSAGE.email properly. Then calls curl_callback two more times (three total) and hangs.

From my understanding, curl uses the callback function to iterate through the 'string', reading each char. Based on what I am seeing in the debugger, this appears to not be the case.

I'm thinking the issue lies in how I am passing my message to curl, but I don't have a good enough understanding of how curl operates to verify this. The many examples and forums I've looked at seem to all contradict one another as to the best way to prepare and pass the message.

Note: curl_global_init() is used because the functions developed here will be implemented into a multi-threaded program.

Source

header.h

/* Define message structures */
typedef struct emailMsg {
    const char *toEmail;
    const char *toName;
    const char *fromEmail;
    const char *fromName;
    const char *subject;
    const char *body;
    const char **email;
    size_t pos;
} emailMsg;


int sendEmail(emailMsg *data);
static size_t curl_callback(void *buffer, size_t size, size_t nmemb, void *instream);
char *payload[6];

source.c

#include <header.h>    

int main(void){
    curl_global_init(CURL_GLOBAL_NOTHING);

    struct emailMsg TESTMESSAGE;

    /* Definitions */
    TESTMESSAGE.toEmail = "<toEmail@somedomain.com>";
    TESTMESSAGE.toName = "toName";
    TESTMESSAGE.fromEmail = "<fromEmail@somedomain.com>";
    TESTMESSAGE.fromName = "fromName";
    TESTMESSAGE.subject = "Test Message";
    TESTMESSAGE.body = "THIS IS A TEST MESSAGE.\r\n"
                    "\r\n"
                    "DISREGARD IT AND HAVE A NICE DAY!";

    sendEmail(&TESTMESSAGE);

    return(0);
}


int sendEmail(emailMsg *data){
    size_t bodySize = strlen(data->body) + 3;
    char *body = malloc(bodySize);
    snprintf(body, bodySize, "%s%s", data->body, "\r\n");

    size_t subjectSize = strlen(data->subject) + 12;
    char *subject = malloc(subjectSize);
    snprintf(subject, subjectSize, "%s%s%s", "Subject: ", data->subject, "\r\n");

    size_t receiverSize = strlen(data->toName) + strlen(data->toEmail) + 10;
    char *receiver = malloc(receiverSize);
    snprintf(receiver, receiverSize, "%s%s (%s)%s", "To: ", data->toEmail, data->toName, "\r\n");

    size_t senderSize = strlen(data->fromEmail) + strlen(data->fromName) + 12;
    char *sender = malloc(senderSize);
    snprintf(sender, senderSize, "%s%s (%s)%s", "From: ", data->fromEmail, data->fromName, "\r\n");

    size_t timeSize = 40;
    char *timeString = malloc(timeSize);
    time_t rawtime = 0;
    time(&rawtime);
    struct tm *timeinfo = localtime(&rawtime);
    strftime (timeString, timeSize, "Date: %a, %d %b %Y %T %z\r\n", timeinfo);

    /* Create payload (string) */
    size_t total_length = timeSize + receiverSize + senderSize + subjectSize + bodySize + 1;

    char *payload = (char*)malloc(total_length);
    memset(payload,0,strlen(payload));

    strcat(payload, timeString);
    strcat(payload, receiver);
    strcat(payload, sender);
    strcat(payload, subject);
    strcat(payload, body);
    strcat(payload, "\0");

    data->email = &payload;

    CURL *curl;
    CURLcode res = CURLE_OK;
    struct curl_slist *recipients = NULL;

    data->pos = 0;

    curl = curl_easy_init();
    if (curl){
        curl_easy_setopt(curl, CURLOPT_URL, "smtp://somemta");

        curl_easy_setopt(curl, CURLOPT_MAIL_FROM, data->fromEmail);
        recipients = curl_slist_append(recipients, data->toEmail);
        curl_easy_setopt(curl, CURLOPT_MAIL_RCPT, recipients);

        curl_easy_setopt(curl, CURLOPT_READFUNCTION, curl_callback);
        curl_easy_setopt(curl, CURLOPT_READDATA, data);
        curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
        /* useful for debugging encryped traffic */
        curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);

        /* send the message */
        res = curl_easy_perform(curl);

        /* Check for errors */
        if(res != CURLE_OK){
            fprintf(stderr, "curl_easy_perform() failed: %s\n", curl_easy_strerror(res));
        };

        /* Free receipients list and cleanup */
        curl_slist_free_all(recipients);
        curl_easy_cleanup(curl);
    };

    return((int)res);
}


static size_t curl_callback(void *buffer, size_t size, size_t nmemb, void *instream){
/*
 * Pass N bytes of data to libcurl for the transfer requested.
 * This is done 'size' by 'size'.
 *
 * Parameters:
 *  void *buffer:   Pointer to the buffer
 *  size_t size:    Size of each item
 *  size_t nmemb:   Number of items
 *  void *instream: Pointer address of data
 */
    /* Check for invalid arguments */
    if((size == 0) || (nmemb == 0) || ((size * nmemb) < 1)){
        return(0);
    };

    emailMsg *upload = (emailMsg*)instream;
    const char *email = upload->email[upload->pos];

    if(email){
        size_t len = strlen(email);
        if(len > size * nmemb){
            return(CURL_READFUNC_ABORT);
        };
        memcpy(buffer, email, len);
        upload->pos++;

        return(len);
    };

    return(0);
}

Edit: Resolution

Thanks to nephtes for helping point out the flaw in my logic and some other issues with the program. Your solution helped me realize that by concatenating the strings into one, I was removing the NULL element that CURL was looking for to properly terminate the upload. The working source is below.

header.h

#define MTA "somemta"

/* Define message structures */
typedef struct emailMsg {
    const char *toEmail;
    const char *toName;
    const char *fromEmail;
    const char *fromName;
    const char *subject;
    const char *body;
    size_t pos;
    char *email[];
} emailMsg;

int sendEmail(emailMsg *data);
static size_t curl_callback(void *buffer, size_t size, size_t nmemb, void *instream);
char *payload;

source.c

#include <header.h>

int main(void){
    curl_global_init(CURL_GLOBAL_NOTHING

    struct emailMsg TESTMESSAGE;

    /* Definitions */
    TESTMESSAGE.toEmail = "<toEmail@somedomain.com>";
    TESTMESSAGE.toName = "toName";
    TESTMESSAGE.fromEmail = "<fromEmail@somedomain.com>";
    TESTMESSAGE.fromName = "fromName";
    TESTMESSAGE.subject = "Test Message";
    TESTMESSAGE.body = "THIS IS A TEST MESSAGE.\r\n"
                        "\r\n"
                        "DISREGARD IT AND HAVE A NICE DAY!";

    /* Process and send message */
    sendEmail(&TESTMESSAGE);

    /* Return zero for success */
    return(0);
}


int sendEmail(emailMsg *data){
    int i;
    size_t sizes[6];

    /* Date */
    /* Calculate string size */
    sizes[0] = 39;

    /* Allocate memory (+1 for NULL) */
    data->email[0] = malloc(sizes[0] + 1);

    /* Compose string */
    time_t rawtime = 0;
    time(&rawtime);
    struct tm *timeinfo = localtime(&rawtime);
    strftime (data->email[0], sizes[0] + 1, "Date: %a, %d %b %Y %T %z\r\n", timeinfo);


    /* Receiver */
    /* Calculate string size */
    sizes[1] = strlen(data->toName) + strlen(data->toEmail) + 9;

    /* Allocate memory (+1 for NULL) */
    data->email[1] = malloc(sizes[1] + 1);

    /* Compose string */
    snprintf(data->email[1], sizes[1] + 1, "%s%s (%s)%s", "To: ", data->toEmail, data->toName, "\r\n");


    /* Sender */
    /* Calculate string size */
    sizes[2] = strlen(data->fromEmail) + strlen(data->fromName) + 11;

    /* Allocate memory (+1 for NULL) */
    data->email[2] = malloc(sizes[2] + 1);

    /* Compose string */
    snprintf(data->email[2], sizes[2] + 1, "%s%s (%s)%s", "From: ", data->fromEmail, data->fromName, "\r\n");


    /* Subject */
    /* Calculate string size */
    sizes[3] = strlen(data->subject) + 11;

    /* Allocate memory (+1 for NULL) */
    data->email[3] = malloc(sizes[3] + 1);

    /* Compose string */
    snprintf(data->email[3], sizes[3] + 1, "%s%s%s", "Subject: ", data->subject, "\r\n");


    /* Body */
    /* Calculate string size */
    sizes[4] = strlen(data->body) + 2;

    /* Allocate memory (+1 for NULL) */
    data->email[4] = malloc(sizes[4] + 1);

    /* Compose string */
    snprintf(data->email[4], sizes[4] + 1, "%s%s", data->body, "\r\n");

    /* NULL terminated */
    sizes[5] = 1;
    data->email[5] = "";

    /* Reset counter */
    data->pos = 0;

    printf("%s", data->email);

    /* CURL initialization */
    CURL *curl;
    CURLcode res = CURLE_OK;
    struct curl_slist *recipients = NULL;

    curl = curl_easy_init();
    if (curl){
        curl_easy_setopt(curl, CURLOPT_URL, MTA);

        curl_easy_setopt(curl, CURLOPT_MAIL_FROM, data->fromEmail);
        recipients = curl_slist_append(recipients, data->toEmail);
        curl_easy_setopt(curl, CURLOPT_MAIL_RCPT, recipients);

        curl_easy_setopt(curl, CURLOPT_READFUNCTION, curl_callback);
        curl_easy_setopt(curl, CURLOPT_READDATA, data);
        curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
        /* useful for debugging encryped traffic */
        curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);

        /* send the message */
        res = curl_easy_perform(curl);

        /* Free receipients list and cleanup */
        curl_slist_free_all(recipients);
        curl_easy_cleanup(curl);
    };

    /* Return CURL response */
    return((int)res);
}


static size_t curl_callback(void *buffer, size_t size, size_t nmemb, void *instream){
/*
 * Pass N bytes of data to libcurl for the transfer requested.
 * This is done 'size' by 'size'.
 *
 * Parameters:
 *  void *buffer:   Pointer to the buffer
 *  size_t size:    Size of each item
 *  size_t nmemb:   Number of items
 *  void *instream: Pointer address of data
 */
    /* Check for invalid arguments */
    if((size == 0) || (nmemb == 0) || ((size * nmemb) < 1)){
        return(0);
    };

    /* Assign instream to local structure */
    emailMsg *upload = (emailMsg*)instream;

    /* Assign current position to local variable */
    const char *email = upload->email[upload->pos];

    /* Run until NULL reached */
    if(email){
        /* Find length of string */
        size_t len = strlen(email);

        /* len > expected message */
        if(len > size * nmemb){
            return(CURL_READFUNC_ABORT);
        };

        /* Copy to buffer */
        memcpy(buffer, email, len);

        /* Increment */
        upload->pos++;

        /* Return the number of bytes written */
        return(len);
    };

    /* Signify end of upload */
    return(0);
}

Are you deliberately ignoring errors? And do not cast the return value of `malloc()`. If you must cast you are using the wrong programming language or the wrong allocation function. Also, please explain `+12` anf `+10` there. Normally you should not include the null terminator so that you write `malloc(some_length + 1)`, I got all confused with your code. Also, don't just `malloc()` and think that it worked. Please always check for `NULL`, you never know when it's going to happen, and if it does your code is completely unprotected against such failure. — Iharob Al Asimi, Jul 22 '16 at 19:06
And DO NOT use `strcat()` like that, you have all the lengths, use `memcpy()` because `strcat()` like that is going to be very inefficient. Specially `strcat(payload, "\0")` where there are two `'\0'`s since `""` is actualy of size `1` and length `0`, do you think that passing `""` to `strlen()` is defined behavior? If so, how do you think `strlen()` determines it's length and returns `0`? — Iharob Al Asimi, Jul 22 '16 at 19:10
Oh, and another thing that might cause you further issues: you're declaring a global `payload` variable in your header and another variable, also named `payload`, in `sendEmail()`. This is certain to cause confusion. — nephtes, Jul 22 '16 at 21:04
@iharob: I left out my error checking because it bloats the code, [here's](http://pastebin.com/aFwXF8k8) the full source. Thanks for pointing out [the importance of not casting with malloc](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). — Nick, Jul 25 '16 at 19:12

nephtes · Accepted Answer · 2016-07-22T22:37:38.937

1

Here's what you're doing before curl_easy_perform()

char *payload = (char*)malloc(total_length);
data->email = &payload;

And here's what you're doing in the callback:

const char *email = upload->email[upload->pos]

The problem is that upload->email (aka data->email) is not an array, yet you're treating it like one. Index 0 will be fine, but anything else will point to some random memory location.

What you need to do is make email a simple char* buffer instead of char**, and use upload->pos as a position index into that buffer, incrementing its value as you copy data to the buffer provided in the callback argument.

edited Jul 22 '16 at 22:37

answered Jul 22 '16 at 19:17

nephtes

1,319
9
19

@iharob well, that was fudging things a little. Array index 1 will point to whatever happens to be next to `payload` on the stack, and so on. Treating *that* as a `char*` will in fact point to some random memory location: maybe it's another string and looks ok, maybe it's a `size_t` and causes a segmentation fault. Bottom line, it's unpredictable. – nephtes Jul 22 '16 at 19:31
Random is not the same as unpredictable, I object to many things in the OPs code. But this seems to be the immediate reason for the poblem. Although a lot of potential UB in the code too. – Iharob Al Asimi Jul 22 '16 at 20:56
@iharob hahaha, yeah, you made your feelings about the code quite clear! – nephtes Jul 22 '16 at 21:05

libcurl smtp message sending

Background

Problem

Source

Edit: Resolution

1 Answers1