sholsapp / gallocy

A distributed shared memory infrastructure.
27 stars 9 forks source link

Buffering problem in GallocyServer::get_request #28

Open sholsapp opened 8 years ago

sholsapp commented 8 years ago

There is a bug in GallocyServer::get_request when reading a request of the wire that corrupts the payload if the payload is larger than 120 characters (likely because then with the HTTP header, we’re above the buffer size of 256).

POST /raft/appendentries HTTP/1.1 Accept: /_ Content-Length: 120 Content-Type: application/json Host: 172.17.0.3 User-Agent: restclient-cpp/0.4.1 {"entries":[{"command":"hello world","term":0}],"leader_commit":0,"previous_log_index":0,"previous_log_term”:0,...binary shit here...“term":0}

The problem has been temporarily avoided by increasing the socket read buffer size to 512 bytes.

sholsapp commented 8 years ago

@rverdon can you take a peek at this code? Anything just bonehead wrong that I'm doing there?

rverdon commented 8 years ago

When did you see this happen? Is there a test you ran? Also, why the two recv calls? That would break badly for UDP... can only call recv on a UDP socket once per packet :/.

sholsapp commented 8 years ago

You see this happen during transmission of packets between client and server when negotiating large packets. You can reproduce by making the buffer size smaller (e.g., 256) and then running with cthulhu - you'll see the payloads are all messed up. I don't have a unit test that causes this to happen yet.

I'm not sure why I used two recv calls... if this won't work with UDP then we'll have to change this implementation completely. What if a package is larger than the buffer size with UDP? Can those big packets not be read?

rverdon commented 8 years ago

The recv() function shall return the length of the message written to the buffer pointed to by the buffer argument. For message-based sockets, such as SOCK_DGRAM and SOCK_SEQPACKET, the entire message shall be read in a single operation. If a message is too long to fit in the supplied buffer, and MSG_PEEK is not set in the flags argument, the excess bytes shall be discarded. For stream-based sockets, such as SOCK_STREAM, message boundaries shall be ignored. In this case, data shall be returned to the user as soon as it becomes available, and no data shall be discarded.

Source

Need to set the buffer to be the largest packet you want to send/recv.