2
\$\begingroup\$

For learning purposes, I wanted to create a header-only C++ wrapper library around HTTP CURL functionality. At the moment the library only implements GET and POST, but I will add other HTTP methods later. Additionally, right now it only supports calling get() or post() with a fully constructed RequestConfig object. I plan on adding overloads that allow the user to pass only some of the parameters (URL, URL & parameters, etc).

Any pointers or guidance would be greatly appreciated!

httplib.hpp

#pragma once

#include <map>
#include <iterator> 
#include <curl/curl.h>
#include <sstream>
#include <chrono>

namespace easyhttp {

    enum class HttpRequestType { get, post };
    enum class RequestError { none, timeout, socket_error, error_misc };

    struct BasicAuthentication {
        std::string username;
        std::string password;
    };

    struct HttpResponse {
        RequestError error;
        std::string response_code;
        std::string content;
    };

    class Parameters {

    public:

        using iterator = std::map<std::string, std::string>::iterator;

        Parameters() {}

        explicit Parameters(const std::initializer_list<std::pair<std::string, std::string>>& list) {
            for (auto itr = list.begin(); itr != list.end(); itr++) {
                if (!itr->first.empty()) {
                    items_[itr->first] = itr->second;
                }
            }
        }

        explicit Parameters(const std::pair<std::string, std::string>& x) {
            items_[x.first] = x.second;
        }

        explicit Parameters(const std::map<std::string, std::string> x) : items_{ x } {}

        void add(std::pair<std::string, std::string> p) {
            if (!p.first.empty()) {
                items_[p.first] = p.second;
            }
        }

        void remove(std::string key) {
            items_.erase(key);
        }

        size_t size() {
            return items_.size();
        }

        std::string get_value(std::string key) {
            return (items_.find(key) == items_.end()) ? "" : items_[key];
        }

        void clear() {
            items_.clear();
        }

        std::map<std::string, std::string>::iterator begin() {
            return items_.begin();
        }

        std::map<std::string, std::string>::iterator end() {
            return items_.end();
        }

    protected:
        std::map<std::string, std::string> items_;
    };


    class UrlParameters : public Parameters {
    public:

        UrlParameters() : Parameters() {}

        explicit UrlParameters(std::initializer_list<std::pair<std::string, std::string>> list)
            : Parameters(list) {}

        explicit UrlParameters(const std::pair<std::string, std::string>& x)
            : Parameters(x) {}

        explicit UrlParameters(const std::map<std::string, std::string>& x)
            : Parameters(x) {}

        std::string get_string() {
            if (str_.empty()) {
                encode();
                return str_;
            }

            return str_;
        }
        std::string get_encoded_string() {
            if (encoded_str_.empty()) {
                return encode();
            }

            return encoded_str_;
        }

        std::string encode() {

            if (items_.size() == 0) {
                return "";
            }

            str_.clear();
            encoded_str_.clear();

            str_ = "?";
            encoded_str_ = "?";

            for (auto& [k, v] : items_) {
                str_ += ("&" + k + "=" + v);
                encoded_str_ += ("&" + url_escape_str(std::string(k)) + "=" + url_escape_str(std::string(v)));
            }

            str_.erase(1, 1);
            encoded_str_.erase(1, 1);

            return encoded_str_;
        }

    private:

        std::string url_escape_str(std::string& orig) {

            // Think about a possible try catch here
            // Technically, std::strings ctor can throw
            char *res = curl_easy_escape(nullptr, orig.c_str(), orig.length());

            std::string escaped_str = std::string(res);
            curl_free(res);
            return escaped_str;
        }

        std::string str_;
        std::string encoded_str_;

    };

    class Headers : public Parameters {
    public:

        Headers() : Parameters() {}

        explicit Headers(std::initializer_list<std::pair<std::string, std::string>> list)
            : Parameters(list) {}

        explicit Headers(const std::pair<std::string, std::string>& x)
            : Parameters(x) {}

        explicit Headers(const std::map<std::string, std::string>& x)
            : Parameters(x) {}

        std::string encode(const std::string key) {
            return (items_.find(key) == items_.end()) ? "" : key + ": " + items_[key];
        }

    };

