Skip to content

Commit c19b58b

Browse files
CarlSchwanCarlSchwan
committed
perf: Remove MiddlewareUtils overhead
Avoid doing some reflection Signed-off-by: Carl Schwan <carl@carlschwan.eu>
1 parent 083ffeb commit c19b58b

14 files changed

Lines changed: 82 additions & 130 deletions

File tree

‎apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,9 @@ public function beforeController(Controller $controller, string $methodName): vo
5555
* @param string $methodName
5656
* @param \Exception $exception
5757
* @throws \Exception
58-
* @return Response
5958
*/
6059
#[\Override]
61-
public function afterException(Controller $controller, string $methodName, \Exception $exception) {
60+
public function afterException(Controller $controller, string $methodName, \Exception $exception): never {
6261
if ($exception instanceof NotSubAdminException) {
6362
throw new OCSException($exception->getMessage(), Http::STATUS_FORBIDDEN);
6463
}

‎apps/settings/tests/Controller/AuthSettingsControllerTest.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ public function testCreateDisabledBySystemConfig(): void {
152152
$expected = new JSONResponse();
153153
$expected->setStatus(Http::STATUS_SERVICE_UNAVAILABLE);
154154

155-
$this->assertEquals($expected, $this->controller->create($name)->getData());
155+
$this->assertEquals($expected, $this->controller->create($name));
156156
}
157157

158158
public function testCreateSessionNotAvailable(): void {
@@ -165,7 +165,7 @@ public function testCreateSessionNotAvailable(): void {
165165
$expected = new JSONResponse();
166166
$expected->setStatus(Http::STATUS_SERVICE_UNAVAILABLE);
167167

168-
$this->assertEquals($expected, $this->controller->create($name)->getData());
168+
$this->assertEquals($expected, $this->controller->create($name));
169169
}
170170

171171
public function testCreateInvalidToken(): void {
@@ -185,7 +185,7 @@ public function testCreateInvalidToken(): void {
185185
$expected = new JSONResponse();
186186
$expected->setStatus(Http::STATUS_SERVICE_UNAVAILABLE);
187187

188-
$this->assertEquals($expected, $this->controller->create($name)->getData());
188+
$this->assertEquals($expected, $this->controller->create($name));
189189
}
190190

191191
public function testDestroy(): void {

‎build/psalm-baseline.xml‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2293,14 +2293,6 @@
22932293
<code><![CDATA[$groupid === null]]></code>
22942294
</TypeDoesNotContainNull>
22952295
</file>
2296-
<file src="apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php">
2297-
<DeprecatedMethod>
2298-
<code><![CDATA[hasAnnotation]]></code>
2299-
</DeprecatedMethod>
2300-
<InvalidReturnType>
2301-
<code><![CDATA[Response]]></code>
2302-
</InvalidReturnType>
2303-
</file>
23042296
<file src="apps/settings/lib/BackgroundJobs/VerifyUserData.php">
23052297
<DeprecatedConstant>
23062298
<code><![CDATA[IAccountManager::PROPERTY_TWITTER]]></code>

‎lib/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@
9797
'OCP\\AppFramework\\Http\\Attribute\\NoAdminRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/NoAdminRequired.php',
9898
'OCP\\AppFramework\\Http\\Attribute\\NoCSRFRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/NoCSRFRequired.php',
9999
'OCP\\AppFramework\\Http\\Attribute\\NoSameSiteCookieRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/NoSameSiteCookieRequired.php',
100+
'OCP\\AppFramework\\Http\\Attribute\\NoSubAdminRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/NoSubAdminRequired.php',
100101
'OCP\\AppFramework\\Http\\Attribute\\NoTwoFactorRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/NoTwoFactorRequired.php',
101102
'OCP\\AppFramework\\Http\\Attribute\\OpenAPI' => $baseDir . '/lib/public/AppFramework/Http/Attribute/OpenAPI.php',
102103
'OCP\\AppFramework\\Http\\Attribute\\PasswordConfirmationRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/PasswordConfirmationRequired.php',
@@ -1177,7 +1178,6 @@
11771178
'OC\\AppFramework\\Middleware\\AdditionalScriptsMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/AdditionalScriptsMiddleware.php',
11781179
'OC\\AppFramework\\Middleware\\CompressionMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/CompressionMiddleware.php',
11791180
'OC\\AppFramework\\Middleware\\MiddlewareDispatcher' => $baseDir . '/lib/private/AppFramework/Middleware/MiddlewareDispatcher.php',
1180-
'OC\\AppFramework\\Middleware\\MiddlewareUtils' => $baseDir . '/lib/private/AppFramework/Middleware/MiddlewareUtils.php',
11811181
'OC\\AppFramework\\Middleware\\NotModifiedMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/NotModifiedMiddleware.php',
11821182
'OC\\AppFramework\\Middleware\\OCSMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/OCSMiddleware.php',
11831183
'OC\\AppFramework\\Middleware\\PublicShare\\Exceptions\\NeedAuthenticationException' => $baseDir . '/lib/private/AppFramework/Middleware/PublicShare/Exceptions/NeedAuthenticationException.php',

‎lib/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
138138
'OCP\\AppFramework\\Http\\Attribute\\NoAdminRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/NoAdminRequired.php',
139139
'OCP\\AppFramework\\Http\\Attribute\\NoCSRFRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/NoCSRFRequired.php',
140140
'OCP\\AppFramework\\Http\\Attribute\\NoSameSiteCookieRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/NoSameSiteCookieRequired.php',
141+
'OCP\\AppFramework\\Http\\Attribute\\NoSubAdminRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/NoSubAdminRequired.php',
141142
'OCP\\AppFramework\\Http\\Attribute\\NoTwoFactorRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/NoTwoFactorRequired.php',
142143
'OCP\\AppFramework\\Http\\Attribute\\OpenAPI' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/OpenAPI.php',
143144
'OCP\\AppFramework\\Http\\Attribute\\PasswordConfirmationRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/PasswordConfirmationRequired.php',
@@ -1218,7 +1219,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
12181219
'OC\\AppFramework\\Middleware\\AdditionalScriptsMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/AdditionalScriptsMiddleware.php',
12191220
'OC\\AppFramework\\Middleware\\CompressionMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/CompressionMiddleware.php',
12201221
'OC\\AppFramework\\Middleware\\MiddlewareDispatcher' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/MiddlewareDispatcher.php',
1221-
'OC\\AppFramework\\Middleware\\MiddlewareUtils' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/MiddlewareUtils.php',
12221222
'OC\\AppFramework\\Middleware\\NotModifiedMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/NotModifiedMiddleware.php',
12231223
'OC\\AppFramework\\Middleware\\OCSMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/OCSMiddleware.php',
12241224
'OC\\AppFramework\\Middleware\\PublicShare\\Exceptions\\NeedAuthenticationException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/PublicShare/Exceptions/NeedAuthenticationException.php',

‎lib/private/AppFramework/DependencyInjection/DIContainer.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
use OC\AppFramework\Middleware\AdditionalScriptsMiddleware;
1818
use OC\AppFramework\Middleware\CompressionMiddleware;
1919
use OC\AppFramework\Middleware\MiddlewareDispatcher;
20-
use OC\AppFramework\Middleware\MiddlewareUtils;
2120
use OC\AppFramework\Middleware\NotModifiedMiddleware;
2221
use OC\AppFramework\Middleware\OCSMiddleware;
2322
use OC\AppFramework\Middleware\PublicShare\PublicShareMiddleware;
@@ -205,7 +204,7 @@ public function __construct(
205204

206205
$securityMiddleware = new SecurityMiddleware(
207206
$c->get(IRequest::class),
208-
$c->get(MiddlewareUtils::class),
207+
$c->get(ControllerMethodReflector::class),
209208
$c->get(INavigationManager::class),
210209
$c->get(IURLGenerator::class),
211210
$c->get(LoggerInterface::class),

‎lib/private/AppFramework/Middleware/MiddlewareUtils.php‎

Lines changed: 0 additions & 60 deletions
This file was deleted.

‎lib/private/AppFramework/Middleware/Security/CORSMiddleware.php‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88

99
namespace OC\AppFramework\Middleware\Security;
1010

11-
use OC\AppFramework\Middleware\MiddlewareUtils;
1211
use OC\AppFramework\Middleware\Security\Exceptions\SecurityException;
12+
use OC\AppFramework\Utility\ControllerMethodReflector;
1313
use OC\Authentication\Exceptions\PasswordLoginForbiddenException;
1414
use OC\User\Session;
1515
use OCP\AppFramework\Controller;
@@ -35,20 +35,18 @@ class CORSMiddleware extends Middleware {
3535

3636
public function __construct(
3737
private readonly IRequest $request,
38-
private readonly MiddlewareUtils $middlewareUtils,
38+
private readonly ControllerMethodReflector $reflector,
3939
private readonly Session $session,
4040
private readonly IThrottler $throttler,
4141
) {
4242
}
4343

4444
#[Override]
4545
public function beforeController(Controller $controller, string $methodName): void {
46-
$reflectionMethod = new ReflectionMethod($controller, $methodName);
47-
4846
// ensure that @CORS annotated API routes are not used in conjunction
4947
// with session authentication since this enables CSRF attack vectors
50-
if ($this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'CORS', CORS::class)
51-
&& (!$this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'PublicPage', PublicPage::class) || $this->session->isLoggedIn())) {
48+
if ($this->reflector->hasAnnotationOrAttribute('CORS', CORS::class)
49+
&& (!$this->reflector->hasAnnotationOrAttribute('PublicPage', PublicPage::class) || $this->session->isLoggedIn())) {
5250
$user = array_key_exists('PHP_AUTH_USER', $this->request->server) ? $this->request->server['PHP_AUTH_USER'] : null;
5351
$pass = array_key_exists('PHP_AUTH_PW', $this->request->server) ? $this->request->server['PHP_AUTH_PW'] : null;
5452

@@ -77,7 +75,7 @@ public function afterController(Controller $controller, string $methodName, Resp
7775

7876
if (isset($this->request->server['HTTP_ORIGIN'])) {
7977
$reflectionMethod = new ReflectionMethod($controller, $methodName);
80-
if ($this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'CORS', CORS::class)) {
78+
if ($this->reflector->hasAnnotationOrAttribute('CORS', CORS::class)) {
8179
// allow credentials headers must not be true or CSRF is possible
8280
// otherwise
8381
foreach ($response->getHeaders() as $header => $value) {

‎lib/private/AppFramework/Middleware/Security/SameSiteCookieMiddleware.php‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,18 @@
1010
namespace OC\AppFramework\Middleware\Security;
1111

1212
use OC\AppFramework\Http\Request;
13-
use OC\AppFramework\Middleware\MiddlewareUtils;
1413
use OC\AppFramework\Middleware\Security\Exceptions\LaxSameSiteCookieFailedException;
14+
use OC\AppFramework\Utility\ControllerMethodReflector;
1515
use OCP\AppFramework\Controller;
1616
use OCP\AppFramework\Http;
1717
use OCP\AppFramework\Http\Attribute\NoSameSiteCookieRequired;
1818
use OCP\AppFramework\Http\Response;
1919
use OCP\AppFramework\Middleware;
20-
use ReflectionMethod;
2120

2221
class SameSiteCookieMiddleware extends Middleware {
2322
public function __construct(
2423
private readonly Request $request,
25-
private readonly MiddlewareUtils $middlewareUtils,
24+
private readonly ControllerMethodReflector $reflector,
2625
) {
2726
}
2827

@@ -36,8 +35,7 @@ public function beforeController(Controller $controller, string $methodName): vo
3635
return;
3736
}
3837

39-
$reflectionMethod = new ReflectionMethod($controller, $methodName);
40-
$noSSC = $this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'NoSameSiteCookieRequired', NoSameSiteCookieRequired::class);
38+
$noSSC = $this->reflector->hasAnnotationOrAttribute('NoSameSiteCookieRequired', NoSameSiteCookieRequired::class);
4139
if ($noSSC) {
4240
return;
4341
}

0 commit comments

Comments
 (0)