Closed GoogleCodeExporter closed 9 years ago
One more detail. If in the testcase I change the two successive socket.sends
with a one send with the same concatenated data, all works ok.
Original comment by mo...@tushino.ru
on 14 Dec 2011 at 10:48
Thanks for your report!
Chrome should be supported, it sounds like a bug in the frame reading in phpws.
I think I know what could be the cause of this problem. Will try to fix this
during the weekend :)
Original comment by ch...@devristo.com
on 14 Dec 2011 at 3:12
Hi, thanks for the prompt response. It seems I found the reason. A frame can be
large enough to fit into a single packet, so there can be several raw packets
in the frame (and the frame header is only in the first raw packet). In
'decode' method we get a packet which can be less in size than payloadLength
(obtained from the first frame/packet). This means that the subsequent packets
($raw input parameter) are a continuation packets for the exiting frame. In
such cases we should append $raw (as is) to the exisiting frame (using old mask
if necessary). Only after payloadLength is read we can construct entire message.
Original comment by mo...@tushino.ru
on 14 Dec 2011 at 3:56
Correct, there could be multiple frames in $raw. They both have headers though.
I am currently testing a fix (and other fixes). They are already committed. It
should fix the issue.
However incomplete frames in $raw (frames split over multiple packets) are
still not supported (messages spanning multiple frames are though). I will see
if I can fix that issue this weekend.
Original comment by ch...@devristo.com
on 14 Dec 2011 at 5:25
I made it already. Now trying to find a way to post changes in most convenient
way.
Original comment by mo...@tushino.ru
on 14 Dec 2011 at 5:45
Hi, I'm attaching 2 changed files. As you have some edits in your version, it
could be problems with merging by diff file, so you can just compare your
latest source with mine.
Original comment by mo...@tushino.ru
on 14 Dec 2011 at 6:18
Attachments:
Just to be sure, your changes do not solve your initial issue? But the problem
i posed with the frames split accross multiple packets?
Original comment by ch...@devristo.com
on 14 Dec 2011 at 6:30
Yes, you are right. I made the fix for another problem, because it stopped my
development process, and because it has no a workaround, whereas the inital
issue does have a workaround by concatenation of client-side messages. AFAICU,
I can download your fix right now? Great. Thank you.
Original comment by mo...@tushino.ru
on 14 Dec 2011 at 7:28
I'm afraid your approach for handling multipacketed frames is buggy. I suggest
to use my version. It's tested.
Original comment by mo...@tushino.ru
on 14 Dec 2011 at 7:42
I'm attaching next versions of my files - they now contain fixes for both
problems, and they pass my testcases.
Original comment by mo...@tushino.ru
on 14 Dec 2011 at 8:11
Attachments:
Thanks for fixing these issues. I will look into your code tonight and
integrate it with mine :)
Original comment by ch...@devristo.com
on 15 Dec 2011 at 9:36
Pushed your changes to git :)
Original comment by ch...@devristo.com
on 15 Dec 2011 at 8:50
Two points. I'm not sure that an exception should be fired if we run out of
payloadLength (moved inside isReady). Currently this breaks connection.
Perhaps, connection should be kept open, but it's not clear to what state reset
buffer and actualLength.
Also styling is broken. I don't use tabs in my editor, so indents are screwed
up in my fragments of code.
Original comment by mo...@tushino.ru
on 16 Dec 2011 at 8:49
Yeah I noticed about the formatting. Will autoformat everything later :)
I think the specs say that the connection should be closed. But I will need to
check up on that. As you say the big problem is that its hard to reset the
buffer. You could clear it, or try to find a frameheader somewhere in the
middle of the frame. But even then, the client has to be informed something
went wrong, otherwise messages get lost without the clients knowledge.
Original comment by ch...@devristo.com
on 16 Dec 2011 at 4:38
Will close this one, since the original bug has been fixed.
Original comment by ch...@devristo.com
on 5 Jan 2012 at 9:53
Original issue reported on code.google.com by
mo...@tushino.ru
on 14 Dec 2011 at 10:39Attachments: