Xpra: Ticket #2018: html5 support for rencode

There is now a javascript version we can use: https://github.com/PolkaJS/rencode (ISC License)



Mon, 29 Oct 2018 16:06:17 GMT - Antoine Martin: attachment set

almost works?


Tue, 26 Feb 2019 05:14:11 GMT - Antoine Martin: status, milestone changed

See also #2165


Wed, 01 May 2019 09:07:50 GMT - Antoine Martin: status changed; resolution set

Code is incomplete and would need a lot of work.


Tue, 26 May 2020 21:34:20 GMT - Tijs van der Zwaan: status changed; resolution deleted

Hi guys,

I've fixed this issue in my local development environment using the following node module; https://github.com/cinderblock/python-rencode/.

Is anyone still interested in a patch?


Tue, 26 May 2020 23:41:47 GMT - Antoine Martin: owner, status changed

Absolutely, please send it along.


Wed, 27 May 2020 14:29:05 GMT - Tijs van der Zwaan: attachment set


Wed, 27 May 2020 14:30:30 GMT - Tijs van der Zwaan: owner, status changed

Here you go. The patch might need some extra attention, but the basics are there. This will break internet explorer functionality. Is IE 11 currently supported?


Wed, 27 May 2020 14:39:51 GMT - Antoine Martin:

Thanks. Yes, IE 11 is still meant to be supported, so I'll need to find a way to either ignore rencode with IE11 or just fix it.


Fri, 29 May 2020 06:13:34 GMT - Antoine Martin: status changed; resolution set

Merged in r26508, only a minor fix, and the IE11 compatibility workaround.

IE11 already needed build fixes for 4.0: r26507, and other minor fixes: r26509, r26510, etc. There are still other problems with IE, but those aren't new and at least it runs now.

Thanks for the patch!


Sat, 30 May 2020 04:31:07 GMT - Antoine Martin:

Caused a regression: #2787


Sat, 30 May 2020 05:14:47 GMT - Antoine Martin: status changed; resolution deleted


Sat, 30 May 2020 05:19:18 GMT - Antoine Martin: owner, status changed

This happens on small rgb32 screen updates because the pixel data ends up inlined with the packet data.

This worksaround the issue by not using packet inlined data (ugly):

Index: xpra/net/protocol.py
===================================================================
--- xpra/net/protocol.py	(revision 26521)
+++ xpra/net/protocol.py	(working copy)
@@ -562,7 +562,7 @@
                 #(it may now be a "Compressed" item and be processed further)
             if ti in (Compressed, LevelCompressed):
                 #already compressed data (usually pixels, cursors, etc)
-                if not item.can_inline or l>INLINE_SIZE:
+                if True:
                     il = 0
                     if ti==LevelCompressed:
                         #unlike Compressed (usually pixels, decompressed in the paint thread),

@Tijs van der Zwaan: what's happening with the pixel data? Why is it mangled by the PyRencode decoder? I've tried playing with the utf8 flag of the decode function, the result is different - but still broken..

