Requirement check in upgrade.php does not validate OR dependencies correctly

[pprs-it]

Debug mode

• I have enabled debug mode
• I have read checked the Common Issues page

Describe the bug

On a PHP installation which does not have the first module of an OR dependency available (e.g. the gd|imagick dependency with only imagick installed), upgrade.php will claim that both extensions are missing.

I can't recall if this was an issue during the initial installation as well, but it might be worth checking if any other dependency checks also behave the same way as upgrade.php currently does.

Reproduction steps

1. Install an older version of Snipe-IT using only the imagick module.
2. Attempt to upgrade to the latest version using upgrade.php.
3. Receive the error message "✘ MISSING PHP EXTENSION: gd OR imagick"

Expected behavior

upgrade.php correctly identifies the dependency gd|imagick to be present.

Screenshots

No response

Snipe-IT Version

7.0.4

Operating System

Debian 12 (bookworm)

Web Server

nginx

PHP Version

PHP 8.2.20

Operating System

No response

Browser

No response

Version

No response

Device

No response

Operating System

No response

Browser

No response

Version

No response

Error messages

$ php8.2 -m
[PHP Modules]
bcmath
calendar
Core
ctype
curl
date
dom
exif
FFI
fileinfo
filter
ftp
gettext
hash
iconv
imagick
json
ldap
libxml
mbstring
mysqli
mysqlnd
openssl
pcntl
pcre
PDO
pdo_mysql
pdo_pgsql
pgsql
Phar
posix
random
readline
Reflection
session
shmop
SimpleXML
sockets
sodium
SPL
standard
sysvmsg
sysvsem
sysvshm
tokenizer
xml
xmlreader
xmlwriter
xsl
Zend OPcache
zip
zlib

[Zend Modules]
Zend OPcache

Additional context

The error is most likely caused by LL288-295. This piece of the code loops over all optional dependencies in a dependency entry (e.g. gd and imagick), and checks for each of the entries if they are available. If it's available, it exits the loop, but if it's not available, it also exits the loop. This section should instead look similar to the following:

diff --git a/upgrade.php b/upgrade.php
index 798d9d3c8..e8497cffc 100644
--- a/upgrade.php
+++ b/upgrade.php
@@ -283,18 +283,21 @@ foreach ($required_exts_array as $required_ext) {
             $require_either = explode("|", $required_ext);

             // Now loop through the either/or array and see whether any of the options match
+            $matched = false;
             foreach ($require_either as $require_either_value) {

                 if (in_array($require_either_value, $loaded_exts_array)) {
                     $ext_installed .=  '√ '.$require_either_value." is installed!\n";
-                    break;
-                // If no match, add it to the string for errors
-                } else {
-                    $ext_missing .=  '✘ MISSING PHP EXTENSION: '.str_replace("|", " OR ", $required_ext)."\n";
+                    $matched = true;
                     break;
                 }
             }

+            // If no match, add it to the string for errors
+            if (!$matched) {
+                $ext_missing .=  '✘ MISSING PHP EXTENSION: '.str_replace("|", " OR ", $required_ext)."\n";
+            }
+
         // If this isn't an either/or option, just add it to the string of errors conventionally
         } elseif (!in_array($required_ext, $recommended_exts_array)){
             $ext_missing .=  '✘ MISSING PHP EXTENSION: '.$required_ext."\n";

[welcome]

👋 Thanks for opening your first issue here! If you're reporting a 🐞 bug, please make sure you include steps to reproduce it. We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

[snipe]

That hasn't been changed in quite some time though - I'm surprised this hasn't come up before if it's a genuine bug. (Not saying it's not, just strange that this is the first time it's reported.)

[pprs-it]

just strange that this is the first time it's reported

Indeed.

Maybe most people just use PHP GD, but I didn't install the module on our system because ImageMagick was already installed. Reduce attack surface and all that, you know.

[snipe]

That's certainly possible. I haven't used ImageMagick in a decade, since most systems already have gd installed. I'm not sure of a reliable way to test this though. :-/

[marcusmoore]

I'm not sure of a reliable way to test this though

I just created a PR for this #14986 but also didn't fully test it by uninstalling extensions from my local system. I detailed how I "faked" it in the PR.

[snipe]

@marcusmoore i think that’s a good best effort here - Ty!

[marcusmoore]

Thanks for reporting @pprs-it. This should be handled in #14986 so I'm going to close this issue. Please let us know if you still run into issues with it in the future.

[pprs-it]

Will do, thank you! 👍