Document collectData()

[mrdeveloperdude]

Hi!

I'm struggling with implementing a server. I started out with keep-alive example but for some reason the json body received always appears to be 0 bytes long (it is in fact not, I tested with other non-qhttp servers (php and netcat) and they receive the 84 bytes of json data just fine).

I think the bug in my software is caused by my lack of understasnding of the collectData() call. What is the integer parameter it accepts? Can I omit the call to collectData() and still get request body? What are other possible causes of receiving empty body in request? How should I go about debugging this?

Thanks!

PS: Big fan of your work!

[mrdeveloperdude]

OK, so I found the documentation for it, and it was great :) But really hard to find being on the abstract class for server and all. Also I found a bug:

void collectData(int atMost) { icollectCapacity = atMost; icollectedData.clear(); icollectedData.reserve(atMost); }

In declaration it is stated taht atMost can be set to -1 to mean "unlimited" however this generates sigsegv crash because reserve(-1) is not valid.

[azadkuh]

@mrdeveloperdude sorry to be so late!

thank you for reporting the collectData() bug, gonna fix it asap.

[knobtviker]

Handling of reserve(-1) fixes the sigsegv crashes but POST body still isn't collected. Also a fan here, just wanted to pitch in to point out there are others waiting for this bugfix. Thanks for this awesome code.

[hoensr]

Same here! What Knobtviker says...

[hoensr]

Well, it is possible to connect to the signal QHttpRequest::data(QByteArray), which is emitted when the body has been read. But I wanted a signal containing the (completely parsed) request and the response, so I added a new signal QHttpConnection::completeRequest, emitted in QHttpConnectionPrivate::messageComplete. Here is the patch:

Index: qhttpserverconnection.cpp
===================================================================
--- qhttpserverconnection.cpp   (revision 33224)
+++ qhttpserverconnection.cpp   (working copy)
@@ -218,6 +218,7 @@
      // request is ready to be dispatched
     ilastRequest->d_func()->isuccessful = true;
     ilastRequest->d_func()->ireadState  = QHttpRequestPrivate::EComplete;
+    emit q_ptr->completeRequest(ilastRequest, ilastResponse);
     return 0;
 }

Index: qhttpserverconnection.hpp
===================================================================
--- qhttpserverconnection.hpp   (revision 33224)
+++ qhttpserverconnection.hpp   (working copy)
@@ -63,6 +63,12 @@
      */
     void            newRequest(QHttpRequest* req, QHttpResponse* res);

+    /** emitted when a HTTP request is completely read.
+    * @param req incoming request by the client.
+    * @param res outgoing response to the client.
+    */
+    void            completeRequest(QHttpRequest* req, QHttpResponse* res);
+
     /** emitted when the tcp/local socket, disconnects. */
     void            disconnected();

Currently, I connect to QHttpServer::newRequest, and in the handler I call request->collectData(65536) and then connect to QHttpConnection::completeRequest for the actual handling. I feel that this is still a bit cumbersome, so maybe it would be nice to just add a QHttpServer::completeRequest signal.

[knobtviker]

Thanks @hoensr after I patched it in my implementation, I was able to gather POST body data. Original issue with collectData() is still present, just saying for records sake.

[azadkuh]

I'm trying to refactor the qhttp implementation in my free time (if i could find any) and fixing this issue is on my set list.

thanks @hoensr and others, i will be happy if you dudes bring up more suggestions.

[azadkuh]

@mrdeveloperdude @hoensr @knobtviker I made some changes to qhttp without breaking any API.

please check the dev branch and new example/helloworld/main.cpp to see how it's working. as for POST i played with curl (please have a look to example/helloworld/README.md) and was OK.

it would be great if you can find some time and test this branch.

• the new branch requires c++14 as minimum.
• only tested by clang / OS X and gcc5 / ubuntu

[gianks]

Hi all, currently i'm using the dev branch without any patch and it's working fine using onEnd lambda! I'm uploading up to 10MB in my tests without troubles, just to inform you.

Maybe i should open a new bug but i'm now in trouble still working on this topic (form data upload) since the headers of a request appear to be case lowered and this is making impossibile to accept a form-data request since in some cases (as chrome does but not Firefox) the data boundary pattern is given with both lower and uppercase letters making impossible to decode correctly the request (since the value i'm looking for it's different from what i can read into the body request!).

Imho Chrome is absolutely right since the pattern should be any acceptable combinations of letters/numbers that is not present in any part of the binary data the browser will send, so limiting the browser to lower case combinations for the boundary is wrong since may interefere with its actual needs to fulfill the request.

Firefox seems to work fine just because it's sending just numbers... no effect on it! Is there a way to tell QHttp to let the headers as they are? (that should help even with session-id and any other cookie, for example).

Thanks

[azadkuh]

@gianks glad to hear that.

and for http header please have a look at: HTTP/1.1 - sec 4.2

as defined by HTTP standards:

Each header field consists of a name followed by a colon (":") and the field value. Field names are case-insensitive. ...

qhttp converts header names into lower case, just for faster and easier search and validation, so each of Content-Length, content-length, ... produce the same result, and as far as I know it does not violate any standard.

by the way I'm interested to know why some tools rely on case-sensitive HTTP header.

[gianks]

Hi and thanks for tour quick reply! What i was remarking is not about field names but about field values! Both are lowered and that's not fitting the standard!

[azadkuh]

yupp, that's right. but the header values are not required to be case sensitive by standard, it's really confusing, for example the value of Connection must be case-insensitive!! for instance Keep-Alive cause problems and should be altered into keep-alive.

but in general it's good idea to update qhttp from altering values with some exceptions as connection, host and ...

[gianks]

I agree 100%. Just when surely well known headers should be lowered even in values otherwise provided as they are. If i can i'll play with it tomorrow... Let us know if you are planning to do it on a schedule.

Thanks Gianluca

[azadkuh]

yes i will. don't have any schedule for this issue, try to update asap.

[7hibault]

Hi, thanks for all the good work.

I'm also facing some troubles with collectData. The code I'm using is adapted from version-2.1 with the remarks from this thread. I can't use the latest branch as it requires c++14 as minimum, which I can't use in my project.

It seems that there is a timing issue when the data is read. Let me illustrate that with an example. Let's say my server will simply answer a PUT request with a 200 code if it can read the body, and will throw a 400 error code if it can't. The result is: sometimes it works, sometimes it doesn't...

While sniffing the network during an unsuccessful attempt, the following packets were exchanged (in that order):

PUT headers 400 answer PUT body

The server gave its 400 answer before the body arrived. That shows that the server doesn't wait until the message is complete to answer. The only workaround I could find to make it sort of work temporarily is to delay a little bit the parsing (usleep in HttpParser constructor in qhttpbase...). But that's not viable.

Does anyone have a patch/idea to fix this?

[hoensr]

Hi 7hibault, I have switched to the dev branch although I use VS2013 without complete c++14 support. But there are only a few places which use c++14 features and I had to change. See this comment. Maybe that would help with your problem?