fix(file): better uploaded file handling and thumbnail serving

Uploaded files now always have a new name with the client-given extension, even when replacing a previous upload. Thumbnail JPEG files now have the `.jpg` extension and are served with the correct Content-Type. If a plugin happened to have created their own non-JPEG thumbs, they'd now be served with the correct MIME (instead of the original file's MIME type). When an existing file is re-uploaded by a different user, the thumbnails are now created in the correct (owner's) directory. If the file is recognized as an image, but thumbnails can't be created, we no longer allow thumbs from the previous file to be re-used. Fixes #9612 Fixes #9267 Fixes #6677
Elgg · Apr 4, 2016 · 72140cf · 72140cf
1 parent 596300f
commit 72140cf
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 54 deletions.
diff --git a/mod/file/actions/file/upload.php b/mod/file/actions/file/upload.php
@@ -81,19 +81,15 @@
 	$prefix = "file/";
 
 	// if previous file, delete it
-	if ($new_file == false) {
+	if (!$new_file) {
 		$filename = $file->getFilenameOnFilestore();
 		if (file_exists($filename)) {
 			unlink($filename);
 		}
-
-		// use same filename on the disk - ensures thumbnails are overwritten
-		$filestorename = $file->getFilename();
-		$filestorename = elgg_substr($filestorename, elgg_strlen($prefix));
-	} else {
-		$filestorename = elgg_strtolower(time().$_FILES['upload']['name']);
 	}
 
+	$filestorename = elgg_strtolower(time().$_FILES['upload']['name']);
+
 	$file->setFilename($prefix . $filestorename);
 	$file->originalfilename = $_FILES['upload']['name'];
 	$mime_type = $file->detectMimeType($_FILES['upload']['tmp_name'], $_FILES['upload']['type']);
@@ -108,61 +104,76 @@
 
 	$guid = $file->save();
 
-	// if image, we need to create thumbnails (this should be moved into a function)
-	if ($guid && $file->simpletype == "image") {
-		$file->icontime = time();
-
-		$thumbnail = get_resized_image_from_existing_file($file->getFilenameOnFilestore(), 60, 60, true);
-		if ($thumbnail) {
-			$thumb = new ElggFile();
-			$thumb->setMimeType($_FILES['upload']['type']);
+	$thumb = new ElggFile();
+	$thumb->owner_guid = $file->owner_guid;
+
+	$sizes = [
+		'small' => [
+			'w' => 60,
+			'h' => 60,
+			'square' => true,
+			'metadata_name' => 'thumbnail',
+			'filename_prefix' => 'thumb',
+		],
+		'medium' => [
+			'w' => 153,
+			'h' => 153,
+			'square' => true,
+			'metadata_name' => 'smallthumb',
+			'filename_prefix' => 'smallthumb',
+		],
+		'large' => [
+			'w' => 600,
+			'h' => 600,
+			'square' => false,
+			'metadata_name' => 'largethumb',
+			'filename_prefix' => 'largethumb',
+		],
+	];
+
+	$remove_thumbs = function () use ($file, $sizes, $thumb) {
+		if (!$file->guid) {
+			return;
+		}
 
-			$thumb->setFilename($prefix."thumb".$filestorename);
-			$thumb->open("write");
-			$thumb->write($thumbnail);
-			$thumb->close();
+		unset($file->icontime);
 
-			$file->thumbnail = $prefix."thumb".$filestorename;
-			unset($thumbnail);
+		foreach ($sizes as $size => $data) {
+			$filename = $file->{$data['metadata_name']};
+			if ($filename !== null) {
+				$thumb->setFilename($filename);
+				$thumb->delete();
+				unset($file->{$data['metadata_name']});
+			}
 		}
+	};
 
-		$thumbsmall = get_resized_image_from_existing_file($file->getFilenameOnFilestore(), 153, 153, true);
-		if ($thumbsmall) {
-			$thumb->setFilename($prefix."smallthumb".$filestorename);
-			$thumb->open("write");
-			$thumb->write($thumbsmall);
-			$thumb->close();
-			$file->smallthumb = $prefix."smallthumb".$filestorename;
-			unset($thumbsmall);
-		}
+	$remove_thumbs();
+
+	$jpg_filename = pathinfo($filestorename, PATHINFO_FILENAME) . '.jpg';
+
+	if ($guid && $file->simpletype == "image") {
+		$file->icontime = time();
+
+		foreach ($sizes as $size => $data) {
+			$image_bytes = get_resized_image_from_existing_file($file->getFilenameOnFilestore(), $data['w'], $data['h'], $data['square']);
+			if (!$image_bytes) {
+				// bail and remove any thumbs
+				$remove_thumbs();
+				break;
+			}
 
-		$thumblarge = get_resized_image_from_existing_file($file->getFilenameOnFilestore(), 600, 600, false);
-		if ($thumblarge) {
-			$thumb->setFilename($prefix."largethumb".$filestorename);
+			$filename = "{$prefix}{$data['filename_prefix']}{$jpg_filename}";
+			$thumb->setFilename($filename);
 			$thumb->open("write");
-			$thumb->write($thumblarge);
+			$thumb->write($image_bytes);
 			$thumb->close();
-			$file->largethumb = $prefix."largethumb".$filestorename;
-			unset($thumblarge);
+			unset($image_bytes);
+
+			$file->{$data['metadata_name']} = $filename;
 		}
-	} elseif ($file->icontime) {
-		// if it is not an image, we do not need thumbnails
-		unset($file->icontime);
-
-		$thumb = new ElggFile();
-
-		$thumb->setFilename($prefix . "thumb" . $filestorename);
-		$thumb->delete();
-		unset($file->thumbnail);
-
-		$thumb->setFilename($prefix . "smallthumb" . $filestorename);
-		$thumb->delete();
-		unset($file->smallthumb);
-
-		$thumb->setFilename($prefix . "largethumb" . $filestorename);
-		$thumb->delete();
-		unset($file->largethumb);
 	}
+
 } else {
 	// not saving a file but still need to save the entity to push attributes to database
 	$file->save();

diff --git a/mod/file/thumbnail.php b/mod/file/thumbnail.php
@@ -18,6 +18,7 @@
 if (!elgg_instanceof($file, 'object', 'file')) {
 	exit;
 }
+/* @var ElggFile $file */
 
 $simpletype = $file->simpletype;
 if ($simpletype == "image") {
@@ -41,7 +42,10 @@
 		$readfile = new ElggFile();
 		$readfile->owner_guid = $file->owner_guid;
 		$readfile->setFilename($thumbfile);
-		$mime = $file->getMimeType();
+		$mime = $readfile->detectMimeType();
+		if ($mime === 'application/octet-stream') {
+			$mime = 'image/jpeg';
+		}
 		$contents = $readfile->grabFile();
 
 		// caching images for 10 days