27f82c1 fix(tray): reject invalid bus names and drop stale watcher lists · neferbar

M internal/config/config.go +1 -1


@@ -140,7 +140,7 @@ func (c *Config) Validate() error {
140	140		for k, v := range c.Tray.Icons {
141	141		// The tray prints the value on its one output line.
142	142		if strings.ContainsFunc(v, unicode.IsControl) {
143		-	return fmt.Errorf("config: tray.icons.%s contains a control character", k)
	143	+	return fmt.Errorf("config: tray.icons.%q contains a control character", k)
144	144		}
145	145		}
146	146		return nil


@@ -140,7 +140,7 @@ func (c *Config) Validate() error {
140		for k, v := range c.Tray.Icons {	140		for k, v := range c.Tray.Icons {
141		// The tray prints the value on its one output line.	141		// The tray prints the value on its one output line.
142		if strings.ContainsFunc(v, unicode.IsControl) {	142		if strings.ContainsFunc(v, unicode.IsControl) {
143	-	return fmt.Errorf("config: tray.icons.%s contains a control character", k)	143	+	return fmt.Errorf("config: tray.icons.%q contains a control character", k)
144		}	144		}
145		}	145		}
146		return nil	146		return nil

M internal/tray/color.go +7 -2


@@ -29,8 +29,13 @@ func (h *histogram) dominant(argb []byte) (RGB, bool) {
29	29		}
30	30		clear(h.weight[:])
31	31		clear(h.sum[:])
32		-	step := 4 * ((len(argb)/4 + maxSamples - 1) / maxSamples)
33		-	for i := 0; i+3 < len(argb); i += step {
	32	+	// An odd pixel step crosses every column of a power-of-two wide icon,
	33	+	// so samples do not line up in a few columns.
	34	+	step := (len(argb)/4 + maxSamples - 1) / maxSamples
	35	+	if step > 1 {
	36	+	step \|= 1
	37	+	}
	38	+	for i := 0; i+3 < len(argb); i += 4 * step {
34	39		a, r, g, b := uint32(argb[i]), uint32(argb[i+1]), uint32(argb[i+2]), uint32(argb[i+3])
35	40		if a < 128 {
36	41		continue

 	}
 	clear(h.weight[:])
 	clear(h.sum[:])
-	step := 4 * ((len(argb)/4 + maxSamples - 1) / maxSamples)
+	// An odd pixel step crosses every column of a power-of-two wide icon,
-	for i := 0; i+3 < len(argb); i += step {
+	// so samples do not line up in a few columns.
+	step := (len(argb)/4 + maxSamples - 1) / maxSamples
+	if step > 1 {
+		step |= 1
+	}
+	for i := 0; i+3 < len(argb); i += 4 * step {
 		a, r, g, b := uint32(argb[i]), uint32(argb[i+1]), uint32(argb[i+2]), uint32(argb[i+3])
 		if a < 128 {
 			continue

M internal/tray/color_test.go +20 -0


@@ -45,6 +45,26 @@ func TestDominantLargeIcon(t *testing.T) {
45	45		}
46	46		}
47	47
	48	+	func TestDominantLargeIconOffColumns(t *testing.T) {
	49	+	// 512x512, transparent but for a red logo in columns 10..49: a stride of
	50	+	// 64 pixels would sample only columns 0, 64, 128... and miss it.
	51	+	const side = 512
	52	+	icon := make([]byte, 0, sideside4)
	53	+	for range side {
	54	+	for x := range side {
	55	+	if x >= 10 && x < 50 {
	56	+	icon = append(icon, 255, 250, 20, 20)
	57	+	} else {
	58	+	icon = append(icon, 0, 0, 0, 0)
	59	+	}
	60	+	}
	61	+	}
	62	+	var h histogram
	63	+	if got, ok := h.dominant(icon); !ok \|\| got != (RGB{250, 20, 20}) {
	64	+	t.Fatalf("dominant = %v, %v", got, ok)
	65	+	}
	66	+	}
	67	+
48	68		func TestReadable(t *testing.T) {
49	69		bg, fg := RGB{0x1e, 0x1e, 0x2e}, RGB{0xcd, 0xd6, 0xf4}
50	70		dark := RGB{0x17, 0x1a, 0x21} // Steam's near-black blue


@@ -45,6 +45,26 @@ func TestDominantLargeIcon(t *testing.T) {
45	}	45		}
46	}	46		}
47		47
		48	+	func TestDominantLargeIconOffColumns(t *testing.T) {
		49	+	// 512x512, transparent but for a red logo in columns 10..49: a stride of
		50	+	// 64 pixels would sample only columns 0, 64, 128... and miss it.
		51	+	const side = 512
		52	+	icon := make([]byte, 0, sideside4)
		53	+	for range side {
		54	+	for x := range side {
		55	+	if x >= 10 && x < 50 {
		56	+	icon = append(icon, 255, 250, 20, 20)
		57	+	} else {
		58	+	icon = append(icon, 0, 0, 0, 0)
		59	+	}
		60	+	}
		61	+	}
		62	+	var h histogram
		63	+	if got, ok := h.dominant(icon); !ok \|\| got != (RGB{250, 20, 20}) {
		64	+	t.Fatalf("dominant = %v, %v", got, ok)
		65	+	}
		66	+	}
		67	+
48	func TestReadable(t *testing.T) {	68		func TestReadable(t *testing.T) {
49	bg, fg := RGB{0x1e, 0x1e, 0x2e}, RGB{0xcd, 0xd6, 0xf4}	69		bg, fg := RGB{0x1e, 0x1e, 0x2e}, RGB{0xcd, 0xd6, 0xf4}
50	dark := RGB{0x17, 0x1a, 0x21} // Steam's near-black blue	70		dark := RGB{0x17, 0x1a, 0x21} // Steam's near-black blue

M internal/tray/resolve.go +21 -9


@@ -22,7 +22,7 @@ type Resolver struct {
22	22		icons map[string]string // lowercased keys
23	23		dataDirs []string
24	24		apps map[string]*desktopApp // nil until the first lookup
25		-	loaded time.Time // when apps was read
	25	+	mtimes []time.Time // of each applications dir when apps was read
26	26		}
27	27
28	28		// desktopApp is what the resolver uses from a .desktop file.

@@ -259,10 +259,23 @@ func (r Resolver) app(id, iconName, title string) desktopApp {
259	259		}
260	260
261	261		func (r *Resolver) reload() {
262		-	r.loaded = time.Now()
	262	+	r.mtimes = r.dirTimes(r.mtimes[:0])
263	263		r.apps = loadApps(r.dataDirs)
264	264		}
265	265
	266	+	// dirTimes appends the mtime of each applications directory, zero when it
	267	+	// is missing.
	268	+	func (r *Resolver) dirTimes(ts []time.Time) []time.Time {
	269	+	for _, d := range r.dataDirs {
	270	+	var t time.Time
	271	+	if st, err := os.Stat(filepath.Join(d, "applications")); d != "" && err == nil {
	272	+	t = st.ModTime()
	273	+	}
	274	+	ts = append(ts, t)
	275	+	}
	276	+	return ts
	277	+	}
	278	+
266	279		// findApp looks up the whole id, icon name and title, then the words of the
267	280		// id and icon name. Titles are free text: their words are not used.
268	281		func (r Resolver) findApp(id, iconName, title string) desktopApp {

@@ -281,14 +294,13 @@ func (r Resolver) findApp(id, iconName, title string) desktopApp {
281	294		return nil
282	295		}
283	296
284		-	// changed reports whether an applications directory was modified after the
285		-	// index was read.
	297	+	// changed reports whether an applications directory changed since the
	298	+	// index was read. Comparing mtimes with themselves, not with the clock,
	299	+	// is safe from clock skew.
286	300		func (r *Resolver) changed() bool {
287		-	for _, d := range r.dataDirs {
288		-	if d == "" {
289		-	continue
290		-	}
291		-	if st, err := os.Stat(filepath.Join(d, "applications")); err == nil && !st.ModTime().Before(r.loaded) {
	301	+	var buf [8]time.Time
	302	+	for i, t := range r.dirTimes(buf[:0]) {
	303	+	if !t.Equal(r.mtimes[i]) {
292	304		return true
293	305		}
294	306		}

 	icons    map[string]string // lowercased keys
 	dataDirs []string
 	apps     map[string]*desktopApp // nil until the first lookup
-	loaded   time.Time              // when apps was read
+	mtimes   []time.Time            // of each applications dir when apps was read
 }
 // desktopApp is what the resolver uses from a .desktop file.
 }
 func (r *Resolver) reload() {
-	r.loaded = time.Now()
+	r.mtimes = r.dirTimes(r.mtimes[:0])
 	r.apps = loadApps(r.dataDirs)
 }
+// dirTimes appends the mtime of each applications directory, zero when it
+// is missing.
+func (r *Resolver) dirTimes(ts []time.Time) []time.Time {
+	for _, d := range r.dataDirs {
+		var t time.Time
+		if st, err := os.Stat(filepath.Join(d, "applications")); d != "" && err == nil {
+			t = st.ModTime()
+		}
+		ts = append(ts, t)
+	}
+	return ts
+}
 // findApp looks up the whole id, icon name and title, then the words of the
 // id and icon name. Titles are free text: their words are not used.
 func (r *Resolver) findApp(id, iconName, title string) *desktopApp {
 	return nil
 }
-// changed reports whether an applications directory was modified after the
+// changed reports whether an applications directory changed since the
-// index was read.
+// index was read. Comparing mtimes with themselves, not with the clock,
+// is safe from clock skew.
 func (r *Resolver) changed() bool {
-	for _, d := range r.dataDirs {
+	var buf [8]time.Time
-		if d == "" {
+	for i, t := range r.dirTimes(buf[:0]) {
-			continue
+		if !t.Equal(r.mtimes[i]) {
-		}
-		if st, err := os.Stat(filepath.Join(d, "applications")); err == nil && !st.ModTime().Before(r.loaded) {
 			return true
 		}
 	}

M internal/tray/resolve_test.go +19 -3


@@ -93,9 +93,10 @@ func TestResolveFindsAppsInstalledLater(t *testing.T) {
93	93		t.Fatalf("before install: %q", got)
94	94		}
95	95		writeDesktop(t, dir, "racer", "[Desktop Entry]\nName=Racer\nCategories=Game;\n")
96		-	// The directory's mtime must move past the load time.
97		-	future := time.Now().Add(time.Second)
98		-	if err := os.Chtimes(filepath.Join(dir, "applications"), future, future); err != nil {
	96	+	// Make sure the directory's mtime differs from the one at the load,
	97	+	// even on a coarse clock.
	98	+	later := time.Now().Add(time.Hour)
	99	+	if err := os.Chtimes(filepath.Join(dir, "applications"), later, later); err != nil {
99	100		t.Fatal(err)
100	101		}
101	102		if got := r.Resolve("racer", "", ""); got != "G" {

@@ -103,6 +104,21 @@ func TestResolveFindsAppsInstalledLater(t *testing.T) {
103	104		}
104	105		}
105	106
	107	+	func TestResolveDoesNotRescanUnchangedDirs(t *testing.T) {
	108	+	// A directory dated in the future does not make every miss rescan.
	109	+	dir := t.TempDir()
	110	+	writeDesktop(t, dir, "racer", "[Desktop Entry]\nName=Racer\nCategories=Game;\n")
	111	+	future := time.Now().Add(24 * time.Hour)
	112	+	if err := os.Chtimes(filepath.Join(dir, "applications"), future, future); err != nil {
	113	+	t.Fatal(err)
	114	+	}
	115	+	r := NewResolver(lookup, nil, []string{dir})
	116	+	r.Resolve("unknown", "", "")
	117	+	if r.changed() {
	118	+	t.Fatal("an unchanged directory counts as changed")
	119	+	}
	120	+	}
	121	+
106	122		func TestEmptyOverrideHides(t *testing.T) {
107	123		r := NewResolver(lookup, map[string]string{"steam": ""}, []string{t.TempDir()})
108	124		if got := r.Resolve("steam", "", ""); got != "" {


@@ -93,9 +93,10 @@ func TestResolveFindsAppsInstalledLater(t *testing.T) {
93		t.Fatalf("before install: %q", got)	93		t.Fatalf("before install: %q", got)
94		}	94		}
95		writeDesktop(t, dir, "racer", "[Desktop Entry]\nName=Racer\nCategories=Game;\n")	95		writeDesktop(t, dir, "racer", "[Desktop Entry]\nName=Racer\nCategories=Game;\n")
96	-	// The directory's mtime must move past the load time.	96	+	// Make sure the directory's mtime differs from the one at the load,
97	-	future := time.Now().Add(time.Second)	97	+	// even on a coarse clock.
98	-	if err := os.Chtimes(filepath.Join(dir, "applications"), future, future); err != nil {	98	+	later := time.Now().Add(time.Hour)
			99	+	if err := os.Chtimes(filepath.Join(dir, "applications"), later, later); err != nil {
99		t.Fatal(err)	100		t.Fatal(err)
100		}	101		}
101		if got := r.Resolve("racer", "", ""); got != "G" {	102		if got := r.Resolve("racer", "", ""); got != "G" {

@@ -103,6 +104,21 @@ func TestResolveFindsAppsInstalledLater(t *testing.T) {
103		}	104		}
104		}	105		}
105			106
			107	+	func TestResolveDoesNotRescanUnchangedDirs(t *testing.T) {
			108	+	// A directory dated in the future does not make every miss rescan.
			109	+	dir := t.TempDir()
			110	+	writeDesktop(t, dir, "racer", "[Desktop Entry]\nName=Racer\nCategories=Game;\n")
			111	+	future := time.Now().Add(24 * time.Hour)
			112	+	if err := os.Chtimes(filepath.Join(dir, "applications"), future, future); err != nil {
			113	+	t.Fatal(err)
			114	+	}
			115	+	r := NewResolver(lookup, nil, []string{dir})
			116	+	r.Resolve("unknown", "", "")
			117	+	if r.changed() {
			118	+	t.Fatal("an unchanged directory counts as changed")
			119	+	}
			120	+	}
			121	+
106		func TestEmptyOverrideHides(t *testing.T) {	122		func TestEmptyOverrideHides(t *testing.T) {
107		r := NewResolver(lookup, map[string]string{"steam": ""}, []string{t.TempDir()})	123		r := NewResolver(lookup, map[string]string{"steam": ""}, []string{t.TempDir()})
108		if got := r.Resolve("steam", "", ""); got != "" {	124		if got := r.Resolve("steam", "", ""); got != "" {

M internal/tray/tray.go +32 -2


@@ -284,7 +284,8 @@ func (t Tray) signal(m zerobus.Message) {
284	284		t.vanished(name)
285	285		case name == watcherName && old != "" && !t.watcher && owner != t.c.UniqueName():
286	286		// Another watcher replaced the one we used: its items are
287		-	// the ones to show now.
	287	+	// the ones to show now, not a late list from the old one.
	288	+	t.pending = slices.DeleteFunc(t.pending, func(p pend) bool { return p.kind == pendItems })
288	289		t.dropAll()
289	290		t.becomeHost()
290	291		}

@@ -356,6 +357,32 @@ func normService(s string) (service, bus, path string) {
356	357		return s, bus, "/" + p
357	358		}
358	359
	360	+	// validBusName reports whether s is a unique (":1.42") or well-known
	361	+	// ("org.example.App") bus name, as D-Bus defines them.
	362	+	func validBusName(s string) bool {
	363	+	if s == "" \|\| len(s) > 255 {
	364	+	return false
	365	+	}
	366	+	unique := s[0] == ':'
	367	+	if unique {
	368	+	s = s[1:]
	369	+	}
	370	+	n := 0
	371	+	for more := true; more; n++ {
	372	+	var e string
	373	+	e, s, more = strings.Cut(s, ".")
	374	+	if e == "" \|\| !unique && e[0] >= '0' && e[0] <= '9' {
	375	+	return false
	376	+	}
	377	+	for _, c := range []byte(e) {
	378	+	if !(c >= 'a' && c <= 'z' \|\| c >= 'A' && c <= 'Z' \|\| c >= '0' && c <= '9' \|\| c == '_' \|\| c == '-') {
	379	+	return false
	380	+	}
	381	+	}
	382	+	}
	383	+	return n >= 2
	384	+	}
	385	+
359	386		// addResult says what add did.
360	387		type addResult uint8
361	388

@@ -533,8 +560,11 @@ func (t Tray) call(m zerobus.Message) {
533	560		}
534	561		if strings.HasPrefix(s, "/") {
535	562		s = m.Sender + s // registered by object path
536		-	} else {
	563	+	} else if validBusName(s) {
537	564		s += itemPath
	565	+	} else {
	566	+	t.errorReply(m, noReply, "org.freedesktop.DBus.Error.InvalidArgs", "invalid service")
	567	+	return
538	568		}
539	569		// add copies s before the next read, and sends only a GetAll.
540	570		switch t.add(s, m.Sender) {


@@ -284,7 +284,8 @@ func (t Tray) signal(m zerobus.Message) {
284		t.vanished(name)	284		t.vanished(name)
285		case name == watcherName && old != "" && !t.watcher && owner != t.c.UniqueName():	285		case name == watcherName && old != "" && !t.watcher && owner != t.c.UniqueName():
286		// Another watcher replaced the one we used: its items are	286		// Another watcher replaced the one we used: its items are
287	-	// the ones to show now.	287	+	// the ones to show now, not a late list from the old one.
			288	+	t.pending = slices.DeleteFunc(t.pending, func(p pend) bool { return p.kind == pendItems })
288		t.dropAll()	289		t.dropAll()
289		t.becomeHost()	290		t.becomeHost()
290		}	291		}

@@ -356,6 +357,32 @@ func normService(s string) (service, bus, path string) {
356		return s, bus, "/" + p	357		return s, bus, "/" + p
357		}	358		}
358			359
			360	+	// validBusName reports whether s is a unique (":1.42") or well-known
			361	+	// ("org.example.App") bus name, as D-Bus defines them.
			362	+	func validBusName(s string) bool {
			363	+	if s == "" \|\| len(s) > 255 {
			364	+	return false
			365	+	}
			366	+	unique := s[0] == ':'
			367	+	if unique {
			368	+	s = s[1:]
			369	+	}
			370	+	n := 0
			371	+	for more := true; more; n++ {
			372	+	var e string
			373	+	e, s, more = strings.Cut(s, ".")
			374	+	if e == "" \|\| !unique && e[0] >= '0' && e[0] <= '9' {
			375	+	return false
			376	+	}
			377	+	for _, c := range []byte(e) {
			378	+	if !(c >= 'a' && c <= 'z' \|\| c >= 'A' && c <= 'Z' \|\| c >= '0' && c <= '9' \|\| c == '_' \|\| c == '-') {
			379	+	return false
			380	+	}
			381	+	}
			382	+	}
			383	+	return n >= 2
			384	+	}
			385	+
359		// addResult says what add did.	386		// addResult says what add did.
360		type addResult uint8	387		type addResult uint8
361			388

@@ -533,8 +560,11 @@ func (t Tray) call(m zerobus.Message) {
533		}	560		}
534		if strings.HasPrefix(s, "/") {	561		if strings.HasPrefix(s, "/") {
535		s = m.Sender + s // registered by object path	562		s = m.Sender + s // registered by object path
536	-	} else {	563	+	} else if validBusName(s) {
537		s += itemPath	564		s += itemPath
			565	+	} else {
			566	+	t.errorReply(m, noReply, "org.freedesktop.DBus.Error.InvalidArgs", "invalid service")
			567	+	return
538		}	568		}
539		// add copies s before the next read, and sends only a GetAll.	569		// add copies s before the next read, and sends only a GetAll.
540		switch t.add(s, m.Sender) {	570		switch t.add(s, m.Sender) {

M internal/tray/tray_test.go +5 -3


@@ -458,9 +458,11 @@ func TestWatcherRegistration(t *testing.T) {
458	458
459	459		// A path D-Bus refuses is rejected and leaves no trace.
460	460		probe := dial(t, addr)
461		-	probe.NewCall(watcherName, watcherPath, watcherIface, "RegisterStatusNotifierItem", "s").Str("/bad//path")
462		-	if _, err := probe.Call(); err == nil {
463		-	t.Fatal("an invalid path was accepted")
	461	+	for _, bad := range []string{"/bad//path", "not a name", "1.starts.with.digit"} {
	462	+	probe.NewCall(watcherName, watcherPath, watcherIface, "RegisterStatusNotifierItem", "s").Str(bad)
	463	+	if _, err := probe.Call(); err == nil {
	464	+	t.Fatalf("%q was accepted", bad)
	465	+	}
464	466		}
465	467		if items := registered(t, probe); len(items) != 1 \|\| items[0] != steam.name+itemPath {
466	468		t.Fatalf("registered items %q", items)

 	// A path D-Bus refuses is rejected and leaves no trace.
 	probe := dial(t, addr)
-	probe.NewCall(watcherName, watcherPath, watcherIface, "RegisterStatusNotifierItem", "s").Str("/bad//path")
+	for _, bad := range []string{"/bad//path", "not a name", "1.starts.with.digit"} {
-	if _, err := probe.Call(); err == nil {
+		probe.NewCall(watcherName, watcherPath, watcherIface, "RegisterStatusNotifierItem", "s").Str(bad)
-		t.Fatal("an invalid path was accepted")
+		if _, err := probe.Call(); err == nil {
+			t.Fatalf("%q was accepted", bad)
+		}
 	}
 	if items := registered(t, probe); len(items) != 1 || items[0] != steam.name+itemPath {
 		t.Fatalf("registered items %q", items)