pr feedback

Author: guybedford

doc/api/net.md

@@ -1717,10 +1717,6 @@ throw [`ERR_SOCKET_HANDLE_ADOPTED`][]. A handle that is never adopted must be
 closed to avoid leaking the socket. Closing a pipe `BoundSocket` removes its
 file system entry; abstract and TCP binds have none to remove.
 
-The presence of the [`boundSocket.isPipe`][] getter on
-`net.BoundSocket.prototype` is a capability signal that a build honors the
-`path` option rather than silently binding a TCP ephemeral port.
-
 When a pipe `BoundSocket` bound to a source `path` is adopted as a client, that
 path is reported as the socket's `localAddress` once it connects.
 
@@ -1744,6 +1740,10 @@ server.listen(bound); // Adopt as a server, or pass to new net.Socket() instead.
 
 <!-- YAML
 added: v26.4.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/64399
+    description: The `path` option is supported.
 -->
 
 * `options` {Object}
@@ -1767,6 +1767,10 @@ added: v26.4.0
 
 <!-- YAML
 added: v26.4.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/64399
+    description: The bound path is returned for a pipe bind.
 -->
 
 * Returns: {Object|string} For a TCP bind, an object with `address`, `family`,
@@ -2298,7 +2302,6 @@ net.isIPv6('fhqwhgads'); // returns false
 [`ERR_INVALID_ARG_VALUE`]: errors.md#err_invalid_arg_value
 [`ERR_SOCKET_HANDLE_ADOPTED`]: errors.md#err_socket_handle_adopted
 [`EventEmitter`]: events.md#class-eventemitter
-[`boundSocket.isPipe`]: #boundsocketispipe
 [`child_process.fork()`]: child_process.md#child_processforkmodulepath-args-options
 [`dns.lookup()`]: dns.md#dnslookuphostname-options-callback
 [`dns.lookup()` hints]: dns.md#supported-getaddrinfo-flags

lib/net.js

@@ -1557,9 +1557,11 @@ Socket.prototype.connect = function(...args) {
   const { path } = options;
   // An adopted BoundSocket handle already fixes the transport; trust its type
   // rather than inferring pipe-ness from a path option on the connect call.
-  // Other pre-existing handles (e.g. a TLSWrap) are not transport handles, so
-  // fall back to the path option in that case.
-  const pipe = this[kBoundSource] ? this._handle instanceof Pipe : !!path;
+  // Once destroyed the adopted handle is gone (its reservation released), so
+  // fall back to the path option, as for other pre-existing handles (e.g. a
+  // TLSWrap) that are not transport handles.
+  const pipe = this[kBoundSource] && this._handle ?
+    this._handle instanceof Pipe : !!path;
   debug('pipe', pipe, path);
 
   if (!this._handle) {

test/parallel/test-net-boundsocket.js

@@ -341,6 +341,40 @@ if (!common.isWindows) {
   }));
 }
 
+// Reconnecting an adopted pipe BoundSocket after destroy: the adopted handle is
+// gone, so pipe-ness must come from the path option rather than the (now null)
+// handle, otherwise connect() would wrongly attempt a TCP connect.
+if (!common.isWindows) {
+  const path = `${common.PIPE}-reconnect`;
+
+  const server = net.createServer(common.mustCall((socket) => {
+    socket.on('data', (data) => socket.end(data));
+  }, 2));
+
+  const bound = new net.BoundSocket({ path: `${path}-src` });
+  server.listen(path, common.mustCall(() => {
+    const client = new net.Socket({ handle: bound });
+    client.connect({ path });
+    client.once('connect', common.mustCall(() => {
+      client.end('ping');
+      client.once('data', common.mustCall((data) => {
+        assert.strictEqual(data.toString(), 'ping');
+      }));
+      client.once('close', common.mustCall(() => {
+        // The adopted handle is gone; reconnect must still be a pipe.
+        client.connect({ path });
+        client.once('connect', common.mustCall(() => {
+          client.end('pong');
+          client.once('data', common.mustCall((data) => {
+            assert.strictEqual(data.toString(), 'pong');
+          }));
+          client.once('close', common.mustCall(() => server.close()));
+        }));
+      }));
+    }));
+  }));
+}
+
 // Linux abstract namespace: a leading '\0' binds without creating a filesystem
 // entry, and still listens/connects.
 if (isLinux) {