6168668 fix(xwm): harden selection transfers on close, late replies and theme truncation · neferwl

M internal/adapters/wayland/xwm/selection.go +8 -2


@@ -119,6 +119,7 @@ type selBridge struct {
119	119		// active is the transfer that owns the conversion property, -1 none.
120	120		active int
121	121		timerOn bool
	122	+	closing bool // selClose ran: no transfer may start
122	123		cancel func()
123	124		tickFn func()
124	125		}

@@ -320,7 +321,7 @@ func (x *XWM) SelectionReceive(kind int, mime string, fd int) {
320	321		// inStartNext starts the oldest queued transfer when none is converting.
321	322		func (x *XWM) inStartNext() {
322	323		b := &x.sel
323		-	if b.active >= 0 {
	324	+	if b.active >= 0 \|\| b.closing {
324	325		return
325	326		}
326	327		best := -1

@@ -384,7 +385,10 @@ func (x *XWM) onDataNotify(e xproto.SelectionNotifyEvent) {
384	385		return
385	386		}
386	387		t := &x.sel.in[i]
387		-	if t.state != inConverting \|\| uint32(e.Target()) != t.target {
	388	+	if t.state != inConverting \|\| uint32(e.Target()) != t.target \|\| uint32(e.Selection()) != x.sel.st[t.kind].atom {
	389	+	return
	390	+	}
	391	+	if p := uint32(e.Property()); p != 0 && p != x.atoms[atomNeferwlData] {
388	392		return
389	393		}
390	394		if e.Property() == 0 {

@@ -629,6 +633,7 @@ func (x *XWM) selTick() {
629	633
630	634		// selClose aborts every transfer.
631	635		func (x *XWM) selClose() {
	636	+	x.sel.closing = true
632	637		for i := range x.sel.out {
633	638		if x.sel.out[i].state != outFree {
634	639		x.outAbort(i)

@@ -643,6 +648,7 @@ func (x *XWM) selClose() {
643	648		x.sel.cancel()
644	649		x.sel.cancel = nil
645	650		}
	651	+	x.sel.timerOn = false
646	652		}
647	653
648	654		// ---- Wayland -> X ----

 	// active is the transfer that owns the conversion property, -1 none.
 	active  int
 	timerOn bool
+	closing bool // selClose ran: no transfer may start
 	cancel  func()
 	tickFn  func()
 }
 // inStartNext starts the oldest queued transfer when none is converting.
 func (x *XWM) inStartNext() {
 	b := &x.sel
-	if b.active >= 0 {
+	if b.active >= 0 || b.closing {
 		return
 	}
 	best := -1
 		return
 	}
 	t := &x.sel.in[i]
-	if t.state != inConverting || uint32(e.Target()) != t.target {
+	if t.state != inConverting || uint32(e.Target()) != t.target || uint32(e.Selection()) != x.sel.st[t.kind].atom {
+		return
+	}
+	if p := uint32(e.Property()); p != 0 && p != x.atoms[atomNeferwlData] {
 		return
 	}
 	if e.Property() == 0 {
 // selClose aborts every transfer.
 func (x *XWM) selClose() {
+	x.sel.closing = true
 	for i := range x.sel.out {
 		if x.sel.out[i].state != outFree {
 			x.outAbort(i)
 		x.sel.cancel()
 		x.sel.cancel = nil
 	}
+	x.sel.timerOn = false
 }
 // ---- Wayland -> X ----

M internal/adapters/wayland/xwm/selection_test.go +49 -0


@@ -735,3 +735,52 @@ func newTimerLoop(t testing.T) mockloop {
735	735		func(time.Duration, func()) func() { return func() {} }).Maybe()
736	736		return l
737	737		}
	738	+
	739	+	// A late SelectionNotify of an aborted transfer (other selection or property)
	740	+	// is not taken by the next transfer.
	741	+	func TestSelectionLateReplyIgnored(t *testing.T) {
	742	+	r := ownedRig(t)
	743	+	_ = r.receive(t, 0)
	744	+	_, n := pumpReq(t, r.x, r.p, r.skip, func(q xpeerReq) bool { return q.major == 24 })
	745	+	r.skip = n
	746	+	for i := 0; i < xIdleMax; i++ {
	747	+	r.timer()
	748	+	}
	749	+	require.Equal(t, inFree, r.x.sel.in[0].state)
	750	+	_ = r.receive(t, 0)
	751	+	require.Equal(t, inConverting, r.x.sel.in[0].state)
	752	+	prim := r.p.atom("PRIMARY")
	753	+	r.p.selectionNotify(r.x.selWin, prim, r.utf8, r.data)
	754	+	r.p.selectionNotify(r.x.selWin, r.clip, r.utf8, r.p.atom("_NEFERWL_TARGETS"))
	755	+	ev := r.x.Stats().Events
	756	+	readUntil(t, r.x, func() bool { return r.x.Stats().Events >= ev+2 })
	757	+	require.Equal(t, inConverting, r.x.sel.in[0].state)
	758	+	}
	759	+
	760	+	// CloseConn closes the fds of in-flight transfers.
	761	+	func TestSelectionCloseConnClosesFDs(t *testing.T) {
	762	+	r := ownedRig(t)
	763	+	f := r.receive(t, 0)
	764	+	r.x.CloseConn()
	765	+	_, err := f.Read(make([]byte, 8))
	766	+	require.ErrorIs(t, err, io.EOF)
	767	+	}
	768	+
	769	+	// Closing never starts a queued transfer's ConvertSelection.
	770	+	func TestSelectionCloseDoesNotStartQueued(t *testing.T) {
	771	+	r := ownedRig(t)
	772	+	_ = r.receive(t, 0)
	773	+	_ = r.receive(t, 0)
	774	+	_, n := pumpReq(t, r.x, r.p, r.skip, func(q xpeerReq) bool { return q.major == 24 })
	775	+	r.x.selClose()
	776	+	r.x.flush()
	777	+	time.Sleep(50 * time.Millisecond)
	778	+	cs := 0
	779	+	for _, q := range since(r.p, r.skip, 24) {
	780	+	cs++
	781	+	_ = q
	782	+	}
	783	+	_ = n
	784	+	require.Equal(t, 1, cs)
	785	+	require.False(t, r.x.sel.timerOn)
	786	+	}


@@ -735,3 +735,52 @@ func newTimerLoop(t testing.T) mockloop {
735	func(time.Duration, func()) func() { return func() {} }).Maybe()	735		func(time.Duration, func()) func() { return func() {} }).Maybe()
736	return l	736		return l
737	}	737		}
		738	+
		739	+	// A late SelectionNotify of an aborted transfer (other selection or property)
		740	+	// is not taken by the next transfer.
		741	+	func TestSelectionLateReplyIgnored(t *testing.T) {
		742	+	r := ownedRig(t)
		743	+	_ = r.receive(t, 0)
		744	+	_, n := pumpReq(t, r.x, r.p, r.skip, func(q xpeerReq) bool { return q.major == 24 })
		745	+	r.skip = n
		746	+	for i := 0; i < xIdleMax; i++ {
		747	+	r.timer()
		748	+	}
		749	+	require.Equal(t, inFree, r.x.sel.in[0].state)
		750	+	_ = r.receive(t, 0)
		751	+	require.Equal(t, inConverting, r.x.sel.in[0].state)
		752	+	prim := r.p.atom("PRIMARY")
		753	+	r.p.selectionNotify(r.x.selWin, prim, r.utf8, r.data)
		754	+	r.p.selectionNotify(r.x.selWin, r.clip, r.utf8, r.p.atom("_NEFERWL_TARGETS"))
		755	+	ev := r.x.Stats().Events
		756	+	readUntil(t, r.x, func() bool { return r.x.Stats().Events >= ev+2 })
		757	+	require.Equal(t, inConverting, r.x.sel.in[0].state)
		758	+	}
		759	+
		760	+	// CloseConn closes the fds of in-flight transfers.
		761	+	func TestSelectionCloseConnClosesFDs(t *testing.T) {
		762	+	r := ownedRig(t)
		763	+	f := r.receive(t, 0)
		764	+	r.x.CloseConn()
		765	+	_, err := f.Read(make([]byte, 8))
		766	+	require.ErrorIs(t, err, io.EOF)
		767	+	}
		768	+
		769	+	// Closing never starts a queued transfer's ConvertSelection.
		770	+	func TestSelectionCloseDoesNotStartQueued(t *testing.T) {
		771	+	r := ownedRig(t)
		772	+	_ = r.receive(t, 0)
		773	+	_ = r.receive(t, 0)
		774	+	_, n := pumpReq(t, r.x, r.p, r.skip, func(q xpeerReq) bool { return q.major == 24 })
		775	+	r.x.selClose()
		776	+	r.x.flush()
		777	+	time.Sleep(50 * time.Millisecond)
		778	+	cs := 0
		779	+	for _, q := range since(r.p, r.skip, 24) {
		780	+	cs++
		781	+	_ = q
		782	+	}
		783	+	_ = n
		784	+	require.Equal(t, 1, cs)
		785	+	require.False(t, r.x.sel.timerOn)
		786	+	}

M internal/adapters/wayland/xwm/selection_xsettings.go +6 -1


@@ -3,6 +3,7 @@ package xwm
3	3		import (
4	4		"encoding/binary"
5	5		"math"
	6	+	"unicode/utf8"
6	7
7	8		"bnema.dev/xturbo"
8	9		"bnema.dev/xturbo/xproto"

@@ -53,7 +54,11 @@ func (x *XWM) cursorThemeName() string {
53	54		t = "default"
54	55		}
55	56		if len(t) > xsMaxString {
56		-	t = t[:xsMaxString]
	57	+	n := xsMaxString
	58	+	for n > 0 && !utf8.RuneStart(t[n]) { // back off to a rune boundary
	59	+	n--
	60	+	}
	61	+	t = t[:n]
57	62		}
58	63		return t
59	64		}

 import (
 	"encoding/binary"
 	"math"
+	"unicode/utf8"
 	"bnema.dev/xturbo"
 	"bnema.dev/xturbo/xproto"
 		t = "default"
 	}
 	if len(t) > xsMaxString {
-		t = t[:xsMaxString]
+		n := xsMaxString
+		for n > 0 && !utf8.RuneStart(t[n]) { // back off to a rune boundary
+			n--
+		}
+		t = t[:n]
 	}
 	return t
 }

M internal/adapters/wayland/xwm/xsettings_test.go +10 -0


@@ -3,7 +3,9 @@ package xwm
3	3		import (
4	4		"context"
5	5		"encoding/binary"
	6	+	"strings"
6	7		"testing"
	8	+	"unicode/utf8"
7	9
8	10		"github.com/bnema/neferwl/internal/ports"
9	11		"github.com/stretchr/testify/require"

@@ -217,3 +219,11 @@ func TestXSettingsWindowNotAdopted(t *testing.T) {
217	219		x.flush()
218	220		require.Zero(t, x.tab.lookup(x.xs.win))
219	221		}
	222	+
	223	+	func TestCursorThemeNameRuneBoundary(t *testing.T) {
	224	+	x := &XWM{}
	225	+	x.cfg.CursorTheme = strings.Repeat("a", xsMaxString-1) + "é" // é straddles the limit
	226	+	got := x.cursorThemeName()
	227	+	require.True(t, utf8.ValidString(got))
	228	+	require.Equal(t, strings.Repeat("a", xsMaxString-1), got)
	229	+	}


@@ -3,7 +3,9 @@ package xwm
3	import (	3		import (
4	"context"	4		"context"
5	"encoding/binary"	5		"encoding/binary"
		6	+	"strings"
6	"testing"	7		"testing"
		8	+	"unicode/utf8"
7		9
8	"github.com/bnema/neferwl/internal/ports"	10		"github.com/bnema/neferwl/internal/ports"
9	"github.com/stretchr/testify/require"	11		"github.com/stretchr/testify/require"

@@ -217,3 +219,11 @@ func TestXSettingsWindowNotAdopted(t *testing.T) {
217	x.flush()	219		x.flush()
218	require.Zero(t, x.tab.lookup(x.xs.win))	220		require.Zero(t, x.tab.lookup(x.xs.win))
219	}	221		}
		222	+
		223	+	func TestCursorThemeNameRuneBoundary(t *testing.T) {
		224	+	x := &XWM{}
		225	+	x.cfg.CursorTheme = strings.Repeat("a", xsMaxString-1) + "é" // é straddles the limit
		226	+	got := x.cursorThemeName()
		227	+	require.True(t, utf8.ValidString(got))
		228	+	require.Equal(t, strings.Repeat("a", xsMaxString-1), got)
		229	+	}

M internal/adapters/wayland/xwm/xwm.go +1 -0


@@ -427,6 +427,7 @@ func (x *XWM) Close() {
427	427		// CloseConn closes the connection of an XWM that was never attached.
428	428		func (x *XWM) CloseConn() {
429	429		if !x.closed {
	430	+	x.selClose()
430	431		x.closed = true
431	432		_ = x.conn.Close()
432	433		}


@@ -427,6 +427,7 @@ func (x *XWM) Close() {
427	// CloseConn closes the connection of an XWM that was never attached.	427		// CloseConn closes the connection of an XWM that was never attached.
428	func (x *XWM) CloseConn() {	428		func (x *XWM) CloseConn() {
429	if !x.closed {	429		if !x.closed {
		430	+	x.selClose()
430	x.closed = true	431		x.closed = true
431	_ = x.conn.Close()	432		_ = x.conn.Close()
432	}	433		}