Correctly resolve multiple inbound connections to complete-closure
Connect() now prevents Open if there are *any connections* under the assumption that one of them will be resolved valid. (Before connect allowed Open on any error, this was a bug that would occasionally be tripped by multiple Inbounds)
This commit is contained in:
parent
137461e2cd
commit
f1e3f2ca54
|
@ -73,27 +73,44 @@ func (s *BaseOnionService) WaitForCapabilityOrClose(cid string, name tapir.Capab
|
||||||
|
|
||||||
// If We have 2 connections for the same hostname...
|
// If We have 2 connections for the same hostname...
|
||||||
if err != nil {
|
if err != nil {
|
||||||
log.Debugf("found duplicate connections for %v <-> %v", s.id.Hostname(), cid)
|
log.Debugf("found duplicate connections for %v <-> %v %v", s.id.Hostname(), cid, err)
|
||||||
// By convention the lowest lexicographical hostname purges all their outbounds to the higher
|
// By convention the lowest lexicographical hostname purges all their outbounds to the higher
|
||||||
// hostname
|
// hostname
|
||||||
// Which should only leave a single connection remaining (as we dedupe on connect too)
|
// Which should only leave a single connection remaining (as we dedupe on connect too)
|
||||||
// This does allow people to attempt to guarantee being the outbound connection but due to the bidirectional
|
// This does allow people to attempt to guarantee being the outbound connection but due to the bidirectional
|
||||||
// authentication this should never result in an advantage in the protocol.
|
// authentication this should never result in an advantage in the protocol.
|
||||||
|
inboundCount := 0
|
||||||
if s.id.Hostname() < cid {
|
if s.id.Hostname() < cid {
|
||||||
// Close all output connections to connection
|
// Close all outbound connections to connection
|
||||||
s.connections.Range(func(key, value interface{}) bool {
|
s.connections.Range(func(key, value interface{}) bool {
|
||||||
connection := value.(tapir.Connection)
|
connection := value.(tapir.Connection)
|
||||||
if connection.Hostname() == cid {
|
if connection.Hostname() == cid {
|
||||||
if !connection.IsClosed() && connection.IsOutbound() {
|
if !connection.IsClosed() && connection.IsOutbound() {
|
||||||
connection.Close()
|
connection.Close()
|
||||||
|
} else if !connection.IsClosed() && !connection.IsOutbound() {
|
||||||
|
inboundCount++
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return true
|
return true
|
||||||
})
|
})
|
||||||
|
|
||||||
|
// This shouldn't happen honestly, but if it does then it can cause an infinite check here
|
||||||
|
if inboundCount > 1 {
|
||||||
|
s.connections.Range(func(key, value interface{}) bool {
|
||||||
|
connection := value.(tapir.Connection)
|
||||||
|
if connection.Hostname() == cid {
|
||||||
|
if !connection.IsClosed() {
|
||||||
|
connection.Close()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return true
|
||||||
|
})
|
||||||
|
return nil, errors.New("multiple inbound connections found and closed; the only resolution to this is to close them all and try connecting again")
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// We had a connection but it didn't have a cpability...try again
|
// We had a connection but it didn't have a capability...try again
|
||||||
conn, err = s.GetConnection(cid)
|
conn, err = s.GetConnection(cid)
|
||||||
}
|
}
|
||||||
// There are no active connections with that hostname
|
// There are no active connections with that hostname
|
||||||
|
@ -126,8 +143,8 @@ func (s *BaseOnionService) GetConnection(hostname string) (tapir.Connection, err
|
||||||
|
|
||||||
// Connect initializes a new outbound connection to the given peer, using the defined Application
|
// Connect initializes a new outbound connection to the given peer, using the defined Application
|
||||||
func (s *BaseOnionService) Connect(hostname string, app tapir.Application) (bool, error) {
|
func (s *BaseOnionService) Connect(hostname string, app tapir.Application) (bool, error) {
|
||||||
_, err := s.GetConnection(hostname)
|
currconn, _ := s.GetConnection(hostname)
|
||||||
if err == nil {
|
if currconn != nil {
|
||||||
// Note: This check is not 100% reliable. And we may end up with two connections between peers
|
// Note: This check is not 100% reliable. And we may end up with two connections between peers
|
||||||
// This can happen when a client connects to a server as the server is connecting to the client
|
// This can happen when a client connects to a server as the server is connecting to the client
|
||||||
// Because at the start of the connection the server cannot derive the true hostname of the client until it
|
// Because at the start of the connection the server cannot derive the true hostname of the client until it
|
||||||
|
|
Loading…
Reference in New Issue