12
\$\begingroup\$

this is the first time I've requested a code review. I'm writing a piece of software in C++ that will use HTTP for communication. I wanted to do it myself to learn and then have someone review it and tell me where I could make some improvements. I'm having some problems with class member object initialization.

I've broken the code down into http.h , httprequest.cpp , httpresponse.cpp. I would really appreciate it if one of you very smart people could take the time to help me learn!

It functions, but I've found some cases where my implementation felt extremely hacked together. I'm trying to make use of the C++ specific operations and language features when I can but my problem is I just don't have the practice to know how and when to use them appropriately. I'm more interested in general programming practice improvements; specifically around garbage collection. Because I'm a python programmer at heart, my GC knowledge is minimal and I'm kind of just diving head first. Anyways, here is the code:

http.h

#ifndef WIN32_LEAN_AND_MEAN
#define WIN32_LEAN_AND_MEAN
#endif

#ifndef DEFAULT_PORT
#define DEFAULT_PORT "80"
#endif

#ifndef HTTP_VERSION
#define HTTP_VERSION "HTTP/1.1"
#endif

#include <windows.h>
#include <winsock2.h>
#include <ws2tcpip.h>
#include <iphlpapi.h>

#include "util.h"

#pragma comment(lib, "Ws2_32.lib")

#define WEB_PORT "80"
#define DEFAULT_BUFLEN 512

class HTTPResponse {
    std::string response_txt, data, version, reason;
    std::vector<std::string> header_lines;
    std::map<std::string, std::string> headers;
    int http_code;
public:
    HTTPResponse();
    HTTPResponse(std::string);
    std::string get_reason();
    std::string get_version();
    std::string get_data();
    std::string get_response_txt();
    std::string get_header_text();
    std::string get_header_data(std::string);
    std::map<std::string, std::string> get_headers();
    int get_http_code();

    HTTPResponse operator=(const HTTPResponse& _resp) { return _resp; }
private:
    void parse(std::string);
    void parse_header_line(std::string);
    void parse_first_line(std::string);
};

class HTTPRequest {
    HTTPResponse resp;
    std::vector<std::pair<std::string, std::string>> added_headers;
    std::string hostname, method, data, path, port, req_txt, resp_txt;
    std::map<std::string, std::string> args, headers;
public:
    HTTPRequest();
    HTTPRequest(std::string);
    HTTPRequest(std::string, std::string);
    void add_cookie(std::string, std::string);
    void add_arg(std::string, std::string);
    void add_header(std::string, std::string);
    void set_data(std::string);
    void set_port(std::string);
    void set_args(std::map<std::string, std::string>);
    void set_args(std::string);
    void set_headers(std::map<std::string, std::string>);
    void set_headers(std::string);
    void set_method(std::string);
    void set_path(std::string);
    void set_hostname(std::string);
    void reset();

    std::string get_request_txt();
    std::string get_response_txt();
    int send_request();
    HTTPResponse get_response();

    HTTPRequest operator=(const HTTPRequest& _req) { return _req; }
private:
    std::string build_request();
    std::map<std::string, std::string> default_headers(std::string);
    std::map<std::string, std::string> parse_args(std::string);
    std::map<std::string, std::string> parse_headers(std::string);
    std::string random_user_agent();
};

httprequest.cpp

#include "http.h"

// HTTPRequest implementation
HTTPRequest::HTTPRequest() {
    hostname = "www.yahoo.com";
    method = "GET";
    path = "/";
    args = std::map<std::string, std::string>();
    set_headers(default_headers(hostname));

}

HTTPRequest::HTTPRequest(std::string _hostname) {
    hostname = _hostname;
    method = "GET";
    path = "/";
    args = std::map<std::string, std::string>();
    set_headers(default_headers(hostname));

}

HTTPRequest::HTTPRequest(std::string _hostname, std::string _path) {
    hostname = _hostname;
    method = "GET";
    path = _path;
    args = std::map<std::string, std::string>();
    set_headers(default_headers(hostname));

}

void HTTPRequest::reset() {
    args.clear();
    path = "/";
    method = "GET";
    added_headers.clear();
    set_headers(default_headers(hostname));
}

