4
\$\begingroup\$

I have written an HTTP client wrapped the libcurl. It should be able to do HTTP get/post with string/map param, with cookies and proxy.

Can somebody review the code? B.T.W., I'm not sure the way pass a map into HTTP header is correct, maybe I should remove these two interface

CURLcode do_http_post(std::string post_url, std::map<std::string, std::string> map_param, void* user_data);

and

void set_http_header(std::map<std::string, std::string> map_param);.

curl_wrapper.h

#ifndef CURL_WRAPPER
#define CURL_WRAPPER
#include <string>
#include <exception>
#include "curl.h"
#include <map>
#define DATA_MAX_LEN CURL_MAX_WRITE_SIZE*30
struct client_data;

class curl_client{
public:
    curl_client();
    ~curl_client();
    CURLcode do_http_get(std::string get_url, void* user_data);
    CURLcode do_http_post(std::string post_url, std::map<std::string, std::string> map_param, void* user_data);
    CURLcode do_http_post(std::string post_url, std::string post_fields, void* user_data);
    void set_http_header(std::string header_param);
    void set_http_header(std::map<std::string, std::string> map_param);
    void set_http_cookie(std::string cookie_file);
    void set_http_proxy(std::string proxy_url);

private:
    static size_t write_data( char *ptr, size_t size, size_t nmemb, void *user_data);
    void set_common_opt(std::string url, void* user_data);
    void set_post_fields(std::string post_fields);
    void set_post_fields(std::map<std::string, std::string> map_param);
    CURLcode perform();

private:
    CURL* curl_;
    CURLcode res_;
    curl_slist* p_header_list_;
};

struct client_data {
    client_data() {
        size = DATA_MAX_LEN;
        used = 0;
        buf = new char[DATA_MAX_LEN];
    }
    ~client_data(){
        if (buf!=nullptr){
            delete[] buf;
        }
    }
    char *buf;
    int size;
    int used;
};


#endif

curl_wrapper.cpp

#include "curl_wrapper.h"

curl_client::curl_client(){
    curl_ = curl_easy_init();
    p_header_list_ = nullptr;
}

curl_client::~curl_client(){
    curl_easy_cleanup(curl_);
    curl_slist_free_all(p_header_list_);
}

CURLcode curl_client::do_http_get(std::string get_url, void* user_data){
    set_common_opt(get_url, user_data);
    return perform();
}

CURLcode curl_client::do_http_post(std::string post_url, std::map<std::string, std::string> map_param, void* user_data){
    set_common_opt(post_url, user_data);
    set_post_fields(map_param);
    return perform();
}

CURLcode curl_client::do_http_post(std::string post_url, std::string post_fields, void* user_data){
    set_common_opt(post_url, user_data);
    set_post_fields(post_fields);
    return perform();
}

void curl_client::set_common_opt(std::string url, void* user_data){
    curl_easy_setopt(curl_, CURLOPT_URL, url.c_str());  
    curl_easy_setopt(curl_, CURLOPT_WRITEFUNCTION, curl_client::write_data);  
    curl_easy_setopt(curl_, CURLOPT_WRITEDATA, user_data);  
    curl_easy_setopt(curl_, CURLOPT_HEADER, 1); 
    curl_easy_setopt(curl_, CURLOPT_FOLLOWLOCATION, 1);  
    // not verify host and ca for https
    curl_easy_setopt(curl_, CURLOPT_SSL_VERIFYPEER, 0);
    curl_easy_setopt(curl_, CURLOPT_SSL_VERIFYHOST, 0);
#ifdef DEBUG
    curl_easy_setopt(curl_, CURLOPT_VERBOSE, 1);  
#endif
}

void curl_client::set_post_fields(std::map<std::string, std::string> map_param){
    std::string post_fields;
    std::map<std::string, std::string>::iterator it;
    for (it = map_param.begin(); it!=map_param.end(); ++it){
        if (it!=map_param.begin()){
            post_fields += "&";
        }
        post_fields += it->first + "=" + it->second;
    }
    set_post_fields(post_fields);  
}

void curl_client::set_post_fields(std::string post_fields){
    curl_easy_setopt(curl_, CURLOPT_POSTFIELDS, post_fields.c_str()); 
    curl_easy_setopt(curl_, CURLOPT_POST, 1);  
}

