From 70b630710d5af2ce9d49d71e9310c71d95a76f87 Mon Sep 17 00:00:00 2001 From: Joe Ferguson Date: Tue, 14 Jul 2026 10:21:32 -0500 Subject: [PATCH 1/2] Fix downloads.php crash on invalid os parameter Validate the client-supplied ?os= against the whitelist before indexing, fixing ~24k/day 'array_key_exists(): ... null given' fatals. Extract the resolution into a testable OptionResolver class. --- downloads.php | 44 +----- src/Downloads/OptionResolver.php | 82 ++++++++++++ tests/Unit/Downloads/OptionResolverTest.php | 141 ++++++++++++++++++++ 3 files changed, 230 insertions(+), 37 deletions(-) create mode 100644 src/Downloads/OptionResolver.php create mode 100644 tests/Unit/Downloads/OptionResolverTest.php diff --git a/downloads.php b/downloads.php index 21325269b9..a80c8d328b 100644 --- a/downloads.php +++ b/downloads.php @@ -1,4 +1,6 @@ $auto_os ?? 'linux', - 'version' => 'default', -]; - -$options = array_merge($defaults, $_GET); - -if ($auto_osvariant && (!array_key_exists('osvariant', $options) || !array_key_exists($options['osvariant'], $os[$options['os']]['variants']))) { - $options['osvariant'] = $auto_osvariant; -} elseif (!array_key_exists('osvariant', $options) || !array_key_exists($options['osvariant'], $os[$options['os']]['variants'])) { - $options['osvariant'] = array_key_first($os[$options['os']]['variants']); -} +$options = (new OptionResolver($os))->resolve( + $_GET, + $_SERVER['HTTP_SEC_CH_UA_PLATFORM'] ?? '', + $_SERVER['HTTP_USER_AGENT'] ?? '', +); ?>

Downloads & Installation Instructions