std::string HTTPRequest::build_request() {
    std::string req = "";
    if (path.empty())
        path = "/";

    if (hostname.empty())
        throw std::exception("Hostname empty.");

    std::string path_args = "";
    if (!args.empty() && method == "GET")
        path_args = path + "?" + Util::serialize_http_args(args);
    else path_args = path;

    if (!method.empty())
        req += method + " " + path_args + " " + HTTP_VERSION + "\r\n";
    else
        req += "GET " + path_args + " " + HTTP_VERSION + "\r\n";

    req += Util::serialize_http_headers(headers);

    if (method == "POST")
        req += Util::serialize_http_args(args);
    else
        req += data;

    req_txt = req;
    return req;
}

void HTTPRequest::add_cookie(std::string cookie, std::string value) {
    if (headers.find("Cookie") != headers.end()) {
        headers.at("Cookie") = Util::trim_copy(headers.at("Cookie")) + ";" + cookie + "=" + value;
    } else if(headers.find("cookie") != headers.end()) {
        headers.at("cookie") = Util::trim_copy(headers.at("cookie")) + ";" + cookie + "=" + value;
    } else {
        add_header("Cookie", cookie + "=" + value);
    }
}

void HTTPRequest::add_arg(std::string arg, std::string value) {
    args.insert(std::pair<std::string, std::string>(arg, value));
    build_request();
}

void HTTPRequest::add_header(std::string header, std::string value) {
    headers.insert(std::pair<std::string, std::string>(header, value));
    added_headers.push_back(std::pair<std::string, std::string>(header, value));
    build_request();
}

void HTTPRequest::set_args(std::map<std::string, std::string> _args) {
    args = _args;
    build_request();
}

void HTTPRequest::set_args(std::string _args) {
    args = parse_args(_args);
    build_request();
}

void HTTPRequest::set_port(std::string _port) {
    port = _port;
}

void HTTPRequest::set_headers(std::map<std::string, std::string> _headers) {
    headers = _headers;
}

void HTTPRequest::set_headers(std::string _headers) {
    headers = parse_headers(_headers);
}

void HTTPRequest::set_path(std::string _path) {
    path = _path;
}

void HTTPRequest::set_data(std::string _data) {
    data = _data;
    headers = default_headers(hostname);
}

void HTTPRequest::set_hostname(std::string _hostname) {
    hostname = _hostname;
    headers = default_headers(hostname);
}

void HTTPRequest::set_method(std::string _method) {
    method = _method;
    build_request();
}

std::map<std::string, std::string> HTTPRequest::parse_headers(std::string _headers) {
    std::vector<std::string> headerz = Util::split(_headers, "\r\n");
    std::map<std::string, std::string> headermap;
    std::vector<std::string> headersplit;

    for (auto _header : headerz) {
        headersplit = Util::split(_header, ":");
        headermap.insert(std::pair<std::string, std::string>(headersplit.at(0), headersplit.at(1)));
    }

    return headermap;
}

std::map<std::string, std::string> HTTPRequest::parse_args(std::string _args) {
    std::vector<std::string> argz = Util::split(_args, "&");
    std::map<std::string, std::string> argmap;
    std::vector<std::string> argsplit;

    for (auto _arg : argz) {
        argsplit = Util::split_once(_arg, "=");
        argmap.insert(std::pair<std::string, std::string>(argsplit.at(0), argsplit.at(1)));
    }

    return argmap;
}

std::string HTTPRequest::get_request_txt() {
    return build_request();
}

std::string HTTPRequest::get_response_txt() {
    return resp_txt;
}

HTTPResponse HTTPRequest::get_response() {
    return HTTPResponse(get_response_txt());
}

