Keep LAN shortcut from blocking holepunch fallback

lib/connect.js

@@ -219,7 +219,6 @@ async function holepunch(c, opts) {
   let { relayAddress, serverAddress, clientAddress, payload } = c.connect
 
   const remoteHolepunchable = !!(payload.holepunch && payload.holepunch.relays.length)
-
   const relayed = diffAddress(serverAddress, relayAddress)
 
   if (payload.firewall === FIREWALL.OPEN || (relayed && !remoteHolepunchable)) {
@@ -243,7 +242,9 @@ async function holepunch(c, opts) {
     return
   }
 
-  // TODO: would be better to just try local addrs in the background whilst continuing with other strategies...
+  // Race the LAN shortcut with normal holepunch when possible. A blocked LAN ping
+  // should not abort a connection that can still holepunch. If the remote is not
+  // holepunchable, the LAN ping is the only remaining path.
   if (c.lan && relayed && clientAddress.host === serverAddress.host) {
     const serverAddresses = payload.addresses4.filter(onlyNonReserved)
 
@@ -252,14 +253,21 @@ async function holepunch(c, opts) {
       const addr = Holepuncher.matchAddress(myAddresses, serverAddresses) || serverAddresses[0]
 
       const socket = c.dht.io.serverSocket
-      try {
-        await c.dht.ping(addr)
-      } catch {
-        maybeDestroyEncryptedSocket(c, HOLEPUNCH_ABORTED())
-        return
+      c.dht.ping(addr).then(onlanconnect, onlanerror)
+      if (!remoteHolepunchable) return
+
+      function onlanconnect() {
+        // LAN and holepunch completions both converge through onsocket,
+        // which claims c.rawStream once and ignores later winners.
+        if (isDone(c)) return
+        c.onsocket(socket, addr.port, addr.host)
+      }
+
+      function onlanerror() {
+        if (!remoteHolepunchable) {
+          maybeDestroyEncryptedSocket(c, HOLEPUNCH_ABORTED())
+        }
       }
-      c.onsocket(socket, addr.port, addr.host)
-      return
     }
   }
 

lib/server.js

@@ -14,7 +14,6 @@ const {
   closeRelayConnection,
   destroyRelayConnection
 } = require('./relay-connection')
-const { isPrivate } = require('bogon')
 const { ALREADY_LISTENING, NODE_DESTROYED, KEYPAIR_ALREADY_USED } = require('./errors')
 
 const HANDSHAKE_CLEAR_WAIT = 10000
@@ -423,20 +422,6 @@ module.exports = class Server extends EventEmitter {
       if (hs.relayToken === null) hs.rawStream.destroy()
     }
 
-    if (!direct && clientAddress.host === serverAddress.host) {
-      const clientAddresses = remotePayload.addresses4.filter(onlyPrivateHosts)
-
-      if (clientAddresses.length > 0 && this._shareLocalAddress) {
-        const myAddresses = await this._localAddresses()
-        const addr = Holepuncher.matchAddress(myAddresses, clientAddresses)
-
-        if (addr) {
-          hs.prepunching = setTimeout(onabort, HANDSHAKE_INITIAL_TIMEOUT)
-          return hs
-        }
-      }
-    }
-
     if (this._closing || this.suspended) return null
 
     if (ourRemoteAddr || this._neverPunch) {
@@ -714,10 +699,6 @@ function defaultCreateSecretStream(isInitiator, rawStream, opts) {
   return new NoiseSecretStream(isInitiator, rawStream, opts)
 }
 
-function onlyPrivateHosts(addr) {
-  return isPrivate(addr.host)
-}
-
 function isRelay(relaySocket, socket, port, host) {
   const stream = relaySocket.rawStream
   if (!stream) return false

test/connections.js

@@ -181,6 +181,76 @@ test('createServer + connect - force holepunch', async function (t) {
   await b.destroy()
 })
 
+test('createServer + connect - failed LAN ping falls back to holepunch', async function (t) {
+  const [boot] = await swarm(t)
+
+  // Use loopback to deterministically enter the same-LAN shortcut in CI.
+  // This does not model NAT; the injected ping failure below verifies that
+  // a blocked LAN shortcut still falls through to normal holepunch.
+  const bootstrap = [{ host: '127.0.0.1', port: boot.address().port }]
+  const a = createDHT({ bootstrap, quickFirewall: false, ephemeral: true })
+  const b = createDHT({ bootstrap, quickFirewall: false, ephemeral: true })
+
+  await a.fullyBootstrapped()
+  await b.fullyBootstrapped()
+
+  const lc = t.test('socket lifecycle')
+  lc.plan(5)
+
+  const server = a.createServer(function (socket) {
+    lc.pass('server side opened')
+
+    socket.once('data', function (data) {
+      lc.alike(data, Buffer.from('hello'))
+      socket.end('world')
+    })
+  })
+
+  await server.listen()
+
+  const serverPort = a.io.serverSocket.address().port
+  const ping = b.ping.bind(b)
+  let blockedLanPing = false
+
+  b.ping = function (addr, opts) {
+    if (addr.host === '127.0.0.1' && addr.port === serverPort) {
+      blockedLanPing = true
+      return Promise.reject(new Error('LAN ping blocked'))
+    }
+    return ping(addr, opts)
+  }
+
+  const socket = b.connect(server.publicKey, {
+    fastOpen: false,
+    holepunch() {
+      lc.pass('client used normal holepunch')
+      return true
+    }
+  })
+
+  socket.once('open', function () {
+    lc.pass('client side opened')
+    socket.write('hello')
+  })
+
+  socket.once('data', function (data) {
+    lc.alike(data, Buffer.from('world'))
+    socket.end()
+  })
+
+  socket.once('error', function (err) {
+    lc.fail('client should not error: ' + err.code)
+  })
+
+  await lc
+
+  t.ok(blockedLanPing, 'exercised the LAN ping failure path')
+
+  await server.close()
+  await a.destroy()
+  await b.destroy()
+})
+
 test('server choosing to abort holepunch', async function (t) {
   const [boot] = await swarm(t)