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

re-implement clipboard without nested main #812

Closed
totaam opened this issue Feb 16, 2015 · 44 comments
Closed

re-implement clipboard without nested main #812

totaam opened this issue Feb 16, 2015 · 44 comments

Comments

@totaam
Copy link
Collaborator

totaam commented Feb 16, 2015

As it is, it is just too problematic: too many bugs to list, including some unresolved ones: #669, #452 (it is also causing hangs with #598). See also #2172, #2138

We should be able to rip it out and just use plain X11 calls (see ICCCM section 2: Peer-to-Peer Communication by Means of Selections), hopefully GTK won't get too confused by this...

We can keep the existing code, at least client side, for the non-X11 platforms.

Link: x-cut-and-paste

@totaam
Copy link
Collaborator Author

totaam commented Feb 24, 2015

2015-02-24 03:40:35: antoine commented


Another reason for doing this, just hit this today (not sure how):

RuntimeError: maximum recursion depth exceeded
Traceback (most recent call last):
  File "/usr/lib64/python2.7/site-packages/xpra/clipboard/clipboard_base.py", line 167, in _get_clipboard_from_remote_handler
Error in sys.excepthook:
Traceback (most recent call last):
  File "/usr/lib64/python2.7/site-packages/xpra/gtk_common/quit.py", line 67, in gtk_main_quit_on_fatal_exception
    print(traceback.print_exception(etype, val, tb))
  File "/usr/lib64/python2.7/traceback.py", line 125, in print_exception
    print_tb(tb, limit, file)
  File "/usr/lib64/python2.7/traceback.py", line 69, in print_tb
    line = linecache.getline(filename, lineno, f.f_globals)
  File "/usr/lib64/python2.7/linecache.py", line 14, in getline
    lines = getlines(filename, module_globals)
  File "/usr/lib64/python2.7/linecache.py", line 40, in getlines
    return updatecache(filename, module_globals)
RuntimeError: maximum recursion depth exceeded

Original exception was:
Traceback (most recent call last):
  File "/usr/lib64/python2.7/site-packages/xpra/clipboard/clipboard_base.py", line 167, in _get_clipboard_from_remote_handler
    log("get clipboard from remote handler id=%s", request_id)
RuntimeError: maximum recursion depth exceeded

@totaam
Copy link
Collaborator Author

totaam commented Jun 28, 2015

API links:

@totaam
Copy link
Collaborator Author

totaam commented Oct 23, 2015

2015-10-23 07:50:46: antoine commented


Re-scheduling due to lack of time.

Another item worth considering during the re-write would be to make all clipboards "greedy" like the win32 and osx ones, or at least having the option of doing that.

#1018 is a duplicate of this ticket.

@totaam
Copy link
Collaborator Author

totaam commented Feb 15, 2017

2017-02-15 12:42:09: antoine commented


Likely to be quite hard.

@totaam
Copy link
Collaborator Author

totaam commented Jul 20, 2017

2017-07-20 17:43:47: antoine commented


See also: #1589 GTK3 clipboard support

@totaam
Copy link
Collaborator Author

totaam commented Nov 4, 2018

2018-11-04 13:39:50: antoine commented


This should help fix bugs like #2025 and #2172

@totaam
Copy link
Collaborator Author

totaam commented Mar 12, 2019

2019-03-12 04:26:29: antoine commented


More problems (and yet more reported on IRC): #2138, see also #1844.

@totaam
Copy link
Collaborator Author

totaam commented Mar 22, 2019

2019-03-22 14:59:06: antoine commented


Preparatory steps added in r22212.

We can't completely do without GTK because the hooks for X11 events are in the gtk main loop filter, and the events are dispatched as gobject signals.
But at least it should be easier to move away from GTK at some point.

And we also need to use the xfixes API, because it's better.
See also #1494

@totaam
Copy link
Collaborator Author

totaam commented Mar 24, 2019

2019-03-24 13:11:43: antoine commented


Added:

The toy clipboard class can now send a receive clipboard data.

@totaam
Copy link
Collaborator Author

totaam commented Mar 27, 2019

2019-03-27 14:10:18: antoine commented


Partial merge:

  • r22217 add time to property events
  • r22227 move nesting check so we can skip it
  • r22228 cosmetic
  • r22229 undo some of the really ugly original code from 2009! just use native struct formats directly rather than transforming them in the bindings
  • r22230 split clipboard base class so we can re-use the higher level logic without the low-level gtk bits

TODO:

  • rename private fields so the helper can access them without triggering warnings, ie: _can_send
  • handle COMPOUND_TEXT?
  • move code to proxy: local_to_remote?
  • XGetWindowProperty needs to handle large data (continue) - HARD!
  • do_owner_changed() should fire a new token every time? for greedy clients only?
  • emit_token needs to get TARGETS and the data before sending
  • we use the TARGET as part of the property name and maybe we shouldn't: in some cases the property name ends up looking like this: PRIMARY-text/plain;charset=utf-8 - is this always going to be a valid property name? could this be abused?
  • the same request can timeout in two places: the remote request timeout, remotely when the remote client takes too long. Both can trigger warning and delete the request_id..

