Fixes #30062: remove old avatar get endpoint

owncloud · Oct 10, 2018 · 29f8fdc · 29f8fdc
1 parent 438615a
commit 29f8fdc
Show file tree

Hide file tree

Showing 7 changed files with 24 additions and 153 deletions.
diff --git a/core/Controller/AvatarController.php b/core/Controller/AvatarController.php
@@ -103,49 +103,6 @@ public function __construct($appName,
 		$this->logger = $logger;
 	}
 
-	/**
-	 * @NoAdminRequired
-	 * @NoCSRFRequired
-	 *
-	 * @param string $userId
-	 * @param int $size
-	 * @return DataResponse|DataDisplayResponse
-	 */
-	public function getAvatar($userId, $size) {
-		if ($size > 2048) {
-			$size = 2048;
-		} elseif ($size <= 0) {
-			$size = 64;
-		}
-
-		try {
-			$avatar = $this->avatarManager->getAvatar($userId)->getFile($size);
-			$resp = new DataDisplayResponse($avatar->getContent(),
-				Http::STATUS_OK,
-				['Content-Type' => $avatar->getMimeType()]);
-			$resp->setETag($avatar->getEtag());
-		} catch (NotFoundException $e) {
-			$user = $this->userManager->get($userId);
-			$resp = new DataResponse([
-				'data' => [
-					'displayname' => $user->getDisplayName(),
-				],
-			]);
-		} catch (\Exception $e) {
-			$resp = new DataResponse([
-				'data' => [
-					'displayname' => '',
-				],
-			]);
-		}
-
-		$resp->addHeader('Pragma', 'public');
-		$resp->cacheFor(0);
-		$resp->setLastModified(new \DateTime('now', new \DateTimeZone('GMT')));
-
-		return $resp;
-	}
-
 	/**
 	 * @NoAdminRequired
 	 *

diff --git a/core/js/jquery.avatar.js b/core/js/jquery.avatar.js
@@ -75,9 +75,11 @@
 
 		var $div = this;
 
-		var url = OC.generateUrl(
-			'/avatar/{user}/{size}',
-			{user: user, size: Math.ceil(size * window.devicePixelRatio)});
+		var url = OC.getRootPath() +
+			'/remote.php/dav/avatars' +
+			'/' + encodeURIComponent(user) +
+			'/' + encodeURIComponent(Math.ceil(size * window.devicePixelRatio).toString()) +
+			'.jpeg';
 
 		// If the displayname is not defined we use the old code path
 		if (typeof(displayname) === 'undefined') {

diff --git a/core/js/tests/specs/jquery.avatarSpec.js b/core/js/tests/specs/jquery.avatarSpec.js
@@ -126,23 +126,23 @@ describe('jquery.avatar tests', function() {
 			$div.avatar('foo', 32);
 
 			expect(fakeServer.requests[0].method).toEqual('GET');
-			expect(fakeServer.requests[0].url).toEqual('/owncloud/index.php/avatar/foo/32');
+			expect(fakeServer.requests[0].url).toEqual('/owncloud/remote.php/dav/avatars/foo/32.jpeg');
 		});
 
 		it('high DPI icon', function() {
 			window.devicePixelRatio = 4;
 			$div.avatar('foo', 32);
 
 			expect(fakeServer.requests[0].method).toEqual('GET');
-			expect(fakeServer.requests[0].url).toEqual('/owncloud/index.php/avatar/foo/128');
+			expect(fakeServer.requests[0].url).toEqual('/owncloud/remote.php/dav/avatars/foo/128.jpeg');
 		});
 
 		it('high DPI icon round up size', function() {
 			window.devicePixelRatio = 1.9;
 			$div.avatar('foo', 32);
 
 			expect(fakeServer.requests[0].method).toEqual('GET');
-			expect(fakeServer.requests[0].url).toEqual('/owncloud/index.php/avatar/foo/61');
+			expect(fakeServer.requests[0].url).toEqual('/owncloud/remote.php/dav/avatars/foo/61.jpeg');
 		});
 	});
 
@@ -164,7 +164,7 @@ describe('jquery.avatar tests', function() {
 
 			expect(img.height).toEqual(32);
 			expect(img.width).toEqual(32);
-			expect(img.src).toEqual(OC.TestUtil.buildAbsoluteUrl('/owncloud/index.php/avatar/foo/32'));
+			expect(img.src).toEqual(OC.TestUtil.buildAbsoluteUrl('/owncloud/remote.php/dav/avatars/foo/32.jpeg'));
 		});
 
 		it('default high DPI icon', function() {
@@ -182,7 +182,7 @@ describe('jquery.avatar tests', function() {
 
 			expect(img.height).toEqual(32);
 			expect(img.width).toEqual(32);
-			expect(img.src).toEqual(OC.TestUtil.buildAbsoluteUrl('/owncloud/index.php/avatar/foo/61'));
+			expect(img.src).toEqual(OC.TestUtil.buildAbsoluteUrl('/owncloud/remote.php/dav/avatars/foo/61.jpeg'));
 		});
 
 		it('with ie8 fix', function() {
@@ -202,7 +202,7 @@ describe('jquery.avatar tests', function() {
 
 			expect(img.height).toEqual(32);
 			expect(img.width).toEqual(32);
-			expect(img.src).toEqual(OC.TestUtil.buildAbsoluteUrl('/owncloud/index.php/avatar/foo/32#500'));
+			expect(img.src).toEqual(OC.TestUtil.buildAbsoluteUrl('/owncloud/remote.php/dav/avatars/foo/32.jpeg#500'));
 		});
 
 		it('unhide div', function() {

diff --git a/core/routes.php b/core/routes.php
@@ -38,7 +38,6 @@
 		['name' => 'lost#resetform', 'url' => '/lostpassword/reset/form/{token}/{userId}', 'verb' => 'GET'],
 		['name' => 'lost#setPassword', 'url' => '/lostpassword/set/{token}/{userId}', 'verb' => 'POST'],
 		['name' => 'user#getDisplayNames', 'url' => '/displaynames', 'verb' => 'POST'],
-		['name' => 'avatar#getAvatar', 'url' => '/avatar/{userId}/{size}', 'verb' => 'GET'],
 		['name' => 'avatar#deleteAvatar', 'url' => '/avatar/', 'verb' => 'DELETE'],
 		['name' => 'avatar#postCroppedAvatar', 'url' => '/avatar/cropped', 'verb' => 'POST'],
 		['name' => 'avatar#getTmpAvatar', 'url' => '/avatar/tmp', 'verb' => 'GET'],

diff --git a/core/templates/layout.user.php b/core/templates/layout.user.php
@@ -67,8 +67,8 @@
 			} ?>">
 							<?php if ($_['userAvatarSet']): ?>
 								<img alt="" width="32" height="32"
-								src="<?php p(\OC::$server->getURLGenerator()->linkToRoute('core.avatar.getAvatar', ['userId' => $_['user_uid'], 'size' => 32]));?>"
-								srcset="<?php p(\OC::$server->getURLGenerator()->linkToRoute('core.avatar.getAvatar', ['userId' => $_['user_uid'], 'size' => 64]));?> 2x, <?php p(\OC::$server->getURLGenerator()->linkToRoute('core.avatar.getAvatar', ['userId' => $_['user_uid'], 'size' => 128]));?> 4x"
+								src="<?php p(\OC::$server->getURLGenerator()->linkTo('', 'remote.php') . '/dav/avatars/' . \rawurlencode($_['user_uid']) . '/32.jpeg');?>"
+								srcset="<?php p(\OC::$server->getURLGenerator()->linkTo('', 'remote.php') . '/dav/avatars/' . \rawurlencode($_['user_uid']) . '/32.jpeg');?> 2x, <?php p(\OC::$server->getURLGenerator()->linkTo('', 'remote.php') . '/dav/avatars/' . \rawurlencode($_['user_uid']) . '/32.jpeg');?> 4x"
 								>
 							<?php endif; ?>
 						</div>

diff --git a/settings/js/panels/profile.js b/settings/js/panels/profile.js
@@ -77,10 +77,12 @@ function updateAvatar (hidedefault) {
 	}
 	$displaydiv.css({'background-color': ''});
 	$displaydiv.avatar(OC.currentUser, 145, true);
-	$.get(OC.generateUrl(
-		'/avatar/{user}/{size}',
-		{user: OC.currentUser, size: 1}
-	), function (result) {
+	var url = OC.getRootPath() +
+		'/remote.php/dav/avatars' +
+		'/' + encodeURIComponent(OC.getCurrentUser().uid) +
+		'/96.jpeg';
+
+	$.get(url, function (result) {
 		if (typeof(result) === 'string') {
 			// Show the delete button when the avatar is custom
 			$('#removeavatar').removeClass('hidden').addClass('inlineblock');
@@ -334,10 +336,11 @@ $(document).ready(function () {
 	});
 
 	// does the user have a custom avatar? if he does show #removeavatar
-	$.get(OC.generateUrl(
-		'/avatar/{user}/{size}',
-		{user: OC.currentUser, size: 1}
-	), function (result) {
+	var url = OC.getRootPath() +
+		'/remote.php/dav/avatars' +
+		'/' + encodeURIComponent(OC.getCurrentUser().uid) +
+		'/1.jpeg';
+	$.get(url, function (result) {
 		if (typeof(result) === 'string') {
 			// Show the delete button when the avatar is custom
 			$('#removeavatar').removeClass('hidden').addClass('inlineblock');

diff --git a/tests/Core/Controller/AvatarControllerTest.php b/tests/Core/Controller/AvatarControllerTest.php
@@ -130,96 +130,6 @@ public function tearDown() {
 		parent::tearDown();
 	}
 
-	/**
-	 * Fetch an avatar if a user has no avatar
-	 */
-	public function testGetAvatarNoAvatar() {
-		$this->avatarManager->expects($this->any())->method('getAvatar')->willReturn($this->avatarMock);
-		$this->avatarMock->expects($this->any())->method('getFile')->will($this->throwException(new NotFoundException()));
-		$response = $this->avatarController->getAvatar('userId', 32);
-
-		//Comment out until JS is fixed
-		//$this->assertEquals(Http::STATUS_NOT_FOUND, $response->getStatus());
-		$this->assertEquals(Http::STATUS_OK, $response->getStatus());
-		$this->assertEquals('displayName', $response->getData()['data']['displayname']);
-	}
-
-	/**
-	 * Fetch the user's avatar
-	 */
-	public function testGetAvatar() {
-		$this->avatarMock->expects($this->any())->method('getFile')->willReturn($this->avatarFile);
-		$this->avatarManager->expects($this->any())->method('getAvatar')->with('userId')->willReturn($this->avatarMock);
-
-		$response = $this->avatarController->getAvatar('userId', 32);
-
-		$this->assertEquals(Http::STATUS_OK, $response->getStatus());
-		$this->assertArrayHasKey('Content-Type', $response->getHeaders());
-		$this->assertEquals('image type', $response->getHeaders()['Content-Type']);
-
-		$this->assertEquals('my etag', $response->getETag());
-	}
-
-	/**
-	 * Fetch the avatar of a non-existing user
-	 */
-	public function testGetAvatarNoUser() {
-		$this->avatarManager
-			->expects($this->any())
-			->method('getAvatar')
-			->with('userDoesNotExist')
-			->will($this->throwException(new \Exception('user does not exist')));
-
-		$response = $this->avatarController->getAvatar('userDoesNotExist', 32);
-
-		//Comment out until JS is fixed
-		//$this->assertEquals(Http::STATUS_NOT_FOUND, $response->getStatus());
-		$this->assertEquals(Http::STATUS_OK, $response->getStatus());
-		$this->assertEquals('', $response->getData()['data']['displayname']);
-	}
-
-	/**
-	 * 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->expects($this->any())->method('getAvatar')->willReturn($this->avatarMock);
-
-		$this->avatarController->getAvatar('userId', 32);
-	}
-
-	/**
-	 * We cannot get avatars that are 0 or negative
-	 */
-	public function testGetAvatarSizeMin() {
-		$this->avatarMock->expects($this->once())
-			->method('getFile')
-			->with($this->equalTo(64))
-			->willReturn($this->avatarFile);
-
-		$this->avatarManager->expects($this->any())->method('getAvatar')->willReturn($this->avatarMock);
-
-		$this->avatarController->getAvatar('userId', 0);
-	}
-
-	/**
-	 * We do not support avatars larger than 2048*2048
-	 */
-	public function testGetAvatarSizeMax() {
-		$this->avatarMock->expects($this->once())
-			->method('getFile')
-			->with($this->equalTo(2048))
-			->willReturn($this->avatarFile);
-
-		$this->avatarManager->expects($this->any())->method('getAvatar')->willReturn($this->avatarMock);
-
-		$this->avatarController->getAvatar('userId', 2049);
-	}
-
 	/**
 	 * Remove an avatar
 	 */