mirror of
https://github.com/nextcloud/server.git
synced 2026-06-09 08:44:07 -04:00
fix(AppFramework): Log malformed protocol values and unify fallback behavior
Signed-off-by: Josh <josh.t.richards@gmail.com>
This commit is contained in:
parent
6bc6ed95a3
commit
14b4d0327e
2 changed files with 53 additions and 22 deletions
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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')
|
||||
|
|
|
|||
Loading…
Reference in a new issue