fix(installer): parse php-config flags safely, fix gh-36

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));

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<string> */
+    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');

src/Installer/PhpBuildConfiguration.php

@@ -4,18 +4,88 @@ namespace TypePhp\Installer;
 
 final class PhpBuildConfiguration
 {
+    /** @return list<string> */
     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<string> */
+    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<string> $configureOptions
+     * @return list<string>
+     */
+    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;