From 14b4d0327ea38b6c0f22934326aaf180eab35ad1 Mon Sep 17 00:00:00 2001 From: Josh Date: Wed, 8 Jan 2025 13:31:03 -0500 Subject: [PATCH 1/3] fix(AppFramework): Log malformed protocol values and unify fallback behavior Signed-off-by: Josh --- lib/private/AppFramework/Http/Request.php | 49 +++++++++++++-------- tests/lib/AppFramework/Http/RequestTest.php | 26 +++++++++-- 2 files changed, 53 insertions(+), 22 deletions(-) diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php index e662cb8679a..08c6f248bf5 100644 --- a/lib/private/AppFramework/Http/Request.php +++ b/lib/private/AppFramework/Http/Request.php @@ -14,6 +14,7 @@ use OC\Security\TrustedDomainHelper; use OCP\IConfig; use OCP\IRequest; use OCP\IRequestId; +use Psr\Log\LoggerInterface; use Symfony\Component\HttpFoundation\IpUtils; /** @@ -627,36 +628,46 @@ class Request implements \ArrayAccess, \Countable, IRequest { /** * Returns the server protocol. It respects one or more reverse proxies servers - * and load balancers + * and load balancers. Precedence: + * 1. `overwriteprotocol` config value + * 2. `X-Forwarded-Proto` header value + * 3. $_SERVER['HTTPS'] value + * If an invalid protocol is provided, defaults to http, continues, but logs as an error. + * * @return string Server protocol (http or https) */ public function getServerProtocol(): string { + $proto = 'http'; + if ($this->config->getSystemValueString('overwriteprotocol') !== '' - && $this->isOverwriteCondition()) { - return $this->config->getSystemValueString('overwriteprotocol'); - } - - if ($this->fromTrustedProxy() && isset($this->server['HTTP_X_FORWARDED_PROTO'])) { + && $this->isOverwriteCondition() + ) { + $proto = strtolower($this->config->getSystemValueString('overwriteprotocol')); + } elseif ($this->fromTrustedProxy() + && isset($this->server['HTTP_X_FORWARDED_PROTO']) + ) { if (str_contains($this->server['HTTP_X_FORWARDED_PROTO'], ',')) { $parts = explode(',', $this->server['HTTP_X_FORWARDED_PROTO']); $proto = strtolower(trim($parts[0])); } else { $proto = strtolower($this->server['HTTP_X_FORWARDED_PROTO']); } - - // Verify that the protocol is always HTTP or HTTPS - // default to http if an invalid value is provided - return $proto === 'https' ? 'https' : 'http'; - } - - if (isset($this->server['HTTPS']) - && $this->server['HTTPS'] !== null + } elseif (!empty($this->server['HTTPS']) && $this->server['HTTPS'] !== 'off' - && $this->server['HTTPS'] !== '') { - return 'https'; + ) { + $proto = 'https'; } - return 'http'; + if ($proto !== 'https' && $proto !== 'http') { + // log unrecognized value so admin has a chance to fix it + \OC::$server->get(LoggerInterface::class)->critical( + 'Server protocol is malformed [falling back to http] (check overwriteprotocol and/or X-Forwarded-Proto to remedy): ' . $proto, + ['app' => 'core'] + ); + } + + // default to http if provided an invalid value + return $proto === 'https' ? 'https' : 'http'; } /** @@ -743,11 +754,11 @@ class Request implements \ArrayAccess, \Countable, IRequest { } /** - * Get PathInfo from request + * Get PathInfo from request (rawurldecoded) * @throws \Exception * @return string|false Path info or false when not found */ - public function getPathInfo() { + public function getPathInfo(): string|false { $pathInfo = $this->getRawPathInfo(); return \Sabre\HTTP\decodePath($pathInfo); } diff --git a/tests/lib/AppFramework/Http/RequestTest.php b/tests/lib/AppFramework/Http/RequestTest.php index a7ff123fc25..bf1e92a7a3d 100644 --- a/tests/lib/AppFramework/Http/RequestTest.php +++ b/tests/lib/AppFramework/Http/RequestTest.php @@ -777,12 +777,12 @@ class RequestTest extends \Test\TestCase { $this->assertSame($expected, $request->getHttpProtocol()); } - public function testGetServerProtocolWithOverride(): void { + public function testGetServerProtocolWithOverrideValid(): void { $this->config ->expects($this->exactly(3)) ->method('getSystemValueString') ->willReturnMap([ - ['overwriteprotocol', '', 'customProtocol'], + ['overwriteprotocol', '', 'HTTPS'], // should be automatically lowercased ['overwritecondaddr', '', ''], ]); @@ -794,9 +794,29 @@ class RequestTest extends \Test\TestCase { $this->stream ); - $this->assertSame('customProtocol', $request->getServerProtocol()); + $this->assertSame('https', $request->getServerProtocol()); } + public function testGetServerProtocolWithOverrideInValid(): void { + $this->config + ->expects($this->exactly(3)) + ->method('getSystemValueString') + ->willReturnMap([ + ['overwriteprotocol', '', 'bogusProtocol'], // should trigger fallback to http + ['overwritecondaddr', '', ''], + ]); + + $request = new Request( + [], + $this->requestId, + $this->config, + $this->csrfTokenManager, + $this->stream + ); + + $this->assertSame('http', $request->getServerProtocol()); + } + public function testGetServerProtocolWithProtoValid(): void { $this->config ->method('getSystemValue') From 4829ac57c1ae7c42daa3ecd60cf00313828e65f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6?= Date: Fri, 1 Aug 2025 10:58:36 +0200 Subject: [PATCH 2/3] fix: use `OCP\Server` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ --- lib/composer/composer/InstalledVersions.php | 20 +++++- lib/composer/composer/installed.json | 69 ++++++++++++++++++++- lib/composer/composer/installed.php | 15 ++++- lib/private/AppFramework/Http/Request.php | 6 +- tests/lib/AppFramework/Http/RequestTest.php | 2 +- 5 files changed, 101 insertions(+), 11 deletions(-) diff --git a/lib/composer/composer/InstalledVersions.php b/lib/composer/composer/InstalledVersions.php index 6d29bff66aa..2052022fd8e 100644 --- a/lib/composer/composer/InstalledVersions.php +++ b/lib/composer/composer/InstalledVersions.php @@ -26,6 +26,12 @@ use Composer\Semver\VersionParser; */ class InstalledVersions { + /** + * @var string|null if set (by reflection by Composer), this should be set to the path where this class is being copied to + * @internal + */ + private static $selfDir = null; + /** * @var mixed[]|null * @psalm-var array{root: array{name: string, pretty_version: string, version: string, reference: string|null, type: string, install_path: string, aliases: string[], dev: bool}, versions: array}|array{}|null @@ -322,6 +328,18 @@ class InstalledVersions self::$installedIsLocalDir = false; } + /** + * @return string + */ + private static function getSelfDir() + { + if (self::$selfDir === null) { + self::$selfDir = strtr(__DIR__, '\\', '/'); + } + + return self::$selfDir; + } + /** * @return array[] * @psalm-return list}> @@ -336,7 +354,7 @@ class InstalledVersions $copiedLocalDir = false; if (self::$canGetVendors) { - $selfDir = strtr(__DIR__, '\\', '/'); + $selfDir = self::getSelfDir(); foreach (ClassLoader::getRegisteredLoaders() as $vendorDir => $loader) { $vendorDir = strtr($vendorDir, '\\', '/'); if (isset(self::$installedByVendor[$vendorDir])) { diff --git a/lib/composer/composer/installed.json b/lib/composer/composer/installed.json index f20a6c47c6d..13ea12dca2a 100644 --- a/lib/composer/composer/installed.json +++ b/lib/composer/composer/installed.json @@ -1,5 +1,68 @@ { - "packages": [], - "dev": false, - "dev-package-names": [] + "packages": [ + { + "name": "bamarni/composer-bin-plugin", + "version": "1.8.2", + "version_normalized": "1.8.2.0", + "source": { + "type": "git", + "url": "https://github.com/bamarni/composer-bin-plugin.git", + "reference": "92fd7b1e6e9cdae19b0d57369d8ad31a37b6a880" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/bamarni/composer-bin-plugin/zipball/92fd7b1e6e9cdae19b0d57369d8ad31a37b6a880", + "reference": "92fd7b1e6e9cdae19b0d57369d8ad31a37b6a880", + "shasum": "" + }, + "require": { + "composer-plugin-api": "^2.0", + "php": "^7.2.5 || ^8.0" + }, + "require-dev": { + "composer/composer": "^2.0", + "ext-json": "*", + "phpstan/extension-installer": "^1.1", + "phpstan/phpstan": "^1.8", + "phpstan/phpstan-phpunit": "^1.1", + "phpunit/phpunit": "^8.5 || ^9.5", + "symfony/console": "^2.8.52 || ^3.4.35 || ^4.4 || ^5.0 || ^6.0", + "symfony/finder": "^2.8.52 || ^3.4.35 || ^4.4 || ^5.0 || ^6.0", + "symfony/process": "^2.8.52 || ^3.4.35 || ^4.4 || ^5.0 || ^6.0" + }, + "time": "2022-10-31T08:38:03+00:00", + "type": "composer-plugin", + "extra": { + "class": "Bamarni\\Composer\\Bin\\BamarniBinPlugin" + }, + "installation-source": "dist", + "autoload": { + "psr-4": { + "Bamarni\\Composer\\Bin\\": "src" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "description": "No conflicts for your bin dependencies", + "keywords": [ + "composer", + "conflict", + "dependency", + "executable", + "isolation", + "tool" + ], + "support": { + "issues": "https://github.com/bamarni/composer-bin-plugin/issues", + "source": "https://github.com/bamarni/composer-bin-plugin/tree/1.8.2" + }, + "install-path": "../bamarni/composer-bin-plugin" + } + ], + "dev": true, + "dev-package-names": [ + "bamarni/composer-bin-plugin" + ] } diff --git a/lib/composer/composer/installed.php b/lib/composer/composer/installed.php index 1cfe4bf1d74..522ac554e3e 100644 --- a/lib/composer/composer/installed.php +++ b/lib/composer/composer/installed.php @@ -3,21 +3,30 @@ 'name' => '__root__', 'pretty_version' => 'dev-master', 'version' => 'dev-master', - 'reference' => 'b7422ba97b7b42a9955a52031a32457ca521d740', + 'reference' => 'c2cf24c4bc7f69c61df1ac5693311da15716673c', 'type' => 'library', 'install_path' => __DIR__ . '/../../../', 'aliases' => array(), - 'dev' => false, + 'dev' => true, ), 'versions' => array( '__root__' => array( 'pretty_version' => 'dev-master', 'version' => 'dev-master', - 'reference' => 'b7422ba97b7b42a9955a52031a32457ca521d740', + 'reference' => 'c2cf24c4bc7f69c61df1ac5693311da15716673c', 'type' => 'library', 'install_path' => __DIR__ . '/../../../', 'aliases' => array(), 'dev_requirement' => false, ), + 'bamarni/composer-bin-plugin' => array( + 'pretty_version' => '1.8.2', + 'version' => '1.8.2.0', + 'reference' => '92fd7b1e6e9cdae19b0d57369d8ad31a37b6a880', + 'type' => 'composer-plugin', + 'install_path' => __DIR__ . '/../bamarni/composer-bin-plugin', + 'aliases' => array(), + 'dev_requirement' => true, + ), ), ); diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php index 08c6f248bf5..7cc7467675c 100644 --- a/lib/private/AppFramework/Http/Request.php +++ b/lib/private/AppFramework/Http/Request.php @@ -638,7 +638,7 @@ class Request implements \ArrayAccess, \Countable, IRequest { */ public function getServerProtocol(): string { $proto = 'http'; - + if ($this->config->getSystemValueString('overwriteprotocol') !== '' && $this->isOverwriteCondition() ) { @@ -660,12 +660,12 @@ class Request implements \ArrayAccess, \Countable, IRequest { if ($proto !== 'https' && $proto !== 'http') { // log unrecognized value so admin has a chance to fix it - \OC::$server->get(LoggerInterface::class)->critical( + \OCP\Server::get(LoggerInterface::class)->critical( 'Server protocol is malformed [falling back to http] (check overwriteprotocol and/or X-Forwarded-Proto to remedy): ' . $proto, ['app' => 'core'] ); } - + // default to http if provided an invalid value return $proto === 'https' ? 'https' : 'http'; } diff --git a/tests/lib/AppFramework/Http/RequestTest.php b/tests/lib/AppFramework/Http/RequestTest.php index bf1e92a7a3d..7ea2cb31482 100644 --- a/tests/lib/AppFramework/Http/RequestTest.php +++ b/tests/lib/AppFramework/Http/RequestTest.php @@ -816,7 +816,7 @@ class RequestTest extends \Test\TestCase { $this->assertSame('http', $request->getServerProtocol()); } - + public function testGetServerProtocolWithProtoValid(): void { $this->config ->method('getSystemValue') From d2a20ea1bd59cf2c50dca1454ae474f8040cccb1 Mon Sep 17 00:00:00 2001 From: skjnldsv Date: Fri, 1 Aug 2025 17:00:01 +0200 Subject: [PATCH 3/3] chore: update composer Signed-off-by: skjnldsv --- lib/composer/composer/installed.json | 69 ++-------------------------- lib/composer/composer/installed.php | 15 ++---- 2 files changed, 6 insertions(+), 78 deletions(-) diff --git a/lib/composer/composer/installed.json b/lib/composer/composer/installed.json index 13ea12dca2a..f20a6c47c6d 100644 --- a/lib/composer/composer/installed.json +++ b/lib/composer/composer/installed.json @@ -1,68 +1,5 @@ { - "packages": [ - { - "name": "bamarni/composer-bin-plugin", - "version": "1.8.2", - "version_normalized": "1.8.2.0", - "source": { - "type": "git", - "url": "https://github.com/bamarni/composer-bin-plugin.git", - "reference": "92fd7b1e6e9cdae19b0d57369d8ad31a37b6a880" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/bamarni/composer-bin-plugin/zipball/92fd7b1e6e9cdae19b0d57369d8ad31a37b6a880", - "reference": "92fd7b1e6e9cdae19b0d57369d8ad31a37b6a880", - "shasum": "" - }, - "require": { - "composer-plugin-api": "^2.0", - "php": "^7.2.5 || ^8.0" - }, - "require-dev": { - "composer/composer": "^2.0", - "ext-json": "*", - "phpstan/extension-installer": "^1.1", - "phpstan/phpstan": "^1.8", - "phpstan/phpstan-phpunit": "^1.1", - "phpunit/phpunit": "^8.5 || ^9.5", - "symfony/console": "^2.8.52 || ^3.4.35 || ^4.4 || ^5.0 || ^6.0", - "symfony/finder": "^2.8.52 || ^3.4.35 || ^4.4 || ^5.0 || ^6.0", - "symfony/process": "^2.8.52 || ^3.4.35 || ^4.4 || ^5.0 || ^6.0" - }, - "time": "2022-10-31T08:38:03+00:00", - "type": "composer-plugin", - "extra": { - "class": "Bamarni\\Composer\\Bin\\BamarniBinPlugin" - }, - "installation-source": "dist", - "autoload": { - "psr-4": { - "Bamarni\\Composer\\Bin\\": "src" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "description": "No conflicts for your bin dependencies", - "keywords": [ - "composer", - "conflict", - "dependency", - "executable", - "isolation", - "tool" - ], - "support": { - "issues": "https://github.com/bamarni/composer-bin-plugin/issues", - "source": "https://github.com/bamarni/composer-bin-plugin/tree/1.8.2" - }, - "install-path": "../bamarni/composer-bin-plugin" - } - ], - "dev": true, - "dev-package-names": [ - "bamarni/composer-bin-plugin" - ] + "packages": [], + "dev": false, + "dev-package-names": [] } diff --git a/lib/composer/composer/installed.php b/lib/composer/composer/installed.php index 522ac554e3e..cd89ef10785 100644 --- a/lib/composer/composer/installed.php +++ b/lib/composer/composer/installed.php @@ -3,30 +3,21 @@ 'name' => '__root__', 'pretty_version' => 'dev-master', 'version' => 'dev-master', - 'reference' => 'c2cf24c4bc7f69c61df1ac5693311da15716673c', + 'reference' => '3fce359f4c606737b21b1b4213efd5bc5536e867', 'type' => 'library', 'install_path' => __DIR__ . '/../../../', 'aliases' => array(), - 'dev' => true, + 'dev' => false, ), 'versions' => array( '__root__' => array( 'pretty_version' => 'dev-master', 'version' => 'dev-master', - 'reference' => 'c2cf24c4bc7f69c61df1ac5693311da15716673c', + 'reference' => '3fce359f4c606737b21b1b4213efd5bc5536e867', 'type' => 'library', 'install_path' => __DIR__ . '/../../../', 'aliases' => array(), 'dev_requirement' => false, ), - 'bamarni/composer-bin-plugin' => array( - 'pretty_version' => '1.8.2', - 'version' => '1.8.2.0', - 'reference' => '92fd7b1e6e9cdae19b0d57369d8ad31a37b6a880', - 'type' => 'composer-plugin', - 'install_path' => __DIR__ . '/../bamarni/composer-bin-plugin', - 'aliases' => array(), - 'dev_requirement' => true, - ), ), );