fix: quoted email addresses and status responses

lib/Controller/AvatarsController.php

@@ -43,31 +43,22 @@
 	 */
 	#[TrapError]
 	public function url(string $email): JSONResponse {
-		if (empty($email)) {
-			return new JSONResponse([], Http::STATUS_BAD_REQUEST);
-		}
-
-		if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
+		$email = $this->normalizeEmail($email);
+		if ($this->validateEmail($email) === false) {
 			return new JSONResponse([], Http::STATUS_BAD_REQUEST);
 		}
 
 		$avatar = $this->avatarService->getAvatar($email, $this->uid);
 		if (is_null($avatar)) {
 			// No avatar found
 			$response = new JSONResponse([], Http::STATUS_NO_CONTENT);
-
-			// Debounce this a bit
-			// (cache for one day)
-			$response->cacheFor(24 * 60 * 60, false, true);
-
+			$response->cacheFor(60 * 60, false, true);
 			return $response;
 		}
 
 		$response = new JSONResponse($avatar);
-
 		// Let the browser cache this for a week
 		$response->cacheFor(7 * 24 * 60 * 60, false, true);
-
 		return $response;
 	}
 
@@ -80,7 +71,8 @@
 	 */
 	#[TrapError]
 	public function image(string $email): Response {
-		if (empty($email)) {
+		$email = $this->normalizeEmail($email);
+		if ($this->validateEmail($email) === false) {
 			return new JSONResponse([], Http::STATUS_BAD_REQUEST);
 		}
 
@@ -89,23 +81,35 @@
 
 		if (is_null($imageData) || !$avatar->isExternal()) {
 			// This could happen if the cache invalidated meanwhile
-			return $this->noAvatarFoundResponse();
+			$response = new Response(Http::STATUS_NO_CONTENT);
+			// Clear cache
+			$response->cacheFor(0);
+			return $response;
 		}
 
 		$resp = new AvatarDownloadResponse($image);
 		$resp->addHeader('Content-Type', $avatar->getMime());
-
 		// Let the browser cache this for a week
 		$resp->cacheFor(7 * 24 * 60 * 60, false, true);
-
 		return $resp;
 	}
 
-	private function noAvatarFoundResponse(): Response {
-		$response = new Response();
-		$response->setStatus(Http::STATUS_NOT_FOUND);
-		// Clear cache
-		$response->cacheFor(0);
-		return $response;
+	private function normalizeEmail(string $email): string {
+		// remove single quotes and whitespace the might exist
+		// Examples:
+		// [email protected]
+		// '[email protected]'
+		// ' [email protected]'
+		return trim(trim($email, "'"));
+	}
+
+	private function validateEmail(string $email): bool {
+		if (empty($email)) {
+			return false;
+		}
+		if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
+			return false;
+		}
+		return true;
 	}
 }

tests/Unit/Controller/AvatarControllerTest.php

@@ -7,7 +7,6 @@
 
 namespace OCA\Mail\Tests\Unit\Controller;
 
-use ChristophWurst\Nextcloud\Testing\TestCase;
 use OCA\Mail\Contracts\IAvatarService;
 use OCA\Mail\Controller\AvatarsController;
 use OCA\Mail\Http\AvatarDownloadResponse;
@@ -18,6 +17,7 @@
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\IRequest;
 use PHPUnit_Framework_MockObject_MockObject;
+use Test\TestCase;
 
 class AvatarControllerTest extends TestCase {
 	/** @var IAvatarService|PHPUnit_Framework_MockObject_MockObject */
@@ -79,7 +79,7 @@ public function testGetUrlNoAvatarFound() {
 		$resp = $this->controller->url($email);
 
 		$expected = new JSONResponse([], Http::STATUS_NO_CONTENT);
-		$expected->cacheFor(24 * 60 * 60, false, true);
+		$expected->cacheFor(60 * 60, false, true);
 		$this->assertEquals($expected, $resp);
 	}
 
@@ -108,8 +108,32 @@ public function testGetImageNotFound() {
 		$resp = $this->controller->image($email);
 
 		$expected = new Response();
-		$expected->setStatus(Http::STATUS_NOT_FOUND);
+		$expected->setStatus(Http::STATUS_NO_CONTENT);
 		$expected->cacheFor(0);
 		$this->assertEquals($expected, $resp);
 	}
+
+	public function testNormalizeEmailWithExtraSpaces() {
+		$email = '   [email protected]   ';
+		$normalizedEmail = $this->invokePrivate($this->controller, 'normalizeEmail', [$email]);
+		$this->assertEquals('[email protected]', $normalizedEmail);
+	}
+
+	public function testNormalizeEmailWithSingleQuotes() {
+		$email = "'[email protected]'";
+		$normalizedEmail = $this->invokePrivate($this->controller, 'normalizeEmail', [$email]);
+		$this->assertEquals('[email protected]', $normalizedEmail);
+	}
+
+	public function testValidateEmailWithValidEmail() {
+		$email = '[email protected]';
+		$isValid = $this->invokePrivate($this->controller, 'validateEmail', [$email]);
+		$this->assertTrue($isValid);
+	}
+
+	public function testValidateEmailWithInvalidEmail() {
+		$email = 'valid.email.example.com';
+		$isValid = $this->invokePrivate($this->controller, 'validateEmail', [$email]);
+		$this->assertFalse($isValid);
+	}
 }