Framing broken for huge frames

[GoogleCodeExporter]

When using Chrome 16 as a client I got the error of mismatched frame size on 
large frames untilizing several packets. Just looking at this. ;-/

Original issue reported on code.google.com by mo...@tushino.ru on 19 Dec 2011 at 6:12

[GoogleCodeExporter]

Weird stuff.

Did some other fixes during the day, see source on git. I tested sending 3x2MB 
messages, works fine with the PHPWS client. Does PHPWS client pass the server 
tests at your side (test the source from git, not the rar) ?

I will try to sent some messages with Chrome.

Note that phpws now uses non blocking sockets as well. CPU usage here is 0.3%, 
even with blocking connections CPU usage can be low. There was a bug a few 
revisions back that caused high cpu usage.

Original comment by ch...@devristo.com on 25 Dec 2011 at 9:07

[GoogleCodeExporter]

On Chrome i get 'Could not decode a text frame as UTF-8.' That is weird :S

Original comment by ch...@devristo.com on 25 Dec 2011 at 9:14

[GoogleCodeExporter]

Here is my test project. There are 2 handlers: 'servers' and 'clients'. 1 of 
each type is sufficient to reproduce the problem. The 'server' should send 
messages of say approx. 30K, they should be redirected to the 'client'. The 
'client' acknowledges each message by unique ID. Only after ack the phpws can 
send another (most resently received) message from the 'server'. On my computer 
PHPWS stalls after approx. 30 such messages, that is the remote client receives 
0-length message and does not acknowledge it, of course. Remote server 
continues to send messages and they seem received ok by PHPWS.

NB. This test project is adapted for PHPWS version from 24 december.

Original comment by mo...@tushino.ru on 25 Dec 2011 at 9:15

Attachments:

• dualtest.zip

[GoogleCodeExporter]

'Could not decode a text frame as UTF-8'. I think this is because the message 
is 0-length. I saw this also.

Original comment by mo...@tushino.ru on 25 Dec 2011 at 9:17

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Frame decoding is verry slow I think. I just got 20 secs on that :S

Original comment by ch...@devristo.com on 25 Dec 2011 at 10:06

[GoogleCodeExporter]

RotMask is very slow. Without it decode runs very fast.

Original comment by ch...@devristo.com on 25 Dec 2011 at 10:25

[GoogleCodeExporter]

2MB demasking took > 20 sec. Now i got it to 5. But should be even faster.

Original comment by ch...@devristo.com on 25 Dec 2011 at 11:21

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Okay committed. Try client.html . It sends a 2MB message on each click. Here I 
can click it as many times as I want and I get response from the server without 
error.

Original comment by ch...@devristo.com on 26 Dec 2011 at 12:43

[GoogleCodeExporter]

The client.html has an unconditional reference to MozWebSocket, which should be 
cutomized for real work. If corrected, I got the error "Fatal error: Allowed 
memory size of 25165824 bytes exhausted (tried to allocate 35 bytes) in 
websocket.framing.php on line 136". It strange to see a request for 25Mb. I 
changed the size of the message to 1024*1024, and it works (as usual for your 
demo ;-)). But my dualtest still fails, moreover it seems that something is 
broken in decoding/encoding, because my dataurls are broken on receiving end.
P.S. In protocol.php line 280 should be
WebSocketFunctions::say("HIXIE Response SENT!");

Original comment by mo...@tushino.ru on 26 Dec 2011 at 5:40

[GoogleCodeExporter]

Hmm thats the rotmask function again. Seems its now more memory hungry than cpu 
hungry. 

Can you try to disable masking in the framing and then check? Data will be 
corrupted, but you can test if the rest is okay.

Original comment by ch...@devristo.com on 26 Dec 2011 at 6:03

[GoogleCodeExporter]

I don't understand what for disabling masking, because it says in the error 
message that it is exactly in the rotMask function, line 136. Most important 
that data is already broken. The onMessage handler in WebSocketUriHandler 
already gets the message data corrupted, so it is something in decode. I don't 
yet have a way to check encode as well. Note, that I use production browsers as 
clients, so their correctness is undisputable.

Original comment by mo...@tushino.ru on 26 Dec 2011 at 6:22

[GoogleCodeExporter]

I know, but you can check if the number of bytes are correct for each frame / 
message. If there is something wrong with the frame offset you will see it. 
Decoding a single frame seems to work, so it has to be in the handling of 
multiple frames / package or multiple packages / frame.

Original comment by ch...@devristo.com on 26 Dec 2011 at 7:08

[GoogleCodeExporter]

I logged messages from version of 24 Dec and from the latest version. The 
messages which came from the later is lesser in size.

Anyway I don't think it was an issue with CPU in the previous version. I'm 
attaching two html-files which are 'server' and 'client' clients for my 
dualtest. You may use them to test the version from Dec24, and you will see 
that after approx 20-th message sent (without any noticable CPU consumption), 
it becomes corrupted. As a result Chrome says "Could not decode a text frame as 
UTF-8" and drops connection, Safari just shows a message of 0 length.