    struct RequestConfig {
        std::string url;
        UrlParameters params;
        Headers headers;
        BasicAuthentication auth;
        std::chrono::seconds timeout_sec;
    };

    namespace{

        size_t http_request_impl_response_write(char* ptr, size_t size, size_t numb, void* ud) {
            size_t response_size = size * numb;
            std::stringstream* ss = (std::stringstream*)ud;
            ss->write(ptr, response_size);
            return response_size;
        }
    }

    class Request {
    public:

        HttpResponse post(RequestConfig& c) {
            HttpResponse resp = http_request_impl(HttpRequestType::post, c);
            return resp;
        }

        HttpResponse get(RequestConfig& c) {
            HttpResponse resp = http_request_impl(HttpRequestType::get, c);
            return resp;
        }

    private:

        HttpResponse http_request_impl(const HttpRequestType r, RequestConfig& c) {

            CURL* curl;
            struct curl_slist* chunk = NULL;
            curl_global_init(CURL_GLOBAL_ALL);
            curl = curl_easy_init();

            std::stringstream response_stream;
            HttpResponse resp;

            curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, static_cast<long>(c.timeout_sec.count()));
            curl_easy_setopt(curl, CURLOPT_URL, (c.url + c.params.get_encoded_string()).c_str());

            if (c.headers.size() > 0) {
                for (const auto& [k,v] : c.headers) {
                    chunk = curl_slist_append(chunk, c.headers.encode(k).c_str());
                }

                curl_easy_setopt(curl, CURLOPT_HTTPHEADER, chunk);
            }

            if (r == HttpRequestType::post) {
                curl_easy_setopt(curl, CURLOPT_POST, 1);

                if (c.params.size() > 0) {
                    std::string temp = c.params.get_encoded_string();
                    curl_easy_setopt(curl, CURLOPT_POSTFIELDS, temp.c_str());
                }

                else {
                    curl_easy_setopt(curl, CURLOPT_POSTFIELDS, "");
                }
            }

            if (!c.auth.username.empty() && !c.auth.password.empty()) {
                curl_easy_setopt(curl, CURLOPT_USERNAME, c.auth.username.c_str());
                curl_easy_setopt(curl, CURLOPT_PASSWORD, c.auth.password.c_str());
            }

            curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, http_request_impl_response_write);
            curl_easy_setopt(curl, CURLOPT_WRITEDATA, &response_stream);

            CURLcode res = curl_easy_perform(curl);

            auto http_code = 0L;
            curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code);
            curl_easy_cleanup(curl);
            curl_slist_free_all(chunk);

            if (res == CURLE_OK) {
                resp.content = response_stream.str();
                resp.error = RequestError::none;
                resp.response_code = std::to_string(http_code);
            }

            else if (res == CURLE_OPERATION_TIMEDOUT) {
                resp.content = "Operation timed out.";
                resp.error = RequestError::timeout;
                resp.response_code = "-1";
            }

            else {
                resp.content = "Request encountered error: " + std::string(curl_easy_strerror(res));
                resp.error = RequestError::error_misc;
                resp.response_code = "-1";
            }

            return resp;

        }

    };
};

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Quite a bit of code to look at. This is not a complete review, but I did two simple things, which may prompt thought / improvements:

  1. I ran a static analyser over your code (clang-tidy V9). The output is below. It is pointing many of the stylistic things I would say too. And it highlights some actual issues.
  2. I added a main() method and i tried to use your code. ie, I made it "work". This also highlighted a couple of small issues. Try it yourself. Is this the API you want for the users of your library? How could it be improved?

my main():

int main()
{
  auto req_params = easyhttp::UrlParameters{{"q","test"}};
  auto req_config = easyhttp::RequestConfig{
    "https://www.google.com",
    req_params,
    {},
    {},
    std::chrono::duration<long>{10}
  };
  auto req = easyhttp::Request();
  auto response = req.get(req_config);
  std::cout << response.content << "\n";
  return 0;
}

clang-tidy output