diff --git a/src/Downloads/OptionResolver.php b/src/Downloads/OptionResolver.php new file mode 100644 index 0000000000..d7618d6f1c --- /dev/null +++ b/src/Downloads/OptionResolver.php @@ -0,0 +1,82 @@ +}> $osList + */ + public function __construct(private readonly array $osList) + { + } + + /** + * Resolve the selected download options from the request, auto-detecting the + * operating system from client hints / user agent when not explicitly chosen. + * + * @param array $get GET parameters (os, osvariant, version, ...) + * @return array + */ + public function resolve(array $get, string $platformHeader, string $uaHeader): array + { + [$autoOs, $autoOsVariant] = $this->autoDetect($platformHeader, $uaHeader); + + $defaults = [ + 'os' => $autoOs ?? 'linux', + 'version' => 'default', + ]; + + $options = array_merge($defaults, $get); + + // Ignore an invalid or non-string os parameter (e.g. bots requesting + // ?os= or ?os[]=x) so indexing $osList below stays safe. + if (!is_string($options['os']) || !array_key_exists($options['os'], $this->osList)) { + $options['os'] = $defaults['os']; + } + + if ($autoOsVariant && (!array_key_exists('osvariant', $options) || !array_key_exists($options['osvariant'], $this->osList[$options['os']]['variants']))) { + $options['osvariant'] = $autoOsVariant; + } elseif (!array_key_exists('osvariant', $options) || !array_key_exists($options['osvariant'], $this->osList[$options['os']]['variants'])) { + $options['osvariant'] = array_key_first($this->osList[$options['os']]['variants']); + } + + return $options; + } + + /** + * @return array{0: ?string, 1: ?string} [auto os, auto os variant] + */ + private function autoDetect(string $platformHeader, string $uaHeader): array + { + $autoOs = null; + $autoOsVariant = null; + + if ($platformHeader === '' && $uaHeader === '') { + return [$autoOs, $autoOsVariant]; + } + + $platform = strtolower(trim($platformHeader, '"')); + + if ($platform === 'windows' || stripos($uaHeader, 'Windows') !== false) { + $autoOs = 'windows'; + } elseif ($platform === 'macos' || stripos($uaHeader, 'Mac') !== false) { + $autoOs = 'osx'; + } elseif ($platform === 'linux' || stripos($uaHeader, 'Linux') !== false) { + $autoOs = 'linux'; + if (stripos($uaHeader, 'Ubuntu') !== false) { + $autoOsVariant = 'linux-ubuntu'; + } elseif (stripos($uaHeader, 'Debian') !== false) { + $autoOsVariant = 'linux-debian'; + } elseif (stripos($uaHeader, 'Fedora') !== false) { + $autoOsVariant = 'linux-fedora'; + } elseif (stripos($uaHeader, 'Red Hat') !== false || stripos($uaHeader, 'RedHat') !== false) { + $autoOsVariant = 'linux-redhat'; + } + } + + return [$autoOs, $autoOsVariant]; + } +} diff --git a/tests/Unit/Downloads/OptionResolverTest.php b/tests/Unit/Downloads/OptionResolverTest.php new file mode 100644 index 0000000000..d6fd211dc6 --- /dev/null +++ b/tests/Unit/Downloads/OptionResolverTest.php @@ -0,0 +1,141 @@ +}> + */ + private const OS_LIST = [ + 'linux' => [ + 'name' => 'Linux', + 'variants' => [ + 'linux-debian' => 'Debian', + 'linux-ubuntu' => 'Ubuntu', + ], + ], + 'osx' => [ + 'name' => 'macOS', + 'variants' => [ + 'osx-homebrew' => 'Homebrew', + 'osx-macports' => 'MacPorts', + ], + ], + 'windows' => [ + 'name' => 'Windows', + 'variants' => [ + 'windows-downloads' => 'ZIP Downloads', + 'windows-native' => 'Single Line Installer', + ], + ], + ]; + + /** + * Reproduces the production crash: a bot requesting downloads.php?os= + * previously made $os[$options['os']] null and threw + * "array_key_exists(): Argument #2 ($array) must be of type array, null given". + */ + public function testInvalidOsParameterFallsBackToDefault(): void + { + $options = $this->resolver()->resolve(['os' => 'not-a-real-os'], '', ''); + + self::assertSame('linux', $options['os']); + self::assertSame('linux-debian', $options['osvariant']); + } + + public function testArrayOsParameterFallsBackToDefault(): void + { + $options = $this->resolver()->resolve(['os' => ['linux']], '', ''); + + self::assertSame('linux', $options['os']); + } + + public function testValidOsParameterSelectsFirstVariant(): void + { + $options = $this->resolver()->resolve(['os' => 'windows'], '', ''); + + self::assertSame('windows', $options['os']); + self::assertSame('windows-downloads', $options['osvariant']); + } + + public function testValidOsAndVariantAreHonored(): void + { + $options = $this->resolver()->resolve( + ['os' => 'osx', 'osvariant' => 'osx-macports'], + '', + '', + ); + + self::assertSame('osx', $options['os']); + self::assertSame('osx-macports', $options['osvariant']); + } + + public function testInvalidVariantFallsBackToFirstForThatOs(): void + { + $options = $this->resolver()->resolve( + ['os' => 'windows', 'osvariant' => 'linux-debian'], + '', + '', + ); + + self::assertSame('windows', $options['os']); + self::assertSame('windows-downloads', $options['osvariant']); + } + + public function testVersionDefaultsWhenNotSupplied(): void + { + $options = $this->resolver()->resolve([], '', ''); + + self::assertSame('default', $options['version']); + } + + public function testGetParametersArePreserved(): void + { + $options = $this->resolver()->resolve( + ['os' => 'linux', 'version' => '8.4', 'source' => 'Y'], + '', + '', + ); + + self::assertSame('8.4', $options['version']); + self::assertSame('Y', $options['source']); + } + + public function testDetectsWindowsFromUserAgent(): void + { + $options = $this->resolver()->resolve([], '', 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)'); + + self::assertSame('windows', $options['os']); + } + + public function testDetectsUbuntuVariantFromUserAgent(): void + { + $options = $this->resolver()->resolve([], '', 'Mozilla/5.0 (X11; Ubuntu; Linux x86_64)'); + + self::assertSame('linux', $options['os']); + self::assertSame('linux-ubuntu', $options['osvariant']); + } + + public function testExplicitOsParameterOverridesAutoDetection(): void + { + $options = $this->resolver()->resolve( + ['os' => 'osx'], + '', + 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)', + ); + + self::assertSame('osx', $options['os']); + } + + private function resolver(): OptionResolver + { + return new OptionResolver(self::OS_LIST); + } +} From 509adb49070d7574f7122a55c56727521a54951f Mon Sep 17 00:00:00 2001 From: Joe Ferguson Date: Wed, 15 Jul 2026 08:21:33 -0500 Subject: [PATCH 2/2] Redirect downloads.php to auto-detected OS on invalid os param --- downloads.php | 96 +++++++++++---------- src/Downloads/OptionResolver.php | 39 +++++++-- src/Downloads/Resolution.php | 19 ++++ tests/Unit/Downloads/OptionResolverTest.php | 94 +++++++++++++------- 4 files changed, 169 insertions(+), 79 deletions(-) create mode 100644 src/Downloads/Resolution.php diff --git a/downloads.php b/downloads.php index a80c8d328b..8e0cc1a418 100644 --- a/downloads.php +++ b/downloads.php @@ -9,6 +9,58 @@ // Try to make this page non-cached header_nocache(); +$os = [ + 'linux' => [ + 'name' => 'Linux', + 'variants' => [ + 'linux-debian' => 'Debian', + 'linux-fedora' => 'Fedora', + 'linux-redhat' => 'RedHat', + 'linux-ubuntu' => 'Ubuntu', + 'linux-docker-cli' => 'Docker (Command Line Interface)', + 'linux-docker-web' => 'Docker (Web Development)', + ], + ], + 'osx' => [ + 'name' => 'macOS', + 'variants' => [ + 'osx-homebrew' => 'Homebrew', + 'osx-homebrew-php' => 'Homebrew-PHP', + 'osx-docker-cli' => 'Docker (Command Line Interface)', + 'osx-docker-web' => 'Docker (Web Development)', + 'osx-macports' => 'MacPorts', + ], + ], + 'windows' => [ + 'name' => 'Windows', + 'variants' => [ + 'windows-downloads' => 'ZIP Downloads', + 'windows-native' => 'Single Line Installer', + 'windows-chocolatey' => 'Chocolatey', + 'windows-scoop' => 'Scoop', + 'windows-winget' => 'Winget', + 'windows-docker-cli' => 'Docker (Command Line Interface)', + 'windows-docker-web' => 'Docker (Web Development)', + 'windows-wsl-debian' => 'WSL/Debian', + 'windows-wsl-ubuntu' => 'WSL/Ubuntu', + ], + ], +]; + +// An invalid ?os= redirects to the auto-detected results before any output. +$resolution = (new OptionResolver($os))->resolve( + $_GET, + $_SERVER['HTTP_SEC_CH_UA_PLATFORM'] ?? '', + $_SERVER['HTTP_USER_AGENT'] ?? '', +); + +if ($resolution->redirectQuery !== null) { + header('Location: /downloads.php?' . $resolution->redirectQuery, true, 302); + exit; +} + +$options = $resolution->options; + $SIDEBAR_DATA = '
Supported Versions @@ -54,44 +106,6 @@ function option(string $value, string $desc, $attributes = []): string return ''; } -$os = [ - 'linux' => [ - 'name' => 'Linux', - 'variants' => [ - 'linux-debian' => 'Debian', - 'linux-fedora' => 'Fedora', - 'linux-redhat' => 'RedHat', - 'linux-ubuntu' => 'Ubuntu', - 'linux-docker-cli' => 'Docker (Command Line Interface)', - 'linux-docker-web' => 'Docker (Web Development)', - ], - ], - 'osx' => [ - 'name' => 'macOS', - 'variants' => [ - 'osx-homebrew' => 'Homebrew', - 'osx-homebrew-php' => 'Homebrew-PHP', - 'osx-docker-cli' => 'Docker (Command Line Interface)', - 'osx-docker-web' => 'Docker (Web Development)', - 'osx-macports' => 'MacPorts', - ], - ], - 'windows' => [ - 'name' => 'Windows', - 'variants' => [ - 'windows-downloads' => 'ZIP Downloads', - 'windows-native' => 'Single Line Installer', - 'windows-chocolatey' => 'Chocolatey', - 'windows-scoop' => 'Scoop', - 'windows-winget' => 'Winget', - 'windows-docker-cli' => 'Docker (Command Line Interface)', - 'windows-docker-web' => 'Docker (Web Development)', - 'windows-wsl-debian' => 'WSL/Debian', - 'windows-wsl-ubuntu' => 'WSL/Ubuntu', - ], - ], -]; - $versions = [ '8.5' => 'version 8.5', '8.4' => 'version 8.4', @@ -100,12 +114,6 @@ function option(string $value, string $desc, $attributes = []): string 'default' => 'default PHP version for OS', ]; - -$options = (new OptionResolver($os))->resolve( - $_GET, - $_SERVER['HTTP_SEC_CH_UA_PLATFORM'] ?? '', - $_SERVER['HTTP_USER_AGENT'] ?? '', -); ?>