Original comment by mo...@tushino.ru on 26 Dec 2011 at 7:17

Attachments:

• clients.zip

[GoogleCodeExporter]

Though I'm not a zealot of the latest (a little bit experimental) version and 
whould be rather happy with fixing the previous more stable one, I have had a 
look at the latest one and what is the line 148 of framing meaning?

for($i = 0; $i < 0; $i++)

Original comment by mo...@tushino.ru on 27 Dec 2011 at 8:34

[GoogleCodeExporter]

It seems I have fixed the problem (on Dec24 sources). Indeed the issue was with 
the missing offset when applying a mask.

Original comment by mo...@tushino.ru on 27 Dec 2011 at 9:21

[GoogleCodeExporter]

So here is the fix details. The rotMask should be:
    protected static function rotMask($data, $key, $offset = 0) {
        $res = '';
        for ($i = 0; $i < strlen($data); $i++) {
            $j = ($i + $offset) % 4;
            $res .= chr(ord($data[$i]) ^ ord($key[$j]));
        }

        return $res;
    }

The part of decode should be:
        $currentOffset = $frame -> actualLength;
        $fullLength = min($frame->payloadLength - $frame->actualLength, strlen($raw));
        $frame -> actualLength += $fullLength;

        if ($fullLength < strlen($raw)) {
            $frameData = substr($raw, 0, $fullLength);
            $raw = substr($raw, $fullLength);
        } else {
            $frameData = $raw;
            $raw = '';
        }

        if ($frame -> mask)
            $frame -> payloadData .= self::rotMask($frameData, $frame -> maskingKey, $currentOffset);
        else
            $frame -> payloadData .= $frameData;

Original comment by mo...@tushino.ru on 27 Dec 2011 at 9:36

[GoogleCodeExporter]

BTW, the $frames array in readFrame seems excessive for server side (it is 
returned by the function but does not used outside), and can produce potential 
problems in client, because it merges frames regardless of the fact that we can 
return the same frame several times. Yet bear in mind that I dont't use PHPWS 
client and just express my quick thoughts - they can possibly be wrong.

Original comment by mo...@tushino.ru on 27 Dec 2011 at 9:46

[GoogleCodeExporter]

Yeah it is not necessary for the server, its purely for the client. About the 
masking: Why dont you just decode the payloaddata when the frame is completely 
received? Thats whats the latest version is doing.

Anyway I am glad its fixed. Will change the code to match yours though :) 
Thanks for all the help!

Original comment by ch...@devristo.com on 27 Dec 2011 at 10:04

[GoogleCodeExporter]

BTW that for-loop seems incorrect, will have a look at the end of the day to 
see what it should be replaced with :)

Original comment by ch...@devristo.com on 27 Dec 2011 at 10:05

[GoogleCodeExporter]

Well, the latest version seems having too much optimizations, the purposes of 
which are not clear to me. ;-) For example, the simple rotMask function became 
unnecesserily complicated. I don't mention the other portions of code. The 
previous version works fine. This is why I'd prefer to see the previuos (more 
stable) version fixed than the new.

Original comment by mo...@tushino.ru on 27 Dec 2011 at 10:25

[GoogleCodeExporter]

The old rotmask took > 20 seconds on some frames. You dont have that issue?

Original comment by ch...@devristo.com on 27 Dec 2011 at 11:40

[GoogleCodeExporter]

No. Looking at the code, I can't imagine more effective way of applying a mask, 
than in the prerious version.

Original comment by mo...@tushino.ru on 27 Dec 2011 at 11:49

[GoogleCodeExporter]

Well xoring 4/8 bytes together goes just as fast as 1 byte on most machines ;) 
Maybe it was an issue with my PHP version, noticed a speed up after an upgrade 
as well.

Original comment by ch...@devristo.com on 27 Dec 2011 at 1:41

[GoogleCodeExporter]

This could be true. But most important to make it fast and keep correct ;-).

Original comment by mo...@tushino.ru on 27 Dec 2011 at 2:50

[GoogleCodeExporter]

No argueing with that ;)

Original comment by ch...@devristo.com on 27 Dec 2011 at 3:07

[GoogleCodeExporter]

Committed your changes to the 24th version.

Original comment by ch...@devristo.com on 27 Dec 2011 at 3:19

[GoogleCodeExporter]

Is this now completely fixed :) ?

Original comment by ch...@devristo.com on 5 Jan 2012 at 9:53

[GoogleCodeExporter]

Nothing can be fixed completely ;-). But it seems so and the issue is too big 
anyway.

Original comment by mo...@tushino.ru on 5 Jan 2012 at 3:09

[GoogleCodeExporter]

True, will close this one. If there are any related issues a new report can be 
made.

Original comment by ch...@devristo.com on 7 Jan 2012 at 10:36

• Changed state: Fixed