From 1addd35b7831d42c9cd0a8c6e50b08a065fd5784 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcel=20M=C3=BCller?= Date: Tue, 29 Apr 2025 22:14:25 +0200 Subject: [PATCH 1/2] fix: Remove unneccesary etag check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Marcel Müller --- core/Controller/NavigationController.php | 12 ++----- .../Controller/NavigationControllerTest.php | 31 ------------------- 2 files changed, 2 insertions(+), 41 deletions(-) diff --git a/core/Controller/NavigationController.php b/core/Controller/NavigationController.php index de72e41294535..5fc929b4eb486 100644 --- a/core/Controller/NavigationController.php +++ b/core/Controller/NavigationController.php @@ -47,12 +47,8 @@ public function getAppsNavigation(bool $absolute = false): DataResponse { $navigation = $this->rewriteToAbsoluteUrls($navigation); } $navigation = array_values($navigation); - $etag = $this->generateETag($navigation); - if ($this->request->getHeader('If-None-Match') === $etag) { - return new DataResponse([], Http::STATUS_NOT_MODIFIED); - } $response = new DataResponse($navigation); - $response->setETag($etag); + $response->setETag($this->generateETag($navigation)); return $response; } @@ -74,12 +70,8 @@ public function getSettingsNavigation(bool $absolute = false): DataResponse { $navigation = $this->rewriteToAbsoluteUrls($navigation); } $navigation = array_values($navigation); - $etag = $this->generateETag($navigation); - if ($this->request->getHeader('If-None-Match') === $etag) { - return new DataResponse([], Http::STATUS_NOT_MODIFIED); - } $response = new DataResponse($navigation); - $response->setETag($etag); + $response->setETag($this->generateETag($navigation)); return $response; } diff --git a/tests/Core/Controller/NavigationControllerTest.php b/tests/Core/Controller/NavigationControllerTest.php index 7ebfbc139cdfd..acbee5a72f0b9 100644 --- a/tests/Core/Controller/NavigationControllerTest.php +++ b/tests/Core/Controller/NavigationControllerTest.php @@ -7,7 +7,6 @@ namespace Tests\Core\Controller; use OC\Core\Controller\NavigationController; -use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; use OCP\INavigationManager; use OCP\IRequest; @@ -103,34 +102,4 @@ public function testGetSettingsNavigation(bool $absolute): void { $this->assertEquals('/core/img/settings.svg', $actual->getData()[0]['icon']); } } - - public function testGetAppNavigationEtagMatch(): void { - $navigation = [ ['id' => 'files', 'href' => '/index.php/apps/files', 'icon' => 'icon' ] ]; - $this->request->expects($this->once()) - ->method('getHeader') - ->with('If-None-Match') - ->willReturn(md5(json_encode($navigation))); - $this->navigationManager->expects($this->once()) - ->method('getAll') - ->with('link') - ->willReturn($navigation); - $actual = $this->controller->getAppsNavigation(); - $this->assertInstanceOf(DataResponse::class, $actual); - $this->assertEquals(Http::STATUS_NOT_MODIFIED, $actual->getStatus()); - } - - public function testGetSettingsNavigationEtagMatch(): void { - $navigation = [ ['id' => 'logout', 'href' => '/index.php/apps/files', 'icon' => 'icon' ] ]; - $this->request->expects($this->once()) - ->method('getHeader') - ->with('If-None-Match') - ->willReturn(md5(json_encode([ ['id' => 'logout', 'href' => 'logout', 'icon' => 'icon' ] ]))); - $this->navigationManager->expects($this->once()) - ->method('getAll') - ->with('settings') - ->willReturn($navigation); - $actual = $this->controller->getSettingsNavigation(); - $this->assertInstanceOf(DataResponse::class, $actual); - $this->assertEquals(Http::STATUS_NOT_MODIFIED, $actual->getStatus()); - } } From ddd91793bcea23b2bdd0b5ccd8e4113de6e8ee95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcel=20M=C3=BCller?= Date: Wed, 30 Apr 2025 21:48:34 +0200 Subject: [PATCH 2/2] fix: Add etag tests to NavigationControllerTest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Marcel Müller --- .../Controller/NavigationControllerTest.php | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/Core/Controller/NavigationControllerTest.php b/tests/Core/Controller/NavigationControllerTest.php index acbee5a72f0b9..2df705c229671 100644 --- a/tests/Core/Controller/NavigationControllerTest.php +++ b/tests/Core/Controller/NavigationControllerTest.php @@ -102,4 +102,36 @@ public function testGetSettingsNavigation(bool $absolute): void { $this->assertEquals('/core/img/settings.svg', $actual->getData()[0]['icon']); } } + + public function testEtagIgnoresLogout(): void { + $navigation1 = [ + ['id' => 'files', 'href' => '/index.php/apps/files', 'icon' => 'icon' ], + ['id' => 'logout', 'href' => '/index.php/logout?requesttoken=abcd', 'icon' => 'icon' ], + ]; + $navigation2 = [ + ['id' => 'files', 'href' => '/index.php/apps/files', 'icon' => 'icon' ], + ['id' => 'logout', 'href' => '/index.php/logout?requesttoken=1234', 'icon' => 'icon' ], + ]; + $navigation3 = [ + ['id' => 'files', 'href' => '/index.php/apps/files/test', 'icon' => 'icon' ], + ['id' => 'logout', 'href' => '/index.php/logout?requesttoken=1234', 'icon' => 'icon' ], + ]; + $this->navigationManager->expects($this->exactly(3)) + ->method('getAll') + ->with('link') + ->willReturnOnConsecutiveCalls( + $navigation1, + $navigation2, + $navigation3, + ); + + // Changes in the logout url should not change the ETag + $request1 = $this->controller->getAppsNavigation(); + $request2 = $this->controller->getAppsNavigation(); + $this->assertEquals($request1->getETag(), $request2->getETag()); + + // Changes in non-logout urls should result in a different ETag + $request3 = $this->controller->getAppsNavigation(); + $this->assertNotEquals($request2->getETag(), $request3->getETag()); + } }