FYI: the pixel data gets converted to a Uint8Array in Protocol.js near pass to our packet handler. (IIRC because strings can't be sent from the worker)


Sun, 31 May 2020 11:23:48 GMT - Tijs van der Zwaan:

Hi Antoine,

I've figured out that it's an encoding problem between Python and javascript. I've implemented a quick and dirty workaround by using base64 encoding.

Protocol.py

                    #data is small enough, inline it:
                    packet[i] = str(base64.b64encode(item.data), "utf-8")

protocol.js

				if (typeof img_data === 'string') {
					const img_data_raw = atob(img_data);
					const uint = new Uint8Array(img_data_raw.length);
					for(i=0,j=img_data_raw.length;i<j;++i) {
						uint[i] = img_data_raw.charCodeAt(i);
					}
					packet[7] = uint;

I'm going to investigate it further...


Sun, 31 May 2020 12:37:45 GMT - Antoine Martin:

Thanks for looking into it. I don't understand what PyRencode is doing to that binary string that we can't just receive it exactly as it is on the wire, just sliced off. (Uint8Array or whatever)


Sun, 31 May 2020 18:09:23 GMT - Tijs van der Zwaan:

Thanks!!! Your comment '..is doing to that binary string..' pointed me in the right direction :) PyRencoder got a switch for UTF-8. If we set the switch to false, it uses ascii encodig, but we want binary. I've changed the python-rencode node package and the PyRencoder wrapper. It now accepts the encoding parameter as string.

Patch:

@@ -333,7 +333,7 @@
        let packet = null;
        try {
                if (proto_flags==1) {
-                packet = PyRencoder.decode(Buffer.from(packet_data), false);
+                packet = PyRencoder.decode(Buffer.from(packet_data), 'binary');
                } else {
                 packet = bdecode(packet_data);
                }

The new PyRencoder.js is in the attachment. Can you test this? And do you want the ES6 source of the PyRencoder? It's just a wrapper for python-rencode node package, build with webpack/babel.


Sun, 31 May 2020 18:10:51 GMT - Tijs van der Zwaan: attachment set

Specify encoding with string parameter


Mon, 01 Jun 2020 04:19:41 GMT - Antoine Martin:

Thanks, applied in r26562. It seems to work fine.

Can you generate a non-minified version of pyrencode.js? All the other libraries are minified during the build, not in the repository.


Tue, 02 Jun 2020 08:57:22 GMT - Antoine Martin:

Also, the rencode mode should be optional, even if the option is not necessarily shown on the connect dialog page.

Why: it's useful to switch packet encoders to track down bugs and verify that it works as expected... Would have saved me time because it breaks AES encryption - this also needs fixing. Try AES mode from wiki/Clients/HTML5.


Fri, 03 Jul 2020 09:29:15 GMT - Tijs van der Zwaan:

Thanks for the comments, I didn't had the time to try AES mode lately. Will fix this soon.


Fri, 03 Jul 2020 10:56:51 GMT - Antoine Martin:

I didn't had the time to try AES mode lately. Will fix this soon.

FYI: there are other AES options in #2615.


Tue, 05 Jan 2021 14:29:55 GMT - Antoine Martin:

This has caused a regression with file transfers: #2993.


Tue, 05 Jan 2021 15:46:14 GMT - Antoine Martin:

@Tijs, as per comment:14:

Can you generate a non-minified version of pyrencode.js?

(preferably with steps so that I can re-generate it when there are new versions) It would make it a easier to debug. My guess is that it comes from here: https://github.com/cinderblock/python-rencode and the code does this with strings:

function encodeStr(buffs, str) {
    // JS strings are always utf8
    var buff = Buffer.from(str, 'utf8');
    if (buff.length < STR_FIXED_COUNT) {
        writeBufferChar(buffs, STR_FIXED_START + buff.length);
        writeBuffer(buffs, buff);
    }
    else {
        writeBuffer(buffs, Buffer.from(buff.length + ':', 'ascii'));
        writeBuffer(buffs, buff);
    }
}

OTOH, some potential solutions:


Tue, 05 Jan 2021 15:47:02 GMT - Antoine Martin: attachment set

patch to allow the bencoder to handle binary buffers directly


Thu, 14 Jan 2021 08:34:37 GMT - Tijs van der Zwaan:

I'm very sorry for the delay. I've included the code used to build this. Yes, I used python-rencode.

The project is very simple: 1) Unpack 2) npm install 3) npm run build-prod -> this build the library to ./builds with Babel


Thu, 14 Jan 2021 08:35:37 GMT - Tijs van der Zwaan: attachment set


Thu, 14 Jan 2021 16:09:46 GMT - Antoine Martin:

$ git clone https://github.com/cinderblock/python-rencode
$ cd python-rencode
$ npm install
$ npm run build-prod
npm ERR! missing script: build-prod
npm ERR! A complete log of this run can be found in:
npm ERR!     /home/antoine/.npm/_logs/2021-01-14T15_04_41_545Z-debug.log

Can you build the non-minified version and attach it to this ticket?

The more I look at this thing and the more I think it should be re-written in plain Uint8Array vanilla javascript. The code is really not very efficient either.


Thu, 14 Jan 2021 16:19:34 GMT - Tijs van der Zwaan:

Replying to Antoine Martin:

$ git clone https://github.com/cinderblock/python-rencode
$ cd python-rencode
$ npm install
$ npm run build-prod
npm ERR! missing script: build-prod
npm ERR! A complete log of this run can be found in:
npm ERR!     /home/antoine/.npm/_logs/2021-01-14T15_04_41_545Z-debug.log

Can you build the non-minified version and attach it to this ticket?

The more I look at this thing and the more I think it should be re-written in plain Uint8Array vanilla javascript. The code is really not very efficient either.

Hi Antoine,

You need to run the npm commands on the source files i've included in the zip file, not the python-rencode repository. The python-rencode libs will be auto-installed by npm.

The non-minified is still kind of unreadable because of all the polyfills. I will try to create a human-readable version.


Thu, 14 Jan 2021 16:30:37 GMT - Antoine Martin: owner, status changed

You need to run the npm commands on the source files i've included in the zip file, not the python-rencode repository

Doh! I had missed the zip file.. That works fine, but it's impossible to edit the resulting library file.

I might just re-write it in plain JS tomorrow.


Sat, 23 Jan 2021 05:39:56 GMT - migration script:

this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/2018