Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Socket is not released when HTTP parsing fails #49

Open
ii14 opened this issue Apr 24, 2020 · 2 comments
Open

Socket is not released when HTTP parsing fails #49

ii14 opened this issue Apr 24, 2020 · 2 comments

Comments

@ii14
Copy link

ii14 commented Apr 24, 2020

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 added a commit to Targoman/QHttp that referenced this issue Apr 28, 2020
azadkuh#49

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.
@ziabary
Copy link

ziabary commented Apr 28, 2020

Bug fixed on Targoman fork. Same issue fixed on qhttclient

@ii14
Copy link
Author

ii14 commented May 4, 2020

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;
}

paluke pushed a commit to paluke/qhttp that referenced this issue Aug 18, 2021
azadkuh#49

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants