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

HTTP server crashes if processing requests with protocol major v > 1 #21

Open
ghost opened this issue May 16, 2014 · 1 comment
Open

Comments

@ghost
Copy link

ghost commented May 16, 2014

I understand that is not meant to process such requests, like
GET / HTTP/4.0
but, the problem is that the server desperately fails into segmentation fault.
This is an example connection to the echoserver from examples:

rakula@nevada ~/play/mordor $ /home/rakula/play/mordor/mordor/examples/.libs/echoserver
terminate called without an active exception
Аварийный останов
rakula@nevada ~/play/mordor $ 

data send here:

nevada rakula # nc 127.0.0.1 8999
GET / HTTP/2.0

nevada rakula # 

As i assume the invalid version should be handled in mordor/http/server.cpp:1249

        if (m_request.requestLine.ver.major != 1) {
            m_requestState = ERROR;
            respondError(shared_from_this(), HTTP_VERSION_NOT_SUPPORTED, "", true);                                  
            return;
        }

an this particular piece of code works fine. it calls respondError, which calls request->finish(), which calls commit() and there we are:

ServerRequest::commit()
...cut...
    MORDOR_ASSERT(m_response.status.ver == Version(1, 0) ||                                                          
           m_response.status.ver == Version(1, 1));

since version is taken from request (if i am getting it right) - it is still "2.0" here, so assertion fails and the program crashes.

probably it should be sane to add something like
m_request.requestLine.ver = Version();
just before calling
respondError(shared_from_this(), HTTP_VERSION_NOT_SUPPORTED, "", true);
from ServerRequest::doRequest()

@ghost
Copy link
Author

ghost commented May 16, 2014

tested my assumption:

rakula@nevada ~/play/mordor $ PAGER=cat git diff mordor/http/server.cpp
diff --git a/mordor/http/server.cpp b/mordor/http/server.cpp
index 2fc07c6..ecae59d 100644
--- a/mordor/http/server.cpp
+++ b/mordor/http/server.cpp
@@ -504,6 +504,7 @@ ServerRequest::doRequest()

         if (m_request.requestLine.ver.major != 1) {
             m_requestState = ERROR;
+            m_request.requestLine.ver = Version();
             respondError(shared_from_this(), HTTP_VERSION_NOT_SUPPORTED, "", true);
             return;
         }
rakula@nevada ~/play/mordor $ 
nevada rakula # nc 127.0.0.1 8999
GET / HTTP/2.0

HTTP/1.1 505 HTTP Version not supported
Connection: close
Content-Length: 0

nevada rakula # 

looks better.

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

0 participants