Skip to content

Only allow avatars in 64 and 512 pixel size #31010

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nickvergessen merged 1 commit into from
Feb 7, 2022
Merged

Only allow avatars in 64 and 512 pixel size #31010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions build/integration/features/avatar.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ 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
When user "anonymous" gets avatar for user "user0"
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


Expand All @@ -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


Expand All @@ -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
Expand Down Expand Up @@ -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


Expand All @@ -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
Expand All @@ -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


Expand All @@ -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
13 changes: 9 additions & 4 deletions core/Controller/AvatarController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,16 @@ public function __construct($appName,
* @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 {
Expand Down
13 changes: 9 additions & 4 deletions core/Controller/GuestAvatarController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,16 @@ public function __construct(
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 {
Expand Down
62 changes: 51 additions & 11 deletions tests/Core/Controller/AvatarControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,46 +193,86 @@ public function testGetAvatarNoUser() {
$this->assertEquals(Http::STATUS_NOT_FOUND, $response->getStatus());
}

public function testGetAvatarSize64(): 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->never())
->method('debug');

$this->avatarController->getAvatar('userId', 64);
}

public function testGetAvatarSize512(): 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->never())
->method('debug');

$this->avatarController->getAvatar('userId', 512);
}

/**
* Make sure we get the correct size
* Small sizes return 64 and generate a log
*/
public function testGetAvatarSize() {
public function testGetAvatarSizeTooSmall(): void {
$this->avatarMock->expects($this->once())
->method('getFile')
->with($this->equalTo(32))
->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);
}

/**
* We cannot get avatars that are 0 or negative
* Avatars between 64 and 512 are upgraded to 512
*/
public function testGetAvatarSizeMin() {
public function testGetAvatarSizeBetween(): void {
$this->avatarMock->expects($this->once())
->method('getFile')
->with($this->equalTo(64))
->with($this->equalTo(512))
->willReturn($this->avatarFile);

$this->avatarManager->method('getAvatar')->willReturn($this->avatarMock);

$this->avatarController->getAvatar('userId', 0);
$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 2048*2048
* We do not support avatars larger than 512
*/
public function testGetAvatarSizeMax() {
public function testGetAvatarSizeTooBig(): 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->once())
->method('debug')
->with('Avatar requested in deprecated size 513');

$this->avatarController->getAvatar('userId', 513);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/Core/Controller/GuestAvatarControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public function testGetAvatar() {

$this->avatar->expects($this->once())
->method('getFile')
->with(128)
->with(512)
->willReturn($this->file);

$this->file->method('getMimeType')
Expand Down