Have a controller instead ofo avatar.php and fix some cropper-design

avatar.php

@@ -1,83 +0,0 @@
-<?php
-
-require_once 'lib/base.php';
-
-if (!\OC_User::isLoggedIn()) {
-	header("HTTP/1.0 403 Forbidden");
-	\OC_Template::printErrorPage("Permission denied");
-}
-
-if ($_SERVER['REQUEST_METHOD'] === "GET") {
-	if (isset($_GET['user'])) {
-		//SECURITY TODO does this fully eliminate directory traversals?
-		$user = stripslashes($_GET['user']);
-	} else {
-		exit();
-	}
-
-	if (isset($_GET['size']) && ((int)$_GET['size'] > 0)) {
-		$size = (int)$_GET['size'];
-		if ($size > 2048) {
-			$size = 2048;
-		}
-	} else {
-		$size = 64;
-	}
-
-	$image = \OC_Avatar::get($user, $size);
-
-	if ($image instanceof \OC_Image) {
-		$image->show();
-	} elseif ($image === false) {
-		OC_JSON::success(array('user' => $user, 'size' => $size));
-	}
-} elseif ($_SERVER['REQUEST_METHOD'] === "POST") {
-	$user = OC_User::getUser();
-
-	// Select an image from own files
-	if (isset($_POST['path'])) {
-		$path = stripslashes($_POST['path']);
-		$avatar = OC::$SERVERROOT.'/data/'.$user.'/files'.$path;
-	}
-
-	if (isset($_POST['crop'])) {
-		$crop = json_decode($_POST['crop'], true);
-		if (!isset($path)) {
-			// TODO get path to temporarily saved uploaded-avatar
-		}
-		$image = new \OC_Image($avatar);
-		$image->crop($x, $y, $w, $h);
-		$avatar = $image->data();
-	}
-
-	// Upload a new image
-	if (!empty($_FILES)) {
-		$files = $_FILES['files'];
-		if ($files['error'][0] === 0) {
-			$avatar = file_get_contents($files['tmp_name'][0]);
-			unlink($files['tmp_name'][0]);
-			// TODO make the tmp_name reusable, if the uploaded avatar is not square
-		}
-	}
-
-	try {
-		\OC_Avatar::set($user, $avatar);
-		OC_JSON::success();
-	} catch (\OC\NotSquareException $e) {
-		$tmpname = \OC_Util::generate_random_bytes(10);
-		// TODO Save the image temporarily here
-		// TODO add a cronjob that cleans up stale tmpimages
-		OC_JSON::error(array("data" => array("message" => "notsquare", "tmpname" => $tmpname) ));
-	} catch (\Exception $e) {
-		OC_JSON::error(array("data" => array("message" => $e->getMessage()) ));
-	}
-} elseif ($_SERVER['REQUEST_METHOD'] === "DELETE") {
-	$user = OC_User::getUser();
-
-	try {
-		\OC_Avatar::remove($user);
-		OC_JSON::success();
-	} catch (\Exception $e) {
-		OC_JSON::error(array("data" => array ("message" => $e->getMessage()) ));
-	}
-}

core/avatar/controller.php

