From 6a65341e14bc7745c52fe86fe53be08dbd682dd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Camilla=20L=C3=B6wy?= Date: Thu, 16 Mar 2017 23:07:59 +0100 Subject: [PATCH] X11: Fix multiple issues in XDND support The code blindly expected UTF8_STRING for files. It did not downgrade based on source protocol version. It did not handle hostnames in text/uri-list data. It did not specify the source time stamp when converting the selection. It did not search the XdndTypeList when necessary. It did not ignore sources that specified invalid versions. While better, this is still not fully conformant. Hostnames are not validated and it does not guard against source crashes. Fixes #968. --- README.md | 1 + src/x11_init.c | 3 +- src/x11_platform.h | 5 +- src/x11_window.c | 159 ++++++++++++++++++++++++++++++++++----------- 4 files changed, 129 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index 939aba18..76b384a3 100644 --- a/README.md +++ b/README.md @@ -171,6 +171,7 @@ information on what to include when reporting a bug. - [X11] Bugfix: `glfwGetVideoMode` would segfault on Cygwin/X - [X11] Bugfix: Dynamic X11 library loading did not use full sonames (#941) - [X11] Bugfix: Window creation on 64-bit would read past top of stack (#951) +- [X11] Bugfix: XDND support had multiple non-conformance issues (#968) - [Linux] Bugfix: Event processing did not detect joystick disconnection (#932) - [Cocoa] Added support for Vulkan window surface creation via [MoltenVK](https://moltengl.com/moltenvk/) (#870) diff --git a/src/x11_init.c b/src/x11_init.c index 22600c96..734c89cc 100644 --- a/src/x11_init.c +++ b/src/x11_init.c @@ -629,9 +629,10 @@ static GLFWbool initExtensions(void) _glfw.x11.XdndStatus = XInternAtom(_glfw.x11.display, "XdndStatus", False); _glfw.x11.XdndActionCopy = XInternAtom(_glfw.x11.display, "XdndActionCopy", False); _glfw.x11.XdndDrop = XInternAtom(_glfw.x11.display, "XdndDrop", False); - _glfw.x11.XdndLeave = XInternAtom(_glfw.x11.display, "XdndLeave", False); _glfw.x11.XdndFinished = XInternAtom(_glfw.x11.display, "XdndFinished", False); _glfw.x11.XdndSelection = XInternAtom(_glfw.x11.display, "XdndSelection", False); + _glfw.x11.XdndTypeList = XInternAtom(_glfw.x11.display, "XdndTypeList", False); + _glfw.x11.text_uri_list = XInternAtom(_glfw.x11.display, "text/uri-list", False); // ICCCM, EWMH and Motif window property atoms // These can be set safely even without WM support diff --git a/src/x11_platform.h b/src/x11_platform.h index b2e4f5de..4ca5a1da 100644 --- a/src/x11_platform.h +++ b/src/x11_platform.h @@ -208,9 +208,10 @@ typedef struct _GLFWlibraryX11 Atom XdndStatus; Atom XdndActionCopy; Atom XdndDrop; - Atom XdndLeave; Atom XdndFinished; Atom XdndSelection; + Atom XdndTypeList; + Atom text_uri_list; // Selection (clipboard) atoms Atom TARGETS; @@ -253,7 +254,9 @@ typedef struct _GLFWlibraryX11 } saver; struct { + int version; Window source; + Atom format; } xdnd; struct { diff --git a/src/x11_window.c b/src/x11_window.c index cc8ffd97..327bd750 100644 --- a/src/x11_window.c +++ b/src/x11_window.c @@ -48,6 +48,8 @@ #define Button6 6 #define Button7 7 +#define _GLFW_XDND_VERSION 5 + // Wait for data to arrive using select // This avoids blocking other threads via the per-display Xlib lock that also @@ -415,7 +417,12 @@ static char** parseUriList(char* text, int* count) continue; if (strncmp(line, prefix, strlen(prefix)) == 0) + { line += strlen(prefix); + // TODO: Validate hostname + while (*line != '/') + line++; + } (*count)++; @@ -619,10 +626,9 @@ static GLFWbool createNativeWindow(_GLFWwindow* window, XFree(hint); } - if (_glfw.x11.XdndAware) + // Announce support for Xdnd (drag and drop) { - // Announce support for Xdnd (drag and drop) - const Atom version = 5; + const Atom version = _GLFW_XDND_VERSION; XChangeProperty(_glfw.x11.display, window->x11.handle, _glfw.x11.XdndAware, XA_ATOM, 32, PropModeReplace, (unsigned char*) &version, 1); @@ -1307,44 +1313,121 @@ static void processEvent(XEvent *event) else if (event->xclient.message_type == _glfw.x11.XdndEnter) { // A drag operation has entered the window - // TODO: Check if UTF-8 string is supported by the source + unsigned long i, count; + Atom* formats = NULL; + const GLFWbool list = event->xclient.data.l[1] & 1; + + _glfw.x11.xdnd.source = event->xclient.data.l[0]; + _glfw.x11.xdnd.version = event->xclient.data.l[1] >> 24; + _glfw.x11.xdnd.format = None; + + if (_glfw.x11.xdnd.version > _GLFW_XDND_VERSION) + return; + + if (list) + { + count = _glfwGetWindowPropertyX11(_glfw.x11.xdnd.source, + _glfw.x11.XdndTypeList, + XA_ATOM, + (unsigned char**) &formats); + } + else + { + count = 3; + formats = (Atom*) event->xclient.data.l + 2; + } + + for (i = 0; i < count; i++) + { + if (formats[i] == _glfw.x11.text_uri_list) + { + _glfw.x11.xdnd.format = _glfw.x11.text_uri_list; + break; + } + } + + if (list && formats) + XFree(formats); } else if (event->xclient.message_type == _glfw.x11.XdndDrop) { - // The drag operation has finished dropping on - // the window, ask to convert it to a UTF-8 string - _glfw.x11.xdnd.source = event->xclient.data.l[0]; - XConvertSelection(_glfw.x11.display, - _glfw.x11.XdndSelection, - _glfw.x11.UTF8_STRING, - _glfw.x11.XdndSelection, - window->x11.handle, CurrentTime); + // The drag operation has finished by dropping on the window + Time time = CurrentTime; + + if (_glfw.x11.xdnd.version > _GLFW_XDND_VERSION) + return; + + if (_glfw.x11.xdnd.format) + { + if (_glfw.x11.xdnd.version >= 1) + time = event->xclient.data.l[2]; + + // Request the chosen format from the source window + XConvertSelection(_glfw.x11.display, + _glfw.x11.XdndSelection, + _glfw.x11.xdnd.format, + _glfw.x11.XdndSelection, + window->x11.handle, + time); + } + else if (_glfw.x11.xdnd.version >= 2) + { + XEvent reply; + memset(&reply, 0, sizeof(reply)); + + reply.type = ClientMessage; + reply.xclient.window = _glfw.x11.xdnd.source; + reply.xclient.message_type = _glfw.x11.XdndFinished; + reply.xclient.format = 32; + reply.xclient.data.l[0] = window->x11.handle; + reply.xclient.data.l[1] = 0; // The drag was rejected + reply.xclient.data.l[2] = None; + + XSendEvent(_glfw.x11.display, _glfw.x11.xdnd.source, + False, NoEventMask, &reply); + XFlush(_glfw.x11.display); + } } else if (event->xclient.message_type == _glfw.x11.XdndPosition) { // The drag operation has moved over the window - const int absX = (event->xclient.data.l[2] >> 16) & 0xFFFF; - const int absY = (event->xclient.data.l[2]) & 0xFFFF; - int x, y; + const int xabs = (event->xclient.data.l[2] >> 16) & 0xffff; + const int yabs = (event->xclient.data.l[2]) & 0xffff; + Window dummy; + int xpos, ypos; - _glfwPlatformGetWindowPos(window, &x, &y); - _glfwInputCursorPos(window, absX - x, absY - y); + if (_glfw.x11.xdnd.version > _GLFW_XDND_VERSION) + return; + + XTranslateCoordinates(_glfw.x11.display, + window->x11.handle, + _glfw.x11.root, + xabs, yabs, + &xpos, &ypos, + &dummy); + + _glfwInputCursorPos(window, xpos, ypos); - // Reply that we are ready to copy the dragged data XEvent reply; memset(&reply, 0, sizeof(reply)); reply.type = ClientMessage; - reply.xclient.window = event->xclient.data.l[0]; + reply.xclient.window = _glfw.x11.xdnd.source; reply.xclient.message_type = _glfw.x11.XdndStatus; reply.xclient.format = 32; reply.xclient.data.l[0] = window->x11.handle; - reply.xclient.data.l[1] = 1; // Always accept the dnd with no rectangle reply.xclient.data.l[2] = 0; // Specify an empty rectangle reply.xclient.data.l[3] = 0; - reply.xclient.data.l[4] = _glfw.x11.XdndActionCopy; - XSendEvent(_glfw.x11.display, event->xclient.data.l[0], + if (_glfw.x11.xdnd.format) + { + // Reply that we are ready to copy the dragged data + reply.xclient.data.l[1] = 1; // Accept with no rectangle + if (_glfw.x11.xdnd.version >= 2) + reply.xclient.data.l[4] = _glfw.x11.XdndActionCopy; + } + + XSendEvent(_glfw.x11.display, _glfw.x11.xdnd.source, False, NoEventMask, &reply); XFlush(_glfw.x11.display); } @@ -1354,11 +1437,11 @@ static void processEvent(XEvent *event) case SelectionNotify: { - if (event->xselection.property) + if (event->xselection.property == _glfw.x11.XdndSelection) { // The converted data from the drag operation has arrived char* data; - const int result = + const unsigned long result = _glfwGetWindowPropertyX11(event->xselection.requestor, event->xselection.property, event->xselection.target, @@ -1379,21 +1462,23 @@ static void processEvent(XEvent *event) if (data) XFree(data); - XEvent reply; - memset(&reply, 0, sizeof(reply)); + if (_glfw.x11.xdnd.version >= 2) + { + XEvent reply; + memset(&reply, 0, sizeof(reply)); - reply.type = ClientMessage; - reply.xclient.window = _glfw.x11.xdnd.source; - reply.xclient.message_type = _glfw.x11.XdndFinished; - reply.xclient.format = 32; - reply.xclient.data.l[0] = window->x11.handle; - reply.xclient.data.l[1] = result; - reply.xclient.data.l[2] = _glfw.x11.XdndActionCopy; + reply.type = ClientMessage; + reply.xclient.window = _glfw.x11.xdnd.source; + reply.xclient.message_type = _glfw.x11.XdndFinished; + reply.xclient.format = 32; + reply.xclient.data.l[0] = window->x11.handle; + reply.xclient.data.l[1] = result; + reply.xclient.data.l[2] = _glfw.x11.XdndActionCopy; - // Reply that all is well - XSendEvent(_glfw.x11.display, _glfw.x11.xdnd.source, - False, NoEventMask, &reply); - XFlush(_glfw.x11.display); + XSendEvent(_glfw.x11.display, _glfw.x11.xdnd.source, + False, NoEventMask, &reply); + XFlush(_glfw.x11.display); + } } return;