int HTTPRequest::send_request() {
    WSADATA wsaData;

    int iResult = WSAStartup(MAKEWORD(2, 2), &wsaData);
    if (iResult != 0) {
        printf("WSAStartup failed: %d\n", iResult);
        return 1;
    }

    struct addrinfo* result = NULL,
        * ptr = NULL,
        hints;

    ZeroMemory(&hints, sizeof(hints));
    hints.ai_family = AF_UNSPEC;
    hints.ai_socktype = SOCK_STREAM;
    hints.ai_protocol = IPPROTO_TCP;

    // Resolve the server address and port
    iResult = getaddrinfo(hostname.c_str(), port.c_str(), &hints, &result);
    if (iResult != 0) {
        printf("getaddrinfo failed: %d\n", iResult);
        WSACleanup();
        return 1;
    }

    ptr = result;
    SOCKET ConnectSocket = INVALID_SOCKET;
    ConnectSocket = socket(ptr->ai_family, ptr->ai_socktype, ptr->ai_protocol);

    if (ConnectSocket == INVALID_SOCKET) {
        printf("Error at socket(): %ld\n", WSAGetLastError());
        freeaddrinfo(result);
        WSACleanup();
        return 1;
    }

    iResult = connect(ConnectSocket, ptr->ai_addr, (int)ptr->ai_addrlen);
    if (iResult == SOCKET_ERROR) {
        closesocket(ConnectSocket);
        ConnectSocket = INVALID_SOCKET;
    }

    freeaddrinfo(result);
    if (ConnectSocket == INVALID_SOCKET) {
        printf("Unable to connect to server!\n");
        WSACleanup();
        return 1;
    }

    const int recvbuflen = DEFAULT_BUFLEN;
    char recvbuf[recvbuflen];

    std::string req_text = build_request();
    // Send an initial buffer
    iResult = send(ConnectSocket, req_text.c_str(), (int)req_text.length(), 0);
    if (iResult == SOCKET_ERROR) {
        printf("send failed: %d\n", WSAGetLastError());
        closesocket(ConnectSocket);
        WSACleanup();
        return 1;
    }

    // shutdown the connection for sending since no more data will be sent
    // the client can still use the ConnectSocket for receiving data
    iResult = shutdown(ConnectSocket, SD_SEND);
    if (iResult == SOCKET_ERROR) {
        printf("shutdown failed: %d\n", WSAGetLastError());
        closesocket(ConnectSocket);
        WSACleanup();
        return 1;
    }

    std::string out = "";
    // Receive data until the server closes the connection
    do {
        memset(recvbuf, 0, recvbuflen);
        iResult = recv(ConnectSocket, &recvbuf[0], recvbuflen - 1, 0);
        out.append(recvbuf);
    } while (iResult > 0);

    resp_txt = std::string(out.c_str());
    resp = HTTPResponse(resp_txt);

    return 0;
}

std::string HTTPRequest::random_user_agent() {
    std::vector<std::string> uas;

    uas.push_back("Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.113 Safari/537.36");
    uas.push_back("Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322)");
    uas.push_back("Mozilla/5.0 (Windows NT 5.1; rv:7.0.1) Gecko/20100101 Firefox/7.0.1");
    uas.push_back("Opera/9.80 (Android; Opera Mini/12.0.1987/37.7327; U; pl) Presto/2.12.423 Version/12.16");
    uas.push_back("Mozilla/5.0 (Linux; U; Android 2.2) AppleWebKit/533.1 (KHTML, like Gecko) Version/4.0 Mobile Safari/533.1");
    uas.push_back("Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36 OPR/43.0.2442.991");
    uas.push_back("Mozilla/5.0 (Linux; U; Android 6.0.1; zh-CN; F5121 Build/34.0.A.1.247) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/40.0.2214.89 UCBrowser/11.5.1.944 Mobile Safari/537.36");
    uas.push_back("Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_3) AppleWebKit/600.5.17 (KHTML, like Gecko) Version/8.0.5 Safari/600.5.17");
    uas.push_back("Microsoft Office/14.0 (Windows NT 6.1; Microsoft Outlook 14.0.7143; Pro)");
    uas.push_back("Microsoft Office/14.0 (Windows NT 6.1; Microsoft Outlook 14.0.7162; Pro)");
    uas.push_back("Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 6.2; Win64; x64; Trident/6.0; .NET4.0E; .NET4.0C; Microsoft Outlook 15.0.5023; ms-office; MSOffice 15)");
    uas.push_back("Opera/9.80 (J2ME/MIDP; Opera Mini/4.2.20464/28.2144; U; en) Presto/2.8.119 Version/11.10");
    uas.push_back("Opera/9.80 (J2ME/MIDP; Opera Mini/7.1.32052/29.3709; U; en) Presto/2.8.119 Version/11.10");
    uas.push_back("Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 6.0; SLCC1; .NET CLR 2.0.50727; InfoPath.2; .NET CLR 3.5.30729; .NET CLR 3.0.30618)");
    uas.push_back("Mozilla/5.0 (compatible; MSIE 9.0; Windows NT 6.1; Trident/5.0; Trident/5.0)");
    uas.push_back("Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 6.0; WOW64; Trident/4.0; SLCC1; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; .NET4.0C; .NET4.0E)");
    uas.push_back("Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1; .NET CLR 1.1.4322; .NET CLR 2.0.50727)");
    uas.push_back("Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.113 Safari/537.36");
    uas.push_back("Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1)");
    uas.push_back("Mozilla/5.0 (Windows NT 5.1; rv:7.0.1) Gecko/20100101 Firefox/7.0.1");

    return uas.at(Util::randrange(0, uas.size() - 1));
}