void curl_client::set_http_header(std::string header_param){
    if (p_header_list_){
        curl_slist_free_all(p_header_list_);
        p_header_list_ = nullptr;
    }
    p_header_list_ = curl_slist_append(p_header_list_, header_param.c_str());
    curl_easy_setopt(curl_, CURLOPT_HTTPHEADER, p_header_list_);
}

void curl_client::set_http_header(std::map<std::string, std::string> map_param){

    std::string header_param;
    std::map<std::string, std::string>::iterator it;
    for (it = map_param.begin(); it!=map_param.end(); ++it){
        if (it!=map_param.begin()){
            header_param += "&";
        }
        header_param += it->first + "=" + it->second;
    }
    set_http_header(header_param);
}

void curl_client::set_http_cookie(std::string cookie_file) {
    curl_easy_setopt(curl_, CURLOPT_COOKIEJAR, cookie_file.c_str());//save to
    curl_easy_setopt(curl_, CURLOPT_COOKIEFILE, cookie_file.c_str());//read from
}

void curl_client::set_http_proxy(std::string proxy_url){
    curl_easy_setopt(curl_, CURLOPT_PROXY, proxy_url.c_str());
}

size_t curl_client::write_data( char *ptr, size_t size, size_t nmemb, void *user_data){
    client_data *the_buf = (client_data *)user_data;
    int bytes_passed_in = size * nmemb;
    int bytes_written = 0;
    if (the_buf->used + bytes_passed_in < DATA_MAX_LEN){
        memcpy(the_buf->buf + the_buf->used, ptr, bytes_passed_in);
        the_buf->used += bytes_passed_in;
        *(the_buf->buf + the_buf->used) = 0;
        bytes_written = bytes_passed_in;
    }else {
        memcpy(the_buf->buf + the_buf->used, ptr, DATA_MAX_LEN - the_buf->used - 1);
        bytes_written = DATA_MAX_LEN - the_buf->used - 1;
        the_buf->used = DATA_MAX_LEN;
        *(the_buf->buf + DATA_MAX_LEN - 1) = 0;
        // here libcurl will signal an error for intact data written.
    }
    return bytes_written;
}

CURLcode curl_client::perform(){
    if (curl_) {
        res_ = CURL_LAST;
        try {
            res_ = curl_easy_perform(curl_);
        } catch (std::exception e){
        }
        return res_;
    }
}

An HTTP get test case:

 void curl_get_googleplay(){
        curl_client curl;
        client_data user_data;
        CURLcode res = curl.do_http_get(std::string("play.google.com/store/apps/details?id=com.teamviewer.teamviewer.market.mobile"), &user_data);
    }
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

Looking at your header file:

  • Replace the #define DATA_MAX_LEN with a static const variable; Do the same with any other constant that is #define-d.

  • Pass more complex parameters by const reference, to avoid making a copy (urls, parameters maps, etc).

  • You define client_data structure for (I assume) receiving the results; If it is strongly typed, why do you pass it in by void*? This just allows client code to call your API in a way that will corrupt your code (e.g. a client might decide to pass there a pointer to a std::vector<uint8_t> instead).

  • You do not use RAII and smart pointers (you probably should)

  • After looking at your code, it is still unclear to me, if I would be able to use it to read the HTTP response headers (e.g. "I want to know if the response came with the "Cache-Control" header specified, and what was it's value").

  • Consider returning the result data as a return value (instead as an output parameter) and raising an exception in case you get a HTTP error. This would allow you to specify other error conditions as well.

  • Just looking at the interface (not the implementation) I have no idea how your class will behave if I call with invalid parameters.

Code:

curl_client curl;
client_data user_data;
curl.set_http_cookie("\\"); // does this throw? What does it throw?

Looking at your implementation file:

  • Your perform function calls curl_easy_perform (a C function) inside a try/catch block for std::exception. Exceptions are a C++ thing (i.e. curl_easy_perform will never throw an exception -- that's why it's using return codes to signal errors).

  • You are using C-style casts; Don't! (see my point above about removing the void* code).

  • There is no reason at all to use raw pointer and memcpy. Consider using std::vector instead (it will be safer, more efficient, exception-safe and already tested).

  • set_post_fields concatenates strings into a different string, in a loop. Consider using a std::ostringstream instead.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.