0 Fork 0

Files

T

dualfroz 94942b3aff fix: do not read an undefined offset when a value argument is last

The argument parser looked up the next command-line token via $argv[$key + 1]
guarded only by count($argv) === 0. The remaining arguments array is not
reindexed after earlier arguments are removed, so a value-taking argument placed
last (after an unconsumed token) read an undefined offset and raised an Undefined
array key warning under PHP 8, and the argument was set to empty instead of
falling back to its default. Check that the next offset exists with isset().

2026-09-04 09:21:56 +02:00

2.7 KiB

Raw Permalink Blame History

fix: do not read an undefined offset when a value argument is last

Problem

When an argument that expects a value is the last token on the command line and is preceded by an unconsumed non-prefixed token, the parser emits an Undefined array key warning under PHP 8:

$climate = new League\CLImate\CLImate();
$climate->arguments->add([
    'name' => ['prefix' => 'n', 'longPrefix' => 'name', 'defaultValue' => 'default-name'],
]);

$climate->arguments->parse(['script', 'somefile', '-n']);
// PHP Warning: Undefined array key 2 in Parser.php on line 270

On top of the warning, the argument is then set to an empty value instead of falling back to its configured default, which is inconsistent with the behaviour when a value argument is the only remaining token (that case correctly falls back to the default).

Root cause

src/Argument/Parser.php, setArgumentValue():

if (is_null($value)) {
    if (count($argv) === 0) {
        return $argv;
    }

    // If the value wasn't previously defined in "key=value"
    // format then define it from the next command argument.
    $nextArgvValue = $argv[$key + 1];
    ...
}

The remaining arguments array is not reindexed after previously matched arguments are removed with unset(). count($argv) === 0 is therefore not a reliable test for "there is no following token": when an unconsumed token still sits at another offset (for example somefile at offset 0), count($argv) is non-zero even though offset $key + 1 does not exist, so $argv[$key + 1] reads an undefined key.

The fix

Check that the next offset actually exists instead of relying on the overall count. When there is no following token the method returns without setting a value, so the argument falls back to its default (matching the existing last-token behaviour):

if (is_null($value)) {
    if (!isset($argv[$key + 1])) {
        return $argv;
    }

    $nextArgvValue = $argv[$key + 1];
    if ($this->isValidArgumentValue($nextArgvValue)) {
        $argument->setValue($nextArgvValue);
        unset($argv[$key + 1]);
        return $argv;
    }
}

Test

Added ManagerTest::testItParsesAValueTakingArgumentThatIsLastWithoutAWarning, which installs an error handler that turns any warning into an exception, parses ['command', 'somefile', '-n'], and asserts that no warning is raised and the argument falls back to its default value.

Gate

composer install && composer test

composer test runs vendor/bin/phpunit. Result: exit code 0 - OK (274 tests, 152 assertions).

Counterfactual: reverting setArgumentValue() to the count($argv) === 0 guard makes the new test fail with ErrorException: Undefined array key 2 raised at Parser.php:270.

2.7 KiB Raw Permalink Blame History