@totaam
Copy link
Collaborator Author

totaam commented Mar 28, 2019

2019-03-28 09:28:12: antoine commented


Updates:

It works surprisingly well and does not ever lock the main thread!

Still TODO:

  • macos and win32 clipboard code so we can drop the GTK clipboard completely (new ticket? this may help: Writing a cross-platform clipboard library)
  • COMPOUND_TEXT and other targets: let clients specify blacklist?
  • XGetWindowProperty: maybe pass the maximum buffer size (and remove icon hack)
  • token needs data for greedy clients
  • handle timeouts
  • handle win32 token state mismatch issue
    etc

@totaam
Copy link
Collaborator Author

totaam commented Apr 11, 2019

2019-04-11 13:26:20: antoine commented


Updates:

  • r22364 python3 fix
  • r22368 XGetWindowProperty fixed (4MB limit for clipboard transfers)
  • r22375: many improvements (see commit message)

Still TODO:

  • greedy clients and early sending of targets and target data
  • win32 and macos

@totaam
Copy link
Collaborator Author

totaam commented Apr 12, 2019

2019-04-12 16:36:54: @totaam commented


win32 updates:

  • r22378 refactoring
  • r22380 add more clipboard ctypes function definitions

@totaam
Copy link
Collaborator Author

totaam commented Apr 12, 2019

2019-04-12 16:37:20: @totaam uploaded file clipboard-win32.patch (8.7 KiB)

win32 native clipboard work in progress

@totaam
Copy link
Collaborator Author

totaam commented Apr 14, 2019

2019-04-14 17:19:13: @totaam commented


updates:

  • r22401 handles "greedy" clients (ie: win32 and macos)
  • r22404 win32 clipboard implementation (disabled for now)

@totaam
Copy link
Collaborator Author

totaam commented Apr 18, 2019

2019-04-18 13:00:09: antoine commented


Updates:

  • r22466 debug logging
  • r22467 null terminate strings on win32
  • r22468 use new win32 backend by default
  • r22427 better error logging for atom errors

@totaam
Copy link
Collaborator Author

totaam commented Apr 20, 2019

2019-04-20 11:09:38: antoine commented


Updates:

  • r22470 add blacklist for clipit, just send empty reply
  • r22475 don't bother sending events for selections that are not enabled
  • r22476 + r22477: refactoring of translated clipboards (osx and win32)
  • r22478: osx fix
  • r22481 improve win32 clipboard: handle greedy clients, block-owner-change

We do have to deal with the macos clipboard to be able to cleanup the codebase and remove the legacy gdk clipboard classes.

@totaam
Copy link
Collaborator Author

totaam commented Apr 21, 2019

2019-04-21 19:01:10: antoine commented


Caused a regression: #2280.

@totaam
Copy link
Collaborator Author

totaam commented Apr 22, 2019

2019-04-22 05:27:41: antoine commented


#2280 fixed in r22501, we don't release the selection on exit to avoid this GTK3 crash.
Ideally, we should find a way to release the selection without crashing GTK3 but this will do for now.

@totaam
Copy link
Collaborator Author

totaam commented Apr 22, 2019

2019-04-22 16:06:25: antoine commented


Updates:

Still TODO:

@totaam
Copy link
Collaborator Author

totaam commented Apr 30, 2019

2019-04-30 13:23:44: antoine changed status from assigned to new

@totaam
Copy link
Collaborator Author

totaam commented Apr 30, 2019

2019-04-30 13:23:44: antoine changed owner from antoine to encodedEntropy

@totaam
Copy link
Collaborator Author

totaam commented Apr 30, 2019

2019-04-30 13:23:44: antoine commented


Updates:

  • translated clipboard fixes in r22569
  • fix token warnings in r22570
  • backwards compatibility verified with 2.5.x clients

This will do for this ticket, will follow up in #273.

@encodedEntropy: this really needs testing, ideally before #1844 so if anything is broken then we will know if it's here or there.

@totaam
Copy link
Collaborator Author

totaam commented May 31, 2019

2019-05-31 13:43:52: antoine changed status from new to assigned

@totaam
Copy link
Collaborator Author

totaam commented May 31, 2019

2019-05-31 13:43:52: antoine changed owner from encodedEntropy to antoine

@totaam
Copy link
Collaborator Author

totaam commented May 31, 2019

2019-05-31 13:43:52: antoine commented


