search

ghedipunk / PHP-Websockets

A Websockets server written in PHP.
BSD 3-Clause "New" or "Revised" License
913 stars 376 forks source link

deframe fails #22

Closed merged-packets closed 8 years ago

merged-packets commented 9 years ago

PHP websocket server cannot correctly deframe websocket messages if a burst of small messages was sent to the server and some TCP packets were merged head-to-tail into bigger TCP packets before PHP receives packet data (http://en.wikipedia.org/wiki/Nagle's_algorithm)

ghedipunk commented 9 years ago

Looking at the available options without doing any testing yet, it looks like the best way for me to go is to implement TCP_NODELAY first, which will prevent data loss for all cases, then work on a way to parse the merged packets.

After work on parsing the merged packets, the TCP_NODELAY option will be an opt-in option for applications that really and truly need as little latency as possible, but parsing the merged packets will be default because the vast majority of applications don't need sub-200ms reaction times.

merged-packets commented 9 years ago

TCP_NODELAY would be a nice to have option. I would also add SO_KEEPALIVE, if possible.

kfchoong commented 9 years ago

I am facing the same problem. May I know if a new version will be released?

Xaraknid commented 9 years ago

I have the same issue. When from my client I "spam" the server sending multiple socket.send(). I search and found the problem. The problem is that deframe assume there will be only one frame per packet sending to him. That's not true sending 100 socket.send to the server I end up sending 9 packets who have respectively (5,35,14,9,9,7,8,7,6) frames inside of the packet.

So deframe() is to deframe the frame not packet. I made a little function to split packet into frame and send frame to the deframe function and instead of packet.

I'll pull my code.

edit : I'm new with github, but I guess you can see my update to websockets.php ?

Xaraknid commented 9 years ago

fix an issue I have with split_packet() typo + now read the right headers for each frame within the packet.

ghedipunk commented 9 years ago

I'll review, test, and merge as soon as possible. Hopefully that means a merge today.

amb3rl4nn commented 9 years ago

I've tested the update made today and it resolves the issue from yesterday where sending certain size packets were getting a header read issue. It appears to be working correctly. Also "Length" is still spelled incorrectly.

Xaraknid commented 9 years ago

Thanks, I fix it in my directory also. Let me dig a little more into deframe I'll shortly pull modification because I didn't like the fact that I add a function with alot of similarity only change is the args and return value ( calcoffset() and extractPayload() ) also as deframe now process only one frame at time now need a rework. I will pull something shortly.

EDIT : pull #27