From 6dd60b6d304648acf542a84ba76ad5fc0b693b43 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 4 Feb 2022 12:12:48 +0100 Subject: [PATCH] Only allow avatars in 64 and 512 pixel size Signed-off-by: Joas Schilling --- build/integration/features/avatar.feature | 26 +++--- core/Controller/AvatarController.php | 13 ++- core/Controller/GuestAvatarController.php | 13 ++- .../Core/Controller/AvatarControllerTest.php | 90 +++++++++++++------ .../Controller/GuestAvatarControllerTest.php | 2 +- 5 files changed, 97 insertions(+), 47 deletions(-) diff --git a/build/integration/features/avatar.feature b/build/integration/features/avatar.feature index f7926615c01..2d6cb66548f 100644 --- a/build/integration/features/avatar.feature +++ b/build/integration/features/avatar.feature @@ -8,7 +8,7 @@ Feature: avatar Then The following headers should be set | Content-Type | image/png | | X-NC-IsCustomAvatar | 0 | - And last avatar is a square of size 128 + And last avatar is a square of size 512 And last avatar is not a single color Scenario: get default user avatar as an anonymous user @@ -16,7 +16,7 @@ Feature: avatar Then The following headers should be set | Content-Type | image/png | | X-NC-IsCustomAvatar | 0 | - And last avatar is a square of size 128 + And last avatar is a square of size 512 And last avatar is not a single color @@ -39,7 +39,7 @@ Feature: avatar Then The following headers should be set | Content-Type | image/png | | X-NC-IsCustomAvatar | 0 | - And last avatar is a square of size 128 + And last avatar is a square of size 512 And last avatar is not a single color @@ -57,13 +57,13 @@ Feature: avatar And The following headers should be set | Content-Type | image/png | | X-NC-IsCustomAvatar | 1 | - And last avatar is a square of size 128 + And last avatar is a square of size 512 And last avatar is a single "#FF0000" color And user "anonymous" gets avatar for user "user0" And The following headers should be set | Content-Type | image/png | | X-NC-IsCustomAvatar | 1 | - And last avatar is a square of size 128 + And last avatar is a square of size 512 And last avatar is a single "#FF0000" color Scenario: set user avatar from internal path @@ -112,26 +112,26 @@ Feature: avatar And The following headers should be set | Content-Type | image/png | | X-NC-IsCustomAvatar | 1 | - And last avatar is a square of size 128 + And last avatar is a square of size 512 And last avatar is a single "#FF0000" color And user "anonymous" gets avatar for user "user0" And The following headers should be set | Content-Type | image/png | | X-NC-IsCustomAvatar | 1 | - And last avatar is a square of size 128 + And last avatar is a square of size 512 And last avatar is a single "#FF0000" color When logged in user deletes the user avatar Then user "user0" gets avatar for user "user0" And The following headers should be set | Content-Type | image/png | | X-NC-IsCustomAvatar | 0 | - And last avatar is a square of size 128 + And last avatar is a square of size 512 And last avatar is not a single color And user "anonymous" gets avatar for user "user0" And The following headers should be set | Content-Type | image/png | | X-NC-IsCustomAvatar | 0 | - And last avatar is a square of size 128 + And last avatar is a square of size 512 And last avatar is not a single color @@ -148,7 +148,7 @@ Feature: avatar Then The following headers should be set | Content-Type | image/png | | X-NC-IsCustomAvatar | 1 | - And last avatar is a square of size 192 + And last avatar is a square of size 512 And last avatar is a single "#FF0000" color Scenario: get user avatar with a smaller size than the original one @@ -163,7 +163,7 @@ Feature: avatar Then The following headers should be set | Content-Type | image/png | | X-NC-IsCustomAvatar | 1 | - And last avatar is a square of size 96 + And last avatar is a square of size 512 And last avatar is a single "#FF0000" color @@ -172,12 +172,12 @@ Feature: avatar When user "user0" gets avatar for guest "guest0" Then The following headers should be set | Content-Type | image/png | - And last avatar is a square of size 128 + And last avatar is a square of size 512 And last avatar is not a single color Scenario: get default guest avatar as an anonymous user When user "anonymous" gets avatar for guest "guest0" Then The following headers should be set | Content-Type | image/png | - And last avatar is a square of size 128 + And last avatar is a square of size 512 And last avatar is not a single color diff --git a/core/Controller/AvatarController.php b/core/Controller/AvatarController.php index 5acfe9cd404..ed8d0c3cdeb 100644 --- a/core/Controller/AvatarController.php +++ b/core/Controller/AvatarController.php @@ -114,11 +114,16 @@ class AvatarController extends Controller { * @return JSONResponse|FileDisplayResponse */ public function getAvatar($userId, $size) { - // min/max size - if ($size > 2048) { - $size = 2048; - } elseif ($size <= 0) { + if ($size <= 64) { + if ($size !== 64) { + $this->logger->debug('Avatar requested in deprecated size ' . $size); + } $size = 64; + } else { + if ($size !== 512) { + $this->logger->debug('Avatar requested in deprecated size ' . $size); + } + $size = 512; } try { diff --git a/core/Controller/GuestAvatarController.php b/core/Controller/GuestAvatarController.php index 57900af810d..6d14474ed44 100644 --- a/core/Controller/GuestAvatarController.php +++ b/core/Controller/GuestAvatarController.php @@ -76,11 +76,16 @@ class GuestAvatarController extends Controller { public function getAvatar($guestName, $size) { $size = (int) $size; - // min/max size - if ($size > 2048) { - $size = 2048; - } elseif ($size <= 0) { + if ($size <= 64) { + if ($size !== 64) { + $this->logger->debug('Avatar requested in deprecated size ' . $size); + } $size = 64; + } else { + if ($size !== 512) { + $this->logger->debug('Avatar requested in deprecated size ' . $size); + } + $size = 512; } try { diff --git a/tests/Core/Controller/AvatarControllerTest.php b/tests/Core/Controller/AvatarControllerTest.php index 35d89c24a45..8d3cd73a656 100644 --- a/tests/Core/Controller/AvatarControllerTest.php +++ b/tests/Core/Controller/AvatarControllerTest.php @@ -193,24 +193,7 @@ class AvatarControllerTest extends \Test\TestCase { $this->assertEquals(Http::STATUS_NOT_FOUND, $response->getStatus()); } - /** - * Make sure we get the correct size - */ - public function testGetAvatarSize() { - $this->avatarMock->expects($this->once()) - ->method('getFile') - ->with($this->equalTo(32)) - ->willReturn($this->avatarFile); - - $this->avatarManager->method('getAvatar')->willReturn($this->avatarMock); - - $this->avatarController->getAvatar('userId', 32); - } - - /** - * We cannot get avatars that are 0 or negative - */ - public function testGetAvatarSizeMin() { + public function testGetAvatarSize64(): void { $this->avatarMock->expects($this->once()) ->method('getFile') ->with($this->equalTo(64)) @@ -218,21 +201,78 @@ class AvatarControllerTest extends \Test\TestCase { $this->avatarManager->method('getAvatar')->willReturn($this->avatarMock); - $this->avatarController->getAvatar('userId', 0); + $this->logger->expects($this->never()) + ->method('debug'); + + $this->avatarController->getAvatar('userId', 64); } - /** - * We do not support avatars larger than 2048*2048 - */ - public function testGetAvatarSizeMax() { + public function testGetAvatarSize512(): void { $this->avatarMock->expects($this->once()) ->method('getFile') - ->with($this->equalTo(2048)) + ->with($this->equalTo(512)) ->willReturn($this->avatarFile); $this->avatarManager->method('getAvatar')->willReturn($this->avatarMock); - $this->avatarController->getAvatar('userId', 2049); + $this->logger->expects($this->never()) + ->method('debug'); + + $this->avatarController->getAvatar('userId', 512); + } + + /** + * Small sizes return 64 and generate a log + */ + public function testGetAvatarSizeTooSmall(): void { + $this->avatarMock->expects($this->once()) + ->method('getFile') + ->with($this->equalTo(64)) + ->willReturn($this->avatarFile); + + $this->avatarManager->method('getAvatar')->willReturn($this->avatarMock); + + $this->logger->expects($this->once()) + ->method('debug') + ->with('Avatar requested in deprecated size 32'); + + $this->avatarController->getAvatar('userId', 32); + } + + /** + * Avatars between 64 and 512 are upgraded to 512 + */ + public function testGetAvatarSizeBetween(): void { + $this->avatarMock->expects($this->once()) + ->method('getFile') + ->with($this->equalTo(512)) + ->willReturn($this->avatarFile); + + $this->avatarManager->method('getAvatar')->willReturn($this->avatarMock); + + $this->logger->expects($this->once()) + ->method('debug') + ->with('Avatar requested in deprecated size 65'); + + $this->avatarController->getAvatar('userId', 65); + } + + /** + * We do not support avatars larger than 512 + */ + public function testGetAvatarSizeTooBig(): void { + $this->avatarMock->expects($this->once()) + ->method('getFile') + ->with($this->equalTo(512)) + ->willReturn($this->avatarFile); + + $this->avatarManager->method('getAvatar')->willReturn($this->avatarMock); + + $this->logger->expects($this->once()) + ->method('debug') + ->with('Avatar requested in deprecated size 513'); + + $this->avatarController->getAvatar('userId', 513); } /** diff --git a/tests/Core/Controller/GuestAvatarControllerTest.php b/tests/Core/Controller/GuestAvatarControllerTest.php index e14a5416c49..b5682d2d716 100644 --- a/tests/Core/Controller/GuestAvatarControllerTest.php +++ b/tests/Core/Controller/GuestAvatarControllerTest.php @@ -77,7 +77,7 @@ class GuestAvatarControllerTest extends \Test\TestCase { $this->avatar->expects($this->once()) ->method('getFile') - ->with(128) + ->with(512) ->willReturn($this->file); $this->file->method('getMimeType')