LibVNC / libvncserver

LibVNCServer/LibVNCClient are cross-platform C libraries that allow you to easily implement VNC server or client functionality in your program.
GNU General Public License v2.0
1.08k stars 483 forks source link

Libvncclient clipboard utf8 support #552

Closed wuhanck closed 1 year ago

wuhanck commented 1 year ago

It is a limited support of extended clipboard psedudo encoding. It only support text/provide(notify as needed). It is to support utf8 clipboard and cost as little as we can. Full extended clipboard pseudo encoding list here: https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#extended-clipboard-pseudo-encoding

This patch add clipboardCap and ClipboardEnabledCap in rfbclient.appData. They both are bitset as spec.

If upper rely on corrent configuration, it can set rfbclient.appData.clipboardCap = rfbExtendedClipboard_Text, and then all will work, utf8 can in/out through client-cut-text.

If upper want smart behavier, it can set rfbclient.appData.clipboardCap = rfbExtendedClipboard_Text, and check the ClipboardEnabledCap whether server support clipboardCap, if server support, it can use utf8 to/from, if server dont support, it can use latin-1 to/from.

When server announced clipboard support on a session, but after that, sometime it still send latin-1 in old way. The default behavior is to log it and ignore. This patch add alse add GotXCutTextFallback callback in rfbClient If upper want to handle this, upper can setup GotXCutTextFallback, it will always called with latin-1 in old way.

wuhanck commented 1 year ago

The extra byte to make libvncserver happy(it dont check Z_STREAM_END) would not harm others as far as I see.

bk138 commented 1 year ago

thanks @wuhanck, I'll have a look ASAP

wuhanck commented 1 year ago

Sorry for delay your job. Github's clean commit, I thought it was clean merge commits(some of them are annoying), but it is some kind of 'reset --hard'. Some old implementation had free the memory by itself, maybe we need some explantation when fix such issue(it cause exsting code break with double free). I dont know where to mention this. So I put here.

wuhanck commented 1 year ago

@bk138, I scan the fed2c8b. there is two independent changes.

  1. remove the client's ability to announce if it is support utf8. It is always announce it support utf8. Some old client or limited client do not support utf8. These change will break them. Not only ABI, even if they recompile the code, things changed. I think at lease the code need be recompiled with no surprise behavior change.

  2. Add two API for utf8text. I also see the modify of example If both introduce first change, you will see that change of on old send-text,: if enable_utf8: target_to_uft8_data on old recv-text: if enable_utf8: utf8_data_to_target That is much clear and easy to understand than add two API.

And these API allow user to choose at freedom which way it used, these will introduce more complex. Really, user don't choose the way they send text but they abbey simple rule to encode/decode there content. this way is much more clear.

And for 1 change, client who don't have utf8 support will be hard to conform to SPEC. A server who agree on extclipboard have right to always send utf8....which is not support by client(not the sending way but the utf8 coding).

wuhanck commented 1 year ago

@bk138 And if apply 1 change. And see the example. Use the way that don't add two API, there is no need to change the example at all. And example will work the same as you modified version. Please re-think the whole scene. It may look good (or look straight to add two API), but really we don't harvest on that.

bk138 commented 1 year ago

@bk138, I scan the fed2c8b. there is two independent changes.

1. remove the client's ability to announce if it is support utf8. It is always announce it support utf8.
   Some old client or limited client do not support utf8.
   These change will break them. Not only ABI, even if they recompile the code, things changed.
   I think at lease the code need be recompiled with no surprise behavior change.

You're right, taken care of in 28b72437.

wuhanck commented 1 year ago

@bk138 28b7243 make things more crazy...... If we need to support ext-clipboard to for example cut/paste image, how can we treat these code? My english is not good. let me explain good/bad from beginning.

The SPEC choose to re-use the cut-text-message (not device a new one) this decision give a big hole for all of us. For cut/paste text, that means we must separate thing into two part

  1. How the text is sent, old way or new way
  2. What's the text encoding.

It is not good to device a API such as send-utf8-using-new-way, for the new way may kill the old server. It is better to let the lib to choose the correct sending-way.

Let us check your modification of the sample. (The text coding is not concerned here and there is no code modification for text coding.) All your modification is to let sample code to choose the correct sending-way... And all your modification is just let client use new way if both agree on ext-clipboard. No more than that. So why not just let this behavior to be fixed in the lib? And let user can just follow this way? That is what I said, in the original patch, there is no need to modify the code (if we apply change 1).

And for future extension, client need write fallback code for sendutf8/sendlatin1 to make it correct work; but client don't need write fallback for sendimage....that is also strange.

Really, original patch is more extensible and easy for old code to evolve.

  1. the lib choose the correct way to send
  2. the user choose correct content to send and it always use one flag to check

The modified patch

1.the user need choose the correct way to send and it can do wrong. 2.the use choose correct content to send and it will need the result of its fallback code to check

please re-think it as a whole. intuitive or straight design is not always working. this is the case.

bk138 commented 1 year ago

OK, I'll put in on my agenda, but it'll take a few days.

wuhanck commented 1 year ago

@bk138 the difficulty is introduced by the SPEC (choose re-use the old message type) and history(big core data structure as ABI and client do check internal state and even some client do free internal resource because lib don't.....). Thanks for your patient.