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>

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

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';
 	}
 
 	/**

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

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

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

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'.

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',

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 {

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',

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