Fix the correctness findings from the Qodana scan

docs/changelog.270.txt

@@ -4,6 +4,11 @@ XOOPS 2.7.x Changelog (Language changes: see: /docs/lang_diff.txt)
 2.7.4 NOT RELEASED
 ===================================
 Bug fixes:
+- image.php no longer crashes on PHP 8.3+ when an angle is requested: imagerotate() is called with three arguments, the fourth was removed in PHP 8.3 (image.php)
+- LDAP and Active Directory logins on PHP 8.3+ no longer emit a deprecation: ldap_connect() receives one ldap:// URI built from the configured server and port, a bare IPv6 address is bracketed, a server already written with an ldap:// or ldaps:// scheme is used as is, and STARTTLS is not attempted on an ldaps:// connection (class/auth/auth_ldap.php, class/auth/auth_ads.php)
+- a cache engine without read() or write() throws LogicException instead of trigger_error(E_USER_ERROR), which PHP 8.4 deprecates (class/cache/xoopscache.php)
+- xoops_aw_encode() and xoops_aw_decode() take their element by reference, so the array_walk() calls that encode and decode XOBJ_DTYPE_UNICODE_ARRAY values act on the array; write.php and XoopsObject::setVar() serialize the walked array instead of array_walk()'s boolean, and XoopsObject::getVar() returns the decoded array instead of true (include/functions.encoding.php, class/model/write.php, kernel/object.php)
+- xoops_getUrlDomain() checks the host key only when parse_url() returned an array (include/functions.php)
 - templates can use |sprintf without a Smarty 4 deprecation notice: XoopsTpl registers sprintf as a modifier, as it already does for strstr, trim and file_exists (class/template.php) (#264)
 
 Libraries and assets:

htdocs/class/auth/auth_ads.php

@@ -66,11 +66,19 @@ public function authenticate($uname, $pwd = null)
 
             return $authenticated;
         }
