Request->getHeader() should always return a string

PHPDoc (of the public API) says that this method returns string but it also returns null, which is not allowed in some method calls. This fixes that behaviour and returns an empty string and fixes all code paths that explicitly checked for null to be still compliant.

Found while enabling the strict_typing for lib/private for the PHP7+ migration.

Signed-off-by: Morris Jobke <hey@morrisjobke.de>

10 files changed, 38 insertions, 16 deletions

diff --git a/apps/workflowengine/lib/Check/FileSize.php b/apps/workflowengine/lib/Check/FileSize.php
index 1744793dec7..7e48f0f6038 100644
--- a/apps/workflowengine/lib/Check/FileSize.php
+++ b/apps/workflowengine/lib/Check/FileSize.php
@@ -103,13 +103,13 @@ class FileSize implements ICheck {
 		}
 
 		$size = $this->request->getHeader('OC-Total-Length');
-		if ($size === null) {
+		if ($size === '') {
 			if (in_array($this->request->getMethod(), ['POST', 'PUT'])) {
 				$size = $this->request->getHeader('Content-Length');
 			}
 		}
 
-		if ($size === null) {
+		if ($size === '') {
 			$size = false;
 		}
 
diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php
index 0e7fbf892b6..23bd42a0f18 100644
--- a/core/Controller/ClientFlowLoginController.php
+++ b/core/Controller/ClientFlowLoginController.php
@@ -115,7 +115,7 @@ class ClientFlowLoginController extends Controller {
 	 */
 	private function getClientName() {
 		$userAgent = $this->request->getHeader('USER_AGENT');
-		return $userAgent !== null ? $userAgent : 'unknown';
+		return $userAgent !== '' ? $userAgent : 'unknown';
 	}
 
 	/**
diff --git a/core/Controller/CssController.php b/core/Controller/CssController.php
index 95a41f8dd15..43a4f453b0e 100644
--- a/core/Controller/CssController.php
+++ b/core/Controller/CssController.php
@@ -98,7 +98,7 @@ class CssController extends Controller {
 	private function getFile(ISimpleFolder $folder, $fileName, &$gzip) {
 		$encoding = $this->request->getHeader('Accept-Encoding');
 
-		if ($encoding !== null && strpos($encoding, 'gzip') !== false) {
+		if (strpos($encoding, 'gzip') !== false) {
 			try {
 				$gzip = true;
 				return $folder->getFile($fileName . '.gzip'); # Safari doesn't like .gz
diff --git a/core/Controller/JsController.php b/core/Controller/JsController.php
index f02948e47a0..670ca997257 100644
--- a/core/Controller/JsController.php
+++ b/core/Controller/JsController.php
@@ -96,7 +96,7 @@ class JsController extends Controller {
 	private function getFile(ISimpleFolder $folder, $fileName, &$gzip) {
 		$encoding = $this->request->getHeader('Accept-Encoding');
 
-		if ($encoding !== null && strpos($encoding, 'gzip') !== false) {
+		if (strpos($encoding, 'gzip') !== false) {
 			try {
 				$gzip = true;
 				return $folder->getFile($fileName . '.gzip'); # Safari doesn't like .gz
diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php
index 77ecb02165b..975c4255d5a 100644
--- a/lib/private/AppFramework/Http/Request.php
+++ b/lib/private/AppFramework/Http/Request.php
@@ -324,7 +324,7 @@ class Request implements \ArrayAccess, \Countable, IRequest {
 
 		}
 
-		return null;
+		return '';
 	}
 
 	/**
@@ -404,8 +404,7 @@ class Request implements \ArrayAccess, \Countable, IRequest {
 	protected function getContent() {
 		// If the content can't be parsed into an array then return a stream resource.
 		if ($this->method === 'PUT'
-			&& $this->getHeader('Content-Length') !== 0
-			&& $this->getHeader('Content-Length') !== null
+			&& $this->getHeader('Content-Length') !== '0'
 			&& $this->getHeader('Content-Length') !== ''
 			&& strpos($this->getHeader('Content-Type'), 'application/x-www-form-urlencoded') === false
 			&& strpos($this->getHeader('Content-Type'), 'application/json') === false
diff --git a/lib/private/L10N/Factory.php b/lib/private/L10N/Factory.php
index e277a00e653..8268be05d4b 100644
--- a/lib/private/L10N/Factory.php
+++ b/lib/private/L10N/Factory.php
@@ -247,7 +247,7 @@ class Factory implements IFactory {
 	 */
 	private function getLanguageFromRequest($app) {
 		$header = $this->request->getHeader('ACCEPT_LANGUAGE');
-		if ($header) {
+		if ($header !== '') {
 			$available = $this->findAvailableLanguages($app);
 
 			// E.g. make sure that 'de' is before 'de_DE'.
diff --git a/lib/private/Log/File.php b/lib/private/Log/File.php
index b6a208ad68a..290f7897c9d 100644
--- a/lib/private/Log/File.php
+++ b/lib/private/Log/File.php
@@ -105,7 +105,10 @@ class File {
 		} else {
 			$user = '--';
 		}
-		$userAgent = $request->getHeader('User-Agent') ?: '--';
+		$userAgent = $request->getHeader('User-Agent');
+		if ($userAgent === '') {
+			$userAgent = '--';
+		}
 		$version = $config->getValue('version', '');
 		$entry = compact(
 			'reqId',
diff --git a/lib/private/Memcache/APCu.php b/lib/private/Memcache/APCu.php
index 70f0d73d2d4..3e96acdecb7 100644
--- a/lib/private/Memcache/APCu.php
+++ b/lib/private/Memcache/APCu.php
@@ -158,8 +158,8 @@ class APCu extends Cache implements IMemcache {
 		} elseif (!\OC::$server->getIniWrapper()->getBool('apc.enable_cli') && \OC::$CLI) {
 			return false;
 		} elseif (
-				version_compare(phpversion('apc'), '4.0.6') === -1 &&
-				version_compare(phpversion('apcu'), '5.1.0') === -1
+				version_compare(phpversion('apc') ?: '0.0.0', '4.0.6') === -1 &&
+				version_compare(phpversion('apcu') ?: '0.0.0', '5.1.0') === -1
 		) {
 			return false;
 		} else {
diff --git a/tests/Core/Controller/ClientFlowLoginControllerTest.php b/tests/Core/Controller/ClientFlowLoginControllerTest.php
index 0e048538223..7fe87df026f 100644
--- a/tests/Core/Controller/ClientFlowLoginControllerTest.php
+++ b/tests/Core/Controller/ClientFlowLoginControllerTest.php
@@ -431,6 +431,10 @@ class ClientFlowLoginControllerTest extends TestCase {
 			->expects($this->once())
 			->method('getServerHost')
 			->willReturn('example.com');
+		$this->request
+			->expects($this->any())
+			->method('getHeader')
+			->willReturn('');
 
 		$expected = new Http\RedirectResponse('nc://login/server:http://example.com&user:MyLoginName&password:MyGeneratedToken');
 		$this->assertEquals($expected, $this->clientFlowLoginController->generateAppPassword('MyStateToken'));
@@ -583,6 +587,10 @@ class ClientFlowLoginControllerTest extends TestCase {
 			->expects($this->once())
 			->method('getServerHost')
 			->willReturn('example.com');
+		$this->request
+			->expects($this->any())
+			->method('getHeader')
+			->willReturn('');
 
 		$expected = new Http\RedirectResponse('nc://login/server:http://example.com&user:MyLoginName&password:MyGeneratedToken');
 		$this->assertEquals($expected, $this->clientFlowLoginController->generateAppPassword('MyStateToken'));
@@ -594,6 +602,7 @@ class ClientFlowLoginControllerTest extends TestCase {
 				[
 					['X-Forwarded-Proto', 'http'],
 					['X-Forwarded-Ssl', 'off'],
+					['USER_AGENT', ''],
 				],
 				'http',
 				'http',
@@ -602,6 +611,7 @@ class ClientFlowLoginControllerTest extends TestCase {
 				[
 					['X-Forwarded-Proto', 'http'],
 					['X-Forwarded-Ssl', 'off'],
+					['USER_AGENT', ''],
 				],
 				'https',
 				'https',
@@ -610,6 +620,7 @@ class ClientFlowLoginControllerTest extends TestCase {
 				[
 					['X-Forwarded-Proto', 'https'],
 					['X-Forwarded-Ssl', 'off'],
+					['USER_AGENT', ''],
 				],
 				'http',
 				'https',
@@ -618,6 +629,7 @@ class ClientFlowLoginControllerTest extends TestCase {
 				[
 					['X-Forwarded-Proto', 'https'],
 					['X-Forwarded-Ssl', 'on'],
+					['USER_AGENT', ''],
 				],
 				'http',
 				'https',
@@ -626,6 +638,7 @@ class ClientFlowLoginControllerTest extends TestCase {
 				[
 					['X-Forwarded-Proto', 'http'],
 					['X-Forwarded-Ssl', 'on'],
+					['USER_AGENT', ''],
 				],
 				'http',
 				'https',
diff --git a/tests/lib/L10N/FactoryTest.php b/tests/lib/L10N/FactoryTest.php
index 602967b1626..4f31784337a 100644
--- a/tests/lib/L10N/FactoryTest.php
+++ b/tests/lib/L10N/FactoryTest.php
@@ -55,9 +55,16 @@ class FactoryTest extends TestCase {
 
 	/**
 	 * @param array $methods
+	 * @param bool $mockRequestGetHeaderMethod
 	 * @return Factory|\PHPUnit_Framework_MockObject_MockObject
 	 */
-	protected function getFactory(array $methods = []) {
+	protected function getFactory(array $methods = [], $mockRequestGetHeaderMethod = false) {
+		if ($mockRequestGetHeaderMethod) {
+			$this->request->expects($this->any())
+				->method('getHeader')
+				->willReturn('');
+		}
+
 		if (!empty($methods)) {
 			return $this->getMockBuilder(Factory::class)
 				->setConstructorArgs([
@@ -137,7 +144,7 @@ class FactoryTest extends TestCase {
 	}
 
 	public function testFindLanguageWithNotExistingRequestLanguageAndNotExistingStoredUserLanguage() {
-		$factory = $this->getFactory(['languageExists']);
+		$factory = $this->getFactory(['languageExists'], true);
 		$this->invokePrivate($factory, 'requestLanguage', ['de']);
 		$factory->expects($this->at(0))
 				->method('languageExists')
@@ -180,7 +187,7 @@ class FactoryTest extends TestCase {
 	}
 
 	public function testFindLanguageWithNotExistingRequestLanguageAndNotExistingStoredUserLanguageAndNotExistingDefault() {
-		$factory = $this->getFactory(['languageExists']);
+		$factory = $this->getFactory(['languageExists'], true);
 		$this->invokePrivate($factory, 'requestLanguage', ['de']);
 		$factory->expects($this->at(0))
 				->method('languageExists')
@@ -226,7 +233,7 @@ class FactoryTest extends TestCase {
 	}
 
 	public function testFindLanguageWithNotExistingRequestLanguageAndNotExistingStoredUserLanguageAndNotExistingDefaultAndNoAppInScope() {
-		$factory = $this->getFactory(['languageExists']);
+		$factory = $this->getFactory(['languageExists'], true);
 		$this->invokePrivate($factory, 'requestLanguage', ['de']);
 		$factory->expects($this->at(0))
 				->method('languageExists')