@@ -0,0 +1,88 @@
+<?php
+/**
+ * Copyright (c) 2013 Christopher Schäpers <christopher@schaepers.it>
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
+ */
+
+class CoreAvatarController {
+	public static function getAvatar($args) {
+		if (!\OC_User::isLoggedIn()) {
+			header("HTTP/1.0 403 Forbidden");
+			\OC_Template::printErrorPage("Permission denied");
+			return;
+		}
+
+		$user = stripslashes($args['user']);
+		$size = (int)$args['size'];
+		if ($size > 2048) {
+			$size = 2048;
+		}
+		// Undefined size
+		elseif ($size === 0) {
+			$size = 64;
+		}
+
+		$image = \OC_Avatar::get($user, $size);
+
+		if ($image instanceof \OC_Image) {
+			$image->show();
+		} elseif ($image === false) {
+			\OC_JSON::success(array('user' => $user, 'size' => $size));
+		}
+	}
+
+	public static function postAvatar($args) {
+		$user = \OC_User::getUser();
+
+		if (isset($_POST['path'])) {
+			$path = stripslashes($_POST['path']);
+			$avatar = OC::$SERVERROOT.'/data/'.$user.'/files'.$path;
+		}
+
+		if (!empty($_FILES)) {
+			$files = $_FILES['files'];
+			if ($files['error'][0] === 0) {
+				$avatar = file_get_contents($files['tmp_name'][0]);
+				unlink($files['tmp_name'][0]);
+			}
+		}
+
+		try {
+			\OC_Avatar::set($user, $avatar);
+			\OC_JSON::success();
+		} catch (\OC\NotSquareException $e) {
+			// TODO move unfitting avatar to /datadir/$user/tmpavatar{png.jpg} here
+			\OC_JSON::error(array("data" => array("message" => "notsquare") ));
+		} catch (\Exception $e) {
+			\OC_JSON::error(array("data" => array("message" => $e->getMessage()) ));
+		}
+	}
+
+	public static function deleteAvatar($args) {
+		$user = OC_User::getUser();
+
+		try {
+			\OC_Avatar::remove($user);
+			\OC_JSON::success();
+		} catch (\Exception $e) {
+			\OC_JSON::error(array("data" => array ("message" => $e->getMessage()) ));
+		}
+	}
+
+	public static function getTmpAvatar($args) {
+		// TODO deliver /datadir/$user/tmpavatar.{png|jpg} here, filename may include a timestamp
+		// TODO make a cronjob that cleans up the tmpavatar after it's older than 2 hours, should be run every hour
+		$user = OC_User::getUser();
+	}
+
+	public static function postCroppedAvatar($args) {
+		$user = OC_User::getUser();
+		$crop = json_decode($_POST['crop'], true);
+		$image = new \OC_Image($avatar);
+		$image->crop($x, $y, $w, $h);
+		$avatar = $image->data();
+		$cropped = true;
+	}
+}

core/routes.php

