4
\$\begingroup\$

This works fine so far, but I am looking for some advice as to how to better write this. My goal is to learn and incorporate current/latest C++ practices, and become a better programmer overall.

#include "HTTPRequest.h"


/*
    This function initalized Winsock
*/
bool HTTPRequest::InitWinsock()
{
    WSADATA WsaDat;

    if (WSAStartup(MAKEWORD(2, 2), &WsaDat) != 0)
    {
        std::cout << "Failed To Start Winsocket With WSAStartup()\n";
        WSACleanup();
        return(false);
    }

    return(true);
}

std::string HTTPRequest::loopRecieve(SOCKET Sock)
{
    char recvBuf[256];  // Small 16bit Char For Transporting Data
    std::string outBuf; // Output String
    unsigned int nret = SOCKET_ERROR;

    while (nret != 0)
    {
        nret = recv(Sock, recvBuf, sizeof(recvBuf), 0);

        if ( (nret == SOCKET_ERROR) && (errno == EAGAIN || errno == EWOULDBLOCK) )
        {
            std::cout << "Error Reading From Recv()\n";
        }
        else
        {
            outBuf.append(recvBuf, nret);
        }
    }

    return(outBuf);
}

void HTTPRequest::getWebPage(std::string Host, int Port, std::string Path)
{
    int nret;
    WSADATA WsaDat;
    SOCKET theSocket;

    if (WSAStartup(MAKEWORD(2, 2), &WsaDat) != 0)
    {
        std::cout << "Failed To Initialize Winsock\n";
        return;
    }

    ADDRINFO hints;
    ADDRINFO* pResult               = nullptr;
    memset(&hints, 0, sizeof(hints));
    hints.ai_flags                  = AI_ALL;
    hints.ai_family                 = PF_INET;
    hints.ai_protocol               = IPPROTO_IPV4;

    nret = getaddrinfo((LPCSTR)Host.c_str(), nullptr, &hints, &pResult);
    if (nret != 0)
    {
        std::cout << "Failed To Do GetAddrInfo()\n";
        return;
    }

    struct sockaddr_in servAddr;
    memset(&servAddr, 0, sizeof(servAddr));
    servAddr.sin_family             = AF_INET;
    servAddr.sin_addr.S_un.S_addr   = *((ULONG*)&(((sockaddr_in*)pResult->ai_addr)->sin_addr));
    servAddr.sin_port               = htons(Port);

    theSocket = socket(AF_INET, SOCK_STREAM, 0);
    if (theSocket == INVALID_SOCKET)
    {
        std::cout << "Socket Is Invalid, Is Winsock Initialized?\n";
        return;
    }

    nret = connect(theSocket, (struct sockaddr*)&servAddr, sizeof(servAddr));
    if (nret == SOCKET_ERROR)
    {
        std::cout << "Failed To Connect To Host\n";
        return;
    }

    std::string request;
    request  = "GET " + Path + " HTTP/1.1"  + "\r\n";
    request += "Host: " + Host              + "\r\n";
    request += "Accept: */*\r\n";
    request += "Accept-Language: en-us\r\n";
    request += "Connection: close\r\n";
    request += "User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Firefox/40.0\r\n";
    request += "Referer: http://" + Host    + "\r\n";
    request += "\r\n";

    nret = send(theSocket, request.c_str(), request.length(), 0);
    if (nret == SOCKET_ERROR)
    {
        std::cout << "Failed To Send To Host\n";
        return;
    }

    char recvBuf[16];   // Small 16bit Char For Transporting Data
    std::string outBuf; // Output String
    while (true)
    {
        nret = recv(theSocket, recvBuf, 16, 0);

        if (nret == SOCKET_ERROR) {
            if (errno == EAGAIN || errno == EWOULDBLOCK) {
                std::cout << "Error Reading From Recv()\n";
                break;
            }
            else {
                std::cout << "Unknown Error Occured\n";
                break;
            }
        }
        if (nret == 0) {
            std::cout << "Recv() Socket Closed. So We're Done!\n";
            break;
        }
        else {
            outBuf.append(recvBuf, nret);
        }
    }

    // Display Data
    std::cout << "Data:\n" << outBuf << "\n";

    closesocket(theSocket);
    WSACleanup();
}

