#2072 closed defect (fixed)
Restricting the amount of clipboard data copying from the server clipboard in/out of the client
Reported by: | Calvin Ko | Owned by: | J. Max Mena |
---|---|---|---|
Priority: | major | Milestone: | 2.5 |
Component: | android | Version: | 2.4.x |
Keywords: | Cc: |
Description
The goal is to restrict the amount of data being transferred from server in the client (or vice versa - out) through clipboard copy/paste operation.
Two options specifying the max size
- max_clipboard_copyin_size (into client - raw to wire)
- max_clipboard_copyout_size (out from client - wire to raw)
In addition, notify the client when the data is truncated.
Attachments (7)
Change History (28)
comment:1 Changed 2 years ago by
Owner: | changed from Antoine Martin to Calvin Ko |
---|
Changed 2 years ago by
Attachment: | clipboard.patch added |
---|
comment:3 Changed 2 years ago by
The updated patch changes:
- makes the default send and receive limits configurable via env vars
- indentation tweaks
- simplify max datalen calculation and make it python3 friendly: use integer division
@calvinko: this modifies a private clipboard method, please include the changes that call it.
Changed 2 years ago by
Attachment: | clipboard-size-v2.patch added |
---|
patch with control command to set the size limits
comment:4 Changed 2 years ago by
The update patch - clipboard-size-v2.patch
- add the private function set_limits in clipboard proxy for changing the limits
- add the control command clipboard_limits for changing the limits
comment:5 Changed 2 years ago by
OK, some minor changes are still needed:
- it should be possible to go back to the default values (-1 and -1), so use None default values for
max_send_size
andmax_receive_size
inset_direction
and check for None inset_limits
- then you can just doxpra control :100 clipboard-limits -1 -1
- have you tested the control command? unless I am missing something, without input validation the control values will be strings and then this breaks badly, in python2 the size check fails silently (removing the restrictions):
> 3 > "1" False
with python3 you probably get an error in the clipboard code on every request (which may be worse as this is known to cause GTK to hang)
- in
control_command_clipboard_limits
, you alias the clipboard helper to a local variable (good), but then you don't use it - the
setting_changed
method takes native values, not strings, either send a pair of values or a dictionary
Changed 2 years ago by
Attachment: | clipboard-size-v3.patch added |
---|
address the minor changes stated in comment:5
comment:6 Changed 2 years ago by
As per comment:5:
use None default values for max_send_size and max_receive_size in set_direction
Otherwise regular clipboard clients, which call set_direction
without the extra arguments will end up removing the size restrictions.
IMO max_send
in setting_changed
is redundant and should be called send
comment:7 Changed 2 years ago by
Did you test this? Ideally this feature should be added to the unit tests.
I found at least two major problems during my quick testing:
- r21202 fixes a long standing bug that is uncovered by the extra "truncated" attribute in the "clipboard-contents" packets - do you really need this? (it will create backwards compatibility issues with older versions - as some users just don't update as often as they should..)
- the checks assume that a limit is always set:
2018-12-11 13:56:49,818 Data copied out truncated because of clipboard policy 1 to -1
And this ends up truncating all clipboard packets by 1 character.
How I tested:
- xpra start then xpra info:
xpra start :100
$ xpra info | grep clipboard | grep max_ clipboard.max_recv_size=-1 clipboard.max_send_size=-1 clipboard.max_size=4194304
- change it via control channel
$ xpra control :13 clipboard-limits 8 8 clipboard send limit set to 8, recv limit set to 8 (single copy/paste)
$ xpra info | grep clipboard | grep max_ clipboard.max_recv_size=8 clipboard.max_send_size=8 clipboard.max_size=4194304
- same with env vars:
XPRA_MAX_CLIPBOARD_RECEIVE_SIZE=10 XPRA_MAX_CLIPBOARD_SEND_SIZE=10 xpra start
$ xpra info | grep clipboard | grep max_ clipboard.max_recv_size=10 clipboard.max_send_size=10 clipboard.max_size=4194304
Then check that the client does receive a truncated string:
- from the server:
echo hello12345 | xclip -i -selection CLIPBOARD
- on the client:
xclip -o -selection CLIPBOARD
Then the other way (client to server) - preferably after changing the limits to ensure that send and receive limits are used the right way around.
Also worth testing: client limits via env vars.
comment:8 Changed 2 years ago by
r21202 fixes a long standing bug that is uncovered by the extra "truncated" >attribute in the "clipboard-contents" packets - do you really need this? (it will create backwards compatibility issues with older versions - as some users just don't update as often as they should..)
You are right. I am actually thinking to add a clipboard-content-truncated message instead of changing the clipboard-content packets. All I need is to notify the client in some way. What do you think?
comment:9 Changed 2 years ago by
I have not test the code since I don't have an environment set up to test xpra.
Lets ask Max to help testing.
comment:10 Changed 2 years ago by
I am actually thinking to add a clipboard-content-truncated message instead of changing the clipboard-content packets. All I need is to notify the client in some way. What do you think?
If you need to notify the client, then I would rather not add a new packet type.
We can workaround older clients by adding a new capability flag in the hello packet - something like clipboard-contents-slice-fix=true
.
comment:11 Changed 2 years ago by
clipboard-size-v5.patch
- check whether limits are set (only truncated when limits are set, i.e., not -1)
- add clipboard.contents_slice_fix capability for backward compatibility. New client should set this to False.
comment:12 Changed 2 years ago by
Owner: | changed from Calvin Ko to J. Max Mena |
---|
comment:14 Changed 2 years ago by
Merged in r21213 with the following changes:
- added
get_format_size
function to ensure we return the correct size for the given data format (becauseCARD32
can actually be 64-bit...) - renamed capability to
clipboard.contents-slice-fix
, capabilities areFalse
by default - expose the capability from both server and client, and call
set_clipboard_contents_slice_fix
from both too - log an informational message if the other end is missing that clipboard fix
- catch "clipboard-limits" setting change in client to avoid warning
Backports:
comment:15 Changed 2 years ago by
Fired up a trunk r21225 server, and followed the steps for using the control channel - primarily xpra control :DISPLAY clipboard-limits 8 8
and it broke the server side clipboard. I could copy text from my client into the server clipboard, but could not copy anything on the server. If I did, the clipboard would be empty, and I couldn't paste anything client side.
I ran a session with -d clipboard
demonstrating what I saw. My repro steps are pretty easy:
- Start a server with
xfce4-terminal
or some terminal emulator with a Copy/Paste? menu
- Run
xpra control :DISPLAY clipboard-limits 8 8
- Copy some text from the client into the server
- Type something into the terminal, highlight it, and try to paste it into a client application (I just used kwrite)
I tried that on three different sessions, and it failed each time. I'll work on using the Environment variables and using xclip
.
comment:16 Changed 2 years ago by
I tried that on three different sessions, and it failed each time.
Failed how? Wrong data? No data?
Do you have logs?
comment:17 Changed 2 years ago by
When I copy from the client to server, I get as much text as I should within the limits - 8 characters. But when I highlight text on the server, copy it, and then paste it client-side, there's no data.
I just installed xclip
, and I restarted my server and now it's working?
comment:18 Changed 2 years ago by
I just installed xclip, and I restarted my server and now it's working?
What steps?
The ones from comment:7 do use xclip so it should not come as a surprise that you need to have it installed.
comment:19 Changed 2 years ago by
Always test with the most basic tools, like xclip.
Do not trust applications to do the right thing with the clipboard.
comment:20 Changed 2 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
Noted.
Both the control channel and the environment variables work with xclip
, so as far as I can tell there isn't anything else to do.
I'm gonna go ahead and close this, unless there's any follow-ups.
comment:21 Changed 3 months ago by
this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/2072
Please attach the patch.