Respect `mod_unique_id` and refactor `OC_Request::getRequestId`

When `mod_unique_id` is enabled the ID generated by it will be used for logging. This allows for correlation of the Apache logs and the ownCloud logs.

Testplan:

- [ ] When `mod_unique_id` is enabled the request ID equals the one generated by `mod_unique_id`.
- [ ] When `mod_unique_id` is not available the request ID is a 20 character long random string
- [ ] The generated Id is stable over the lifespan of one request

Changeset looks a little bit larger since I had to adjust every unit test using the HTTP\Request class for proper DI.

Fixes https://github.com/owncloud/core/issues/13366

10 files changed, 195 insertions, 96 deletions

diff --git a/tests/lib/appframework/controller/ApiControllerTest.php b/tests/lib/appframework/controller/ApiControllerTest.php
index 3055fbe0da8..014ddb62437 100644
--- a/tests/lib/appframework/controller/ApiControllerTest.php
+++ b/tests/lib/appframework/controller/ApiControllerTest.php
@@ -25,18 +25,19 @@
 namespace OCP\AppFramework;
 
 use OC\AppFramework\Http\Request;
-use OCP\AppFramework\Http\TemplateResponse;
 
 
 class ChildApiController extends ApiController {};
 
 
 class ApiControllerTest extends \Test\TestCase {
-
+    /** @var ChildApiController */
+    protected $controller;
 
     public function testCors() {
         $request = new Request(
-            array('server' => array('HTTP_ORIGIN' => 'test'))
+            ['server' => ['HTTP_ORIGIN' => 'test']],
+            $this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
         );
         $this->controller = new ChildApiController('app', $request, 'verbs',
             'headers', 100);
diff --git a/tests/lib/appframework/controller/ControllerTest.php b/tests/lib/appframework/controller/ControllerTest.php
index 18d47d00f6b..58395d05914 100644
--- a/tests/lib/appframework/controller/ControllerTest.php
+++ b/tests/lib/appframework/controller/ControllerTest.php
@@ -66,15 +66,16 @@ class ControllerTest extends \Test\TestCase {
 		parent::setUp();
 
 		$request = new Request(
-			array(
-				'get' => array('name' => 'John Q. Public', 'nickname' => 'Joey'),
-				'post' => array('name' => 'Jane Doe', 'nickname' => 'Janey'),
-				'urlParams' => array('name' => 'Johnny Weissmüller'),
-				'files' => array('file' => 'filevalue'),
-				'env' => array('PATH' => 'daheim'),
-				'session' => array('sezession' => 'kein'),
+			[
+				'get' => ['name' => 'John Q. Public', 'nickname' => 'Joey'],
+				'post' => ['name' => 'Jane Doe', 'nickname' => 'Janey'],
+				'urlParams' => ['name' => 'Johnny Weissmüller'],
+				'files' => ['file' => 'filevalue'],
+				'env' => ['PATH' => 'daheim'],
+				'session' => ['sezession' => 'kein'],
 				'method' => 'hi',
-			)
+			],
+			$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
 		);
 
 		$this->app = $this->getMock('OC\AppFramework\DependencyInjection\DIContainer',
diff --git a/tests/lib/appframework/dependencyinjection/DIContainerTest.php b/tests/lib/appframework/dependencyinjection/DIContainerTest.php
index 08e72aff984..43309f64e63 100644
--- a/tests/lib/appframework/dependencyinjection/DIContainerTest.php
+++ b/tests/lib/appframework/dependencyinjection/DIContainerTest.php
@@ -71,7 +71,10 @@ class DIContainerTest extends \Test\TestCase {
 
 
 	public function testMiddlewareDispatcherIncludesSecurityMiddleware(){
-		$this->container['Request'] = new Request(array('method' => 'GET'));
+		$this->container['Request'] = new Request(
+			['method' => 'GET'],
+			$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
+		);
 		$security = $this->container['SecurityMiddleware'];
 		$dispatcher = $this->container['MiddlewareDispatcher'];
 
diff --git a/tests/lib/appframework/http/DispatcherTest.php b/tests/lib/appframework/http/DispatcherTest.php
index 3933e00804b..832cd80e60a 100644
--- a/tests/lib/appframework/http/DispatcherTest.php
+++ b/tests/lib/appframework/http/DispatcherTest.php
@@ -276,13 +276,16 @@ class DispatcherTest extends \Test\TestCase {
 
 
 	public function testControllerParametersInjected() {
-		$this->request = new Request(array(
-			'post' => array(
+		$this->request = new Request(
+			[
+				'post' => [
 				'int' => '3',
 				'bool' => 'false'
-			),
-			'method' => 'POST'
-		));
+				],
+				'method' => 'POST'
+			],
+			$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
+		);
 		$this->dispatcher = new Dispatcher(
 			$this->http, $this->middlewareDispatcher, $this->reflector,
 			$this->request
@@ -298,14 +301,17 @@ class DispatcherTest extends \Test\TestCase {
 
 
 	public function testControllerParametersInjectedDefaultOverwritten() {
-		$this->request = new Request(array(
-			'post' => array(
-				'int' => '3',
-				'bool' => 'false',
-				'test2' => 7
-			),
-			'method' => 'POST'
-		));
+		$this->request = new Request(
+			[
+				'post' => [
+					'int' => '3',
+					'bool' => 'false',
+					'test2' => 7
+				],
+				'method' => 'POST',
+			],
+			$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
+		);
 		$this->dispatcher = new Dispatcher(
 			$this->http, $this->middlewareDispatcher, $this->reflector,
 			$this->request
@@ -322,16 +328,19 @@ class DispatcherTest extends \Test\TestCase {
 
 
 	public function testResponseTransformedByUrlFormat() {
-		$this->request = new Request(array(
-			'post' => array(
-				'int' => '3',
-				'bool' => 'false'
-			),
-			'urlParams' => array(
-				'format' => 'text'
-			),
-			'method' => 'GET'
-		));
+		$this->request = new Request(
+			[
+				'post' => [
+					'int' => '3',
+					'bool' => 'false'
+				],
+				'urlParams' => [
+					'format' => 'text'
+				],
+				'method' => 'GET'
+			],
+			$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
+		);
 		$this->dispatcher = new Dispatcher(
 			$this->http, $this->middlewareDispatcher, $this->reflector,
 			$this->request
@@ -347,16 +356,19 @@ class DispatcherTest extends \Test\TestCase {
 
 
 	public function testResponseTransformsDataResponse() {
-		$this->request = new Request(array(
-			'post' => array(
-				'int' => '3',
-				'bool' => 'false'
-			),
-			'urlParams' => array(
-				'format' => 'json'
-			),
-			'method' => 'GET'
-		));
+		$this->request = new Request(
+			[
+				'post' => [
+					'int' => '3',
+					'bool' => 'false'
+				],
+				'urlParams' => [
+					'format' => 'json'
+				],
+				'method' => 'GET'
+			],
+			$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
+		);
 		$this->dispatcher = new Dispatcher(
 			$this->http, $this->middlewareDispatcher, $this->reflector,
 			$this->request
@@ -372,17 +384,20 @@ class DispatcherTest extends \Test\TestCase {
 
 
 	public function testResponseTransformedByAcceptHeader() {
-		$this->request = new Request(array(
-			'post' => array(
-				'int' => '3',
-				'bool' => 'false'
-			),
-			'server' => array(
-				'HTTP_ACCEPT' => 'application/text, test',
-				'HTTP_CONTENT_TYPE' => 'application/x-www-form-urlencoded'
-			),
-			'method' => 'PUT'
-		));
+		$this->request = new Request(
+			[
+				'post' => [
+					'int' => '3',
+					'bool' => 'false'
+				],
+				'server' => [
+					'HTTP_ACCEPT' => 'application/text, test',
+					'HTTP_CONTENT_TYPE' => 'application/x-www-form-urlencoded'
+				],
+				'method' => 'PUT'
+			],
+			$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
+		);
 		$this->dispatcher = new Dispatcher(
 			$this->http, $this->middlewareDispatcher, $this->reflector,
 			$this->request
@@ -398,19 +413,22 @@ class DispatcherTest extends \Test\TestCase {
 
 
 	public function testResponsePrimarilyTransformedByParameterFormat() {
-		$this->request = new Request(array(
-			'post' => array(
-				'int' => '3',
-				'bool' => 'false'
-			),
-			'get' => array(
-				'format' => 'text'
-			),
-			'server' => array(
-				'HTTP_ACCEPT' => 'application/json, test'
-			),
-			'method' => 'POST'
-		));
+		$this->request = new Request(
+			[
+				'post' => [
+					'int' => '3',
+					'bool' => 'false'
+				],
+				'get' => [
+					'format' => 'text'
+				],
+				'server' => [
+					'HTTP_ACCEPT' => 'application/json, test'
+				],
+				'method' => 'POST'
+			],
+			$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
+		);
 		$this->dispatcher = new Dispatcher(
 			$this->http, $this->middlewareDispatcher, $this->reflector,
 			$this->request
diff --git a/tests/lib/appframework/http/RequestTest.php b/tests/lib/appframework/http/RequestTest.php
index 85db76efe71..eeba64b7f69 100644
--- a/tests/lib/appframework/http/RequestTest.php
+++ b/tests/lib/appframework/http/RequestTest.php
@@ -8,9 +8,13 @@
 
 namespace OC\AppFramework\Http;
 
-global $data;
+use OCP\Security\ISecureRandom;
 
 class RequestTest extends \Test\TestCase {
+	/** @var string */
+	protected $stream = 'fakeinput://data';
+	/** @var ISecureRandom */
+	protected $secureRandom;
 
 	protected function setUp() {
 		parent::setUp();
@@ -20,7 +24,8 @@ class RequestTest extends \Test\TestCase {
 			stream_wrapper_unregister('fakeinput');
 		}
 		stream_wrapper_register('fakeinput', 'RequestStream');
-		$this->stream = 'fakeinput://data';
+
+		$this->secureRandom = $this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock();
 	}
 
 	protected function tearDown() {
@@ -34,7 +39,7 @@ class RequestTest extends \Test\TestCase {
 			'method' => 'GET',
 		);
 
-		$request = new Request($vars, $this->stream);
+		$request = new Request($vars, $this->secureRandom, $this->stream);
 
 		// Countable
 		$this->assertEquals(2, count($request));
@@ -61,7 +66,7 @@ class RequestTest extends \Test\TestCase {
 			'method' => 'GET'
 		);
 
-		$request = new Request($vars, $this->stream);
+		$request = new Request($vars, $this->secureRandom, $this->stream);
 
 		$this->assertEquals(3, count($request));
 		$this->assertEquals('Janey', $request->{'nickname'});
@@ -78,7 +83,7 @@ class RequestTest extends \Test\TestCase {
 			'method' => 'GET'
 		);
 
-		$request = new Request($vars, $this->stream);
+		$request = new Request($vars, $this->secureRandom, $this->stream);
 		$request['nickname'] = 'Janey';
 	}
 
@@ -91,7 +96,7 @@ class RequestTest extends \Test\TestCase {
 			'method' => 'GET'
 		);
 
-		$request = new Request($vars, $this->stream);
+		$request = new Request($vars, $this->secureRandom, $this->stream);
 		$request->{'nickname'} = 'Janey';
 	}
 
@@ -104,7 +109,7 @@ class RequestTest extends \Test\TestCase {
 			'method' => 'GET',
 		);
 
-		$request = new Request($vars, $this->stream);
+		$request = new Request($vars, $this->secureRandom, $this->stream);
 		$result = $request->post;
 	}
 
@@ -114,7 +119,7 @@ class RequestTest extends \Test\TestCase {
 			'method' => 'GET',
 		);
 
-		$request = new Request($vars, $this->stream);
+		$request = new Request($vars, $this->secureRandom, $this->stream);
 		$this->assertEquals('GET', $request->method);
 		$result = $request->get;
 		$this->assertEquals('John Q. Public', $result['name']);
@@ -129,7 +134,7 @@ class RequestTest extends \Test\TestCase {
 			'server' => array('CONTENT_TYPE' => 'application/json; utf-8')
 		);
 
-		$request = new Request($vars, $this->stream);
+		$request = new Request($vars, $this->secureRandom, $this->stream);
 		$this->assertEquals('POST', $request->method);
 		$result = $request->post;
 		$this->assertEquals('John Q. Public', $result['name']);
@@ -147,7 +152,7 @@ class RequestTest extends \Test\TestCase {
 			'server' => array('CONTENT_TYPE' => 'application/x-www-form-urlencoded'),
 		);
 
-		$request = new Request($vars, $this->stream);
+		$request = new Request($vars, $this->secureRandom, $this->stream);
 
 		$this->assertEquals('PATCH', $request->method);
 		$result = $request->patch;
@@ -166,7 +171,7 @@ class RequestTest extends \Test\TestCase {
 			'server' => array('CONTENT_TYPE' => 'application/json; utf-8'),
 		);
 
-		$request = new Request($vars, $this->stream);
+		$request = new Request($vars, $this->secureRandom, $this->stream);
 
 		$this->assertEquals('PUT', $request->method);
 		$result = $request->put;
@@ -181,7 +186,7 @@ class RequestTest extends \Test\TestCase {
 			'server' => array('CONTENT_TYPE' => 'application/json; utf-8'),
 		);
 
-		$request = new Request($vars, $this->stream);
+		$request = new Request($vars, $this->secureRandom, $this->stream);
 
 		$this->assertEquals('PATCH', $request->method);
 		$result = $request->patch;
@@ -200,7 +205,7 @@ class RequestTest extends \Test\TestCase {
 			'server' => array('CONTENT_TYPE' => 'image/png'),
 		);
 
-		$request = new Request($vars, $this->stream);
+		$request = new Request($vars, $this->secureRandom, $this->stream);
 		$this->assertEquals('PUT', $request->method);
 		$resource = $request->put;
 		$contents = stream_get_contents($resource);
@@ -223,7 +228,7 @@ class RequestTest extends \Test\TestCase {
 			'urlParams' => array('id' => '2'),
 		);
 
-		$request = new Request($vars, $this->stream);
+		$request = new Request($vars, $this->secureRandom, $this->stream);
 
 		$newParams = array('id' => '3', 'test' => 'test2');
 		$request->setUrlParameters($newParams);
@@ -231,4 +236,39 @@ class RequestTest extends \Test\TestCase {
 		$this->assertEquals('3', $request->getParam('id'));
 		$this->assertEquals('3', $request->getParams()['id']);
 	}
+
+	public function testGetIdWithModUnique() {
+		$vars = [
+			'server' => [
+				'UNIQUE_ID' => 'GeneratedUniqueIdByModUnique'
+			],
+		];
+
+		$request = new Request($vars, $this->secureRandom, $this->stream);
+		$this->assertSame('GeneratedUniqueIdByModUnique', $request->getId());
+	}
+
+	public function testGetIdWithoutModUnique() {
+		$lowRandomSource = $this->getMockBuilder('\OCP\Security\ISecureRandom')
+			->disableOriginalConstructor()->getMock();
+		$lowRandomSource->expects($this->once())
+			->method('generate')
+			->with('20')
+			->will($this->returnValue('GeneratedByOwnCloudItself'));
+
+		$this->secureRandom
+			->expects($this->once())
+			->method('getLowStrengthGenerator')
+			->will($this->returnValue($lowRandomSource));
+
+		$request = new Request([], $this->secureRandom, $this->stream);
+		$this->assertSame('GeneratedByOwnCloudItself', $request->getId());
+	}
+
+	public function testGetIdWithoutModUniqueStable() {
+		$request = new Request([], \OC::$server->getSecureRandom(), $this->stream);
+		$firstId = $request->getId();
+		$secondId = $request->getId();
+		$this->assertSame($firstId, $secondId);
+	}
 }
diff --git a/tests/lib/appframework/middleware/MiddlewareDispatcherTest.php b/tests/lib/appframework/middleware/MiddlewareDispatcherTest.php
index be8765afd39..078543c7b59 100644
--- a/tests/lib/appframework/middleware/MiddlewareDispatcherTest.php
+++ b/tests/lib/appframework/middleware/MiddlewareDispatcherTest.php
@@ -126,8 +126,16 @@ class MiddlewareDispatcherTest extends \Test\TestCase {
 
 
 	private function getControllerMock(){
-		return $this->getMock('OCP\AppFramework\Controller', array('method'),
-			array('app', new Request(array('method' => 'GET'))));
+		return $this->getMock(
+			'OCP\AppFramework\Controller',
+			['method'],
+			['app',
+				new Request(
+					['method' => 'GET'],
+					$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
+				)
+			]
+		);
 	}
 
 
diff --git a/tests/lib/appframework/middleware/MiddlewareTest.php b/tests/lib/appframework/middleware/MiddlewareTest.php
index b41ec33eb15..fcc0c300a8a 100644
--- a/tests/lib/appframework/middleware/MiddlewareTest.php
+++ b/tests/lib/appframework/middleware/MiddlewareTest.php
@@ -51,8 +51,14 @@ class MiddlewareTest extends \Test\TestCase {
 				->disableOriginalConstructor()
 				->getMock();
 
-		$this->controller = $this->getMock('OCP\AppFramework\Controller',
-				array(), array($this->api, new Request()));
+		$this->controller = $this->getMock(
+			'OCP\AppFramework\Controller',
+			[],
+			[
+				$this->api,
+				new Request([], $this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock())
+			]
+		);
 		$this->exception = new \Exception();
 		$this->response = $this->getMock('OCP\AppFramework\Http\Response');
 	}
diff --git a/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php b/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php
index b4bbcce5ad7..57a7c524abe 100644
--- a/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php
+++ b/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php
@@ -32,7 +32,12 @@ class CORSMiddlewareTest extends \Test\TestCase {
 	 */
 	public function testSetCORSAPIHeader() {
 		$request = new Request(
-			array('server' => array('HTTP_ORIGIN' => 'test'))
+			[
+				'server' => [
+					'HTTP_ORIGIN' => 'test'
+				]
+			],
+			$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
 		);
 		$this->reflector->reflect($this, __FUNCTION__);
 		$middleware = new CORSMiddleware($request, $this->reflector);
@@ -45,7 +50,12 @@ class CORSMiddlewareTest extends \Test\TestCase {
 
 	public function testNoAnnotationNoCORSHEADER() {
 		$request = new Request(
-			array('server' => array('HTTP_ORIGIN' => 'test'))
+			[
+				'server' => [
+					'HTTP_ORIGIN' => 'test'
+				]
+			],
+			$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
 		);
 		$middleware = new CORSMiddleware($request, $this->reflector);
 
@@ -59,7 +69,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
 	 * @CORS
 	 */
 	public function testNoOriginHeaderNoCORSHEADER() {
-		$request = new Request();
+		$request = new Request([], $this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock());
 		$this->reflector->reflect($this, __FUNCTION__);
 		$middleware = new CORSMiddleware($request, $this->reflector);
 
@@ -75,7 +85,12 @@ class CORSMiddlewareTest extends \Test\TestCase {
 	 */
 	public function testCorsIgnoredIfWithCredentialsHeaderPresent() {
 		$request = new Request(
-			array('server' => array('HTTP_ORIGIN' => 'test'))
+			[
+				'server' => [
+					'HTTP_ORIGIN' => 'test'
+				]
+			],
+			$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
 		);
 		$this->reflector->reflect($this, __FUNCTION__);
 		$middleware = new CORSMiddleware($request, $this->reflector);
diff --git a/tests/lib/appframework/middleware/security/SecurityMiddlewareTest.php b/tests/lib/appframework/middleware/security/SecurityMiddlewareTest.php
index a8925403a95..3acba7ce1d8 100644
--- a/tests/lib/appframework/middleware/security/SecurityMiddlewareTest.php
+++ b/tests/lib/appframework/middleware/security/SecurityMiddlewareTest.php
@@ -314,10 +314,14 @@ class SecurityMiddlewareTest extends \Test\TestCase {
 
 	public function testAfterExceptionReturnsRedirect(){
 		$this->request = new Request(
-			array('server' =>
-				array('HTTP_ACCEPT' => 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8',
-					'REQUEST_URI' => 'owncloud/index.php/apps/specialapp')
-			)
+			[
+				'server' =>
+				[
+					'HTTP_ACCEPT' => 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8',
+					'REQUEST_URI' => 'owncloud/index.php/apps/specialapp'
+				]
+			],
+			$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
 		);
 		$this->middleware = $this->getMiddleware(true, true);
 		$response = $this->middleware->afterException($this->controller, 'test',
diff --git a/tests/lib/appframework/middleware/sessionmiddlewaretest.php b/tests/lib/appframework/middleware/sessionmiddlewaretest.php
index 344b555ec3c..c417225d908 100644
--- a/tests/lib/appframework/middleware/sessionmiddlewaretest.php
+++ b/tests/lib/appframework/middleware/sessionmiddlewaretest.php
@@ -33,7 +33,10 @@ class SessionMiddlewareTest extends \Test\TestCase {
 	protected function setUp() {
 		parent::setUp();
 
-		$this->request = new Request();
+		$this->request = new Request(
+			[],
+			$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
+		);
 		$this->reflector = new ControllerMethodReflector();
 	}