@@ -57,6 +57,26 @@ $this->create('core_lostpassword_reset_password', '/lostpassword/reset/{token}/{
 	->post()
 	->action('OC_Core_LostPassword_Controller', 'resetPassword');
 
+// Avatar routes
+OC::$CLASSPATH['CoreAvatarController'] = 'core/avatar/controller.php';
+$this->create('core_avatar_get', '/avatar/{user}/{size}')
+	->defaults(array('user' => '', 'size' => 64))
+	->get()
+	->action('CoreAvatarController', 'getAvatar');
+$this->create('core_avatar_post', '/avatar/')
+	->post()
+	->action('CoreAvatarController', 'postAvatar');
+$this->create('core_avatar_delete', '/avatar/')
+	->delete()
+	->action('CoreAvatarController', 'deleteAvatar');
+$this->create('core_avatar_get_tmp', '/avatar/tmp/{size}')
+	->defaults(array('size' => 64))
+	->get()
+	->action('CoreAvatarController', 'getTmpAvatar');
+$this->create('core_avatar_post_cropped', '/avatar/cropped')
+	->post()
+	->action('CoreAvatarController', 'postCroppedAvatar');
+
 // Not specifically routed
 $this->create('app_css', '/apps/{app}/{file}')
 	->requirements(array('file' => '.*.css'))

lib/templatelayout.php

@@ -20,7 +20,7 @@ class OC_TemplateLayout extends OC_Template {
 
 			// display avatars if they are enabled
 			if (OC_Config::getValue('avatar') === 'gravatar' || OC_Config::getValue('avatar', 'local') === 'local') {
-				$this->assign('avatar', '<img class="avatar" src="'.link_to('', 'avatar.php').'?user='.OC_User::getUser().'&size=32">');
+				$this->assign('avatar', '<img class="avatar" src="'.\OC_Helper::linkToRoute('core_avatar_get').'/'.OC_User::getUser().'/32">');
 			}
 
 			// Update notification

settings/js/personal.js

@@ -45,7 +45,7 @@ function changeDisplayName(){
 }
 
 function selectAvatar (path) {
-	$.post(OC.filePath('', '', 'avatar.php'), {path: path}, avatarResponseHandler);
+	$.post(OC.router_base_url+'/avatar/', {path: path}, avatarResponseHandler);
 }
 
 function updateAvatar () {
@@ -54,22 +54,30 @@ function updateAvatar () {
 }
 
 function showAvatarCropper() {
-	OC.dialogs.message('', t('settings', 'Crop'), undefined, OCdialogs.OK_BUTTON, sendCropData);
-	var $dialog = $('#oc-dialog-'+(OC.dialogs.dialogs_counter-1)+'-content');
+	var $dlg = $('<div id="cropperbox" title="'+t('settings', 'Crop')+'"></div>');
+	$('body').append($dlg);
+	$('#cropperbox').ocdialog({
+		width: '600px',
+		height: '600px',
+		buttons: [{
+			text: t('settings', 'Crop'),
+			click: sendCropData,
+			defaultButton: true
+		}]
+	});
 	var cropper = new Image();
 	$(cropper).load(function() {
 		$(this).attr('id', 'cropper');
-		$('#oc-dialog-'+(OC.dialogs.dialogs_counter-1)+'-content').html(this);
+		$('#cropperbox').html(this);
 		$(this).Jcrop({
 			onChange: saveCoords,
 			onSelect: saveCoords,
 			aspectRatio: 1
 		});
-	}).attr('src', OC.filePath('', '', 'avatar.php')+"?user="+OC.currentUser+"&size=512&tmp="+$('#avatar').data('tmpname'));
+	}).attr('src', OC.router_base_url+'/avatar/tmp/512');
 }
 
 function sendCropData() {
-	var tmp = $('#avatar').data('tmpname');
 	var cropperdata = $('#cropper').data();
 	var data = {
 		x: cropperdata.x,
@@ -77,7 +85,7 @@ function sendCropData() {
 		w: cropperdata.w,
 		h: cropperdata.h
 	};
-	$.post(OC.filePath('', '', 'avatar.php'), {tmp:tmp, crop: data}, avatarResponseHandler);
+	$.post(OC.router_base_url+'/avatar/', {crop: data}, avatarResponseHandler);
 }
 
 function saveCoords(c) {
@@ -90,7 +98,6 @@ function avatarResponseHandler(data) {
 	if (data.status === "success") {
 		updateAvatar();
 	} else if (data.data.message === "notsquare") {
-		$('#avatar').data('tmpname', data.data.tmpname);
 		showAvatarCropper();
 	} else {
 		$warning.show();
@@ -206,7 +213,7 @@ $(document).ready(function(){
 	$('#removeavatar').click(function(){
 		$.ajax({
 			type:	'DELETE',
-			url:	OC.filePath('', '', 'avatar.php'),
+			url:	OC.router_base_url+'/avatar/',
 			success: function(msg) {
 				updateAvatar();
 			}

settings/templates/personal.php

@@ -83,10 +83,10 @@ if($_['passwordChangeSupported']) {
 }
 ?>
 
-<form id="avatar" method="post" action="<?php p(\OC_Helper::linkTo('', 'avatar.php')); ?>">
+<form id="avatar" method="post" action="<?php p(\OC_Helper::linkToRoute('core_avatar_post')); ?>">
 	<fieldset class="personalblock">
 		<legend><strong><?php p($l->t('Profile Image')); ?></strong></legend>
-		<img src="<?php print_unescaped(link_to('', 'avatar.php').'?user='.OC_User::getUser().'&size=128'); ?>"><br>
+		<img src="<?php print_unescaped(\OC_Helper::linkToRoute('core_avatar_get').'/'.OC_User::getUser().'/128'); ?>"><br>
 		<em><?php p($l->t('Has to be square and either PNG or JPG')); ?></em><br>
 		<div class="warning hidden"></div>
 		<div class="inlineblock button" id="uploadavatarbutton"><?php p($l->t('Upload new')); ?></div>

settings/templates/users.php

@@ -97,7 +97,7 @@ $_['subadmingroups'] = array_flip($items);
 		<?php foreach($_["users"] as $user): ?>
 		<tr data-uid="<?php p($user["name"]) ?>"
 			data-displayName="<?php p($user["displayName"]) ?>">
-			<td class="avatar"><img src="<?php print_unescaped(link_to('', 'avatar.php')); ?>?user=<?php p($user['name']); ?>&size=32"></td>
+			<td class="avatar"><img src="<?php print_unescaped(\OC_Helper::linkToRoute('core_avatar_get')); ?>/<?php p($user['name']); ?>/32"></td>
 			<td class="name"><?php p($user["name"]); ?></td>
 			<td class="displayName"><span><?php p($user["displayName"]); ?></span> <img class="svg action"
 				src="<?php p(image_path('core', 'actions/rename.svg'))?>"