mirrors/kitty
1
0
Fork 0
mirror of https://github.com/kovidgoyal/kitty synced 2026-06-08 22:28:24 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'copilot/fix-memory-leaks-and-add-tests' of https://github.com/kovidgoyal/kitty

Browse Source
This commit is contained in:
Kovid Goyal
2026-04-14 17:47:00 +05:30
parent 5e8ca630d4 48523efc7f
commit a9e56d52a0
2 changed files with 142 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

83
kitty/dnd.c
View File

@@ -284,7 +284,8 @@ queue_payload_to_child(id_type id, uint32_t client_id, PendingData *pending, con
ensure_space_for(pending, items, PendingEntry, pending->count + 1, capacity, 32, true);
char *buf = malloc(header_sz + data_sz - offset);
if (!buf) fatal("Out of memory");
memcpy(buf, header, header_sz); memcpy(buf + header_sz, data, data_sz - offset);
memcpy(buf, header, header_sz);
if (data_sz - offset) memcpy(buf + header_sz, data, data_sz - offset);
PendingEntry *e = &pending->items[pending->count++];
e->buf = buf; e->header_sz = header_sz; e->data_sz = data_sz - offset;
e->as_base64 = as_base64; e->client_id = client_id;
@@ -429,8 +430,9 @@ drop_register_window(Window *w, const uint8_t *payload, size_t payload_sz, bool
if (!payload || !payload_sz) return;
size_t sz = w->drop.registered_mimes ? strlen(w->drop.registered_mimes) : 0;
if (sz + payload_sz > MIME_LIST_SIZE_CAP) return;
w->drop.registered_mimes = realloc(w->drop.registered_mimes, sz + payload_sz + 1);
if (w->drop.registered_mimes) {
char *tmp = realloc(w->drop.registered_mimes, sz + payload_sz + 1);
if (tmp) {
w->drop.registered_mimes = tmp;
memcpy(w->drop.registered_mimes + sz, payload, payload_sz);
sz += payload_sz;
w->drop.registered_mimes[sz] = 0;
@@ -628,9 +630,9 @@ drop_dispatch_data(Window *w, const char *mime, const char *data, ssize_t sz) {
queue_payload_to_child(w->id, w->drop.client_id, &w->drop.pending, buf, header_size, sz ? data : NULL, sz, true);
if (is_uri_list) {
w->drop.uri_list_sz += sz;
w->drop.uri_list = realloc(w->drop.uri_list, w->drop.uri_list_sz);
if (w->drop.uri_list) memcpy(w->drop.uri_list + w->drop.uri_list_sz - sz, data, sz);
else w->drop.uri_list_sz = 0;
char *tmp = realloc(w->drop.uri_list, w->drop.uri_list_sz);
if (tmp) { w->drop.uri_list = tmp; memcpy(w->drop.uri_list + w->drop.uri_list_sz - sz, data, sz); }
else { free(w->drop.uri_list); w->drop.uri_list = NULL; w->drop.uri_list_sz = 0; }
}
if (sz == 0) { drop_pop_request(w); drop_process_queue(w); }
}
@@ -1195,6 +1197,7 @@ drag_free_remote_item(DragRemoteItem *x) {
if (x->top_level_parent_dir_fd_plus_one) safe_close(x->top_level_parent_dir_fd_plus_one-1, __FILE__, __LINE__);
if (x->children) {
for (size_t i = 0; i < x->children_sz; i++) drag_free_remote_item(x->children + i);
free(x->children);
}
zero_at_ptr(x);
}
@@ -1230,6 +1233,7 @@ drag_free_offer(Window *w) {
if (ds.images[i].data) free(ds.images[i].data);
zero_at_ptr(ds.images + i);
}
free_pending(&ds.pending);
ds.allowed_operations = 0;
ds.state = DRAG_SOURCE_NONE;
ds.pre_sent_total_sz = 0;
@@ -1275,8 +1279,9 @@ drag_add_mimes(Window *w, int allowed_operations, uint32_t client_id, const char
ds.client_id = client_id;
size_t new_sz = ds.bufsz + sz;
if (new_sz > MIME_LIST_SIZE_CAP) abrt(EFBIG);
ds.mimes_buf = realloc(ds.mimes_buf, ds.bufsz + sz + 1);
if (!ds.mimes_buf) abrt(ENOMEM);
char *tmp = realloc(ds.mimes_buf, ds.bufsz + sz + 1);
if (!tmp) abrt(ENOMEM);
ds.mimes_buf = tmp;
memcpy(ds.mimes_buf + ds.bufsz, data, sz);
ds.bufsz = new_sz;
ds.mimes_buf[ds.bufsz] = 0;
@@ -1315,8 +1320,9 @@ drag_add_pre_sent_data(Window *w, unsigned idx, const uint8_t *payload, size_t s
}
if (item.data_capacity < sz + item.data_size) {
size_t newcap = MAX(item.data_capacity * 2, sz + item.data_size);
item.optional_data = realloc(item.optional_data, newcap);
if (!item.optional_data) abrt(ENOMEM);
uint8_t *tmp = realloc(item.optional_data, newcap);
if (!tmp) abrt(ENOMEM);
item.optional_data = tmp;
item.data_capacity = newcap;
}
size_t outlen = item.data_capacity - item.data_size;
@@ -1343,8 +1349,9 @@ drag_add_image(Window *w, unsigned idx, int fmt, int width, int height, const ui
}
if (img.capacity < sz + img.sz) {
size_t newcap = MAX(img.capacity * 2, sz + img.sz);
img.data = realloc(img.data, newcap);
if (!img.data) abrt(ENOMEM);
uint8_t *tmp = realloc(img.data, newcap);
if (!tmp) abrt(ENOMEM);
img.data = tmp;
img.capacity = newcap;
}
size_t outlen = img.capacity - img.sz;
@@ -1362,7 +1369,7 @@ static bool
expand_rgb_data(Window *w, size_t idx) {
#define fail(code) { cancel_drag(w, code); return false; }
if (img.sz != (size_t)img.width * (size_t)img.height * 3) fail(EINVAL);
const size_t sz = img.width * img.height * 4;
const size_t sz = (size_t)img.width * (size_t)img.height * 4u;
RAII_ALLOC(uint8_t, expanded, malloc(sz));
if (!expanded) fail(ENOMEM);
memset(expanded, 0xff, sz);
@@ -1434,24 +1441,24 @@ void
drag_notify(Window *w, DragNotifyType type) {
if (ds.state < DRAG_SOURCE_STARTED) return;
char buf[128];
size_t sz = snprintf(buf, sizeof(buf), "t=e:x=%d", type + 1);
size_t sz = snprintf(buf, sizeof(buf), "\x1b]%d;t=e:x=%d", DND_CODE, type + 1);
switch(type) {
case DRAG_NOTIFY_ACCEPTED:
for (size_t i = 0; i < ds.num_mimes; i++) {
if (strcmp(ds.items[i].mime_type, global_state.drag_source.accepted_mime_type) == 0) {
sz += snprintf(buf + sz, sizeof(buf) - sz, "y=%zu", i); break;
sz += snprintf(buf + sz, sizeof(buf) - sz, ":y=%zu", i); break;
}
} break;
case DRAG_NOTIFY_ACTION_CHANGED:
switch (global_state.drag_source.action) {
case GLFW_DRAG_OPERATION_MOVE:
sz += snprintf(buf + sz, sizeof(buf) - sz, "o=2"); break;
sz += snprintf(buf + sz, sizeof(buf) - sz, ":o=2"); break;
default:
sz += snprintf(buf + sz, sizeof(buf) - sz, "o=1"); break;
sz += snprintf(buf + sz, sizeof(buf) - sz, ":o=1"); break;
} break;
case DRAG_NOTIFY_DROPPED: ds.state = DRAG_SOURCE_DROPPED; break;
case DRAG_NOTIFY_FINISHED:
sz += snprintf(buf + sz, sizeof(buf) - sz, "y=%d", global_state.drag_source.was_canceled ? 1 : 0); break;
sz += snprintf(buf + sz, sizeof(buf) - sz, ":y=%d", global_state.drag_source.was_canceled ? 1 : 0); break;
}
queue_payload_to_child(w->id, w->drag_source.client_id, &w->drag_source.pending, buf, sz, NULL, 0, false);
if (type == DRAG_NOTIFY_FINISHED) drag_free_offer(w);
@@ -1758,8 +1765,9 @@ add_payload(Window *w, DragRemoteItem *ri, bool has_more, const uint8_t *payload
if (ri->data_sz + payload_sz > ri->data_capacity) {
size_t cap = MAX(ri->data_capacity * 2, ri->data_sz + payload_sz + 4096);
if (cap > PRESENT_DATA_CAP) abrt(EMFILE);
ri->data = realloc(ri->data, cap);
if (!ri->data) abrt(ENOMEM);
uint8_t *tmp = realloc(ri->data, cap);
if (!tmp) abrt(ENOMEM);
ri->data = tmp;
ri->data_capacity = cap;
}
size_t outlen = ri->data_capacity - ri->data_sz;
@@ -1777,6 +1785,13 @@ add_payload(Window *w, DragRemoteItem *ri, bool has_more, const uint8_t *payload
ri->fd_plus_one = 0;
break;
case 1:
// Ensure room for the null terminator needed by symlinkat
if (ri->data_sz >= ri->data_capacity) {
uint8_t *tmp = realloc(ri->data, ri->data_sz + 1);
if (!tmp) abrt(ENOMEM);
ri->data = tmp;
ri->data_capacity = ri->data_sz + 1;
}
ri->data[ri->data_sz] = 0;
if (symlinkat((char*)ri->data, dirfd, ri->dir_entry_name) != 0) abrt(errno);
break;
@@ -1823,6 +1838,7 @@ toplevel_data_for_drag(
int fd = safe_openat(mi.base_dir_fd_plus_one - 1, path, O_RDONLY | O_DIRECTORY, 0);
if (fd < 0) abrt(errno);
ri->top_level_parent_dir_fd_plus_one = fd + 1;
free(mi.uri_list[uri_item_idx]);
mi.uri_list[uri_item_idx] = as_file_url(mi.base_dir_for_remote_items, path, ri->dir_entry_name);
}
add_payload(w, ri, has_more, payload, payload_sz, ri->top_level_parent_dir_fd_plus_one - 1);
@@ -2115,6 +2131,32 @@ dnd_test_request_drag_data(PyObject *self UNUSED, PyObject *args) {
Py_RETURN_NONE;
}
static PyObject *
dnd_test_drag_notify(PyObject *self UNUSED, PyObject *args) {
// Call drag_notify with a specific type for testing the protocol output.
// type: 0=ACCEPTED, 1=ACTION_CHANGED, 2=DROPPED, 3=FINISHED
// accepted_mime: the MIME type to set in global_state.drag_source.accepted_mime_type (for ACCEPTED)
// action: the action to set in global_state.drag_source.action (for ACTION_CHANGED)
// was_canceled: whether the drag was canceled (for FINISHED)
unsigned long long window_id;
int type;
const char *accepted_mime = NULL;
int action = 0, was_canceled = 0;
if (!PyArg_ParseTuple(args, "Ki|sii", &window_id, &type, &accepted_mime, &action, &was_canceled)) return NULL;
Window *w = window_for_window_id((id_type)window_id);
if (!w) { PyErr_SetString(PyExc_ValueError, "Window not found"); return NULL; }
if (type < 0 || type > 3) { PyErr_SetString(PyExc_ValueError, "Invalid type"); return NULL; }
if (accepted_mime && *accepted_mime) {
free(global_state.drag_source.accepted_mime_type);
global_state.drag_source.accepted_mime_type = strdup(accepted_mime);
if (!global_state.drag_source.accepted_mime_type) { PyErr_NoMemory(); return NULL; }
}
global_state.drag_source.action = action;
global_state.drag_source.was_canceled = was_canceled;
drag_notify(w, (DragNotifyType)type);
Py_RETURN_NONE;
}
static PyMethodDef dnd_methods[] = {
{"dnd_set_test_write_func", (PyCFunction)py_dnd_set_test_write_func, METH_VARARGS, ""},
METHODB(dnd_test_create_fake_window, METH_NOARGS),
@@ -2124,6 +2166,7 @@ static PyMethodDef dnd_methods[] = {
METHODB(dnd_test_fake_drop_data, METH_VARARGS),
METHODB(dnd_test_force_drag_dropped, METH_VARARGS),
METHODB(dnd_test_request_drag_data, METH_VARARGS),
METHODB(dnd_test_drag_notify, METH_VARARGS),
{NULL, NULL, 0, NULL}
};

79
kitty_tests/dnd.py
View File

@@ -14,6 +14,7 @@ from kitty.fast_data_types import (
dnd_set_test_write_func,
dnd_test_cleanup_fake_window,
dnd_test_create_fake_window,
dnd_test_drag_notify,
dnd_test_fake_drop_data,
dnd_test_fake_drop_event,
dnd_test_force_drag_dropped,
@@ -2989,3 +2990,81 @@ class TestDnDProtocol(BaseTest):
events = self._get_events(cap, wid)
# Should get a move event
self.assertTrue(len(events) >= 1, events)
def test_drag_notify_colon_separators(self) -> None:
"""drag_notify output has proper colon separators between metadata keys."""
with dnd_test_window() as (osw, wid, screen, cap):
self._setup_drag_offer(screen, wid, cap, 'text/plain text/html')
dnd_test_force_drag_dropped(wid)
# DRAG_NOTIFY_ACCEPTED (type=0) should produce t=e:x=1:y=<idx>
dnd_test_drag_notify(wid, 0, 'text/html')
events = self._get_events(cap, wid)
self.assertEqual(len(events), 1, events)
self.ae(events[0]['type'], 'e')
# Verify proper key formatting with colons
self.ae(events[0]['meta'].get('x'), '1')
self.ae(events[0]['meta'].get('y'), '1') # text/html is index 1
def test_drag_notify_action_changed_colon_separator(self) -> None:
"""drag_notify ACTION_CHANGED output has proper colon separators."""
from kitty.fast_data_types import GLFW_DRAG_OPERATION_MOVE
with dnd_test_window() as (osw, wid, screen, cap):
self._setup_drag_offer(screen, wid, cap, 'text/plain')
dnd_test_force_drag_dropped(wid)
# DRAG_NOTIFY_ACTION_CHANGED (type=1) with MOVE action
dnd_test_drag_notify(wid, 1, '', GLFW_DRAG_OPERATION_MOVE)
events = self._get_events(cap, wid)
self.assertEqual(len(events), 1, events)
self.ae(events[0]['type'], 'e')
self.ae(events[0]['meta'].get('x'), '2') # ACTION_CHANGED = type+1 = 2
self.ae(events[0]['meta'].get('o'), '2') # MOVE = o=2
def test_drag_notify_finished_colon_separator(self) -> None:
"""drag_notify FINISHED output has proper colon separators."""
with dnd_test_window() as (osw, wid, screen, cap):
self._setup_drag_offer(screen, wid, cap, 'text/plain')
dnd_test_force_drag_dropped(wid)
# DRAG_NOTIFY_FINISHED (type=3) with was_canceled=0
dnd_test_drag_notify(wid, 3, '', 0, 0)
events = self._get_events(cap, wid)
self.assertEqual(len(events), 1, events)
self.ae(events[0]['type'], 'e')
self.ae(events[0]['meta'].get('x'), '4') # FINISHED = type+1 = 4
self.ae(events[0]['meta'].get('y'), '0') # was_canceled = 0
def test_remote_drag_children_freed_on_cleanup(self) -> None:
"""Remote drag with directories properly frees the children array on cleanup."""
uri_list = b'file:///home/user/mydir\r\n'
with dnd_test_window() as (osw, wid, screen, cap):
self._setup_remote_drag(screen, wid, cap, uri_list)
# Create a directory entry (X=2 means directory handle=2)
dir_listing = standard_b64encode(b'file1.txt\x00subdir').decode()
parse_bytes(screen, client_remote_file(1, dir_listing, item_type=2))
self._assert_no_output(cap, wid)
# Finish the directory entry
parse_bytes(screen, client_remote_file(1, '', item_type=2))
self._assert_no_output(cap, wid)
# Now send file data for the first child (entry_num=1, Y=handle)
file_data = standard_b64encode(b'hello').decode()
parse_bytes(screen, client_remote_file(1, file_data, item_type=0, parent_handle=2, entry_num=1))
parse_bytes(screen, client_remote_file(1, '', item_type=0, parent_handle=2, entry_num=1))
self._assert_no_output(cap, wid)
# Cleanup happens when context manager exits - no crash means children are freed
def test_remote_drag_uri_replaced_without_leak(self) -> None:
"""Remote drag replaces URI string without leaking the original."""
uri_list = b'file:///home/user/hello.txt\r\n'
file_content = b'test content'
with dnd_test_window() as (osw, wid, screen, cap):
self._setup_remote_drag(screen, wid, cap, uri_list)
b64 = standard_b64encode(file_content).decode()
# Send file data - this replaces the URI string in the uri_list
parse_bytes(screen, client_remote_file(1, b64, item_type=0))
self._assert_no_output(cap, wid)
# End of data for this file
parse_bytes(screen, client_remote_file(1, '', item_type=0))
self._assert_no_output(cap, wid)
# Completion signal
parse_bytes(screen, client_remote_file_finish())
self._assert_no_output(cap, wid)
# No crash or leak - cleanup happens in context manager exit