curl2.cpp:18:8: warning: constructor does not initialize these fields: error [hicpp-member-init]
struct HttpResponse {
       ^
curl2.cpp:29:3: warning: use '= default' to define a trivial default constructor [hicpp-use-equals-default]
  Parameters() {}
  ^            ~~
               = default;
curl2.cpp:31:12: warning: initializer-list constructor should not be declared explicit [google-explicit-constructor]
  explicit Parameters(const std::initializer_list<std::pair<std::string, std::string>>& list) {
  ~~~~~~~~~^
curl2.cpp:32:5: warning: use range-based for loop instead [modernize-loop-convert]
    for (auto itr = list.begin(); itr != list.end(); itr++) {
    ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        (const auto & itr : list)
curl2.cpp:41:64: warning: the const qualified parameter 'x' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]
  explicit Parameters(const std::map<std::string, std::string> x) : items_{x} {}
                                                               ^
                                                              &
curl2.cpp:43:48: warning: the parameter 'p' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
  void add(std::pair<std::string, std::string> p) {
                                               ^
           const                              &
curl2.cpp:49:27: warning: the parameter 'key' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
  void remove(std::string key) { items_.erase(key); }
                          ^
              const      &
curl2.cpp:53:37: warning: the parameter 'key' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
  std::string get_value(std::string key) {
                                    ^
                        const      &
curl2.cpp:64:38: warning: member variable 'items_' has protected visibility [cppcoreguidelines-non-private-member-variables-in-classes]
  std::map<std::string, std::string> items_;
                                     ^
curl2.cpp:69:21: warning: initializer for base class 'easyhttp::Parameters' is redundant [readability-redundant-member-init]
  UrlParameters() : Parameters() {}
                    ^~~~~~~~~~~~~
curl2.cpp:71:12: warning: initializer-list constructor should not be declared explicit [google-explicit-constructor]
  explicit UrlParameters(std::initializer_list<std::pair<std::string, std::string>> list)
  ~~~~~~~~~^
curl2.cpp:96:9: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
    if (items_.size() == 0) {
        ^~~~~~~~~~~~~~~~~~
        items_.empty()
/usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/stl_map.h:463:7: note: method 'map'::empty() defined here
      empty() const _GLIBCXX_NOEXCEPT
      ^
curl2.cpp:107:30: warning: string concatenation results in allocation of unnecessary temporary strings; consider using 'operator+=' or 'string::append()' instead [performance-inefficient-string-concatenation]
      str_ += ("&" + k + "=" + v);
                             ^
curl2.cpp:118:15: warning: method 'url_escape_str' can be made static [readability-convert-member-functions-to-static]
  std::string url_escape_str(const std::string& orig) {
              ^
  static
curl2.cpp:135:15: warning: initializer for base class 'easyhttp::Parameters' is redundant [readability-redundant-member-init]
  Headers() : Parameters() {}
              ^~~~~~~~~~~~~
curl2.cpp:137:12: warning: initializer-list constructor should not be declared explicit [google-explicit-constructor]
  explicit Headers(std::initializer_list<std::pair<std::string, std::string>> list)
  ~~~~~~~~~^
curl2.cpp:144:40: warning: the const qualified parameter 'key' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]
  std::string encode(const std::string key) {
                                       ^
                                      &
curl2.cpp:161:3: warning: use auto when initializing with a cast to avoid duplicating the type name [hicpp-use-auto]
  std::stringstream* ss            = (std::stringstream*)ud;
  ^~~~~~~~~~~~~~~~~
  auto
curl2.cpp:161:38: warning: C-style casts are discouraged; use static_cast [google-readability-casting]
  std::stringstream* ss            = (std::stringstream*)ud;
                                     ^~~~~~~~~~~~~~~~~~~~
                                     static_cast<std::stringstream*>( )
curl2.cpp:161:38: warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]
curl2.cpp:180:16: warning: method 'http_request_impl' can be made static [readability-convert-member-functions-to-static]
  HttpResponse http_request_impl(const HttpRequestType r, RequestConfig& c) {
               ^
  static
curl2.cpp:183:32: warning: use nullptr [modernize-use-nullptr]
    struct curl_slist* chunk = NULL;
                               ^~~~
                               nullptr

\$\endgroup\$
0

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.