Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed extension requirement checking in upgrade script #14986

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

marcusmoore
Copy link
Collaborator

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context, providing screenshots where practical. List any dependencies that are required for this change.

Fixes #14972

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

I tested this locally by copying out the relevant part of the upgrade.php script into a new script and running it in isolation:

Original Isolated Script:
echo "Checking Required PHP extensions... \n\n";

// Get the list of installed extensions
$loaded_exts_array = get_loaded_extensions();

// The PHP extensions PHP is *required* to have enabled in order to run
$required_exts_array =
    [
        'bcmath',
        'curl',
        'exif',
        'fileinfo',
        'gd|imagick',
        'json',
        'ldap',
        'mbstring',
        'mysqli|pgsql',
        'openssl',
        'PDO',
        'sodium',
        'tokenizer',
        'xml',
        'zip',
    ];

$recommended_exts_array =
    [
        'sodium', //note that extensions need to be in BOTH the $required_exts_array and this one to be 'optional'
    ];
$ext_missing = '';
$ext_installed = '';

// Loop through the required extensions
foreach ($required_exts_array as $required_ext) {

    // If we don't find the string in the array....
    if (!in_array($required_ext, $loaded_exts_array)) {

        // Let's check for any options with pipes in them - those mean you can have either or
        if (strpos($required_ext, '|')) {

            // Split the either/ors by their pipe and put them into an array
            $require_either = explode("|", $required_ext);

            // Now loop through the either/or array and see whether any of the options match
            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";
                    break;
                }
            }

        // 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";
        } else {
            $ext_installed .= '- '.$required_ext." is *NOT* installed, but is recommended...\n";
        }

    // The required extension string was found in the array of installed extensions - yay!
    } else {
        $ext_installed .=  ''.$required_ext." is installed!\n";
    }
}

// Print out a useful error message and abort the install
if ($ext_missing!='') {
    echo "--------------------------------------------------------\n";
    echo "You have the following extensions installed: \n";
    echo "--------------------------------------------------------\n";

    foreach ($loaded_exts_array as $loaded_ext) {
       echo "- ".$loaded_ext."\n";
    }

    echo "--------------------- !! ERROR !! ----------------------\n";
    echo $ext_missing;
    echo "--------------------------------------------------------\n";
    echo "ABORTING THE INSTALLER  \n";
    echo "Please install the extensions above and re-run this script.\n";
    echo "--------------------------------------------------------\n";
    exit(1);
} else {
    echo $ext_installed."\n";

}

To confirm the issue I added 'not_real_extension|gd|imagick', to $required_exts_array and ran the script:

CleanShot 2024-06-27 at 12 45 18@2x

I made the adjustments in this PR and was able to see not_real_extension didn't throw the error and gd was accepted:

CleanShot 2024-06-27 at 12 46 21@2x

To make sure I didn't accidentally break the script I added 'one|two', to $required_exts_array and it gave an error like expected:

CleanShot 2024-06-27 at 12 47 34@2x

From there I copy/pasted the wrapping foreach ($required_exts_array as $required_ext) { from my isolated script into upgrade.php. BUT! I didn't run upgrade.php because I didn't want to actually do the "upgrade" on my local system.

This is the final result of my isolated script I copied from for reference:

Original Isolated Script:
<?php

echo "Checking Required PHP extensions... \n\n";

// Get the list of installed extensions
$loaded_exts_array = get_loaded_extensions();

// The PHP extensions PHP is *required* to have enabled in order to run
$required_exts_array =
    [
        'bcmath',
        'curl',
        'exif',
        'fileinfo',
        'one|two',
        'not_real_extension|gd|imagick',
        'json',
        'ldap',
        'mbstring',
        'mysqli|pgsql',
        'openssl',
        'PDO',
        'sodium',
        'tokenizer',
        'xml',
        'zip',
    ];

$recommended_exts_array =
    [
        'sodium', //note that extensions need to be in BOTH the $required_exts_array and this one to be 'optional'
    ];
$ext_missing = '';
$ext_installed = '';

// Loop through the required extensions
foreach ($required_exts_array as $required_ext) {

    // If we don't find the string in the array....
    if (!in_array($required_ext, $loaded_exts_array)) {

        // Let's check for any options with pipes in them - those mean you can have either or
        if (strpos($required_ext, '|')) {

            // Split the either/ors by their pipe and put them into an array
            $require_either = explode("|", $required_ext);

            $has_one_required_ext = false;

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

                if (in_array($require_either_value, $loaded_exts_array)) {
                    $ext_installed .=  ''.$require_either_value." is installed!\n";
                    $has_one_required_ext = true;
                    break;
                }
            }

            // If no match, add it to the string for errors
            if (!$has_one_required_ext) {
                $ext_missing .=  '✘ MISSING PHP EXTENSION: '.str_replace("|", " OR ", $required_ext)."\n";
                break;
            }

            // 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";
        } else {
            $ext_installed .= '- '.$required_ext." is *NOT* installed, but is recommended...\n";
        }

        // The required extension string was found in the array of installed extensions - yay!
    } else {
        $ext_installed .=  ''.$required_ext." is installed!\n";
    }
}

// Print out a useful error message and abort the install
if ($ext_missing!='') {
    echo "--------------------------------------------------------\n";
    echo "You have the following extensions installed: \n";
    echo "--------------------------------------------------------\n";

    foreach ($loaded_exts_array as $loaded_ext) {
        echo "- ".$loaded_ext."\n";
    }

    echo "--------------------- !! ERROR !! ----------------------\n";
    echo $ext_missing;
    echo "--------------------------------------------------------\n";
    echo "ABORTING THE INSTALLER  \n";
    echo "Please install the extensions above and re-run this script.\n";
    echo "--------------------------------------------------------\n";
    exit(1);
} else {
    echo $ext_installed."\n";

}

@marcusmoore marcusmoore requested a review from snipe as a code owner June 27, 2024 19:52
Copy link

what-the-diff bot commented Jun 27, 2024

PR Summary

  • Introduction of PHP extension prerequisite
    This change includes an added safeguard to confirm the presence of at least one essential PHP extension. If these extensions are missing, the system now presents an error message to alert the user. This helps in averting potential operational issues while also aiding in easier problem troubleshooting.

@snipe snipe merged commit f027fd5 into snipe:develop Jul 2, 2024
9 checks passed
@marcusmoore marcusmoore deleted the bug/sc-25926 branch July 2, 2024 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants