Skip to content

Commit

Permalink
[v15] Fix panic in app dialing (#42786)
Browse files Browse the repository at this point in the history
* Fix panic in app dialing

#35501 incorrectly
checked the length on the local servers variable instead of
`t.c.servers` which could lead to panics like the one below.

```bash

panic: runtime error: slice bounds out of range [1:0]
goroutine 6558252 [running]:
github.com/gravitational/teleport/lib/web/app.(*transport).DialContext(0xc00296d5f0, {0xae8bbd8, 0xc002596a20}, {0x42c525?, 0xc001dc4d80?}, {0x120?, 0x118?})
    github.com/gravitational/teleport/lib/web/app/transport.go:264 +0x5dc
net/http.(*Transport).dial(0xc002596a20?, {0xae8bbd8?, 0xc002596a20?}, {0x92ff0a2?, 0x4e4fc13?}, {0xc000b8dfc0?, 0xc00226f000?})
    net/http/transport.go:1183 +0xd2
net/http.(*Transport).dialConn(0xc002f9cb40, {0xae8bbd8, 0xc002596a20}, {{}, 0x0, {0x9301a85, 0x5}, {0xc000b8dfc0, 0x1a}, 0x0})
    net/http/transport.go:1625 +0x7e8
net/http.(*Transport).dialConnFor(0xae8bc10?, 0xc003340370)
    net/http/transport.go:1467 +0x9f
created by net/http.(*Transport).queueForDial in goroutine 6562349
    net/http/transport.go:1436 +0x3cb

```

* prevent modifying servers if changed
  • Loading branch information
rosstimothy authored Jun 11, 2024
1 parent d546641 commit 40fe0ad
Showing 1 changed file with 11 additions and 5 deletions.
16 changes: 11 additions & 5 deletions lib/web/app/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,12 +281,18 @@ func (t *transport) DialContext(ctx context.Context, _, _ string) (conn net.Conn
break
}

// eliminate any servers from the head of the list that were unreachable
t.mu.Lock()
if i < len(servers) {
t.c.servers = t.c.servers[i:]
} else {
t.c.servers = nil
// Only attempt to tidy up the list of servers if they weren't altered
// while the dialing happened. Since the lock is only held initially when
// making the servers copy and released during the dials, another dial attempt
// may have already happened and modified the list of servers.
if len(servers) == len(t.c.servers) {
// eliminate any servers from the head of the list that were unreachable
if i < len(t.c.servers) {
t.c.servers = t.c.servers[i:]
} else {
t.c.servers = nil
}
}
t.mu.Unlock()

Expand Down

0 comments on commit 40fe0ad

Please sign in to comment.