Skip to content

Commit 9c4b4c0

Browse files
committed
fix(clipboard): address critical bugs and design issues
X11 ICCCM compliance (#4/#5): - SetSelectionOwner now uses real server timestamp via PropertyNotify probe, not XCB_CURRENT_TIME (violates ICCCM §2.1). Added get_server_timestamp_locked() which fires a zero-byte ChangeProperty to trigger timestamp event. - TIMESTAMP replies now return g_own_ts (real value) instead of truncated 0. - Verified: xclip -o -t TIMESTAMP returns non-zero after our clipboard write. INCR cleanup (#3): - On INCR read timeout, delete property to unblock sender waiting for PropertyNotify=Delete (ICCCM compliant termination). Process lifecycle (#7): - Wayland: runCaptureBytes, runSilently, writeBytes now escalate to destroyForcibly() if SIGTERM doesn't terminate after 500ms grace. AccessBehavior mapping (#12): - Kotlin: explicit when() mapping (0→AlwaysAllow, 1→AskEveryTime, 2→AlwaysDeny) instead of ordinal/entries.getOrNull (fragile with future macOS versions). - ObjC: validate input 0..2 on set; return -1 if get() returns out-of-range. Documentation & robustness (#13, #1): - Clipboard.watch() doc: clarify poll interval is always honored; source of counter differs by backend (Mach IPC / XFixes / wl-paste). - Re-check isActive after slow availableFormats() to avoid emitting to cancelled flow. Added X11TimestampSmokeTest to verify real timestamps are used.
1 parent eccbad6 commit 9c4b4c0

7 files changed

Lines changed: 173 additions & 27 deletions

File tree

clipboard-common/src/main/kotlin/io/github/kdroidfilter/nucleus/clipboard/Clipboard.kt

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,10 @@ object Clipboard {
132132
* Cold [Flow] that emits whenever the clipboard contents change.
133133
*
134134
* The event carries only format metadata and a monotonic `changeCount`, not
135-
* payload bytes — call a `readXxx` method to fetch content. Polls the backend's
136-
* change counter (cheap Mach IPC on macOS, WM_CLIPBOARDUPDATE on Windows,
137-
* XFixes on X11, data-control events on Wayland).
135+
* payload bytes — call a `readXxx` method to fetch content. The flow polls
136+
* `changeCount()` on every [pollInterval] tick regardless of backend; what
137+
* differs is the source that bumps the counter: Mach IPC on macOS, a native
138+
* event thread backed by XFixes on X11, and `wl-paste --watch` on Wayland.
138139
*
139140
* The baseline is captured on `collect`, so the flow does not emit the current
140141
* clipboard state — only subsequent changes.
@@ -149,7 +150,13 @@ object Clipboard {
149150
val cur = backend.changeCount()
150151
if (cur != last) {
151152
last = cur
152-
trySend(ClipboardEvent(formats = availableFormats(), changeCount = cur))
153+
// availableFormats() may be slow on Wayland (spawns a process);
154+
// re-check isActive so a cancellation during the await
155+
// does not leak a ghost emission after close.
156+
val formats = availableFormats()
157+
if (isActive) {
158+
trySend(ClipboardEvent(formats = formats, changeCount = cur))
159+
}
153160
}
154161
}
155162
}

clipboard-linux/src/main/kotlin/io/github/kdroidfilter/nucleus/clipboard/linux/WaylandClipboardDelegate.kt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import java.util.concurrent.atomic.AtomicLong
1414
private const val PROCESS_WRITE_TIMEOUT_SECONDS: Long = 3L
1515
private const val IMAGE_READ_TIMEOUT_MS: Long = 5000L
1616
private const val DESTROY_DRAIN_MS: Long = 200L
17+
private const val DESTROY_GRACE_MS: Long = 500L
1718

1819
private val IMAGE_MIME_PREFERENCE: List<String> =
1920
listOf("image/png", "image/jpeg", "image/jpg", "image/webp", "image/bmp", "image/gif", "image/tiff")
@@ -219,6 +220,9 @@ internal class WaylandClipboardDelegate {
219220
p.waitFor(PROCESS_WRITE_TIMEOUT_SECONDS, TimeUnit.SECONDS)
220221
if (p.isAlive) {
221222
p.destroy()
223+
if (!p.waitFor(DESTROY_GRACE_MS, TimeUnit.MILLISECONDS)) {
224+
p.destroyForcibly()
225+
}
222226
false
223227
} else {
224228
p.exitValue() == 0
@@ -266,6 +270,9 @@ internal class WaylandClipboardDelegate {
266270
}
267271
if (!p.waitFor(timeoutMs, TimeUnit.MILLISECONDS)) {
268272
p.destroy()
273+
if (!p.waitFor(DESTROY_GRACE_MS, TimeUnit.MILLISECONDS)) {
274+
p.destroyForcibly()
275+
}
269276
t.join(DESTROY_DRAIN_MS)
270277
return null
271278
}
@@ -296,6 +303,9 @@ internal class WaylandClipboardDelegate {
296303
val p = ProcessBuilder(cmd).redirectErrorStream(true).start()
297304
if (!p.waitFor(timeoutMs, TimeUnit.MILLISECONDS)) {
298305
p.destroy()
306+
if (!p.waitFor(DESTROY_GRACE_MS, TimeUnit.MILLISECONDS)) {
307+
p.destroyForcibly()
308+
}
299309
false
300310
} else {
301311
p.exitValue() == 0

clipboard-linux/src/main/native/linux/nucleus_clipboard_linux.c

Lines changed: 72 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ static xcb_atom_t a_CLIPBOARD, a_TARGETS, a_MULTIPLE, a_TIMESTAMP, a_SAVE_TARGET
313313
a_INCR, a_ATOM_PAIR, a_UTF8_STRING, a_STRING, a_TEXT, a_COMPOUND_TEXT,
314314
a_text_plain, a_text_plain_utf8, a_text_html, a_text_rtf, a_application_rtf,
315315
a_image_png, a_text_uri_list, a_x_special_gnome_copied_files,
316-
a_application_x_kde_cutselection, a_clipboard_manager, a_prop;
316+
a_application_x_kde_cutselection, a_clipboard_manager, a_prop, a_ts_probe;
317317

318318
/* Mutex protecting everything below. */
319319
static pthread_mutex_t g_mutex = PTHREAD_MUTEX_INITIALIZER;
@@ -327,7 +327,15 @@ typedef struct {
327327

328328
static payload_t g_offers[OFFER__COUNT];
329329
static bool g_offer_is_cut = false;
330-
static xcb_timestamp_t g_own_ts = 0;
330+
331+
/* Latest X11 server timestamp observed on our own window. Used instead of
332+
* XCB_CURRENT_TIME for SetSelectionOwner / TIMESTAMP replies, as ICCCM §2.1
333+
* forbids CurrentTime for those. Fetched via a zero-byte ChangeProperty probe
334+
* which triggers a PropertyNotify that the event thread captures. */
335+
static xcb_timestamp_t g_latest_ts = 0;
336+
static bool g_ts_pending = false;
337+
static pthread_cond_t g_ts_cond = PTHREAD_COND_INITIALIZER;
338+
static xcb_timestamp_t g_own_ts = 0;
331339

332340
/* Requestor-side pending read. */
333341
typedef struct {
@@ -444,6 +452,46 @@ static void intern_all_atoms(void) {
444452
a_application_x_kde_cutselection = intern("application/x-kde-cutselection", false);
445453
a_clipboard_manager = intern("CLIPBOARD_MANAGER", false);
446454
a_prop = intern(PROP_NAME, false);
455+
a_ts_probe = intern("NUCLEUS_CLIPBOARD_TS_PROBE", false);
456+
}
457+
458+
/* --------------------------------------------------------------------- */
459+
/* Deadline helper */
460+
/* --------------------------------------------------------------------- */
461+
462+
static void add_ms(struct timespec *ts, long ms) {
463+
ts->tv_sec += ms / 1000;
464+
ts->tv_nsec += (ms % 1000) * 1000000L;
465+
if (ts->tv_nsec >= 1000000000L) { ts->tv_sec++; ts->tv_nsec -= 1000000000L; }
466+
}
467+
468+
/* --------------------------------------------------------------------- */
469+
/* ICCCM-compliant server timestamp fetch */
470+
/* --------------------------------------------------------------------- */
471+
472+
/* Forces the server to emit a PropertyNotify on our window and captures its
473+
* `time` field via the event thread. Must be called with g_mutex held; the
474+
* condvar wait releases it transiently. Falls back to g_latest_ts on timeout,
475+
* or 0 if no event has ever been observed. */
476+
static xcb_timestamp_t get_server_timestamp_locked(void) {
477+
g_ts_pending = true;
478+
/* Append 0 bytes to a dedicated property → unconditional PropertyNotify. */
479+
p_xcb_change_property(g_conn, XCB_PROP_MODE_APPEND, g_window, a_ts_probe,
480+
XCB_ATOM_STRING, 8, 0, NULL);
481+
p_xcb_flush(g_conn);
482+
483+
struct timespec deadline;
484+
clock_gettime(CLOCK_REALTIME, &deadline);
485+
add_ms(&deadline, 500);
486+
487+
while (g_ts_pending) {
488+
int rc = pthread_cond_timedwait(&g_ts_cond, &g_mutex, &deadline);
489+
if (rc == ETIMEDOUT) {
490+
g_ts_pending = false;
491+
break;
492+
}
493+
}
494+
return g_latest_ts;
447495
}
448496

449497
/* --------------------------------------------------------------------- */
@@ -616,13 +664,6 @@ static uint8_t *fetch_property(xcb_atom_t prop, bool *out_incr, size_t *out_len)
616664
return out;
617665
}
618666

619-
/* Deadline helper. */
620-
static void add_ms(struct timespec *ts, long ms) {
621-
ts->tv_sec += ms / 1000;
622-
ts->tv_nsec += (ms % 1000) * 1000000L;
623-
if (ts->tv_nsec >= 1000000000L) { ts->tv_sec++; ts->tv_nsec -= 1000000000L; }
624-
}
625-
626667
/* Perform a full read for the given target (atom). Returns malloc'd bytes or NULL. */
627668
static uint8_t *read_selection(xcb_atom_t target, size_t *out_len) {
628669
*out_len = 0;
@@ -665,6 +706,11 @@ static uint8_t *read_selection(xcb_atom_t target, size_t *out_len) {
665706
while (!g_read.incr_done) {
666707
int rc = pthread_cond_timedwait(&g_read_cond, &g_mutex, &incr_deadline);
667708
if (rc == ETIMEDOUT) {
709+
/* Delete the property so the sender is unblocked for its next
710+
* chunk (ICCCM requires the requestor to ack each chunk by
711+
* deleting the property). We then abandon the transfer. */
712+
p_xcb_delete_property(g_conn, g_window, a_prop);
713+
p_xcb_flush(g_conn);
668714
g_read.waiting = false;
669715
read_reset_locked();
670716
pthread_mutex_unlock(&g_mutex);
@@ -720,8 +766,18 @@ static void on_selection_notify_locked(const xcb_selection_notify_event_t *ev) {
720766
}
721767

722768
static void on_property_notify_locked(const xcb_property_notify_event_t *ev) {
769+
/* Any PropertyNotify on our own window gives us a fresh server timestamp
770+
* we can use for subsequent SetSelectionOwner calls (ICCCM §2.1). */
771+
if (ev->window == g_window) {
772+
g_latest_ts = ev->time;
773+
if (ev->atom == a_ts_probe && g_ts_pending) {
774+
g_ts_pending = false;
775+
pthread_cond_broadcast(&g_ts_cond);
776+
}
777+
}
778+
723779
/* state 0 = NewValue, 1 = Delete. We only care about NewValue on our
724-
* own property during INCR. */
780+
* own read property during INCR. */
725781
if (!g_read.waiting || !g_read.incr || g_read.incr_done) return;
726782
if (ev->window != g_window || ev->atom != a_prop || ev->state != 0) return;
727783

@@ -1248,10 +1304,10 @@ Java_io_github_kdroidfilter_nucleus_clipboard_linux_NativeX11ClipboardBridge_nat
12481304

12491305
g_offer_is_cut = (isCut == JNI_TRUE);
12501306

1251-
/* Use CurrentTime; xcb_set_selection_owner is routed via server and will
1252-
* be our implicit timestamp. Record it for TIMESTAMP replies. */
1253-
g_own_ts = XCB_CURRENT_TIME;
1254-
p_xcb_set_selection_owner(g_conn, g_window, a_CLIPBOARD, XCB_CURRENT_TIME);
1307+
/* ICCCM §2.1: SetSelectionOwner must use a real server timestamp,
1308+
* never CurrentTime. Probe one via a zero-byte ChangeProperty. */
1309+
g_own_ts = get_server_timestamp_locked();
1310+
p_xcb_set_selection_owner(g_conn, g_window, a_CLIPBOARD, g_own_ts);
12551311
p_xcb_flush(g_conn);
12561312

12571313
pthread_mutex_unlock(&g_mutex);
@@ -1271,7 +1327,8 @@ Java_io_github_kdroidfilter_nucleus_clipboard_linux_NativeX11ClipboardBridge_nat
12711327
* often dies silently anyway. */
12721328

12731329
free_offers_locked();
1274-
p_xcb_set_selection_owner(g_conn, XCB_ATOM_NONE, a_CLIPBOARD, XCB_CURRENT_TIME);
1330+
xcb_timestamp_t ts = get_server_timestamp_locked();
1331+
p_xcb_set_selection_owner(g_conn, XCB_ATOM_NONE, a_CLIPBOARD, ts);
12751332
p_xcb_flush(g_conn);
12761333

12771334
pthread_mutex_unlock(&g_mutex);
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
package io.github.kdroidfilter.nucleus.clipboard.linux
2+
3+
import kotlin.test.Test
4+
import kotlin.test.assertTrue
5+
6+
/**
7+
* Verifies that SetSelectionOwner uses a real X11 server timestamp (ICCCM §2.1),
8+
* not XCB_CURRENT_TIME. We can't introspect the native timestamp directly; the
9+
* proof is indirect: `xclip -selection clipboard -o -t TIMESTAMP` returns the
10+
* selection owner's timestamp, which must be > 0 when the protocol is respected.
11+
*
12+
* Skipped when DISPLAY is unset or when xclip is not installed.
13+
*/
14+
class X11TimestampSmokeTest {
15+
@Test
16+
fun selectionTimestampIsNonZero() {
17+
val display = System.getenv("DISPLAY") ?: return
18+
if (display.isEmpty()) return
19+
if (!NativeX11ClipboardBridge.isLoaded) return
20+
if (!NativeX11ClipboardBridge.nativeInit()) return
21+
if (!hasBinary("xclip")) return
22+
23+
NativeX11ClipboardBridge.nativeWrite(
24+
"nucleus-ts-probe-${System.currentTimeMillis()}",
25+
null,
26+
null,
27+
null,
28+
null,
29+
false,
30+
)
31+
Thread.sleep(150)
32+
33+
val output =
34+
ProcessBuilder("xclip", "-selection", "clipboard", "-o", "-t", "TIMESTAMP")
35+
.redirectErrorStream(true)
36+
.start()
37+
.inputStream
38+
.bufferedReader()
39+
.readText()
40+
.trim()
41+
42+
println("TIMESTAMP output=$output")
43+
val ts = output.toLongOrNull()
44+
assertTrue(ts != null && ts > 0L, "selection timestamp must be non-zero, got '$output'")
45+
}
46+
47+
private fun hasBinary(name: String): Boolean {
48+
val path = System.getenv("PATH") ?: return false
49+
return path.split(':').any { dir ->
50+
dir.isNotEmpty() && java.io.File(dir, name).canExecute()
51+
}
52+
}
53+
}

clipboard-macos/src/main/kotlin/io/github/kdroidfilter/nucleus/clipboard/macos/MacClipboardBackend.kt

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,23 @@ class MacClipboardBackend : ClipboardBackend {
7676

7777
override fun setAccessBehavior(behavior: AccessBehavior) {
7878
if (!NativeMacClipboardBridge.isLoaded) return
79-
NativeMacClipboardBridge.nativeSetAccessBehavior(behavior.ordinal)
79+
val raw =
80+
when (behavior) {
81+
AccessBehavior.AlwaysAllow -> 0
82+
AccessBehavior.AskEveryTime -> 1
83+
AccessBehavior.AlwaysDeny -> 2
84+
}
85+
NativeMacClipboardBridge.nativeSetAccessBehavior(raw)
8086
}
8187

8288
override fun accessBehavior(): AccessBehavior? {
8389
if (!NativeMacClipboardBridge.isLoaded) return null
84-
val raw = NativeMacClipboardBridge.nativeGetAccessBehavior()
85-
return AccessBehavior.entries.getOrNull(raw)
90+
return when (NativeMacClipboardBridge.nativeGetAccessBehavior()) {
91+
0 -> AccessBehavior.AlwaysAllow
92+
1 -> AccessBehavior.AskEveryTime
93+
2 -> AccessBehavior.AlwaysDeny
94+
else -> null
95+
}
8696
}
8797

8898
override fun isAccessBehaviorSupported(): Boolean =

clipboard-macos/src/main/kotlin/io/github/kdroidfilter/nucleus/clipboard/macos/NativeMacClipboardBridge.kt

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,17 @@ internal object NativeMacClipboardBridge {
5858
external fun nativeClear(): Boolean
5959

6060
/**
61-
* Sets `NSPasteboard.accessBehavior`. Values match [AccessBehavior] ordinal:
62-
* 0 = always allow, 1 = ask every time, 2 = always deny. No-op on macOS < 15.4.
61+
* Sets `NSPasteboard.accessBehavior`. Accepts 0 = always allow,
62+
* 1 = ask every time, 2 = always deny. Other values are refused.
63+
* No-op on macOS < 15.4.
6364
*/
6465
@JvmStatic
6566
external fun nativeSetAccessBehavior(value: Int)
6667

6768
/**
68-
* Reads `NSPasteboard.accessBehavior`. Returns the raw enum value on
69-
* macOS 15.4+, or `-1` when the property is not exposed by the runtime.
69+
* Reads `NSPasteboard.accessBehavior`. Returns 0/1/2 on macOS 15.4+,
70+
* or `-1` when the property is not exposed or the value is outside the
71+
* documented range (future macOS may add new cases).
7072
*/
7173
@JvmStatic
7274
external fun nativeGetAccessBehavior(): Int

clipboard-macos/src/main/native/macos/nucleus_clipboard.m

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,10 +315,13 @@ static jobjectArray toJStringArray(JNIEnv *env, NSArray<NSString *> *items) {
315315
JNIEnv *env, jclass cls, jint value) {
316316
(void)env; (void)cls;
317317
@autoreleasepool {
318+
// macOS 15.4 enum layout: 0 = alwaysAllow, 1 = askEveryTime, 2 = alwaysDeny.
319+
// Refuse out-of-range values rather than propagating them to AppKit where
320+
// they would produce undefined behavior.
321+
if (value < 0 || value > 2) return;
318322
NSPasteboard *pb = [NSPasteboard generalPasteboard];
319323
SEL sel = NSSelectorFromString(@"setAccessBehavior:");
320324
if (![pb respondsToSelector:sel]) return;
321-
// macOS 15.4 enum layout: 0 = alwaysAllow, 1 = askEveryTime, 2 = alwaysDeny.
322325
NSInteger mapped = (NSInteger)value;
323326
NSMethodSignature *sig = [pb methodSignatureForSelector:sel];
324327
NSInvocation *inv = [NSInvocation invocationWithMethodSignature:sig];
@@ -344,6 +347,10 @@ static jobjectArray toJStringArray(JNIEnv *env, NSArray<NSString *> *items) {
344347
[inv invoke];
345348
NSInteger result = 0;
346349
[inv getReturnValue:&result];
350+
// Only forward the three documented enum values. A future macOS
351+
// release may add intermediate cases — surface them as -1 so the
352+
// Kotlin side can degrade to null rather than silently misreport.
353+
if (result < 0 || result > 2) return -1;
347354
return (jint)result;
348355
}
349356
}

0 commit comments

Comments
 (0)