From f16262142e4d56a6227ec525f8abb4ff35e64ed6 Mon Sep 17 00:00:00 2001 From: tianfenghan Date: Mon, 31 Aug 2026 12:08:02 +0800 Subject: [PATCH] fix(installer): parse php-config flags safely, fix gh-36 --- .../Installer/PhpBuildConfigurationTest.php | 61 ++++++++++++ src/Installer/LibPhpInstaller.php | 17 ++-- src/Installer/PhpBuildConfiguration.php | 92 +++++++++++++++++-- 3 files changed, 154 insertions(+), 16 deletions(-) diff --git a/phpunit/src/Installer/PhpBuildConfigurationTest.php b/phpunit/src/Installer/PhpBuildConfigurationTest.php index 049eabd2..5498a20d 100644 --- a/phpunit/src/Installer/PhpBuildConfigurationTest.php +++ b/phpunit/src/Installer/PhpBuildConfigurationTest.php @@ -25,6 +25,67 @@ final class PhpBuildConfigurationTest extends TestCase self::assertNotContains('--enable-fpm', $options); } + public function testParseShellWordsRemovesQuotesAroundAssignmentValues(): void + { + self::assertSame( + ['CFLAGS=-g -O2', 'CPPFLAGS=-DNAME="Type PHP"', '--with-zlib=/opt/php libs'], + PhpBuildConfiguration::parseShellWords( + 'CFLAGS=\'-g -O2\' CPPFLAGS="-DNAME=\"Type PHP\"" --with-zlib=/opt/php\ libs' + ) + ); + self::assertSame( + ['CFLAGS=-g -O2'], + PhpBuildConfiguration::parseShellWords('CFLAGS="-g -O2"') + ); + } + + public function testDeriveDropsUnquotedBuildFlagsFromPhpConfig(): void + { + $parsed = PhpBuildConfiguration::parsePhpConfigOptions( + '--includedir=/usr/include --disable-all --with-zlib=/usr ' . + 'build_alias=x86_64-linux-gnu host_alias=x86_64-linux-gnu ' . + 'CFLAGS=-g -O2 -Werror=implicit-function-declaration -fno-omit-frame-pointer ' . + '-fstack-protector-strong --param=ssp-buffer-size=4 -O2 -Wall -pedantic -g ' . + 'PHP_BUILD_PROVIDER=Ubuntu' + ); + $options = PhpBuildConfiguration::derive( + $parsed, + '/home/test/.typephp' + ); + + self::assertSame( + ['--includedir=/usr/include', '--disable-all', '--with-zlib=/usr'], + $parsed + ); + self::assertContains('--includedir=/usr/include', $options); + self::assertContains('--disable-all', $options); + self::assertContains('--with-zlib=/usr', $options); + self::assertNotContains('CFLAGS=-g', $options); + self::assertNotContains('-O2', $options); + self::assertNotContains('--param=ssp-buffer-size=4', $options); + self::assertNotContains('PHP_BUILD_PROVIDER=Ubuntu', $options); + } + + public function testDeriveDropsConfigureExecutableAndQuotedBuildAssignments(): void + { + $options = PhpBuildConfiguration::derive( + "../configure '--enable-cli' CFLAGS='-g -O2' PHP_BUILD_PROVIDER=Ubuntu", + '/home/test/.typephp' + ); + + self::assertNotContains('../configure', $options); + self::assertNotContains('CFLAGS=-g -O2', $options); + self::assertContains('--enable-cli', $options); + } + + public function testParseShellWordsRejectsIncompleteInput(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Unterminated quote'); + + PhpBuildConfiguration::parseShellWords("CFLAGS='-g -O2"); + } + public function testDetectPackageManagerUsesSupportedPriority(): void { $manager = LinuxPackageManager::detect(static fn(string $command): bool => in_array($command, ['dnf', 'yum'], true)); diff --git a/src/Installer/LibPhpInstaller.php b/src/Installer/LibPhpInstaller.php index 1c2e7e55..ad9ed030 100644 --- a/src/Installer/LibPhpInstaller.php +++ b/src/Installer/LibPhpInstaller.php @@ -174,20 +174,19 @@ final class LibPhpInstaller $this->console->write("libphp.so installed successfully in {$prefix}/lib"); } - private function currentConfigureOptions(): string + /** @return list */ + private function currentConfigureOptions(): array { // Prefer the "Configure Command:" output from `PHP_BINARY -i`, which preserves - // the quoting of each argument and can therefore handle values containing spaces - // such as `CFLAGS=-g -O2`. In contrast, `php-config --configure-options` drops the - // quotes, causing space-containing values to be split incorrectly (for example, - // `-O2` being passed to configure as a separate argument). + // exact argument boundaries. In contrast, `php-config --configure-options` drops + // quotes around space-containing build assignments such as `CFLAGS=-g -O2`. $info = $this->capture([PHP_BINARY, '-n', '-i']); if (preg_match('/^Configure Command =>\s*(.+)$/mi', $info, $match)) { $words = PhpBuildConfiguration::parseShellWords(trim($match[1])); - if (($words[0] ?? null) === './configure') { + if (isset($words[0]) && basename($words[0]) === 'configure') { array_shift($words); } - return implode(' ', array_map('escapeshellarg', $words)); + return $words; } // Fallback: php-config --configure-options. On PPA multi-version installations @@ -203,7 +202,9 @@ final class LibPhpInstaller $phpConfig = trim((string) shell_exec('command -v php-config 2>/dev/null')); } if ($phpConfig !== '') { - return trim($this->capture([$phpConfig, '--configure-options'])); + return PhpBuildConfiguration::parsePhpConfigOptions( + trim($this->capture([$phpConfig, '--configure-options'])) + ); } throw new \RuntimeException('Unable to determine the current PHP configure options from php-config or php -i'); diff --git a/src/Installer/PhpBuildConfiguration.php b/src/Installer/PhpBuildConfiguration.php index d7b8e3f5..afc8d766 100644 --- a/src/Installer/PhpBuildConfiguration.php +++ b/src/Installer/PhpBuildConfiguration.php @@ -4,18 +4,88 @@ namespace TypePhp\Installer; final class PhpBuildConfiguration { + /** @return list */ public static function parseShellWords(string $value): array { - preg_match_all('/(?:[^\s"\']+|"[^"]*"|\'[^\']*\')+/', $value, $matches); - return array_map(static function (string $word): string { - if (strlen($word) >= 2 && (($word[0] === "'" && $word[-1] === "'") || ($word[0] === '"' && $word[-1] === '"'))) { - return substr($word, 1, -1); + $words = []; + $word = ''; + $quote = null; + $wordStarted = false; + $length = strlen($value); + + for ($index = 0; $index < $length; $index++) { + $char = $value[$index]; + if ($quote === null) { + if (str_contains(" \t\r\n\v\f", $char)) { + if ($wordStarted) { + $words[] = $word; + $word = ''; + $wordStarted = false; + } + continue; + } + if ($char === "'" || $char === '"') { + $quote = $char; + $wordStarted = true; + continue; + } + if ($char === '\\') { + if (++$index >= $length) { + throw new \InvalidArgumentException('Incomplete escape sequence in configure options'); + } + $word .= $value[$index]; + $wordStarted = true; + continue; + } + $word .= $char; + $wordStarted = true; + continue; + } + + if ($char === $quote) { + $quote = null; + continue; + } + if ($quote === '"' && $char === '\\' && $index + 1 < $length + && str_contains('\"$`', $value[$index + 1]) + ) { + $word .= $value[++$index]; + continue; + } + $word .= $char; + } + + if ($quote !== null) { + throw new \InvalidArgumentException('Unterminated quote in configure options'); + } + if ($wordStarted) { + $words[] = $word; + } + return $words; + } + + /** @return list */ + public static function parsePhpConfigOptions(string $value): array + { + $options = []; + foreach (self::parseShellWords($value) as $option) { + // php-config --configure-options loses the quoting of trailing build + // assignments. Once the first assignment is reached, tokens that follow + // may be either part of its value or another assignment, so none of them + // can safely be reused as configure arguments. + if (preg_match('/^[A-Za-z_][A-Za-z0-9_]*=/', $option)) { + break; } - return $word; - }, $matches[0]); + $options[] = $option; + } + return $options; } - public static function derive(string $configureOptions, string $prefix): array + /** + * @param string|list $configureOptions + * @return list + */ + public static function derive(string|array $configureOptions, string $prefix): array { $replace = [ '--prefix', '--with-config-file-path', '--with-config-file-scan-dir', @@ -23,7 +93,13 @@ final class PhpBuildConfiguration ]; $drop = ['--with-apxs', '--with-apxs2', '--enable-fpm', '--with-fpm-systemd']; $result = []; - foreach (self::parseShellWords($configureOptions) as $option) { + $options = is_string($configureOptions) ? self::parseShellWords($configureOptions) : $configureOptions; + foreach ($options as $option) { + // Build assignments are not PHP feature configuration. Keep only long + // configure options; callers can override CFLAGS through the environment. + if (!str_starts_with($option, '--')) { + continue; + } $name = explode('=', $option, 2)[0]; if (in_array($name, $replace, true) || in_array($name, $drop, true)) { continue;