[rc-1.6.0] FOGSSH: null the link on disconnect, and close only a live session

packages/web/src/Net/FOGSSH.php

@@ -152,14 +152,21 @@ public function sftp()
      */
     public function disconnect()
     {
+        // Null, never unset(). An unset declared property routes the next
+        // read of it to __get(), which looks it up in $data and warns
+        // 'Undefined array key "_link"' on every reuse of this shared
+        // instance -- the next connect() or disconnect() reads both.
         if ($this->_sftp) {
             @ssh2_disconnect($this->_sftp);
             $this->_sftp = null;
-            unset($this->_sftp);
         }
-        $test = @ssh2_disconnect($this->_link);
+        // No session to close after a failed connect (false) or an earlier
+        // disconnect (null). ssh2_disconnect() takes only a resource, and on
+        // PHP 8 anything else is a TypeError that @ does not suppress.
+        $test = is_resource($this->_link)
+            ? @ssh2_disconnect($this->_link)
+            : false;
         $this->_link = null;
-        unset($this->_link);
         return $test;
     }
     /**

tests/fogssh-reuse-after-disconnect.test.php

@@ -0,0 +1,56 @@
+<?php
+/**
+ * FOGSSH can be used again after disconnect() without a warning.
+ *
+ * FOGSSH is one shared instance (LoadGlobals). disconnect() used to unset()
+ * its declared $_link and $_sftp. PHP then routes the next read of an unset
+ * property to __get(), which looks it up in $data and warns
+ * 'Undefined array key "_link"'. Every daemon pass that disconnected and
+ * reused the instance wrote that warning to servicemaster.log.
+ *
+ * disconnect() also passed a null or false link to ssh2_disconnect(). On
+ * PHP 8 that is a TypeError, which @ does not suppress, so a disconnect()
+ * after a failed connect() was fatal.
+ *
+ * Runs with or without the ssh2 extension: with it, the first disconnect()
+ * on a never-connected instance is the TypeError case.
+ *
+ * Usage: php tests/fogssh-reuse-after-disconnect.test.php
+ * Exit status 0 = pass, 1 = fail.
+ */
+
+require __DIR__ . '/lib/fog-test-harness.php';
+
+FogTestHarness::boot('fogssh-reuse-after-disconnect');
+
+$t = new FogChecks();
+
+$warnings = [];
+set_error_handler(function ($no, $str, $file, $line) use (&$warnings) {
+    $warnings[] = "$str at " . basename($file) . ":$line";
+    return true;
+});
+
+$ssh = new \FOG\Net\FOGSSH();
+$fatal = '';
+try {
+    $ssh->disconnect();
+    $ssh->disconnect();
+} catch (\Throwable $e) {
+    $fatal = get_class($e) . ': ' . $e->getMessage();
+}
+
+restore_error_handler();
+
+$t->check(
+    'disconnect() without a live session does not throw'
+    . ($fatal ? " ($fatal)" : ''),
+    '' === $fatal
+);
+$t->check(
+    'a second disconnect() reads $_link and $_sftp without a warning'
+    . ($warnings ? ' (' . implode('; ', $warnings) . ')' : ''),
+    [] === $warnings
+);
+
+$t->finish();