fix(tray): parse protocol numbers with ParseUint and dial the control connection outside the lock
- commit
- 040c5886f0b7c193eb7384c9d0bfec3b55457b1e
- author
- brice · · 2026-10-09 04:01 UTC
- parent
- e357010
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +98 -37
M internal/tray/control.go +40 -24
| @@ -63,10 +63,14 @@ func parseCommand(line string) (command, bool) { | |||
| 63 | 63 | if len(f) == 0 { | |
| 64 | 64 | return command{}, false | |
| 65 | 65 | } | |
| 66 | - | num := func(s string, limit int) (int, bool) { | |
| 67 | - | n, err := strconv.Atoi(s) | |
| 68 | - | return n, err == nil && n >= 0 && n <= limit && s[0] != '+' | |
| 66 | + | // Plain decimal digits only: ParseUint takes no sign, and the bit size | |
| 67 | + | // keeps the value in range on 32-bit platforms, where int is 32 bits. | |
| 68 | + | num := func(s string, bits int, limit uint64) (uint64, bool) { | |
| 69 | + | n, err := strconv.ParseUint(s, 10, bits) | |
| 70 | + | return n, err == nil && n <= limit | |
| 69 | 71 | } | |
| 72 | + | // Columns are bounded by 1<<20; tokens use the full uint32. | |
| 73 | + | col := func(s string) (int, bool) { n, ok := num(s, 31, 1<<20); return int(n), ok } | |
| 70 | 74 | c := command{verb: f[0]} | |
| 71 | 75 | switch f[0] { | |
| 72 | 76 | case "click": | |
| @@ -83,12 +87,12 @@ func parseCommand(line string) (command, bool) { | |||
| 83 | 87 | default: | |
| 84 | 88 | return command{}, false | |
| 85 | 89 | } | |
| 86 | - | col, ok1 := num(f[2], 1<<20) | |
| 87 | - | tok, ok2 := num(f[3], 1<<32-1) | |
| 90 | + | cl, ok1 := col(f[2]) | |
| 91 | + | tok, ok2 := num(f[3], 32, 1<<32-1) | |
| 88 | 92 | if !ok1 || !ok2 { | |
| 89 | 93 | return command{}, false | |
| 90 | 94 | } | |
| 91 | - | c.col, c.token = col, uint32(tok) | |
| 95 | + | c.col, c.token = cl, uint32(tok) | |
| 92 | 96 | case "scroll": | |
| 93 | 97 | if len(f) != 4 { | |
| 94 | 98 | return command{}, false | |
| @@ -105,18 +109,18 @@ func parseCommand(line string) (command, bool) { | |||
| 105 | 109 | default: | |
| 106 | 110 | return command{}, false | |
| 107 | 111 | } | |
| 108 | - | steps, ok1 := num(f[2], maxSteps) | |
| 109 | - | col, ok2 := num(f[3], 1<<20) | |
| 112 | + | steps, ok1 := num(f[2], 31, maxSteps) | |
| 113 | + | cl, ok2 := col(f[3]) | |
| 110 | 114 | if !ok1 || !ok2 || steps == 0 { | |
| 111 | 115 | return command{}, false | |
| 112 | 116 | } | |
| 113 | - | c.delta, c.col = sign*int32(steps), col | |
| 117 | + | c.delta, c.col = sign*int32(steps), cl | |
| 114 | 118 | case "hover": | |
| 115 | - | col, ok := num(f[len(f)-1], 1<<20) | |
| 119 | + | cl, ok := col(f[len(f)-1]) | |
| 116 | 120 | if len(f) != 2 || !ok { | |
| 117 | 121 | return command{}, false | |
| 118 | 122 | } | |
| 119 | - | c.col = col | |
| 123 | + | c.col = cl | |
| 120 | 124 | case "leave": | |
| 121 | 125 | if len(f) != 1 { | |
| 122 | 126 | return command{}, false | |
| @@ -125,14 +129,14 @@ func parseCommand(line string) (command, bool) { | |||
| 125 | 129 | if len(f) != 3 { | |
| 126 | 130 | return command{}, false | |
| 127 | 131 | } | |
| 128 | - | tok, ok1 := num(f[1], 1<<32-1) | |
| 129 | - | id, ok2 := num(f[2], 1<<31-1) | |
| 132 | + | tok, ok1 := num(f[1], 32, 1<<32-1) | |
| 133 | + | id, ok2 := num(f[2], 31, 1<<31-1) | |
| 130 | 134 | if !ok1 || !ok2 { | |
| 131 | 135 | return command{}, false | |
| 132 | 136 | } | |
| 133 | - | c.token, c.col = uint32(tok), id // col carries the menu item id | |
| 137 | + | c.token, c.col = uint32(tok), int(id) // col carries the menu item id | |
| 134 | 138 | case "menu-closed": | |
| 135 | - | tok, ok := num(f[len(f)-1], 1<<32-1) | |
| 139 | + | tok, ok := num(f[len(f)-1], 32, 1<<32-1) | |
| 136 | 140 | if len(f) != 2 || !ok { | |
| 137 | 141 | return command{}, false | |
| 138 | 142 | } | |
| @@ -215,21 +219,33 @@ func (c *control) close() { | |||
| 215 | 219 | } | |
| 216 | 220 | } | |
| 217 | 221 | ||
| 218 | - | // conn2 returns the control connection, dialing it when needed. | |
| 222 | + | // conn2 returns the control connection, dialing it when needed. The dial runs | |
| 223 | + | // outside the lock, so shutdown never waits for it: if shutdown began meanwhile | |
| 224 | + | // the new connection is closed and ErrClosed is returned. | |
| 219 | 225 | func (c *control) conn2() (*zerobus.Conn, error) { | |
| 220 | 226 | c.mu.Lock() | |
| 221 | - | defer c.mu.Unlock() | |
| 222 | 227 | if c.done { | |
| 228 | + | c.mu.Unlock() | |
| 223 | 229 | return nil, zerobus.ErrClosed | |
| 224 | 230 | } | |
| 225 | - | if c.conn == nil { | |
| 226 | - | conn, err := c.dial() | |
| 227 | - | if err != nil { | |
| 228 | - | return nil, err | |
| 229 | - | } | |
| 230 | - | c.conn = conn | |
| 231 | + | if conn := c.conn; conn != nil { | |
| 232 | + | c.mu.Unlock() | |
| 233 | + | return conn, nil | |
| 234 | + | } | |
| 235 | + | c.mu.Unlock() | |
| 236 | + | ||
| 237 | + | conn, err := c.dial() | |
| 238 | + | if err != nil { | |
| 239 | + | return nil, err | |
| 240 | + | } | |
| 241 | + | c.mu.Lock() | |
| 242 | + | defer c.mu.Unlock() | |
| 243 | + | if c.done { | |
| 244 | + | _ = conn.Close() | |
| 245 | + | return nil, zerobus.ErrClosed | |
| 231 | 246 | } | |
| 232 | - | return c.conn, nil | |
| 247 | + | c.conn = conn | |
| 248 | + | return conn, nil | |
| 233 | 249 | } | |
| 234 | 250 | ||
| 235 | 251 | // drop forgets conn after a transport error; the next call dials again. | |
| @@ -63,10 +63,14 @@ func parseCommand(line string) (command, bool) { | |||||
| 63 | if len(f) == 0 { | 63 | if len(f) == 0 { | ||
| 64 | return command{}, false | 64 | return command{}, false | ||
| 65 | } | 65 | } | ||
| 66 | - | num := func(s string, limit int) (int, bool) { | 66 | + | // Plain decimal digits only: ParseUint takes no sign, and the bit size |
| 67 | - | n, err := strconv.Atoi(s) | 67 | + | // keeps the value in range on 32-bit platforms, where int is 32 bits. |
| 68 | - | return n, err == nil && n >= 0 && n <= limit && s[0] != '+' | 68 | + | num := func(s string, bits int, limit uint64) (uint64, bool) { |
| 69 | + | n, err := strconv.ParseUint(s, 10, bits) | |||
| 70 | + | return n, err == nil && n <= limit | |||
| 69 | } | 71 | } | ||
| 72 | + | // Columns are bounded by 1<<20; tokens use the full uint32. | |||
| 73 | + | col := func(s string) (int, bool) { n, ok := num(s, 31, 1<<20); return int(n), ok } | |||
| 70 | c := command{verb: f[0]} | 74 | c := command{verb: f[0]} | ||
| 71 | switch f[0] { | 75 | switch f[0] { | ||
| 72 | case "click": | 76 | case "click": | ||
| @@ -83,12 +87,12 @@ func parseCommand(line string) (command, bool) { | |||||
| 83 | default: | 87 | default: | ||
| 84 | return command{}, false | 88 | return command{}, false | ||
| 85 | } | 89 | } | ||
| 86 | - | col, ok1 := num(f[2], 1<<20) | 90 | + | cl, ok1 := col(f[2]) |
| 87 | - | tok, ok2 := num(f[3], 1<<32-1) | 91 | + | tok, ok2 := num(f[3], 32, 1<<32-1) |
| 88 | if !ok1 || !ok2 { | 92 | if !ok1 || !ok2 { | ||
| 89 | return command{}, false | 93 | return command{}, false | ||
| 90 | } | 94 | } | ||
| 91 | - | c.col, c.token = col, uint32(tok) | 95 | + | c.col, c.token = cl, uint32(tok) |
| 92 | case "scroll": | 96 | case "scroll": | ||
| 93 | if len(f) != 4 { | 97 | if len(f) != 4 { | ||
| 94 | return command{}, false | 98 | return command{}, false | ||
| @@ -105,18 +109,18 @@ func parseCommand(line string) (command, bool) { | |||||
| 105 | default: | 109 | default: | ||
| 106 | return command{}, false | 110 | return command{}, false | ||
| 107 | } | 111 | } | ||
| 108 | - | steps, ok1 := num(f[2], maxSteps) | 112 | + | steps, ok1 := num(f[2], 31, maxSteps) |
| 109 | - | col, ok2 := num(f[3], 1<<20) | 113 | + | cl, ok2 := col(f[3]) |
| 110 | if !ok1 || !ok2 || steps == 0 { | 114 | if !ok1 || !ok2 || steps == 0 { | ||
| 111 | return command{}, false | 115 | return command{}, false | ||
| 112 | } | 116 | } | ||
| 113 | - | c.delta, c.col = sign*int32(steps), col | 117 | + | c.delta, c.col = sign*int32(steps), cl |
| 114 | case "hover": | 118 | case "hover": | ||
| 115 | - | col, ok := num(f[len(f)-1], 1<<20) | 119 | + | cl, ok := col(f[len(f)-1]) |
| 116 | if len(f) != 2 || !ok { | 120 | if len(f) != 2 || !ok { | ||
| 117 | return command{}, false | 121 | return command{}, false | ||
| 118 | } | 122 | } | ||
| 119 | - | c.col = col | 123 | + | c.col = cl |
| 120 | case "leave": | 124 | case "leave": | ||
| 121 | if len(f) != 1 { | 125 | if len(f) != 1 { | ||
| 122 | return command{}, false | 126 | return command{}, false | ||
| @@ -125,14 +129,14 @@ func parseCommand(line string) (command, bool) { | |||||
| 125 | if len(f) != 3 { | 129 | if len(f) != 3 { | ||
| 126 | return command{}, false | 130 | return command{}, false | ||
| 127 | } | 131 | } | ||
| 128 | - | tok, ok1 := num(f[1], 1<<32-1) | 132 | + | tok, ok1 := num(f[1], 32, 1<<32-1) |
| 129 | - | id, ok2 := num(f[2], 1<<31-1) | 133 | + | id, ok2 := num(f[2], 31, 1<<31-1) |
| 130 | if !ok1 || !ok2 { | 134 | if !ok1 || !ok2 { | ||
| 131 | return command{}, false | 135 | return command{}, false | ||
| 132 | } | 136 | } | ||
| 133 | - | c.token, c.col = uint32(tok), id // col carries the menu item id | 137 | + | c.token, c.col = uint32(tok), int(id) // col carries the menu item id |
| 134 | case "menu-closed": | 138 | case "menu-closed": | ||
| 135 | - | tok, ok := num(f[len(f)-1], 1<<32-1) | 139 | + | tok, ok := num(f[len(f)-1], 32, 1<<32-1) |
| 136 | if len(f) != 2 || !ok { | 140 | if len(f) != 2 || !ok { | ||
| 137 | return command{}, false | 141 | return command{}, false | ||
| 138 | } | 142 | } | ||
| @@ -215,21 +219,33 @@ func (c *control) close() { | |||||
| 215 | } | 219 | } | ||
| 216 | } | 220 | } | ||
| 217 | 221 | ||||
| 218 | - | // conn2 returns the control connection, dialing it when needed. | 222 | + | // conn2 returns the control connection, dialing it when needed. The dial runs |
| 223 | + | // outside the lock, so shutdown never waits for it: if shutdown began meanwhile | |||
| 224 | + | // the new connection is closed and ErrClosed is returned. | |||
| 219 | func (c *control) conn2() (*zerobus.Conn, error) { | 225 | func (c *control) conn2() (*zerobus.Conn, error) { | ||
| 220 | c.mu.Lock() | 226 | c.mu.Lock() | ||
| 221 | - | defer c.mu.Unlock() | |||
| 222 | if c.done { | 227 | if c.done { | ||
| 228 | + | c.mu.Unlock() | |||
| 223 | return nil, zerobus.ErrClosed | 229 | return nil, zerobus.ErrClosed | ||
| 224 | } | 230 | } | ||
| 225 | - | if c.conn == nil { | 231 | + | if conn := c.conn; conn != nil { |
| 226 | - | conn, err := c.dial() | 232 | + | c.mu.Unlock() |
| 227 | - | if err != nil { | 233 | + | return conn, nil |
| 228 | - | return nil, err | 234 | + | } |
| 229 | - | } | 235 | + | c.mu.Unlock() |
| 230 | - | c.conn = conn | 236 | + | |
| 237 | + | conn, err := c.dial() | |||
| 238 | + | if err != nil { | |||
| 239 | + | return nil, err | |||
| 240 | + | } | |||
| 241 | + | c.mu.Lock() | |||
| 242 | + | defer c.mu.Unlock() | |||
| 243 | + | if c.done { | |||
| 244 | + | _ = conn.Close() | |||
| 245 | + | return nil, zerobus.ErrClosed | |||
| 231 | } | 246 | } | ||
| 232 | - | return c.conn, nil | 247 | + | c.conn = conn |
| 248 | + | return conn, nil | |||
| 233 | } | 249 | } | ||
| 234 | 250 | ||||
| 235 | // drop forgets conn after a transport error; the next call dials again. | 251 | // drop forgets conn after a transport error; the next call dials again. | ||
M internal/tray/control_test.go +58 -13
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package tray | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "errors" | |
| 4 | 5 | "io" | |
| 5 | 6 | "strings" | |
| 6 | 7 | "testing" | |
| @@ -11,18 +12,21 @@ import ( | |||
| 11 | 12 | ||
| 12 | 13 | func TestParseCommand(t *testing.T) { | |
| 13 | 14 | good := map[string]command{ | |
| 14 | - | "click left 0 1": {verb: "click", button: btnLeft, col: 0, token: 1}, | |
| 15 | - | "click middle 3 42": {verb: "click", button: btnMiddle, col: 3, token: 42}, | |
| 16 | - | "click right 12 7": {verb: "click", button: btnRight, col: 12, token: 7}, | |
| 17 | - | "scroll up 2 0": {verb: "scroll", vert: true, delta: -2}, | |
| 18 | - | "scroll down 1 4": {verb: "scroll", vert: true, delta: 1, col: 4}, | |
| 19 | - | "scroll left 3 1": {verb: "scroll", delta: -3, col: 1}, | |
| 20 | - | "scroll right 5 2": {verb: "scroll", delta: 5, col: 2}, | |
| 21 | - | "hover 4": {verb: "hover", col: 4}, | |
| 22 | - | "leave": {verb: "leave"}, | |
| 23 | - | "menu-activate 9 17": {verb: "menu-activate", token: 9, col: 17}, | |
| 24 | - | "menu-closed 9": {verb: "menu-closed", token: 9}, | |
| 25 | - | " click left 1 2 ": {verb: "click", button: btnLeft, col: 1, token: 2}, | |
| 15 | + | "click left 0 1": {verb: "click", button: btnLeft, col: 0, token: 1}, | |
| 16 | + | "click left 0 4294967295": {verb: "click", button: btnLeft, token: 1<<32 - 1}, | |
| 17 | + | "click left 1048576 1": {verb: "click", button: btnLeft, col: 1 << 20, token: 1}, | |
| 18 | + | "menu-activate 4294967295 2147483647": {verb: "menu-activate", token: 1<<32 - 1, col: 1<<31 - 1}, | |
| 19 | + | "click middle 3 42": {verb: "click", button: btnMiddle, col: 3, token: 42}, | |
| 20 | + | "click right 12 7": {verb: "click", button: btnRight, col: 12, token: 7}, | |
| 21 | + | "scroll up 2 0": {verb: "scroll", vert: true, delta: -2}, | |
| 22 | + | "scroll down 1 4": {verb: "scroll", vert: true, delta: 1, col: 4}, | |
| 23 | + | "scroll left 3 1": {verb: "scroll", delta: -3, col: 1}, | |
| 24 | + | "scroll right 5 2": {verb: "scroll", delta: 5, col: 2}, | |
| 25 | + | "hover 4": {verb: "hover", col: 4}, | |
| 26 | + | "leave": {verb: "leave"}, | |
| 27 | + | "menu-activate 9 17": {verb: "menu-activate", token: 9, col: 17}, | |
| 28 | + | "menu-closed 9": {verb: "menu-closed", token: 9}, | |
| 29 | + | " click left 1 2 ": {verb: "click", button: btnLeft, col: 1, token: 2}, | |
| 26 | 30 | } | |
| 27 | 31 | for line, want := range good { | |
| 28 | 32 | got, ok := parseCommand(line) | |
| @@ -36,6 +40,11 @@ func TestParseCommand(t *testing.T) { | |||
| 36 | 40 | "scroll up", "scroll up 0 0", "scroll up -1 0", "scroll up 2", "scroll diagonal 1 0", "scroll up 1000000 0", | |
| 37 | 41 | "hover", "hover x", "hover -1", "hover 1 2", "leave now", "menu-activate 1", "menu-activate 1 -2", | |
| 38 | 42 | "menu-closed", "menu-closed x", "menu-closed 1 2", "CLICK left 0 1", | |
| 43 | + | // Signs are refused, and so is anything past 31 bits for columns and steps. | |
| 44 | + | "click left 0 +1", "click left +0 1", "click left 0 -0", "scroll up +2 0", "scroll up 2 +0", "hover +1", | |
| 45 | + | "menu-activate +1 2", "menu-activate 1 +2", "menu-closed +1", "menu-closed -1", | |
| 46 | + | "click left 2147483648 1", "scroll up 1 2147483648", "hover 2147483648", "hover 1048577", | |
| 47 | + | "menu-activate 1 2147483648", "click left 0 0x1", "click left 0 1_0", "click left 0 1e3", | |
| 39 | 48 | } { | |
| 40 | 49 | if got, ok := parseCommand(line); ok { | |
| 41 | 50 | t.Errorf("parseCommand(%q) = %+v, want it refused", line, got) | |
| @@ -172,10 +181,46 @@ func TestFrozenApplicationDoesNotBlockNextClick(t *testing.T) { | |||
| 172 | 181 | in.send(t, "click left 0 1", "click left 2 2") | |
| 173 | 182 | expectCall(t, steam, "Activate(0,0)") | |
| 174 | 183 | expectCall(t, obs, "Activate(0,0)") | |
| 175 | - | if d := time.Since(start); d < 1500*time.Millisecond || d > 2500*time.Millisecond { | |
| 184 | + | if d := time.Since(start); d < 1500*time.Millisecond || d > 3500*time.Millisecond { | |
| 176 | 185 | t.Fatalf("the second click arrived after %v, want about 2s (the watchdog)", d) | |
| 177 | 186 | } | |
| 178 | 187 | // And again on the dropped item: the connection was redialed. | |
| 179 | 188 | in.send(t, "click middle 2 3") | |
| 180 | 189 | expectCall(t, obs, "SecondaryActivate(0,0)") | |
| 181 | 190 | } | |
| 191 | + | ||
| 192 | + | // If shutdown begins while the control connection is being dialed, the new | |
| 193 | + | // connection is closed and not kept. | |
| 194 | + | func TestConn2ClosesADialThatRacesShutdown(t *testing.T) { | |
| 195 | + | addr := privateBus(t) | |
| 196 | + | dialing, release := make(chan struct{}), make(chan struct{}) | |
| 197 | + | var dialed *zerobus.Conn | |
| 198 | + | c := &control{dial: func() (*zerobus.Conn, error) { | |
| 199 | + | close(dialing) | |
| 200 | + | <-release | |
| 201 | + | conn, err := zerobus.Dial(addr) | |
| 202 | + | dialed = conn | |
| 203 | + | return conn, err | |
| 204 | + | }} | |
| 205 | + | errc := make(chan error, 1) | |
| 206 | + | go func() { _, err := c.conn2(); errc <- err }() | |
| 207 | + | <-dialing | |
| 208 | + | closed := make(chan struct{}) | |
| 209 | + | go func() { c.close(); close(closed) }() | |
| 210 | + | select { | |
| 211 | + | case <-closed: // close must not wait for the dial | |
| 212 | + | case <-time.After(time.Second): | |
| 213 | + | t.Fatal("close waited for the dial") | |
| 214 | + | } | |
| 215 | + | close(release) | |
| 216 | + | if err := <-errc; !errors.Is(err, zerobus.ErrClosed) { | |
| 217 | + | t.Fatalf("conn2 = %v, want ErrClosed", err) | |
| 218 | + | } | |
| 219 | + | if c.conn != nil { | |
| 220 | + | t.Fatal("the connection was kept after shutdown") | |
| 221 | + | } | |
| 222 | + | dialed.NewCall("org.freedesktop.DBus", "/org/freedesktop/DBus", "org.freedesktop.DBus", "GetId", "") | |
| 223 | + | if _, err := dialed.Call(); err == nil { | |
| 224 | + | t.Fatal("the connection dialed during shutdown is still open") | |
| 225 | + | } | |
| 226 | + | } | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package tray | 1 | package tray | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "errors" | |||
| 4 | "io" | 5 | "io" | ||
| 5 | "strings" | 6 | "strings" | ||
| 6 | "testing" | 7 | "testing" | ||
| @@ -11,18 +12,21 @@ import ( | |||||
| 11 | 12 | ||||
| 12 | func TestParseCommand(t *testing.T) { | 13 | func TestParseCommand(t *testing.T) { | ||
| 13 | good := map[string]command{ | 14 | good := map[string]command{ | ||
| 14 | - | "click left 0 1": {verb: "click", button: btnLeft, col: 0, token: 1}, | 15 | + | "click left 0 1": {verb: "click", button: btnLeft, col: 0, token: 1}, |
| 15 | - | "click middle 3 42": {verb: "click", button: btnMiddle, col: 3, token: 42}, | 16 | + | "click left 0 4294967295": {verb: "click", button: btnLeft, token: 1<<32 - 1}, |
| 16 | - | "click right 12 7": {verb: "click", button: btnRight, col: 12, token: 7}, | 17 | + | "click left 1048576 1": {verb: "click", button: btnLeft, col: 1 << 20, token: 1}, |
| 17 | - | "scroll up 2 0": {verb: "scroll", vert: true, delta: -2}, | 18 | + | "menu-activate 4294967295 2147483647": {verb: "menu-activate", token: 1<<32 - 1, col: 1<<31 - 1}, |
| 18 | - | "scroll down 1 4": {verb: "scroll", vert: true, delta: 1, col: 4}, | 19 | + | "click middle 3 42": {verb: "click", button: btnMiddle, col: 3, token: 42}, |
| 19 | - | "scroll left 3 1": {verb: "scroll", delta: -3, col: 1}, | 20 | + | "click right 12 7": {verb: "click", button: btnRight, col: 12, token: 7}, |
| 20 | - | "scroll right 5 2": {verb: "scroll", delta: 5, col: 2}, | 21 | + | "scroll up 2 0": {verb: "scroll", vert: true, delta: -2}, |
| 21 | - | "hover 4": {verb: "hover", col: 4}, | 22 | + | "scroll down 1 4": {verb: "scroll", vert: true, delta: 1, col: 4}, |
| 22 | - | "leave": {verb: "leave"}, | 23 | + | "scroll left 3 1": {verb: "scroll", delta: -3, col: 1}, |
| 23 | - | "menu-activate 9 17": {verb: "menu-activate", token: 9, col: 17}, | 24 | + | "scroll right 5 2": {verb: "scroll", delta: 5, col: 2}, |
| 24 | - | "menu-closed 9": {verb: "menu-closed", token: 9}, | 25 | + | "hover 4": {verb: "hover", col: 4}, |
| 25 | - | " click left 1 2 ": {verb: "click", button: btnLeft, col: 1, token: 2}, | 26 | + | "leave": {verb: "leave"}, |
| 27 | + | "menu-activate 9 17": {verb: "menu-activate", token: 9, col: 17}, | |||
| 28 | + | "menu-closed 9": {verb: "menu-closed", token: 9}, | |||
| 29 | + | " click left 1 2 ": {verb: "click", button: btnLeft, col: 1, token: 2}, | |||
| 26 | } | 30 | } | ||
| 27 | for line, want := range good { | 31 | for line, want := range good { | ||
| 28 | got, ok := parseCommand(line) | 32 | got, ok := parseCommand(line) | ||
| @@ -36,6 +40,11 @@ func TestParseCommand(t *testing.T) { | |||||
| 36 | "scroll up", "scroll up 0 0", "scroll up -1 0", "scroll up 2", "scroll diagonal 1 0", "scroll up 1000000 0", | 40 | "scroll up", "scroll up 0 0", "scroll up -1 0", "scroll up 2", "scroll diagonal 1 0", "scroll up 1000000 0", | ||
| 37 | "hover", "hover x", "hover -1", "hover 1 2", "leave now", "menu-activate 1", "menu-activate 1 -2", | 41 | "hover", "hover x", "hover -1", "hover 1 2", "leave now", "menu-activate 1", "menu-activate 1 -2", | ||
| 38 | "menu-closed", "menu-closed x", "menu-closed 1 2", "CLICK left 0 1", | 42 | "menu-closed", "menu-closed x", "menu-closed 1 2", "CLICK left 0 1", | ||
| 43 | + | // Signs are refused, and so is anything past 31 bits for columns and steps. | |||
| 44 | + | "click left 0 +1", "click left +0 1", "click left 0 -0", "scroll up +2 0", "scroll up 2 +0", "hover +1", | |||
| 45 | + | "menu-activate +1 2", "menu-activate 1 +2", "menu-closed +1", "menu-closed -1", | |||
| 46 | + | "click left 2147483648 1", "scroll up 1 2147483648", "hover 2147483648", "hover 1048577", | |||
| 47 | + | "menu-activate 1 2147483648", "click left 0 0x1", "click left 0 1_0", "click left 0 1e3", | |||
| 39 | } { | 48 | } { | ||
| 40 | if got, ok := parseCommand(line); ok { | 49 | if got, ok := parseCommand(line); ok { | ||
| 41 | t.Errorf("parseCommand(%q) = %+v, want it refused", line, got) | 50 | t.Errorf("parseCommand(%q) = %+v, want it refused", line, got) | ||
| @@ -172,10 +181,46 @@ func TestFrozenApplicationDoesNotBlockNextClick(t *testing.T) { | |||||
| 172 | in.send(t, "click left 0 1", "click left 2 2") | 181 | in.send(t, "click left 0 1", "click left 2 2") | ||
| 173 | expectCall(t, steam, "Activate(0,0)") | 182 | expectCall(t, steam, "Activate(0,0)") | ||
| 174 | expectCall(t, obs, "Activate(0,0)") | 183 | expectCall(t, obs, "Activate(0,0)") | ||
| 175 | - | if d := time.Since(start); d < 1500*time.Millisecond || d > 2500*time.Millisecond { | 184 | + | if d := time.Since(start); d < 1500*time.Millisecond || d > 3500*time.Millisecond { |
| 176 | t.Fatalf("the second click arrived after %v, want about 2s (the watchdog)", d) | 185 | t.Fatalf("the second click arrived after %v, want about 2s (the watchdog)", d) | ||
| 177 | } | 186 | } | ||
| 178 | // And again on the dropped item: the connection was redialed. | 187 | // And again on the dropped item: the connection was redialed. | ||
| 179 | in.send(t, "click middle 2 3") | 188 | in.send(t, "click middle 2 3") | ||
| 180 | expectCall(t, obs, "SecondaryActivate(0,0)") | 189 | expectCall(t, obs, "SecondaryActivate(0,0)") | ||
| 181 | } | 190 | } | ||
| 191 | + | ||||
| 192 | + | // If shutdown begins while the control connection is being dialed, the new | |||
| 193 | + | // connection is closed and not kept. | |||
| 194 | + | func TestConn2ClosesADialThatRacesShutdown(t *testing.T) { | |||
| 195 | + | addr := privateBus(t) | |||
| 196 | + | dialing, release := make(chan struct{}), make(chan struct{}) | |||
| 197 | + | var dialed *zerobus.Conn | |||
| 198 | + | c := &control{dial: func() (*zerobus.Conn, error) { | |||
| 199 | + | close(dialing) | |||
| 200 | + | <-release | |||
| 201 | + | conn, err := zerobus.Dial(addr) | |||
| 202 | + | dialed = conn | |||
| 203 | + | return conn, err | |||
| 204 | + | }} | |||
| 205 | + | errc := make(chan error, 1) | |||
| 206 | + | go func() { _, err := c.conn2(); errc <- err }() | |||
| 207 | + | <-dialing | |||
| 208 | + | closed := make(chan struct{}) | |||
| 209 | + | go func() { c.close(); close(closed) }() | |||
| 210 | + | select { | |||
| 211 | + | case <-closed: // close must not wait for the dial | |||
| 212 | + | case <-time.After(time.Second): | |||
| 213 | + | t.Fatal("close waited for the dial") | |||
| 214 | + | } | |||
| 215 | + | close(release) | |||
| 216 | + | if err := <-errc; !errors.Is(err, zerobus.ErrClosed) { | |||
| 217 | + | t.Fatalf("conn2 = %v, want ErrClosed", err) | |||
| 218 | + | } | |||
| 219 | + | if c.conn != nil { | |||
| 220 | + | t.Fatal("the connection was kept after shutdown") | |||
| 221 | + | } | |||
| 222 | + | dialed.NewCall("org.freedesktop.DBus", "/org/freedesktop/DBus", "org.freedesktop.DBus", "GetId", "") | |||
| 223 | + | if _, err := dialed.Call(); err == nil { | |||
| 224 | + | t.Fatal("the connection dialed during shutdown is still open") | |||
| 225 | + | } | |||
| 226 | + | } | |||