void HTTPRequest::postWebPage(std::string Host, int Port, std::string Path, std::vector<std::pair<std::string, std::string>> vPostReqs)
{
    int nret;
    WSADATA WsaDat;
    SOCKET theSocket;

    if (WSAStartup(MAKEWORD(2, 2), &WsaDat) != 0)
    {
        std::cout << "Failed To Initialize Winsock\n";
        return;
    }

    ADDRINFO hints;
    ADDRINFO* pResult = nullptr;
    memset(&hints, 0, sizeof(hints));
    hints.ai_flags = AI_ALL;
    hints.ai_family = PF_INET;
    hints.ai_protocol = IPPROTO_IPV4;

    nret = getaddrinfo((LPCSTR)Host.c_str(), nullptr, &hints, &pResult);
    if (nret != 0)
    {
        std::cout << "Failed To Do GetAddrInfo()\n";
        return;
    }

    struct sockaddr_in servAddr;
    memset(&servAddr, 0, sizeof(servAddr));
    servAddr.sin_family = AF_INET;
    servAddr.sin_addr.S_un.S_addr = *((ULONG*)&(((sockaddr_in*)pResult->ai_addr)->sin_addr));
    servAddr.sin_port = htons(Port);

    theSocket = socket(AF_INET, SOCK_STREAM, 0);
    if (theSocket == INVALID_SOCKET)
    {
        std::cout << "Socket Is Invalid, Is Winsock Initialized?\n";
        return;
    }

    nret = connect(theSocket, (struct sockaddr*)&servAddr, sizeof(servAddr));
    if (nret == SOCKET_ERROR)
    {
        std::cout << "Failed To Connect To Host\n";
        return;
    }

    // Structure POST Data Properly
    std::string concatPostData;
    for (auto i : vPostReqs)
    {
        concatPostData += i.first + "=" + i.second + "&";
    }
    concatPostData.pop_back(); // Pop Off Extra &

    // Construct HEADER
    std::string header;
    header = "POST " + Path + " HTTP/1.1\r\n";
    header += "Host: " + Host + ":" + std::to_string(Port) + "\r\n";
    header += "User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Firefox/40.0\r\n";
    header += "Referer: http://" + Host + "\r\n";
    header += "Content-Type: application/x-www-form-urlencoded\r\n";
    header += "Content-Length: " + std::to_string(concatPostData.length()) + "\r\n";
    header += "Accept-Charset: utf-8\r\n";
    header += "Connection: close\r\n\r\n";
    header += concatPostData + "\r\n";
    header += "\r\n";

    nret = send(theSocket, header.c_str(), header.length(), 0);
    if (nret == SOCKET_ERROR)
    {
        std::cout << "Failed To Send To Host\n";
        return;
    }

    std::string outBuf = loopRecieve(theSocket);

    // Display Data
    std::cout << "Data:\n" << outBuf << "\n";

    closesocket(theSocket);
    WSACleanup();
}


int main()
{
    HTTPRequest hReq;

    std::vector<std::pair<std::string, std::string>> vPostReqs;

    vPostReqs.push_back(std::make_pair("ajax_login", "yes"));
    vPostReqs.push_back(std::make_pair("ACT", "10"));
    vPostReqs.push_back(std::make_pair("RET", "-2"));
    vPostReqs.push_back(std::make_pair("site_id", "1"));
    vPostReqs.push_back(std::make_pair("username", "Boobin"));
    vPostReqs.push_back(std::make_pair("password", "inboob"));
    vPostReqs.push_back(std::make_pair("auto_login", "1"));
    vPostReqs.push_back(std::make_pair("submit", "Submit"));

    hReq.postWebPage("postcatcher.in", 80, "/catchers/55f09e1c23e9dc0300001628", vPostReqs);

    system("pause");
    return(0);
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ ITs hard to review without the header file as there is lots of type information in there that would be nice. \$\endgroup\$ Sep 9, 2015 at 22:58

1 Answer 1

2
\$\begingroup\$

Don't use system(). It's generally considered bad practice and pause does not have a place in most programs. If you do intend on using system(), keep in mind that you have to explicitly flush std::cout before the call.

Even then, it's likely that you want an explicit flush for your error messages. Either use std::cerr or std::cout.flush() after each message.

You have InitWinsock(), you never use it

WsaData is a local variable in InitWinsock() making it completely useless.

You also never use WsaData. However after reading the documentation, I'm not convinced you need to call InitWinsock() more than once. So just remove every instance of WsaStartup(). Here's example code from MSDN:

WORD wVersionRequested;
WSADATA wsaData;
int err;

wVersionRequested = MAKEWORD( 2, 2 );

err = WSAStartup( wVersionRequested, &wsaData );
if ( err != 0 ) {
    /* Tell the user that we could not find a usable */
    /* WinSock DLL.                                  */
    return;
}

/* Confirm that the WinSock DLL supports 2.2.*/
/* Note that if the DLL supports versions greater    */
/* than 2.2 in addition to 2.2, it will still return */
/* 2.2 in wVersion since that is the version we      */
/* requested.                                        */

if ( LOBYTE( wsaData.wVersion ) != 2 ||
        HIBYTE( wsaData.wVersion ) != 2 ) {
    /* Tell the user that we could not find a usable */
    /* WinSock DLL.                                  */
    WSACleanup( );
    return; 
}

/* The WinSock DLL is acceptable. Proceed. */

Error messages

The recommended way to get an error is to use WSAGetLastError(), not errno:

Error codes set by Windows Sockets are not made available through the errno variable. Additionally, for the getXbyY class of functions, error codes are not made available through the h_errno variable. The WSAGetLastError function is intended to provide a reliable way for a thread in a multithreaded process to obtain per-thread error information.

Furthermore, the error constants prefixed by WSA are preferred:

// Example from MSDN
r = recv(...);
if (r == -1       /* (but see below) */
    && WSAGetLastError() == WSAEWOULDBLOCK)
    {...}

Furthermore, the error messages should be more descriptive. You may either choose to convert the error code to a string yourself or use FormatMessage as MSDN recommends.

Minor issue

memset(&servAddr, 0, sizeof(servAddr)); is not necessary in C++. You can zero initialize a struct with servAddr = {0};.

struct some_struct is redundant in C++. You can omit the struct keyword.

(LPCSTR)some_string.c_str() is redundant. LPCSTR is just a typedef to const char*.

\$\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.