Something's not right.
When copying an image from eog on the server (testing for #1494):

do_owner_changed()
_send_clipboard_token_handler(X11ClipboardProxy(CLIPBOARD), ())

requesting local XConvertSelection from 'Image Viewer' for 'TARGETS' into 'CLIPBOARD-TARGETS'
client @25.162 got token, selection=CLIPBOARD, targets=None, target data=None, claim=True, can-receive=True
_send_clipboard_token_handler(X11ClipboardProxy(CLIPBOARD), (('TIMESTAMP', 'TARGETS', 'MULTIPLE', 'image/png', 'image/bmp', 'image/x-bmp', 'image/x-MS-bmp', 'image/x-icon', 'image/x-ico', 'image/x

Why do we request the targets and send a clipboard-token to the client without them? Do one or the other!

Then immediately, gedit and Terminal are being greedy and requesting the targets:

client @25.164 clipboard request for CLIPBOARD from window 0x3400125: 'gedit'
client @25.166 clipboard request for CLIPBOARD from window 0x2400002: 'Terminal'

We get the targets again:

proxy_got_contents(6, CLIPBOARD, TARGETS, ATOM, 32, <class 'bytes'>:192) data=0xa001...

And send them to the client:

client @25.167 got token, selection=CLIPBOARD, targets=(b'TIMESTAMP', b'TARGETS', b'MULTIPLE', b'image/png', b'image/bmp', b'image/x-bmp', b'image/x-MS-bmp', b'image/x-icon', b'image/x-ico', b'image/x-win-bitmap', b'image/vnd.microsoft.icon', b'application/ico', b'image/ico', b'image/icon', b'text/ico', b'image/jpeg', b'image/tiff', b'UTF8_STRING', b'COMPOUND_TEXT', b'TEXT', b'STRING', b'text/plain;charset=utf-8', b'text/plain', b'text/uri-list'), target data=None, claim=True, can-receive=True

We should filter the image/FORMAT list to only contain formats that we can process through pillow.

We tell both gedit and Terminal about the targets:

client @25.168 setting response b'\x99\x01\x00\x00.. ..\x00\x00' to property GDK_SELECTION of window 'gedit' as ATOM
client @25.168 setting response b'\x99\x01\x00\x00.. ..\x00\x00' to property GDK_SELECTION of window 'Terminal' as ATOM

Probably because we claim the clipboard again when handling this token, they request the targets again, and we respond with the cached value:

client @25.171 clipboard request for CLIPBOARD from window 0x3400125: 'gedit'
client @25.171 using existing TARGETS value as response
client @25.172 clipboard request for CLIPBOARD from window 0x2400002: 'Terminal'
client @25.172 using existing TARGETS value as response

When trying to paste it into the gimp, it requests the data in image/png format:

client @19.259 clipboard request for CLIPBOARD from window 0x42029cd: 'GNU Image Manipulation Program'
client @19.259 using existing TARGETS value as response
client @19.264 clipboard request for CLIPBOARD from window 0x42029cd: 'GNU Image Manipulation Program'
client @19.264 send_clipboard_request_handler(X11ClipboardProxy(CLIPBOARD), 'CLIPBOARD', 'image/png')

The server tries to honour it:

requesting local XConvertSelection from 'Image Viewer' for 'image/png' into 'CLIPBOARD-image/png'
proxy_got_contents(1, CLIPBOARD, image/png, INCR, 32, <class 'bytes'>:8) data=0x470b040000000000..

This fails:

xpra.x11.bindings.window_bindings.BadPropertyType: incomplete data: 196608 bytes after

Because we don't handle INCR properties.

@totaam
Copy link
Collaborator Author

totaam commented Jun 1, 2019

2019-06-01 06:43:55: antoine commented


r22824 adds support for incremental transfers IN. (+minor fixup in r22825)

Still TODO:

  • incremental transfers OUT? (meh - works without)
  • from comment:26, "why do we request the targets and send a clipboard-token to the client without them? Do one or the other!"
  • we should not cache the target data for so long - I've seen it get wedged with outdated buffers
  • targets filters: allow whitelisted mimetypes?
  • poc image filter

@totaam
Copy link
Collaborator Author

totaam commented Jun 1, 2019

2019-06-01 07:43:08: antoine commented


added PoC overlay code in r22826 from both images and timestamp, only for png images for now.

@totaam
Copy link
Collaborator Author

totaam commented Jun 1, 2019

2019-06-01 12:50:05: antoine commented


Supporting this in the html5 client is going to be difficult, #1844 may help.
See:

  • chromium bug: Support programmatical copying of images to clipboard: The CL enables a feature that has been available under the "Enable experimental Web Platform features" flag since M75, so we think that the code is solid and there are no crashers lurking in there. (updated today!)
  • Unblocking Clipboard Access: There are more generic read() and write() methods in the specification, but these come with additional implementation complexity and security concerns (remember those image bombs?). For now, Chrome is rolling out the simpler text parts of the API. - we can deal with the image bombs by using the pillow image filter
  • Clipboard API and events : Async Clipboard API: An image that is specially crafted to exploit bugs in the core OS image handling code can be written to the clipboard (same)

@totaam
Copy link
Collaborator Author

totaam commented Jun 3, 2019

2019-06-03 05:01:07: antoine commented


Moving html5 to #2312.

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

No branches or pull requests

1 participant