From f93a82b8b09109e391b0314f92d02d285c356ad6 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Tue, 1 Aug 2017 17:32:03 +0200 Subject: [PATCH] Remove explicit type hints for Controller This is public API and breaks the middlewares of existing apps. Since this also requires maintaining two different code paths for 12 and 13 I'm at the moment voting for reverting this change. Signed-off-by: Lukas Reschke --- apps/federation/lib/Middleware/AddServerMiddleware.php | 2 +- .../lib/Middleware/OCSShareAPIMiddleware.php | 4 ++-- .../lib/Middleware/SharingCheckMiddleware.php | 4 ++-- .../lib/Middleware/ProvisioningApiMiddleware.php | 4 ++-- core/Middleware/TwoFactorMiddleware.php | 4 ++-- lib/private/AppFramework/Middleware/OCSMiddleware.php | 6 +++--- .../Middleware/Security/BruteForceMiddleware.php | 4 ++-- .../AppFramework/Middleware/Security/CORSMiddleware.php | 6 +++--- .../Middleware/Security/RateLimitingMiddleware.php | 4 ++-- .../Middleware/Security/SecurityMiddleware.php | 6 +++--- lib/private/AppFramework/Middleware/SessionMiddleware.php | 4 ++-- lib/public/AppFramework/Middleware.php | 8 ++++---- settings/Middleware/SubadminMiddleware.php | 4 ++-- .../AppFramework/Middleware/MiddlewareDispatcherTest.php | 8 ++++---- 14 files changed, 34 insertions(+), 34 deletions(-) diff --git a/apps/federation/lib/Middleware/AddServerMiddleware.php b/apps/federation/lib/Middleware/AddServerMiddleware.php index 5f67ad6b7a..3c2e6daf60 100644 --- a/apps/federation/lib/Middleware/AddServerMiddleware.php +++ b/apps/federation/lib/Middleware/AddServerMiddleware.php @@ -59,7 +59,7 @@ class AddServerMiddleware extends Middleware { * @return JSONResponse * @throws \Exception */ - public function afterException(Controller $controller, $methodName, \Exception $exception) { + public function afterException($controller, $methodName, \Exception $exception) { if (($controller instanceof SettingsController) === false) { throw $exception; } diff --git a/apps/files_sharing/lib/Middleware/OCSShareAPIMiddleware.php b/apps/files_sharing/lib/Middleware/OCSShareAPIMiddleware.php index dd33701250..9df0229ab8 100644 --- a/apps/files_sharing/lib/Middleware/OCSShareAPIMiddleware.php +++ b/apps/files_sharing/lib/Middleware/OCSShareAPIMiddleware.php @@ -28,7 +28,7 @@ class OCSShareAPIMiddleware extends Middleware { * * @throws OCSNotFoundException */ - public function beforeController(Controller $controller, $methodName) { + public function beforeController($controller, $methodName) { if ($controller instanceof ShareAPIController) { if (!$this->shareManager->shareApiEnabled()) { throw new OCSNotFoundException($this->l->t('Share API is disabled')); @@ -42,7 +42,7 @@ class OCSShareAPIMiddleware extends Middleware { * @param Response $response * @return Response */ - public function afterController(Controller $controller, $methodName, Response $response) { + public function afterController($controller, $methodName, Response $response) { if ($controller instanceof ShareAPIController) { /** @var ShareAPIController $controller */ $controller->cleanup(); diff --git a/apps/files_sharing/lib/Middleware/SharingCheckMiddleware.php b/apps/files_sharing/lib/Middleware/SharingCheckMiddleware.php index 3dd4ad718b..e1a77fdec3 100644 --- a/apps/files_sharing/lib/Middleware/SharingCheckMiddleware.php +++ b/apps/files_sharing/lib/Middleware/SharingCheckMiddleware.php @@ -92,7 +92,7 @@ class SharingCheckMiddleware extends Middleware { * @throws S2SException * @throws ShareNotFound */ - public function beforeController(Controller $controller, $methodName) { + public function beforeController($controller, $methodName) { if(!$this->isSharingEnabled()) { throw new NotFoundException('Sharing is disabled.'); } @@ -119,7 +119,7 @@ class SharingCheckMiddleware extends Middleware { * @return NotFoundResponse * @throws \Exception */ - public function afterException(Controller $controller, $methodName, \Exception $exception) { + public function afterException($controller, $methodName, \Exception $exception) { if(is_a($exception, '\OCP\Files\NotFoundException')) { return new NotFoundResponse(); } diff --git a/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php b/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php index 6245d2be90..e21f9b2287 100644 --- a/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php +++ b/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php @@ -42,7 +42,7 @@ class ProvisioningApiMiddleware extends Middleware { * * @throws NotSubAdminException */ - public function beforeController(Controller $controller, $methodName) { + public function beforeController($controller, $methodName) { if (!$this->isAdmin && !$this->reflector->hasAnnotation('NoSubAdminRequired') && !$this->isSubAdmin) { throw new NotSubAdminException(); } @@ -55,7 +55,7 @@ class ProvisioningApiMiddleware extends Middleware { * @throws \Exception * @return Response */ - public function afterException(Controller $controller, $methodName, \Exception $exception) { + public function afterException($controller, $methodName, \Exception $exception) { if ($exception instanceof NotSubAdminException) { throw new OCSException($exception->getMessage(), \OCP\API::RESPOND_UNAUTHORISED); } diff --git a/core/Middleware/TwoFactorMiddleware.php b/core/Middleware/TwoFactorMiddleware.php index b8edda4db6..a4e0d7219e 100644 --- a/core/Middleware/TwoFactorMiddleware.php +++ b/core/Middleware/TwoFactorMiddleware.php @@ -79,7 +79,7 @@ class TwoFactorMiddleware extends Middleware { * @param Controller $controller * @param string $methodName */ - public function beforeController(Controller $controller, $methodName) { + public function beforeController($controller, $methodName) { if ($this->reflector->hasAnnotation('PublicPage')) { // Don't block public pages return; @@ -122,7 +122,7 @@ class TwoFactorMiddleware extends Middleware { } } - public function afterException(Controller $controller, $methodName, Exception $exception) { + public function afterException($controller, $methodName, Exception $exception) { if ($exception instanceof TwoFactorAuthRequiredException) { $params = []; if (isset($this->request->server['REQUEST_URI'])) { diff --git a/lib/private/AppFramework/Middleware/OCSMiddleware.php b/lib/private/AppFramework/Middleware/OCSMiddleware.php index 50ee40b7b4..46f2881b07 100644 --- a/lib/private/AppFramework/Middleware/OCSMiddleware.php +++ b/lib/private/AppFramework/Middleware/OCSMiddleware.php @@ -55,7 +55,7 @@ class OCSMiddleware extends Middleware { * @param Controller $controller * @param string $methodName */ - public function beforeController(Controller $controller, $methodName) { + public function beforeController($controller, $methodName) { if ($controller instanceof OCSController) { if (substr_compare($this->request->getScriptName(), '/ocs/v2.php', -strlen('/ocs/v2.php')) === 0) { $this->ocsVersion = 2; @@ -73,7 +73,7 @@ class OCSMiddleware extends Middleware { * @throws \Exception * @return BaseResponse */ - public function afterException(Controller $controller, $methodName, \Exception $exception) { + public function afterException($controller, $methodName, \Exception $exception) { if ($controller instanceof OCSController && $exception instanceof OCSException) { $code = $exception->getCode(); if ($code === 0) { @@ -92,7 +92,7 @@ class OCSMiddleware extends Middleware { * @param Response $response * @return \OCP\AppFramework\Http\Response */ - public function afterController(Controller $controller, $methodName, Response $response) { + public function afterController($controller, $methodName, Response $response) { /* * If a different middleware has detected that a request unauthorized or forbidden * we need to catch the response and convert it to a proper OCS response. diff --git a/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php b/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php index b7ec137062..e349960115 100644 --- a/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php @@ -59,7 +59,7 @@ class BruteForceMiddleware extends Middleware { /** * {@inheritDoc} */ - public function beforeController(Controller $controller, $methodName) { + public function beforeController($controller, $methodName) { parent::beforeController($controller, $methodName); if($this->reflector->hasAnnotation('BruteForceProtection')) { @@ -71,7 +71,7 @@ class BruteForceMiddleware extends Middleware { /** * {@inheritDoc} */ - public function afterController(Controller $controller, $methodName, Response $response) { + public function afterController($controller, $methodName, Response $response) { if($this->reflector->hasAnnotation('BruteForceProtection') && $response->isThrottled()) { $action = $this->reflector->getAnnotationParameter('BruteForceProtection', 'action'); $ip = $this->request->getRemoteAddress(); diff --git a/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php b/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php index 77ad743059..4b50b0d20b 100644 --- a/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php @@ -80,7 +80,7 @@ class CORSMiddleware extends Middleware { * @throws SecurityException * @since 6.0.0 */ - public function beforeController(Controller $controller, $methodName){ + public function beforeController($controller, $methodName){ // ensure that @CORS annotated API routes are not used in conjunction // with session authentication since this enables CSRF attack vectors if ($this->reflector->hasAnnotation('CORS') && @@ -110,7 +110,7 @@ class CORSMiddleware extends Middleware { * @return Response a Response object * @throws SecurityException */ - public function afterController(Controller $controller, $methodName, Response $response){ + public function afterController($controller, $methodName, Response $response){ // only react if its a CORS request and if the request sends origin and if(isset($this->request->server['HTTP_ORIGIN']) && @@ -143,7 +143,7 @@ class CORSMiddleware extends Middleware { * @throws \Exception the passed in exception if it can't handle it * @return Response a Response object or null in case that the exception could not be handled */ - public function afterException(Controller $controller, $methodName, \Exception $exception){ + public function afterException($controller, $methodName, \Exception $exception){ if($exception instanceof SecurityException){ $response = new JSONResponse(['message' => $exception->getMessage()]); if($exception->getCode() !== 0) { diff --git a/lib/private/AppFramework/Middleware/Security/RateLimitingMiddleware.php b/lib/private/AppFramework/Middleware/Security/RateLimitingMiddleware.php index c73b31a617..28ef8b43ff 100644 --- a/lib/private/AppFramework/Middleware/Security/RateLimitingMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/RateLimitingMiddleware.php @@ -77,7 +77,7 @@ class RateLimitingMiddleware extends Middleware { * {@inheritDoc} * @throws RateLimitExceededException */ - public function beforeController(Controller $controller, $methodName) { + public function beforeController($controller, $methodName) { parent::beforeController($controller, $methodName); $anonLimit = $this->reflector->getAnnotationParameter('AnonRateThrottle', 'limit'); @@ -105,7 +105,7 @@ class RateLimitingMiddleware extends Middleware { /** * {@inheritDoc} */ - public function afterException(Controller $controller, $methodName, \Exception $exception) { + public function afterException($controller, $methodName, \Exception $exception) { if($exception instanceof RateLimitExceededException) { if (stripos($this->request->getHeader('Accept'),'html') === false) { $response = new JSONResponse( diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php index becbd7b9ca..4e41c94643 100644 --- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php @@ -136,7 +136,7 @@ class SecurityMiddleware extends Middleware { * @param string $methodName the name of the method * @throws SecurityException when a security check fails */ - public function beforeController(Controller $controller, $methodName) { + public function beforeController($controller, $methodName) { // this will set the current navigation entry of the app, use this only // for normal HTML requests and not for AJAX requests @@ -205,7 +205,7 @@ class SecurityMiddleware extends Middleware { * @param Response $response * @return Response */ - public function afterController(Controller $controller, $methodName, Response $response) { + public function afterController($controller, $methodName, Response $response) { $policy = !is_null($response->getContentSecurityPolicy()) ? $response->getContentSecurityPolicy() : new ContentSecurityPolicy(); if (get_class($policy) === EmptyContentSecurityPolicy::class) { @@ -234,7 +234,7 @@ class SecurityMiddleware extends Middleware { * @throws \Exception the passed in exception if it can't handle it * @return Response a Response object or null in case that the exception could not be handled */ - public function afterException(Controller $controller, $methodName, \Exception $exception) { + public function afterException($controller, $methodName, \Exception $exception) { if($exception instanceof SecurityException) { if($exception instanceof StrictCookieMissingException) { return new RedirectResponse(\OC::$WEBROOT); diff --git a/lib/private/AppFramework/Middleware/SessionMiddleware.php b/lib/private/AppFramework/Middleware/SessionMiddleware.php index f2545653e8..dd2029bf05 100644 --- a/lib/private/AppFramework/Middleware/SessionMiddleware.php +++ b/lib/private/AppFramework/Middleware/SessionMiddleware.php @@ -59,7 +59,7 @@ class SessionMiddleware extends Middleware { * @param Controller $controller * @param string $methodName */ - public function beforeController(Controller $controller, $methodName) { + public function beforeController($controller, $methodName) { $useSession = $this->reflector->hasAnnotation('UseSession'); if (!$useSession) { $this->session->close(); @@ -72,7 +72,7 @@ class SessionMiddleware extends Middleware { * @param Response $response * @return Response */ - public function afterController(Controller $controller, $methodName, Response $response){ + public function afterController($controller, $methodName, Response $response){ $useSession = $this->reflector->hasAnnotation('UseSession'); if ($useSession) { $this->session->close(); diff --git a/lib/public/AppFramework/Middleware.php b/lib/public/AppFramework/Middleware.php index fbd75981b5..677e5c2e7e 100644 --- a/lib/public/AppFramework/Middleware.php +++ b/lib/public/AppFramework/Middleware.php @@ -52,7 +52,7 @@ abstract class Middleware { * the controller * @since 6.0.0 */ - public function beforeController(Controller $controller, $methodName){ + public function beforeController($controller, $methodName){ } @@ -72,7 +72,7 @@ abstract class Middleware { * @return Response a Response object in case that the exception was handled * @since 6.0.0 */ - public function afterException(Controller $controller, $methodName, \Exception $exception){ + public function afterException($controller, $methodName, \Exception $exception){ throw $exception; } @@ -88,7 +88,7 @@ abstract class Middleware { * @return Response a Response object * @since 6.0.0 */ - public function afterController(Controller $controller, $methodName, Response $response){ + public function afterController($controller, $methodName, Response $response){ return $response; } @@ -104,7 +104,7 @@ abstract class Middleware { * @return string the output that should be printed * @since 6.0.0 */ - public function beforeOutput(Controller $controller, $methodName, $output){ + public function beforeOutput($controller, $methodName, $output){ return $output; } diff --git a/settings/Middleware/SubadminMiddleware.php b/settings/Middleware/SubadminMiddleware.php index 23f5ebac6b..9914d65af0 100644 --- a/settings/Middleware/SubadminMiddleware.php +++ b/settings/Middleware/SubadminMiddleware.php @@ -59,7 +59,7 @@ class SubadminMiddleware extends Middleware { * @param string $methodName * @throws \Exception */ - public function beforeController(Controller $controller, $methodName) { + public function beforeController($controller, $methodName) { if(!$this->reflector->hasAnnotation('NoSubadminRequired')) { if(!$this->isSubAdmin) { throw new NotAdminException('Logged in user must be a subadmin'); @@ -75,7 +75,7 @@ class SubadminMiddleware extends Middleware { * @return TemplateResponse * @throws \Exception */ - public function afterException(Controller $controller, $methodName, \Exception $exception) { + public function afterException($controller, $methodName, \Exception $exception) { if($exception instanceof NotAdminException) { $response = new TemplateResponse('core', '403', array(), 'guest'); $response->setStatus(Http::STATUS_FORBIDDEN); diff --git a/tests/lib/AppFramework/Middleware/MiddlewareDispatcherTest.php b/tests/lib/AppFramework/Middleware/MiddlewareDispatcherTest.php index 9eca127cb8..f948e184f5 100644 --- a/tests/lib/AppFramework/Middleware/MiddlewareDispatcherTest.php +++ b/tests/lib/AppFramework/Middleware/MiddlewareDispatcherTest.php @@ -62,7 +62,7 @@ class TestMiddleware extends Middleware { $this->beforeControllerThrowsEx = $beforeControllerThrowsEx; } - public function beforeController(Controller $controller, $methodName){ + public function beforeController($controller, $methodName){ self::$beforeControllerCalled++; $this->beforeControllerOrder = self::$beforeControllerCalled; $this->controller = $controller; @@ -72,7 +72,7 @@ class TestMiddleware extends Middleware { } } - public function afterException(Controller $controller, $methodName, \Exception $exception){ + public function afterException($controller, $methodName, \Exception $exception){ self::$afterExceptionCalled++; $this->afterExceptionOrder = self::$afterExceptionCalled; $this->controller = $controller; @@ -81,7 +81,7 @@ class TestMiddleware extends Middleware { parent::afterException($controller, $methodName, $exception); } - public function afterController(Controller $controller, $methodName, Response $response){ + public function afterController($controller, $methodName, Response $response){ self::$afterControllerCalled++; $this->afterControllerOrder = self::$afterControllerCalled; $this->controller = $controller; @@ -90,7 +90,7 @@ class TestMiddleware extends Middleware { return parent::afterController($controller, $methodName, $response); } - public function beforeOutput(Controller $controller, $methodName, $output){ + public function beforeOutput($controller, $methodName, $output){ self::$beforeOutputCalled++; $this->beforeOutputOrder = self::$beforeOutputCalled; $this->controller = $controller;