add libvncclient extended clipboard pseudo-encoding support

[wuhanck]

add libvncclient support for notify and text only.
although it is not full support, it is working for utf-8 clipboard.

[bk138]

Thanks for the contribution! Please document in the code

• what is missing as you indicate
• please remove the unrelated whitespace changes.

[wuhanck]

extended clipboard support
0 | text
1 | rtf
2 | html
3 | dib
4 | files
and with combination of
24 | cap
25 | request
26 | peek
27 | notify
28 | provide
It is a big feature.
But in reality, at most cases(if it is not all), client just need to support text with notify/provide to get utf-8 clipboard working.

[wuhanck]

extended clipboard support need zlib. It is possible to use libvncclient without zlib? Or please help me to protect the code by some Macro.

[bk138]

extended clipboard support need zlib. It is possible to use libvncclient without zlib? Or please help me to protect the code by some Macro.

Yes, I added some hints for you in the review comments. Thanks for the contribution, I think with some more work we can get this in!

[bk138]

@wuhanck I now understoof your point about the config flag. Only two minor things missing AFAICT.

[bk138]

Sorry, two more questions. Thanks for keeping up with this PR!

[bk138]

@wuhanck I think some more work is required:

• upfront, please remove the commits following 80aacdc (80aacdc is good, rest we might add later, but let's keep it simple for now...)
• please rename appData.clipboardCap to somehting more descriptive like appData.useExtendedClipBoard
• most important, please ensure that libvnvclient works with libvncserver when this is enabled. Here's what I did:
  • built your branch
  • added cl->appData.clipboardCap = TRUE; in examples/client/SDLvncviewer.c
  • built
  • started ./examples/server/example
  • connected to it with SDLvncviewer via ./examples/client/SDLvncviewer
  • clipboard is exchanged on window leave/enter -> this crashes the client at the moment

[wuhanck]

@bk138. clipboardCap is not a bool. It is a cap bitset....Maybe current name is descriptive. useclipboardcap is confusing.
I test badc53a, it works for old way(before indicate extend clipboard)
I add "cl->appData.clipboardCap = rfbExtendedClipboard_Text;" before calling rfbInitClient.
here is my log.
22/01/2023 09:44:58 Least significant byte first in each pixel.
22/01/2023 09:44:58 TRUE colour: max red 255 green 255 blue 255, shift red 0 green 8 blue 16
22/01/2023 09:44:58 sending clipboard text 'cl->appData.clipboardCap = rfbExtendedClipboard_Text;'
22/01/2023 09:44:58 ignore SDL event: 0x302
22/01/2023 09:44:58 rfbClientProcessExtServerCutText. default caps.
22/01/2023 09:44:58 rfbClientProcessExtServerCutText. default caps.
22/01/2023 09:44:58 client2server supported messages (bit flags)
22/01/2023 09:44:58 00: 00ff 0081 0000 0000 - 0000 0000 0000 0000
22/01/2023 09:44:58 08: 0000 0000 0000 0000 - 0000 0000 0000 0000
22/01/2023 09:44:58 10: 0000 0000 0000 0000 - 0000 0000 0000 0000
22/01/2023 09:44:58 18: 0000 0000 0000 0000 - 0000 0000 0000 0008
22/01/2023 09:44:58 server2client supported messages (bit flags)
22/01/2023 09:44:58 00: 001f 0080 0000 0000 - 0000 0000 0000 0000
22/01/2023 09:44:58 08: 0000 0000 0000 0000 - 0000 0000 0000 0000
22/01/2023 09:44:58 10: 0000 0000 0000 0000 - 0000 0000 0000 0000
22/01/2023 09:44:58 18: 0000 0000 0000 0000 - 0000 0000 0000 0000
After that, server handle extend clipboard meet some error.
maybe some digging need for that.
It is not client crash, it is that server cannot handle message and close the connection.

if we want to ignore and fallback to old sytle latin-1, we need the logic after 80aacdc.
we need tell the difference of server support or not extend clipboard cap and let the client to choose how to handle it.
so we need add clipboardenabledcap. (it is also a bitset of cap, not a boolean)
(if we just block the old path, we dont need this logic)

[wuhanck]

@bk138 it is simple and solid to just block old path and let the upper layer to setup corret conf.
if we want to be flexible, current logic is good to handle almost all cases of old/new server.
if a server had indicate that it support extended clipboardcap(with text/provide), but it use the old way to send text after that, we just fallback and not handle it nicely. (it is a rare and strange behavior)
We need to change the runtime api, for example add new callback to handle this.
But it changes too much.
let us just stay here with limitations.

[wuhanck]

@bk138 I test sdlvncviewer with tigervnc standalone server.
Although the ui is not correct working, the clipboard part is working use the extended clipboard.
examples/server/client all not ready for extend clipboard, maybe it is not suitable to check with each other.
it is better to use x11vnc/tigervnc server to check

[bk138]

@wuhanck I now understood your point about clipboardCap. If you want to get this merged, please

• use 80aacdc as a starting point
• add a comment in rfbclient.h what the cap is and how to set it, i.e. some kind of API doc
• make the libvncclient implementation work with the libvncserver one. I know that the bug might as well be in the server implementation, but we cannot have a two impls in the project that don't work together.

[wuhanck]

OK. I refine the patch.
For we want be smart at clipboard, we add clipboardEnabledCap for upper to check whether vnc-server support our cap.
For upper who rely on correct configuration, it still can set clipboardCap to rfbExtendedClipboard_Text, and everything will be OK.
For upper who want smart, it set clipboardCap to rfbExtendedClipboard_Text and then in callback to check whether text is supproted by server, if supported, it can use utf-8 to/from os system, if not supported, it can use latin-1 to/from os system.
There is one left issue, if server annouced that it support text, but later, it still send latin-1 using old way, for now we log and ignore it, for we dont know how to handle it and this is not a rational behavior(if server send text clipboard cap, it may better use utf8 text).
And we let it be.

Refine the patch for zlib check.
Add comment for clipboardCap/clipboardEnabledCap.
Add extra byte for some impl to flush data (maybe they dont check Z_STREAM_END), now libvncserver works with libvncclient ( we dont touch libvncserver)

[wuhanck]

@bk138 some commits after f135856 in libvncserver:master break things.
commit bba8f57 in wuhanck:master works.
Please check/review based on bba8f57.

[pdlan]

Please note that according to rfbproto, it is still allowed to use a positive value of the length for the clipboard message (i.e., still use Latin-1) even if the Extended Clipboard Pseudo Encoding is enabled. So I would suggest handling this case.

[wuhanck]

Please note that according to rfbproto, it is still allowed to use a positive value of the length for the clipboard message (i.e., still use Latin-1) even if the Extended Clipboard Pseudo Encoding is enabled. So I would suggest handling this case.

I know server can do this. It is designed to let it improve in future.
Our choice is to let current client to support the utf8 easily.
For client that already support utf8 to/from os, if it choose to rely on conf, it only add one or two lines to support.
For client that want smart, it can check clipboardenabledcap to use latin-1/utf-8 automatically.

The limitation is not global. It is when a server announce clipboardcap on a session, then it use old way to send on this session.
As coder, a server can support utf8 and on a session it annouce the cap, it will change the state to use new way, that is nature. And it send some in old way and send some in new way, it is not easy to code...
Still, let log that behavior and ignore it for now.
In future, we can add new callback, for example add xxx_fallback for client to handle this issue.
But it will break more things, this patch already break some.
let us first make this patch working.

as you known, we still lack lot of features which extended clipboard pseudo encoding support....
we currently only want to talk in text using utf8 and cost as little as we can. we need let things go.

[bk138]

@wuhanck I saw you closed this PR and emptied the branch. I deem this to be a bit sad, as the contribution was almost ready to go in. I can understand that code review and rework can be frustrating at times, but id needs to be done for the sake of quality of the product. As this is all about open-source and voluntary contributions, your decision is OK with me.

I just want to state what I deem mergable (and the PR was close to this TBH):

• ok to only implement a subset of functionality (in this case only text) ✅
• good-enough code quality ✅
• needs to work with other implementations ✅
• needs to work within the project itself (i.e. a libvncclient talking to libvncserver) ⛔

So 3 of 4 ticked in my opinion.

[wuhanck]

Sorry, I am clean repo from github.com… I will restore and reopen a pull request. Sorry about that. Please wait. I am working on that now. 从 Windows 版邮件<https://go.microsoft.com/fwlink/?LinkId=550986>发送 发件人: Christian ***@***.***> 发送时间: 2023年1月24日 22:31 收件人: ***@***.***> 抄送: ***@***.***>; ***@***.***> 主题: Re: [LibVNC/libvncserver] add libvncclient extended clipboard pseudo-encoding support (PR #545) @wuhanck<https://github.com/wuhanck> I saw you closed this PR and emptied the branch. I deem this to be a bit sad, as the contribution was almost ready to go in. I can understand that code review and rework can be frustrating at times, but id needs to be done for the sake of quality of the product. As this is all about open-source and voluntary contributions, your decision is OK with me. I just want to state what I deem mergable (and the PR was close to this TBH): * ok to only implement a subset of functionality (in this case only text) ✅ * good-enough code quality ✅ * needs to work with other implementations ✅ * needs to work within the project itself (i.e. a libvncclient talking to libvncserver) ⛔ So 3 of 4 ticked in my opinion. — Reply to this email directly, view it on GitHub<#545 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAQSUC77HWKTK6W5OI2VQNTWT7RSVANCNFSM6AAAAAATT4TVXI>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[wuhanck]

I reopen the pull request in a new branch. I add support for fallback for some guy seems need it. Libvncserver don’t check Z_STREAM_END，so I add extra byte to make it happy. It would harm others as far as I can see. And I found In the master, after commit f135856, some commit break things. Maybe bug or api broken, I don’t know. Please recheck them. 从 Windows 版邮件<https://go.microsoft.com/fwlink/?LinkId=550986>发送 发件人: Christian ***@***.***> 发送时间: 2023年1月24日 22:31 收件人: ***@***.***> 抄送: ***@***.***>; ***@***.***> 主题: Re: [LibVNC/libvncserver] add libvncclient extended clipboard pseudo-encoding support (PR #545) @wuhanck<https://github.com/wuhanck> I saw you closed this PR and emptied the branch. I deem this to be a bit sad, as the contribution was almost ready to go in. I can understand that code review and rework can be frustrating at times, but id needs to be done for the sake of quality of the product. As this is all about open-source and voluntary contributions, your decision is OK with me. I just want to state what I deem mergable (and the PR was close to this TBH): * ok to only implement a subset of functionality (in this case only text) ✅ * good-enough code quality ✅ * needs to work with other implementations ✅ * needs to work within the project itself (i.e. a libvncclient talking to libvncserver) ⛔ So 3 of 4 ticked in my opinion. — Reply to this email directly, view it on GitHub<#545 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAQSUC77HWKTK6W5OI2VQNTWT7RSVANCNFSM6AAAAAATT4TVXI>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[wuhanck]

从 Windows 版邮件<https://go.microsoft.com/fwlink/?LinkId=550986>发送 发件人: Christian ***@***.***> 发送时间: 2023年1月24日 22:31 收件人: ***@***.***> 抄送: ***@***.***>; ***@***.***> 主题: Re: [LibVNC/libvncserver] add libvncclient extended clipboard pseudo-encoding support (PR #545) @wuhanck<https://github.com/wuhanck> I saw you closed this PR and emptied the branch. I deem this to be a bit sad, as the contribution was almost ready to go in. I can understand that code review and rework can be frustrating at times, but id needs to be done for the sake of quality of the product. As this is all about open-source and voluntary contributions, your decision is OK with me. I just want to state what I deem mergable (and the PR was close to this TBH): * ok to only implement a subset of functionality (in this case only text) ✅ * good-enough code quality ✅ * needs to work with other implementations ✅ * needs to work within the project itself (i.e. a libvncclient talking to libvncserver) ⛔ So 3 of 4 ticked in my opinion. — Reply to this email directly, view it on GitHub<#545 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAQSUC77HWKTK6W5OI2VQNTWT7RSVANCNFSM6AAAAAATT4TVXI>. You are receiving this because you were mentioned.Message ID: ***@***.***>