Downloads & Installation Instructions

diff --git a/src/Downloads/OptionResolver.php b/src/Downloads/OptionResolver.php index d7618d6f1c..8c946b85e4 100644 --- a/src/Downloads/OptionResolver.php +++ b/src/Downloads/OptionResolver.php @@ -17,10 +17,14 @@ public function __construct(private readonly array $osList) * Resolve the selected download options from the request, auto-detecting the * operating system from client hints / user agent when not explicitly chosen. * + * When the client supplies an invalid os parameter (e.g. bots requesting + * ?os= or ?os[]=x, which previously crashed while indexing the os + * list), the returned Resolution carries a redirect query pointing at the + * auto-detected results instead of silently rendering a default. + * * @param array $get GET parameters (os, osvariant, version, ...) - * @return array */ - public function resolve(array $get, string $platformHeader, string $uaHeader): array + public function resolve(array $get, string $platformHeader, string $uaHeader): Resolution { [$autoOs, $autoOsVariant] = $this->autoDetect($platformHeader, $uaHeader); @@ -31,9 +35,10 @@ public function resolve(array $get, string $platformHeader, string $uaHeader): a $options = array_merge($defaults, $get); - // Ignore an invalid or non-string os parameter (e.g. bots requesting - // ?os= or ?os[]=x) so indexing $osList below stays safe. - if (!is_string($options['os']) || !array_key_exists($options['os'], $this->osList)) { + $invalidOs = array_key_exists('os', $get) + && (!is_string($get['os']) || !array_key_exists($get['os'], $this->osList)); + + if ($invalidOs) { $options['os'] = $defaults['os']; } @@ -43,7 +48,29 @@ public function resolve(array $get, string $platformHeader, string $uaHeader): a $options['osvariant'] = array_key_first($this->osList[$options['os']]['variants']); } - return $options; + return new Resolution( + $options, + $invalidOs ? $this->redirectQuery($options) : null, + ); + } + + /** + * Build the canonical query string for the auto-detected results, preserving + * the resolved os/variant/version so the redirect lands on a valid page. + * + * @param array $options + */ + private function redirectQuery(array $options): string + { + $query = []; + + foreach (['os', 'osvariant', 'version'] as $key) { + if (array_key_exists($key, $options) && is_string($options[$key])) { + $query[$key] = $options[$key]; + } + } + + return http_build_query($query); } /** diff --git a/src/Downloads/Resolution.php b/src/Downloads/Resolution.php new file mode 100644 index 0000000000..0638ecbb0f --- /dev/null +++ b/src/Downloads/Resolution.php @@ -0,0 +1,19 @@ + $options resolved download options for rendering + * @param ?string $redirectQuery query string to redirect to (without leading + * '?'), or null when the request should render + */ + public function __construct( + public readonly array $options, + public readonly ?string $redirectQuery = null, + ) { + } +} diff --git a/tests/Unit/Downloads/OptionResolverTest.php b/tests/Unit/Downloads/OptionResolverTest.php index d6fd211dc6..7d379e4cc3 100644 --- a/tests/Unit/Downloads/OptionResolverTest.php +++ b/tests/Unit/Downloads/OptionResolverTest.php @@ -5,9 +5,11 @@ namespace phpweb\Test\Unit\Downloads; use phpweb\Downloads\OptionResolver; +use phpweb\Downloads\Resolution; use PHPUnit\Framework; #[Framework\Attributes\CoversClass(OptionResolver::class)] +#[Framework\Attributes\CoversClass(Resolution::class)] class OptionResolverTest extends Framework\TestCase { /** @@ -41,97 +43,131 @@ class OptionResolverTest extends Framework\TestCase * Reproduces the production crash: a bot requesting downloads.php?os= * previously made $os[$options['os']] null and threw * "array_key_exists(): Argument #2 ($array) must be of type array, null given". + * It now redirects to the auto-detected results instead. */ - public function testInvalidOsParameterFallsBackToDefault(): void + public function testInvalidOsParameterRedirectsToDefault(): void { - $options = $this->resolver()->resolve(['os' => 'not-a-real-os'], '', ''); + $resolution = $this->resolver()->resolve(['os' => 'not-a-real-os'], '', ''); - self::assertSame('linux', $options['os']); - self::assertSame('linux-debian', $options['osvariant']); + self::assertSame('linux', $resolution->options['os']); + self::assertSame('linux-debian', $resolution->options['osvariant']); + self::assertSame('os=linux&osvariant=linux-debian&version=default', $resolution->redirectQuery); } - public function testArrayOsParameterFallsBackToDefault(): void + public function testArrayOsParameterRedirectsToDefault(): void { - $options = $this->resolver()->resolve(['os' => ['linux']], '', ''); + $resolution = $this->resolver()->resolve(['os' => ['linux']], '', ''); - self::assertSame('linux', $options['os']); + self::assertSame('linux', $resolution->options['os']); + self::assertSame('os=linux&osvariant=linux-debian&version=default', $resolution->redirectQuery); } - public function testValidOsParameterSelectsFirstVariant(): void + public function testInvalidOsRedirectsToAutoDetectedResults(): void { - $options = $this->resolver()->resolve(['os' => 'windows'], '', ''); + $resolution = $this->resolver()->resolve( + ['os' => 'error'], + '', + 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)', + ); + + self::assertSame('windows', $resolution->options['os']); + self::assertSame('os=windows&osvariant=windows-downloads&version=default', $resolution->redirectQuery); + } + + public function testInvalidOsRedirectPreservesRequestedVersion(): void + { + $resolution = $this->resolver()->resolve(['os' => 'error', 'version' => '8.4'], '', ''); + + self::assertSame('os=linux&osvariant=linux-debian&version=8.4', $resolution->redirectQuery); + } + + public function testValidOsParameterDoesNotRedirect(): void + { + $resolution = $this->resolver()->resolve(['os' => 'windows'], '', ''); + + self::assertNull($resolution->redirectQuery); + self::assertSame('windows', $resolution->options['os']); + self::assertSame('windows-downloads', $resolution->options['osvariant']); + } + + public function testAbsentOsParameterDoesNotRedirect(): void + { + $resolution = $this->resolver()->resolve([], '', ''); - self::assertSame('windows', $options['os']); - self::assertSame('windows-downloads', $options['osvariant']); + self::assertNull($resolution->redirectQuery); + self::assertSame('linux', $resolution->options['os']); } public function testValidOsAndVariantAreHonored(): void { - $options = $this->resolver()->resolve( + $resolution = $this->resolver()->resolve( ['os' => 'osx', 'osvariant' => 'osx-macports'], '', '', ); - self::assertSame('osx', $options['os']); - self::assertSame('osx-macports', $options['osvariant']); + self::assertNull($resolution->redirectQuery); + self::assertSame('osx', $resolution->options['os']); + self::assertSame('osx-macports', $resolution->options['osvariant']); } public function testInvalidVariantFallsBackToFirstForThatOs(): void { - $options = $this->resolver()->resolve( + $resolution = $this->resolver()->resolve( ['os' => 'windows', 'osvariant' => 'linux-debian'], '', '', ); - self::assertSame('windows', $options['os']); - self::assertSame('windows-downloads', $options['osvariant']); + self::assertNull($resolution->redirectQuery); + self::assertSame('windows', $resolution->options['os']); + self::assertSame('windows-downloads', $resolution->options['osvariant']); } public function testVersionDefaultsWhenNotSupplied(): void { - $options = $this->resolver()->resolve([], '', ''); + $resolution = $this->resolver()->resolve([], '', ''); - self::assertSame('default', $options['version']); + self::assertSame('default', $resolution->options['version']); } public function testGetParametersArePreserved(): void { - $options = $this->resolver()->resolve( + $resolution = $this->resolver()->resolve( ['os' => 'linux', 'version' => '8.4', 'source' => 'Y'], '', '', ); - self::assertSame('8.4', $options['version']); - self::assertSame('Y', $options['source']); + self::assertSame('8.4', $resolution->options['version']); + self::assertSame('Y', $resolution->options['source']); } public function testDetectsWindowsFromUserAgent(): void { - $options = $this->resolver()->resolve([], '', 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)'); + $resolution = $this->resolver()->resolve([], '', 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)'); - self::assertSame('windows', $options['os']); + self::assertSame('windows', $resolution->options['os']); } public function testDetectsUbuntuVariantFromUserAgent(): void { - $options = $this->resolver()->resolve([], '', 'Mozilla/5.0 (X11; Ubuntu; Linux x86_64)'); + $resolution = $this->resolver()->resolve([], '', 'Mozilla/5.0 (X11; Ubuntu; Linux x86_64)'); - self::assertSame('linux', $options['os']); - self::assertSame('linux-ubuntu', $options['osvariant']); + self::assertSame('linux', $resolution->options['os']); + self::assertSame('linux-ubuntu', $resolution->options['osvariant']); } public function testExplicitOsParameterOverridesAutoDetection(): void { - $options = $this->resolver()->resolve( + $resolution = $this->resolver()->resolve( ['os' => 'osx'], '', 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)', ); - self::assertSame('osx', $options['os']); + self::assertNull($resolution->redirectQuery); + self::assertSame('osx', $resolution->options['os']); } private function resolver(): OptionResolver