std::map<std::string, std::string> HTTPRequest::default_headers(std::string _hostname) {
    std::map<std::string, std::string> dheaders;
    dheaders.insert(std::pair<std::string, std::string>("Host", _hostname));
    dheaders.insert(std::pair<std::string, std::string>("User-Agent", random_user_agent()));
    dheaders.insert(std::pair<std::string, std::string>("Accept-Language", "en-us"));
    dheaders.insert(std::pair<std::string, std::string>("Connection", "Keep-Alive"));
    dheaders.insert(std::pair<std::string, std::string>("Content-Length", Util::int2str(data.length())));
    if (added_headers.size() > 0) {
        for (auto h : added_headers)
            dheaders.insert(h);
    }

    return dheaders;
}

httpresponse.cpp

#include "http.h"

HTTPResponse::HTTPResponse() {
    http_code = 0;
}

HTTPResponse::HTTPResponse(std::string _response) {
    HTTPResponse();
    response_txt = _response;
    parse(response_txt);
}

void HTTPResponse::parse(std::string _response) {
    std::vector<std::string> blocks = Util::split(_response, "\r\n\r\n");
    response_txt = _response;
    int block_count = blocks.size();

    //split into headers and data
    if (block_count > 0) {
        header_lines = Util::split(blocks.at(0), "\r\n");
        if (block_count > 1) {
            data = Util::trim_copy(blocks.at(1));
        }

        for (size_t i = 0; i < header_lines.size(); ++i) {
            if (i == 0)
                parse_first_line(Util::trim_copy(header_lines.at(i)));
            else
                parse_header_line(Util::trim_copy(header_lines.at(i)));
        }
    }
}

void HTTPResponse::parse_header_line(std::string line) {
    std::vector<std::string> values = Util::split_once(line, ":");
    if (values.size() >= 2) {
        headers.insert(std::pair<std::string, std::string>(
            Util::lowercase(values.at(0)), Util::ltrim_copy(values.at(1))));
    }
}

void HTTPResponse::parse_first_line(std::string line) {
    std::vector<std::string> sections = Util::split(line, " ");
    int size = sections.size();

    if (size >= 3) {
        version = sections.at(0);
        http_code = std::stoi(sections.at(1).c_str());
        reason = sections.at(2);
        if (size > 3) {
            for (int i = 3; i < size; ++i)
                reason += " " + sections.at(i);
        }
    } else {
        throw std::exception("Invalid HTTP response. Looking for 3 header elements, found %d", size);
    }
}

std::map<std::string, std::string> HTTPResponse::get_headers() {
    return headers;
}

std::string HTTPResponse::get_header_text() {
    return Util::serialize_http_headers(headers);
}

std::string HTTPResponse::get_header_data(std::string header) {
    if (headers.find(Util::lowercase(header)) != headers.end()) {
        return headers.at(Util::lowercase(header));
    } else {
        return "";
    }
}

std::string HTTPResponse::get_response_txt() {
    return response_txt;
}

std::string HTTPResponse::get_data() {
    return data;
}

