Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Large clipboard pastes fail #1839

Closed
vscfreire opened this issue Mar 23, 2021 · 4 comments · Fixed by #2810
Closed

Large clipboard pastes fail #1839

vscfreire opened this issue Mar 23, 2021 · 4 comments · Fixed by #2810
Labels
clipboard confirmed confirmed reproduction

Comments

@vscfreire
Copy link

vscfreire commented Mar 23, 2021

When copying a large amount of text containing non-ascii characters locally, and then trying to paste it on the remote linux session, the paste operation will output a single character instead of the clipboard content. Since reproducing this issue requires a large amount of text, I won't include it here, but essentially copying the beginning section of Mary Shelley's Frankenstein (beginning of Letter 1 until the end of Letter 4) reproduces it consistently for me.

I found a commit from 2012 (60322a3) that introduced code meant to split clipboard data passing between client and server in small chunks when data size was larger than 32KB. This caused some long clipboard data text blocks to be interpreted as "COMPOUND_TEXT" instead of "UTF8_STRING" by the X server. I tried handling "COMPOUNT_TEXT" correctly, but some multi-byte Unicode characters were being mangled.

I eventually just disabled the splitting code in clipboard_data_in() (the g_clip_c2s.doing_response_ss and g_clip_c2s.in_request ifs) and it worked. I don't know if there's been any change to the RDP protocol since 2012, but every client I tried (native Microsoft RDP client and FreeRDP client) already seems to split large bodies of text into chunks, so handling it on the server-side no longer seems necessary.

Let me know if you want me to provide more information or run some more tests.

@matt335672
Copy link
Member

Hi @vscfreire

This is really great detective work - thanks.

I'll add for the casual reader that the 'letters' referred to above are distinct blocks of text rather than letters in the sense of characters. That confused me on my first reading.

In order to prevent any variations caused by browser, I've extracted the section you suggest above as frankenstein.txt which is 31357 characters long and according to the file command is "UTF-8 Unicode text, with very long lines, with CRLF line terminators".

  • If I open that in Windows 10 notepad, hit CTRL-A, CTRL-C and paste it across the link, I get a single upper-case 'L', as far as I can tell. This is independent of the back-end I use.
  • If I restrict myself to letters 1-3 which is roughly half the length, all seems to work OK.

I've not gone into the rest of the information you provide, but it seems pretty comprehensive.

@matt335672 matt335672 added the confirmed confirmed reproduction label Mar 24, 2021
@vscfreire
Copy link
Author

Hi @matt335672, sorry for the confusion, and thanks for providing the section so that it's easier to reproduce.

So far, since disabling that code, I haven't noticed any problems. Let me know if you need any further clarification.

@matt335672
Copy link
Member

@vscfreire - sorry about the delay.

After some investigation I've proposed that this code is removed. There might be a good reason why the code is present, so a confirmation that the removal is a goos idea would be good.

Are you still running without this code? If so have you noticed any issues?

@vscfreire
Copy link
Author

Hi @matt335672, thanks for getting back to me.

Yes, I've been running without this code in the latest versions of xrdp, since this ticket was submitted. No issues so far.

I took a look at the pull request, and it's essentially the same change I did.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clipboard confirmed confirmed reproduction
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants