Skip to content

fix(core): Stop abusing the cache for avatar upload #55134

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
merged 2 commits into from
Sep 25, 2025
Merged

fix(core): Stop abusing the cache for avatar upload #55134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bfb4fe8
fix(core): Stop abusing the cache for avatar upload
provokateurin Jul 1, 2025
dfa9fce
chore(assets): Recompile assets
provokateurin Sep 16, 2025
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
3 changes: 1 addition & 2 deletions apps/settings/src/components/PersonalInfo/AvatarSection.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,7 @@ export default {
if (data.status === 'success') {
this.handleAvatarUpdate(false)
} else if (data.data === 'notsquare') {
const tempAvatar = generateUrl('/avatar/tmp') + '?requesttoken=' + encodeURIComponent(OC.requestToken) + '#' + Math.floor(Math.random() * 1000)
this.$refs.cropper.replace(tempAvatar)
this.$refs.cropper.replace(data.image)
this.showCropper = true
} else {
showError(data.data.message)
Expand Down
128 changes: 5 additions & 123 deletions build/integration/features/avatar.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,32 +21,9 @@ Feature: avatar
And last avatar is a square of size 512
And last avatar is not a single color



Scenario: get temporary non-square user avatar before cropping it
Given Logging in using web as "user0"
And logged in user posts temporary avatar from file "data/coloured-pattern-non-square.png"
When logged in user gets temporary avatar
Then The following headers should be set
| Content-Type | image/png |
# "last avatar" also includes the last temporary avatar
And last avatar is not a square
And last avatar is not a single color

Scenario: get non-square user avatar before cropping it
Given Logging in using web as "user0"
And logged in user posts temporary avatar from file "data/coloured-pattern-non-square.png"
# Avatar needs to be cropped to finish setting it
When user "user0" 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 512
And last avatar is not a single color

Scenario: set square user avatar from file
Given Logging in using web as "user0"
When logged in user posts temporary avatar from file "data/green-square-256.png"
When logged in user posts avatar from file "data/green-square-256.png"
And user "user0" gets avatar for user "user0"
And The following headers should be set
| Content-Type | image/png |
Expand All @@ -64,7 +41,7 @@ Feature: avatar
Scenario: set square user avatar from internal path
Given user "user0" uploads file "data/green-square-256.png" to "/internal-green-square-256.png"
And Logging in using web as "user0"
When logged in user posts temporary avatar from internal path "internal-green-square-256.png"
When logged in user posts avatar from internal path "internal-green-square-256.png"
And user "user0" gets avatar for user "user0" with size "64"
And The following headers should be set
| Content-Type | image/png |
Expand All @@ -78,82 +55,21 @@ Feature: avatar
And last avatar is a square of size 64
And last avatar is a single "#00FF00" color

Scenario: set non-square user avatar from file
Given Logging in using web as "user0"
When logged in user posts temporary avatar from file "data/coloured-pattern-non-square.png"
And logged in user crops temporary avatar
| x | 384 |
| y | 256 |
| w | 128 |
| h | 128 |
Then logged in user gets temporary avatar with 404
And user "user0" 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 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 512
And last avatar is a single "#FF0000" color

Scenario: set non-square user avatar from internal path
Given user "user0" uploads file "data/coloured-pattern-non-square.png" to "/internal-coloured-pattern-non-square.png"
And Logging in using web as "user0"
When logged in user posts temporary avatar from internal path "internal-coloured-pattern-non-square.png"
And logged in user crops temporary avatar
| x | 704 |
| y | 320 |
| w | 64 |
| h | 64 |
Then logged in user gets temporary avatar with 404
And user "user0" gets avatar for user "user0" with size "64"
And The following headers should be set
| Content-Type | image/png |
| X-NC-IsCustomAvatar | 1 |
And last avatar is a square of size 64
And last avatar is a single "#00FF00" color
And user "anonymous" gets avatar for user "user0" with size "64"
And The following headers should be set
| Content-Type | image/png |
| X-NC-IsCustomAvatar | 1 |
And last avatar is a square of size 64
And last avatar is a single "#00FF00" color

Scenario: cropped user avatar needs to be squared
Given Logging in using web as "user0"
And logged in user posts temporary avatar from file "data/coloured-pattern-non-square.png"
When logged in user crops temporary avatar with 400
| x | 384 |
| y | 256 |
| w | 192 |
| h | 128 |



Scenario: delete user avatar
Given Logging in using web as "user0"
And logged in user posts temporary avatar from file "data/coloured-pattern-non-square.png"
And logged in user crops temporary avatar
| x | 384 |
| y | 256 |
| w | 128 |
| h | 128 |
And logged in user posts avatar from file "data/green-square-256.png"
And user "user0" 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 512
And last avatar is a single "#FF0000" color
And last avatar is a single "#00FF00" 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 512
And last avatar is a single "#FF0000" color
And last avatar is a single "#00FF00" color
When logged in user deletes the user avatar
Then user "user0" gets avatar for user "user0"
And The following headers should be set
Expand All @@ -168,40 +84,6 @@ Feature: avatar
And last avatar is a square of size 512
And last avatar is not a single color



Scenario: get user avatar with a larger size than the original one
Given Logging in using web as "user0"
And logged in user posts temporary avatar from file "data/coloured-pattern-non-square.png"
And logged in user crops temporary avatar
| x | 384 |
| y | 256 |
| w | 128 |
| h | 128 |
When user "user0" gets avatar for user "user0" with size "192"
Then The following headers should be set
| Content-Type | image/png |
| X-NC-IsCustomAvatar | 1 |
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
Given Logging in using web as "user0"
And logged in user posts temporary avatar from file "data/coloured-pattern-non-square.png"
And logged in user crops temporary avatar
| x | 384 |
| y | 256 |
| w | 128 |
| h | 128 |
When user "user0" gets avatar for user "user0" with size "96"
Then The following headers should be set
| Content-Type | image/png |
| X-NC-IsCustomAvatar | 1 |
And last avatar is a square of size 512
And last avatar is a single "#FF0000" color



Scenario: get default guest avatar
When user "user0" gets avatar for guest "guest0"
Then The following headers should be set
Expand Down
53 changes: 4 additions & 49 deletions build/integration/features/bootstrap/Avatar.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
* SPDX-FileCopyrightText: 2020 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
use Behat\Gherkin\Node\TableNode;
use PHPUnit\Framework\Assert;

require __DIR__ . '/../../vendor/autoload.php';
Expand Down Expand Up @@ -68,30 +67,11 @@ public function userGetsAvatarForGuest(string $user, string $guestAvatar) {
}

/**
* @When logged in user gets temporary avatar
*/
public function loggedInUserGetsTemporaryAvatar() {
$this->loggedInUserGetsTemporaryAvatarWith('200');
}

/**
* @When logged in user gets temporary avatar with :statusCode
*
* @param string $statusCode
*/
public function loggedInUserGetsTemporaryAvatarWith(string $statusCode) {
$this->sendingAToWithRequesttoken('GET', '/index.php/avatar/tmp');
$this->theHTTPStatusCodeShouldBe($statusCode);

$this->getLastAvatar();
}

/**
* @When logged in user posts temporary avatar from file :source
* @When logged in user posts avatar from file :source
*
* @param string $source
*/
public function loggedInUserPostsTemporaryAvatarFromFile(string $source) {
public function loggedInUserPostsAvatarFromFile(string $source) {
$file = \GuzzleHttp\Psr7\Utils::streamFor(fopen($source, 'r'));

$this->sendingAToWithRequesttoken('POST', '/index.php/avatar',
Expand All @@ -107,40 +87,15 @@ public function loggedInUserPostsTemporaryAvatarFromFile(string $source) {
}

/**
* @When logged in user posts temporary avatar from internal path :path
* @When logged in user posts avatar from internal path :path
*
* @param string $path
*/
public function loggedInUserPostsTemporaryAvatarFromInternalPath(string $path) {
public function loggedInUserPostsAvatarFromInternalPath(string $path) {
$this->sendingAToWithRequesttoken('POST', '/index.php/avatar?path=' . $path);
$this->theHTTPStatusCodeShouldBe('200');
}

/**
* @When logged in user crops temporary avatar
*
* @param TableNode $crop
*/
public function loggedInUserCropsTemporaryAvatar(TableNode $crop) {
$this->loggedInUserCropsTemporaryAvatarWith('200', $crop);
}

/**
* @When logged in user crops temporary avatar with :statusCode
*
* @param string $statusCode
* @param TableNode $crop
*/
public function loggedInUserCropsTemporaryAvatarWith(string $statusCode, TableNode $crop) {
$parameters = [];
foreach ($crop->getRowsHash() as $key => $value) {
$parameters[] = 'crop[' . $key . ']=' . $value;
}

$this->sendingAToWithRequesttoken('POST', '/index.php/avatar/cropped?' . implode('&', $parameters));
$this->theHTTPStatusCodeShouldBe($statusCode);
}

/**
* @When logged in user deletes the user avatar
*/
Expand Down
79 changes: 2 additions & 77 deletions core/Controller/AvatarController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,20 @@
namespace OC\Core\Controller;

use OC\AppFramework\Utility\TimeFactory;
use OC\NotSquareException;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\FrontpageRoute;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
use OCP\AppFramework\Http\Attribute\OpenAPI;
use OCP\AppFramework\Http\Attribute\PublicPage;
use OCP\AppFramework\Http\DataDisplayResponse;
use OCP\AppFramework\Http\FileDisplayResponse;
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Http\Response;
use OCP\Files\File;
use OCP\Files\IRootFolder;
use OCP\Files\NotPermittedException;
use OCP\IAvatarManager;
use OCP\ICache;
use OCP\IL10N;
use OCP\Image;
use OCP\IRequest;
Expand All @@ -41,7 +38,6 @@ public function __construct(
string $appName,
IRequest $request,
protected IAvatarManager $avatarManager,
protected ICache $cache,
protected IL10N $l10n,
protected IUserManager $userManager,
protected IRootFolder $rootFolder,
Expand Down Expand Up @@ -202,8 +198,7 @@ public function postAvatar(?string $path = null): JSONResponse {
Http::STATUS_BAD_REQUEST
);
}
$this->cache->set('avatar_upload', file_get_contents($files['tmp_name'][0]), 7200);
$content = $this->cache->get('avatar_upload');
$content = file_get_contents($files['tmp_name'][0]);
unlink($files['tmp_name'][0]);
} else {
$phpFileUploadErrors = [
Expand Down Expand Up @@ -250,18 +245,15 @@ public function postAvatar(?string $path = null): JSONResponse {
try {
$avatar = $this->avatarManager->getAvatar($this->userId);
$avatar->set($image);
// Clean up
$this->cache->remove('tmpAvatar');
return new JSONResponse(['status' => 'success']);
} catch (\Throwable $e) {
$this->logger->error($e->getMessage(), ['exception' => $e, 'app' => 'core']);
return new JSONResponse(['data' => ['message' => $this->l10n->t('An error occurred. Please contact your admin.')]], Http::STATUS_BAD_REQUEST);
}
}

$this->cache->set('tmpAvatar', $image->data(), 7200);
return new JSONResponse(
['data' => 'notsquare'],
['data' => 'notsquare', 'image' => 'data:' . $mimeType . ';base64,' . base64_encode($image->data())],
Http::STATUS_OK
);
} else {
Expand All @@ -288,71 +280,4 @@ public function deleteAvatar(): JSONResponse {
return new JSONResponse(['data' => ['message' => $this->l10n->t('An error occurred. Please contact your admin.')]], Http::STATUS_BAD_REQUEST);
}
}

/**
* @return JSONResponse|DataDisplayResponse
*/
#[NoAdminRequired]
#[FrontpageRoute(verb: 'GET', url: '/avatar/tmp')]
public function getTmpAvatar() {
$tmpAvatar = $this->cache->get('tmpAvatar');
if (is_null($tmpAvatar)) {
return new JSONResponse(['data' => [
'message' => $this->l10n->t('No temporary profile picture available, try again')
]],
Http::STATUS_NOT_FOUND);
}

$image = new Image();
$image->loadFromData($tmpAvatar);

$resp = new DataDisplayResponse(
$image->data() ?? '',
Http::STATUS_OK,
['Content-Type' => $image->mimeType()]);

$resp->setETag((string)crc32($image->data() ?? ''));
$resp->cacheFor(0);
$resp->setLastModified(new \DateTime('now', new \DateTimeZone('GMT')));
return $resp;
}

#[NoAdminRequired]
#[FrontpageRoute(verb: 'POST', url: '/avatar/cropped')]
public function postCroppedAvatar(?array $crop = null): JSONResponse {
if (is_null($crop)) {
return new JSONResponse(['data' => ['message' => $this->l10n->t('No crop data provided')]],
Http::STATUS_BAD_REQUEST);
}

if (!isset($crop['x'], $crop['y'], $crop['w'], $crop['h'])) {
return new JSONResponse(['data' => ['message' => $this->l10n->t('No valid crop data provided')]],
Http::STATUS_BAD_REQUEST);
}

$tmpAvatar = $this->cache->get('tmpAvatar');
if (is_null($tmpAvatar)) {
return new JSONResponse(['data' => [
'message' => $this->l10n->t('No temporary profile picture available, try again')
]],
Http::STATUS_BAD_REQUEST);
}

$image = new Image();
$image->loadFromData($tmpAvatar);
$image->crop($crop['x'], $crop['y'], (int)round($crop['w']), (int)round($crop['h']));
try {
$avatar = $this->avatarManager->getAvatar($this->userId);
$avatar->set($image);
// Clean up
$this->cache->remove('tmpAvatar');
return new JSONResponse(['status' => 'success']);
} catch (NotSquareException $e) {
return new JSONResponse(['data' => ['message' => $this->l10n->t('Crop is not square')]],
Http::STATUS_BAD_REQUEST);
} catch (\Exception $e) {
$this->logger->error($e->getMessage(), ['exception' => $e, 'app' => 'core']);
return new JSONResponse(['data' => ['message' => $this->l10n->t('An error occurred. Please contact your admin.')]], Http::STATUS_BAD_REQUEST);
}
}
}
4 changes: 2 additions & 2 deletions dist/settings-vue-settings-personal-info.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/settings-vue-settings-personal-info.js.map
View file
Open in desktop

Large diffs are not rendered by default.

Loading
Loading