std::string HTTPResponse::get_version() {
    return version;
}

std::string HTTPResponse::get_reason() {
    return reason;
}

int HTTPResponse::get_http_code() {
    return http_code;
}                   ```

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review, unfortunately this question is off-topic because it has to be working code, please see the guidelines at codereview.stackexchange.com/help/dont-ask and codereview.stackexchange.com/help/how-to-ask. Since it is partially working you might try our sister website stackoverflow.com, they help debug code. \$\endgroup\$ – pacmaninbw May 3 at 19:27
  • \$\begingroup\$ No you're misunderstanding, it works and I'm not looking for debugging help. I can do that myself. I'm looking for someone to review the code and tell me where fundamental improvements can be made to the style or whatever. I'll do my own unit testing. \$\endgroup\$ – leaustinwile May 3 at 19:29
  • \$\begingroup\$ Welcome to Code Review! You stated "It functions, but I've found some cases that it fails.". In what cases does it fail?? Please read this meta post explaining why "It functions, but I've found some cases that it fails." is off-topic \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ May 3 at 19:51
  • \$\begingroup\$ I guess I used the wrong wording. Better would be to say, I feel like I had to hack it together. It works as I need it to, but I think it's sloppy and I think I'm missing some garbage collection stuff but the code is working as expected. You provide it a hostname in the constructor, set the path, args, headers, cookies, and you send the request and get a response. The response gets parsed into an object and is accessible by elements of the HTTP response. If you guys are saying it's off-topic still I'll take my merry ass to stack overflow like you suggested and bring it back when it's ontopic. \$\endgroup\$ – leaustinwile May 3 at 20:20
  • 3
    \$\begingroup\$ Thanks for clarifying. I would advise you to edityour post and remove any wording that makes it sound like there are cases where it fails. We just want to make sure the code works before reviewing it. Also, please remember there is a [Code of Conduct](Code of Conduct) on this network that should be followed. FWIW I have opted to not VTC but instead leave this post open based on your response. \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ May 3 at 20:33
2
\$\begingroup\$

Initialization in Constructors
Currently the constructors perform the initialization within the body of the constructor, this isn't necessary for simple variables, C++ has a shorthand form for initializing variables

HTTPRequest::HTTPRequest(std::string _hostname)
 : hostname{_hostname}, method{"GET"}, path{"/"}
{
    set_args();
    set_default_headers();
}

As show in the example above, there is no need to pass hostname to the function that sets the default headers because hostname is a class member. Since it is a class member every function in the class has access to it. Therefore setting the default headers can be written like this:

void HTTPRequest::set_default_headers()
{
    headers.insert(std::pair<std::string, std::string>("Host", hostname));
    headers.insert(std::pair<std::string, std::string>("User-Agent", random_user_agent()));
    headers.insert(std::pair<std::string, std::string>("Accept-Language", "en-us"));
    headers.insert(std::pair<std::string, std::string>("Connection", "Keep-Alive"));
    headers.insert(std::pair<std::string, std::string>("Content-Length", Util::int2str(data.length())));
    if (added_headers.size() > 0) {
        for (auto h : added_headers)
        {
            headers.insert(h);
        }
    }
}

Note that set_default_headers() is not returning a map as default_headers(str::string hostname) is. There is no reason to create a local map of headers when the class member headers is declared in the class is available to update. This function replaces both set_headers() and default_headers() in the current implementation. The current implementation of set_headers(default_headers(hostname)); wastes memory and execution time.

A similar function should be created to set the args variable as well.

Note:
Memory allocation for both args and headers is handled by this line in the class declaration:

    std::map<std::string, std::string> args, headers;

This code in the current constructor is attempting to reallocate that memory and is incorrect:

    args = std::map<std::string, std::string>();

every argument that is an element of args must be added using an insert statement.

Header Files
It would be better if http.h included the map, string and vector headers, otherwise each cpp file that references http.h needs to include those headers prior to including http.h.

It is fairly common for a class to defined in its own header file, for instance HTTPRequest.h and HTTPResponse.h rather than both class definitions being in http.h. In the case where one class requires the definition of another class the other class header file can be included for example HTTPResponse.h could contain:

#include <string>
#include <map>
#include <vector>

Creating a header for each class limits the number of files that need to be recompiled every time there is an edit to a particular class such as adding a method to a class or changing the parameters of a method.

and HTTPRequest.h could contain

#include <string>
#include <map>
#include <vector>
#include "HTTPResponse.h"

Header files generally contain some mechanism to prevent the contents of the header file from being included twice. Microsoft provides #pragma once for this purpose, but a more common and portable method is

#ifndef HTTPRequest_h
#define HTTPRequest_h
    ...
#endif

Preventing the contents of the header from being included more than once is important because otherwise compiler errors about duplicate definitions will result.

In Visual Studio when a class is added both a header file and a cpp file are generated and the header file starts with #pragma once.

Variable Declarations
Currently http.h contains the following:

    std::string response_txt, data, version, reason;

If for some reason the type of data needed to change it is harder to change it, it needs to be removed from the combined line and inserted on a line by itself. If a variable needs to be added or deleted it is easier to add a single declaration or remove a single line. It might be better if each variable was declared on it's own line both in a class definition in a header file or a local variable in a method. Using a single line improves readability and maintainability.

\$\endgroup\$
  • \$\begingroup\$ Thank you, so about your comment about the headers. I am utilizing a stdafx.h with the: #ifndef STDAFX_INC #define STDAFX_INC #endif pattern, to include headers that are common throughout the larger project, map, vector and string are defined there, what are your thoughts on that? Otherwise, good suggestions and nice catch on the re-initialization in the ctor. \$\endgroup\$ – leaustinwile May 4 at 1:44
  • \$\begingroup\$ @leaustinwile Two reasons not to, if at some point you want to port your solution that won't port, as an experienced developer I prefer to see the header files where needed rather than being surprised by compiler errors. \$\endgroup\$ – pacmaninbw May 4 at 1:56
  • \$\begingroup\$ Fair enough, I'll restructure. Off-topic question: C++ unit testing. I've never done it so I don't really know how, could you point me to a worthy guide? \$\endgroup\$ – leaustinwile May 4 at 2:01
  • \$\begingroup\$ @leaustinwile I have primarily written my own tests, but there are unit test frameworks. I've used phpunit. The nice thing about frameworks is that they tell you the coverage, i.e. the percentage of code that was tested. en.wikipedia.org/wiki/CppUnit, cppunit.sourceforge.net/doc/1.8.0, cppunit.sourceforge.net/doc/cvs/cppunit_cookbook.html, en.wikipedia.org/wiki/List_of_unit_testing_frameworks \$\endgroup\$ – pacmaninbw May 4 at 2:10
  • \$\begingroup\$ Okay, lastly here's an idea I'm exploring, first let me give you some more background. I work for Nike's adversarial simulation team doing red team ops, this HTTP implementation is going to be used in our custom payload for ops. Do you think that functional programming and using typedef/structs would be better suited for something like this? I've only recently been introduced to the idea of it but after reading some stuff, I am starting to think that for two reasons (my skill level, and the project type) that it may be easier to implement in a functional manner as opposed to object oriented. \$\endgroup\$ – leaustinwile May 4 at 8:14
3
\$\begingroup\$

Let me add a few quick comments to the other review(s).

In general, pay attention to const correctness to reduce the chances of making mistakes and also to avoid unnecessary - and potentially expensive - copies.

  • You have plenty of "getters" of the form get_... (e.g. get_version(), and so on) that don't modify the state of the object. Write these as get_version() const.

  • It seems that you pass not-cheap-to-copy objects like std::string always by-value meaning they are copied. Is this really necessary? By default, a better instinct is to always write them by const-ref, like add_arg(const std::string& arg, const std::string& value), and so on. You should have a good reason for not doing this.

  • Your constructors violate good practices. Read through the relevant rules for constructors on C++ Core Guidelines.

  • In build_request(), define std::string req just before the line if(!method.empty()) to improve readability.

  • In random_user_agent(), you don't need push_back but instead initialize the vector to hold these values directly by a constructor call. You can then make the vector const as well.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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