azadkuh / qhttp

a light-weight and asynchronous HTTP library (both server & client) in Qt5 and c++14
MIT License
484 stars 158 forks source link

Socket is not released when HTTP parsing fails #49

Open ii14 opened 4 years ago

ii14 commented 4 years ago

If the incoming HTTP header is invalid, which can be as trivial as providing the HTTP method in lowercase instead of uppercase, the socket is never released and it just hangs the entire connection infinitely.

The reason for this is that there are no checks whether the http_parser_execute function actually succeeded or not.

A quick fix for this is to add a simple check in method onReadyRead in qhttpserverconnection_private.hpp:

void onReadyRead() {
    while ( isocket.bytesAvailable() > 0 ) {
        char buffer[4097] = {0};
        size_t readLength = (size_t) isocket.readRaw(buffer, 4096);

        parse(buffer, readLength);
        if (iparser.http_errno != 0) {
            release(); // release the socket if parsing failed
            return;
        }
    }
}

or even write a hardcoded 400 Bad Request back to the socket, but it probably should be done in a more proper fashion.

ziabary commented 4 years ago

Bug fixed on Targoman fork. Same issue fixed on qhttclient

ii14 commented 4 years ago

Bug fixed on Targoman fork. Same issue fixed on qhttclient

Keep in mind that the server will send back an empty response. If you want to report a bad request back to the client, something like this seems to work:

if (iparser.http_errno != 0) {
    QHttpResponse response(q_ptr);
    response.setStatusCode(qhttp::ESTATUS_BAD_REQUEST);
    response.addHeader("connection", "close");
    response.end("<h1>400 Bad Request</h1>\n");
    release();
    return;
}