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.
This commit is contained in:
Camilla Löwy 2017-03-16 23:07:59 +01:00
parent bff31f006e
commit 6a65341e14
4 changed files with 129 additions and 39 deletions

View File

@ -171,6 +171,7 @@ information on what to include when reporting a bug.
- [X11] Bugfix: `glfwGetVideoMode` would segfault on Cygwin/X - [X11] Bugfix: `glfwGetVideoMode` would segfault on Cygwin/X
- [X11] Bugfix: Dynamic X11 library loading did not use full sonames (#941) - [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: 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) - [Linux] Bugfix: Event processing did not detect joystick disconnection (#932)
- [Cocoa] Added support for Vulkan window surface creation via - [Cocoa] Added support for Vulkan window surface creation via
[MoltenVK](https://moltengl.com/moltenvk/) (#870) [MoltenVK](https://moltengl.com/moltenvk/) (#870)

View File

@ -629,9 +629,10 @@ static GLFWbool initExtensions(void)
_glfw.x11.XdndStatus = XInternAtom(_glfw.x11.display, "XdndStatus", False); _glfw.x11.XdndStatus = XInternAtom(_glfw.x11.display, "XdndStatus", False);
_glfw.x11.XdndActionCopy = XInternAtom(_glfw.x11.display, "XdndActionCopy", False); _glfw.x11.XdndActionCopy = XInternAtom(_glfw.x11.display, "XdndActionCopy", False);
_glfw.x11.XdndDrop = XInternAtom(_glfw.x11.display, "XdndDrop", 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.XdndFinished = XInternAtom(_glfw.x11.display, "XdndFinished", False);
_glfw.x11.XdndSelection = XInternAtom(_glfw.x11.display, "XdndSelection", 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 // ICCCM, EWMH and Motif window property atoms
// These can be set safely even without WM support // These can be set safely even without WM support

View File

@ -208,9 +208,10 @@ typedef struct _GLFWlibraryX11
Atom XdndStatus; Atom XdndStatus;
Atom XdndActionCopy; Atom XdndActionCopy;
Atom XdndDrop; Atom XdndDrop;
Atom XdndLeave;
Atom XdndFinished; Atom XdndFinished;
Atom XdndSelection; Atom XdndSelection;
Atom XdndTypeList;
Atom text_uri_list;
// Selection (clipboard) atoms // Selection (clipboard) atoms
Atom TARGETS; Atom TARGETS;
@ -253,7 +254,9 @@ typedef struct _GLFWlibraryX11
} saver; } saver;
struct { struct {
int version;
Window source; Window source;
Atom format;
} xdnd; } xdnd;
struct { struct {

View File

@ -48,6 +48,8 @@
#define Button6 6 #define Button6 6
#define Button7 7 #define Button7 7
#define _GLFW_XDND_VERSION 5
// Wait for data to arrive using select // Wait for data to arrive using select
// This avoids blocking other threads via the per-display Xlib lock that also // 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; continue;
if (strncmp(line, prefix, strlen(prefix)) == 0) if (strncmp(line, prefix, strlen(prefix)) == 0)
{
line += strlen(prefix); line += strlen(prefix);
// TODO: Validate hostname
while (*line != '/')
line++;
}
(*count)++; (*count)++;
@ -619,10 +626,9 @@ static GLFWbool createNativeWindow(_GLFWwindow* window,
XFree(hint); 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, XChangeProperty(_glfw.x11.display, window->x11.handle,
_glfw.x11.XdndAware, XA_ATOM, 32, _glfw.x11.XdndAware, XA_ATOM, 32,
PropModeReplace, (unsigned char*) &version, 1); PropModeReplace, (unsigned char*) &version, 1);
@ -1307,44 +1313,121 @@ static void processEvent(XEvent *event)
else if (event->xclient.message_type == _glfw.x11.XdndEnter) else if (event->xclient.message_type == _glfw.x11.XdndEnter)
{ {
// A drag operation has entered the window // 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) else if (event->xclient.message_type == _glfw.x11.XdndDrop)
{ {
// The drag operation has finished dropping on // The drag operation has finished by dropping on the window
// the window, ask to convert it to a UTF-8 string Time time = CurrentTime;
_glfw.x11.xdnd.source = event->xclient.data.l[0];
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, XConvertSelection(_glfw.x11.display,
_glfw.x11.XdndSelection, _glfw.x11.XdndSelection,
_glfw.x11.UTF8_STRING, _glfw.x11.xdnd.format,
_glfw.x11.XdndSelection, _glfw.x11.XdndSelection,
window->x11.handle, CurrentTime); window->x11.handle,
time);
} }
else if (event->xclient.message_type == _glfw.x11.XdndPosition) else if (_glfw.x11.xdnd.version >= 2)
{ {
// 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;
_glfwPlatformGetWindowPos(window, &x, &y);
_glfwInputCursorPos(window, absX - x, absY - y);
// Reply that we are ready to copy the dragged data
XEvent reply; XEvent reply;
memset(&reply, 0, sizeof(reply)); memset(&reply, 0, sizeof(reply));
reply.type = ClientMessage; reply.type = ClientMessage;
reply.xclient.window = event->xclient.data.l[0]; 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 xabs = (event->xclient.data.l[2] >> 16) & 0xffff;
const int yabs = (event->xclient.data.l[2]) & 0xffff;
Window dummy;
int xpos, ypos;
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);
XEvent reply;
memset(&reply, 0, sizeof(reply));
reply.type = ClientMessage;
reply.xclient.window = _glfw.x11.xdnd.source;
reply.xclient.message_type = _glfw.x11.XdndStatus; reply.xclient.message_type = _glfw.x11.XdndStatus;
reply.xclient.format = 32; reply.xclient.format = 32;
reply.xclient.data.l[0] = window->x11.handle; 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[2] = 0; // Specify an empty rectangle
reply.xclient.data.l[3] = 0; 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); False, NoEventMask, &reply);
XFlush(_glfw.x11.display); XFlush(_glfw.x11.display);
} }
@ -1354,11 +1437,11 @@ static void processEvent(XEvent *event)
case SelectionNotify: case SelectionNotify:
{ {
if (event->xselection.property) if (event->xselection.property == _glfw.x11.XdndSelection)
{ {
// The converted data from the drag operation has arrived // The converted data from the drag operation has arrived
char* data; char* data;
const int result = const unsigned long result =
_glfwGetWindowPropertyX11(event->xselection.requestor, _glfwGetWindowPropertyX11(event->xselection.requestor,
event->xselection.property, event->xselection.property,
event->xselection.target, event->xselection.target,
@ -1379,6 +1462,8 @@ static void processEvent(XEvent *event)
if (data) if (data)
XFree(data); XFree(data);
if (_glfw.x11.xdnd.version >= 2)
{
XEvent reply; XEvent reply;
memset(&reply, 0, sizeof(reply)); memset(&reply, 0, sizeof(reply));
@ -1390,11 +1475,11 @@ static void processEvent(XEvent *event)
reply.xclient.data.l[1] = result; reply.xclient.data.l[1] = result;
reply.xclient.data.l[2] = _glfw.x11.XdndActionCopy; reply.xclient.data.l[2] = _glfw.x11.XdndActionCopy;
// Reply that all is well
XSendEvent(_glfw.x11.display, _glfw.x11.xdnd.source, XSendEvent(_glfw.x11.display, _glfw.x11.xdnd.source,
False, NoEventMask, &reply); False, NoEventMask, &reply);
XFlush(_glfw.x11.display); XFlush(_glfw.x11.display);
} }
}
return; return;
} }