Peerjs crashes if you send any Unicode characters above U+FFFF

[divec]

Steps to reproduce: Essentially, just conn.send( '𨋢' ) using the default binary serialization. See full example below.

Expected behaviour: Data is sent and received correctly.

Actual behaviour: The receiving peer crashes inside util.unpack, because the sender sent garbled data.

Cause
This happens when sending any Unicode codepoint above U+FFFF:

• In the Javascript string, a single codepoint above U+FFFF is expressed as a UTF-16 surrogate pair made of two code units, e.g. '\uD860\uDEE2'.
• This is a single Unicode codepoint (even though it has Javascript string length 2).
• The function utf8Length wrongly assumes each individual surrogate becomes three UTF-8 bytes (i.e. a total of six bytes for a surrogate pair).
• But in fact a surrogate pair, taken as a whole, becomes four UTF-8 bytes.
• This length mismatch causes the crash in util.unpack.

Fix: #10

Workaround: Using JSON serialization (instead of the default binary) avoids the crash (but has certain other disadvantages).

Full code to reproduce the error

var peer1 = new Peer();
peer1.on( 'open', function ( id ) {
        var peer2 = new Peer();
        peer2.on( 'open', function() {
                var conn2 = peer2.connect( id );
                // But using JSON serialization, instead of the default binary, avoids the crash
                //var conn2 = peer2.connect( id, { serialization: 'json' } );
                conn2.on( 'open', function () {
                        // Send U+282E2, the Cantonese word for 'elevator'
                        // (but it could be any of the 93,000 Unicode codepoints above U+FFFF)
                        conn2.send( '𨋢' );
                } );
        } );
} );
peer1.on( 'connection', function ( conn1 ) {
        conn1.on( 'data', function ( data ) {
                // This never runs: util.unpack crashes before we get this far
                console.log( 'received: ' + data );
        } );
} );

[divec]

Ping: Any chance someone could review my proposed js-binarypack patch? I'm satisfied it fixes the issue (it has unit tests and comprehensive comments). Thanks!

[jonasgloning]

Transferred to the binarypack-repo (issue tracker wasn't enabled before).

Thank you for the bug report and the pull request! Complete with description, analysis, explanation, workaround, fix, and code for reproduction, this is probably the most helpful issue I have seen here!

I could reproduce the crash and confirm your patch works. I'll merge it now, but it will take some time to reach npm. Only the previous maintainer has write-access to the package; I'll ask him to transfer it.

[divec]

Thanks very much!

[divec]

Hi @jonasgloning , is there anything we can do to get this updated in npm? Especially in the package peerjs-js-binarypack used by peerjs. Thanks!

[jonasgloning]

Released now as v1.0.2!

[jonasgloning]

Can you explain to me why expected != text in this line https://github.com/divec/js-binarypack/blob/3c7eed63c1da824e7d6fa5a3e0bf8742db72b96f/test/utf8.html#L7 ?

[divec]

Great question. A Javascript string with unpaired surrogates is not actually valid UTF-16, and so it cannot be round-tripped to UTF-8 and back. The recommended way to handle this is to replace each unpaired surrogate with \uFFFD (the "replacement character"). Note a surrogate pair means two adjacent Javascript characters where the first is in the range \uD800 - \uDBFF and the second is in the range \uDC00 - \uDFFF. To be valid UTF-16, Javascript characters from these ranges must *only* appear in surrogate pairs. An *unpaired* surrogate means any such Javascript character that is not paired up properly.