-        $this->_ds = ldap_connect($this->ldap_server, $this->ldap_port);
+        // PHP 8.3 deprecated the ($host, $port) form; pass one URI and keep a scheme the admin configured.
+        $ldapServer = (string) $this->ldap_server;
+        if (filter_var($ldapServer, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) !== false) {
+            $ldapServer = '[' . $ldapServer . ']';
+        }
+        $ldapUri = preg_match('#^ldaps?://#i', $ldapServer)
+            ? $ldapServer
+            : 'ldap://' . $ldapServer . ':' . $this->ldap_port;
+        $this->_ds = ldap_connect($ldapUri);
         if ($this->_ds) {
             ldap_set_option($this->_ds, LDAP_OPT_PROTOCOL_VERSION, $this->ldap_version);
             ldap_set_option($this->_ds, LDAP_OPT_REFERRALS, 0);
-            if ($this->ldap_use_TLS) { // We use TLS secure connection
+            if ($this->ldap_use_TLS && stripos($ldapUri, 'ldaps://') !== 0) { // STARTTLS; an ldaps:// URI is already encrypted
                 if (!ldap_start_tls($this->_ds)) {
                     $this->setErrors(0, _AUTH_LDAP_START_TLS_FAILED);
                     // TLS was required: never look up or bind over the plain connection.

htdocs/class/auth/auth_ldap.php

@@ -235,10 +235,18 @@ public function authenticate($uname, $pwd = null)
 
             return $authenticated;
         }
-        $this->_ds = ldap_connect($this->ldap_server, $this->ldap_port);
+        // PHP 8.3 deprecated the ($host, $port) form; pass one URI and keep a scheme the admin configured.
+        $ldapServer = (string) $this->ldap_server;
+        if (filter_var($ldapServer, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) !== false) {
+            $ldapServer = '[' . $ldapServer . ']';
+        }
+        $ldapUri = preg_match('#^ldaps?://#i', $ldapServer)
+            ? $ldapServer
+            : 'ldap://' . $ldapServer . ':' . $this->ldap_port;
+        $this->_ds = ldap_connect($ldapUri);
         if ($this->_ds) {
             ldap_set_option($this->_ds, LDAP_OPT_PROTOCOL_VERSION, $this->ldap_version);
-            if ($this->ldap_use_TLS) { // We use TLS secure connection
+            if ($this->ldap_use_TLS && stripos($ldapUri, 'ldaps://') !== 0) { // STARTTLS; an ldaps:// URI is already encrypted
                 if (!ldap_start_tls($this->_ds)) {
                     $this->setErrors(0, _AUTH_LDAP_START_TLS_FAILED);
                     // TLS was required: never look up or bind over the plain connection.

htdocs/class/cache/xoopscache.php

@@ -705,7 +705,7 @@ public function gc()
      */
     public function write($key, $value, $duration = null)
     {
-        trigger_error(sprintf('Method write() not implemented in %s', get_class($this)), E_USER_ERROR);
+        throw new LogicException(sprintf('Method write() not implemented in %s', get_class($this)));
     }
 
     /**
@@ -717,7 +717,7 @@ public function write($key, $value, $duration = null)
      */
     public function read($key)
     {
-        trigger_error(sprintf('Method read() not implemented in %s', get_class($this)), E_USER_ERROR);
+        throw new LogicException(sprintf('Method read() not implemented in %s', get_class($this)));
     }
 
     /**

htdocs/class/model/write.php

@@ -197,7 +197,8 @@ public function cleanVars($object)
                         $cleanv[$key] = str_replace('\\"', '"', addslashes($cleanv[$key]));
                     }
                     // TODO: Not encoding safe, should try base64_encode -- phppp
-                    $cleanv = "'" . serialize(array_walk($cleanv, 'xoops_aw_encode')) . "'";
+                    array_walk($cleanv, 'xoops_aw_encode');
+                    $cleanv = $this->handler->db->quote(serialize($cleanv));
                     break;
 
                 case XOBJ_DTYPE_ARRAY:

htdocs/image.php

@@ -688,7 +688,7 @@ function imageFilenameCheck($imageUrl)
 
 // Imagerotate
 if (ENABLE_IMAGEROTATE && !empty($angle)) {
-    $destination_image = imagerotate($destination_image, $angle, $background, 0);
+    $destination_image = imagerotate($destination_image, $angle, $background);
 }
 
 if ($do_sharpen) {

htdocs/include/functions.encoding.php

@@ -128,25 +128,29 @@ function xoops_convert_decode($data, $store_method = 'urlcode')
 /**
  * xoops_aw_encode()
  *
- * @param  mixed $value value of array
+ * array_walk() callback: encodes the array element in place.
+ *
+ * @param  mixed $value value of array, passed by reference
  * @param  mixed $key key of array
  * @param string $store_method
- * @return boolean|string
+ * @return void
  */
-function xoops_aw_encode($value, $key, $store_method = 'urlcode')
+function xoops_aw_encode(&$value, $key, $store_method = 'urlcode')
 {
     $value = xoops_convert_encode($value, $store_method);
 }
 
 /**
  * xoops_aw_decode()
  *
- * @param mixed $value value of array
+ * array_walk() callback: decodes the array element in place.
+ *
+ * @param mixed $value value of array, passed by reference
  * @param mixed $key key of array
  * @param string $store_method
- * @return boolean|string
+ * @return void
  */
-function xoops_aw_decode($value, $key, $store_method = 'urlcode')
+function xoops_aw_decode(&$value, $key, $store_method = 'urlcode')
 {
     $value = xoops_convert_decode($value, $store_method);
 }

htdocs/include/functions.php

@@ -1381,7 +1381,7 @@ function xoops_getUrlDomain($url)
     $domain = '';
     $_URL   = parse_url($url);
 
-    if (!empty($_URL) || !empty($_URL['host'])) {
+    if (!empty($_URL) && !empty($_URL['host'])) {
         $domain = $_URL['host'];
     }
 

htdocs/kernel/object.php

@@ -550,7 +550,7 @@ public function getVar($key, ?string $format = null)
                             }
                             $ret = is_array($ret) ? $ret : [];
                             if (is_array($ret)) {
-                                $ret = array_walk($ret, 'xoops_aw_decode');
+                                array_walk($ret, 'xoops_aw_decode');
                             }
                         }
 
@@ -885,7 +885,8 @@ public function cleanVars()
                         $cleanv = xoops_convert_encode($cleanv);
                         break;
                     case /** @scrutinizer ignore-deprecated */ XOBJ_DTYPE_UNICODE_ARRAY:
-                        $cleanv = serialize(array_walk($cleanv, 'xoops_aw_encode'));
+                        array_walk($cleanv, 'xoops_aw_encode');
+                        $cleanv = serialize($cleanv);
                         break;
                     default:
                         break;

htdocs/kernel/user.php

@@ -91,7 +91,7 @@ class XoopsUser extends XoopsObject
 
     /**
      * constructor
-     * @param array|null $id ID of the user to be loaded from the database.
+     * @param int|array|null $id ID of the user to be loaded from the database, or an array of values
      */
     public function __construct($id = null)
     {

tests/unit/htdocs/class/LegacyClassLoadTest.php

@@ -160,18 +160,12 @@ public function delete($key)
             }
         };
         $messages = [];
-        set_error_handler(static function (int $no, string $msg) use (&$messages): bool {
-            if ($no === E_USER_ERROR) {
-                $messages[] = $msg;
+        foreach ([static fn () => $engine->write('k', 'v'), static fn () => $engine->read('k')] as $call) {
+            try {
+                $call();
+            } catch (\LogicException $e) {
+                $messages[] = $e->getMessage();
             }
-
-            return true;
-        });
-        try {
-            $engine->write('k', 'v');
-            $engine->read('k');
-        } finally {
-            restore_error_handler();
         }
 
         self::assertSame([