Use dav preview for version preview

Fix public preview

Fix duplicate disposition header and set cache control on previews

return 404 if preview could not be generated

Encode file name properly

Fix preview hooks on public share page

apps/dav/lib/Connector/Sabre/FilesPlugin.php

@@ -241,9 +241,9 @@ function httpGet(RequestInterface $request, ResponseInterface $response) {
 					Request::USER_AGENT_ANDROID_MOBILE_CHROME,
 					Request::USER_AGENT_FREEBOX,
 				])) {
-				$response->addHeader('Content-Disposition', 'attachment; filename="' . rawurlencode($filename) . '"');
+				$response->setHeader('Content-Disposition', 'attachment; filename="' . rawurlencode($filename) . '"');
 			} else {
-				$response->addHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . rawurlencode($filename)
+				$response->setHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . rawurlencode($filename)
 													 . '; filename="' . rawurlencode($filename) . '"');
 			}
 		}

apps/dav/lib/Files/PreviewPlugin.php

@@ -60,6 +60,8 @@ function initialize(Server $server) {
 	 * @param ResponseInterface $response
 	 * @return bool
 	 * @throws NotFound
+	 * @throws \Sabre\DAVACL\Exception\NeedPrivileges
+	 * @throws \Sabre\DAV\Exception\NotAuthenticated
 	 */
 	function httpGet(RequestInterface $request, ResponseInterface $response) {
 
@@ -87,7 +89,7 @@ function httpGet(RequestInterface $request, ResponseInterface $response) {
 
 		if ($image = $fileNode->getThumbnail($queryParams)) {
 			if ($image === null || !$image->valid()) {
-				return false;
+				throw new NotFound();
 			}
 			$type = $image->mimeType();
 			if (!in_array($type, ['image/png', 'image/jpeg', 'image/gif'])) {
@@ -104,8 +106,11 @@ function httpGet(RequestInterface $request, ResponseInterface $response) {
 
 			$response->setHeader('Content-Type', $type);
 			$response->setHeader('Content-Disposition', 'attachment');
-			$response->setStatus(200);
+			// cache 24h
+			$response->setHeader('Cache-Control', 'max-age=86400, must-revalidate');
+			$response->setHeader('Expires', gmdate ("D, d M Y H:i:s", time() + 86400) . " GMT");
 
+			$response->setStatus(200);
 			$response->setBody($imageData);
 
 			// Returning false to break the event chain

apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php

@@ -560,14 +560,18 @@ public function testDownloadHeaders($isClumsyAgent, $contentDispositionHeader) {
 			->will($this->returnValue($isClumsyAgent));
 
 		$response
-			->expects($this->exactly(3))
+			->expects($this->exactly(2))
 			->method('addHeader')
 			->withConsecutive(
-				['Content-Disposition', $contentDispositionHeader],
 				['OC-Checksum', 'abcdefghijkl'],
 				['X-Accel-Buffering', 'no']
 			);
 
+		$response
+			->expects($this->once())
+			->method('setHeader')
+			->with('Content-Disposition', $contentDispositionHeader);
+
 		$this->plugin->httpGet($request, $response);
 	}
 

apps/files/js/filelist.js

@@ -1718,7 +1718,7 @@
 			urlSpec.x = Math.ceil(urlSpec.x);
 			urlSpec.y = Math.ceil(urlSpec.y);
 			urlSpec.forceIcon = 0;
-			var file = urlSpec.file;
+			var file = encodeURIComponent(urlSpec.file);
 			delete urlSpec.file;
 			urlSpec.preview = 1;
 

apps/files_sharing/ajax/publicpreview.php

@@ -81,12 +81,12 @@
 		\OCP\Util::writeLog('core-preview', 'Passed filename is not valid, might be malicious (file:"' . $file . '";ip:"' . \OC::$server->getRequest()->getRemoteAddress() . '")', \OCP\Util::WARN);
 		exit;
 	}
-	$sharedFile = \OC\Files\Filesystem::normalizePath($file);
+	$sharedFile = $node->get($file);
 }
 
 if($linkedItem->getNodeType() === 'file') {
 	$path = $node->getParent()->getPath();
-	$sharedFile = $node->getName();
+	$sharedFile = $node;
 }
 
 $path = ltrim(\OC\Files\Filesystem::normalizePath($path, false), '/');

apps/files_versions/appinfo/app.php

@@ -24,3 +24,5 @@
 \OCP\Util::addStyle('files_versions', 'versions');
 
 \OCA\Files_Versions\Hooks::connectHooks();
+
+$application = new \OCA\Files_Versions\AppInfo\Application();

apps/files_versions/js/versionmodel.js

@@ -46,12 +46,9 @@
 		},
 
 		getPreviewUrl: function() {
-			var url = OC.generateUrl('/apps/files_versions/preview');
-			var params = {
-				file: this.get('fullPath'),
-				version: this.get('timestamp')
-			};
-			return url + '?' + OC.buildQueryString(params);
+			return OC.linkToRemoteBase('dav') + '/meta/' +
+				encodeURIComponent(this.get('fileId')) + '/v/' +
+				encodeURIComponent(this.get('id')) + '?preview';
 		},
 
 		getDownloadUrl: function() {

apps/files_versions/tests/js/versionmodelSpec.js

@@ -39,8 +39,8 @@ describe('OCA.Versions.VersionModel', function() {
 	});
 	it('returns the preview url', function() {
 		expect(model.getPreviewUrl())
-			.toEqual(OC.generateUrl('/apps/files_versions/preview') +
-					'?file=%2Fsubdir%2Fsome%20file.txt&version=10000000'
+			.toEqual(
+				OC.linkToRemoteBase('dav') + '/meta/10000000/v/123456789?preview'
 			);
 	});
 	it('returns the download url', function() {

lib/private/Files/View.php

@@ -1285,6 +1285,9 @@ private function shouldEmitHooks($path = '') {
 	 * @return bool
 	 */
 	private function runHooks($hooks, $path, $post = false) {
+		if (empty($hooks)) {
+			return true;
+		}
 		$relativePath = $path;
 		$path = $this->getHookPath($path);
 		$prefix = ($post) ? 'post_' : '';

lib/private/Preview.php

@@ -31,6 +31,7 @@
  */
 namespace OC;
 
+use OC\Files\Filesystem;
 use OC\Files\View;
 use OCP\Files\File;
 use OCP\Files\FileInfo;
@@ -1237,12 +1238,19 @@ public static function prepare_delete_files($args) {
 	 * @param string $prefix
 	 */
 	public static function prepare_delete(array $args, $prefix = '') {
-		$path = $args['path'];
-		if (substr($path, 0, 1) === '/') {
-			$path = substr($path, 1);
+		$path = Files\Filesystem::normalizePath($args['path']);
+		$user = isset($args['user']) ? $args['user'] : \OC_User::getUser();
+		if ($user === false) {
+			$user = Filesystem::getOwner($path);
+		}
+
+		$userFolder = \OC::$server->getUserFolder($user);
+		if ($userFolder === null) {
+			return;
 		}
-		$node = \OC::$server->getUserFolder()->get($path);
-		self::addPathToDeleteFileMapper($node->getPath(), $node);
+
+		$node = $userFolder->get($path);
+		self::addPathToDeleteFileMapper($path, $node);
 		if ($node->getType() === FileInfo::TYPE_FOLDER) {
 			$children = self::getAllChildren($node);
 			self::$deleteChildrenMapper[$node->getPath()] = $children;
@@ -1301,16 +1309,13 @@ public static function post_delete_versions($args) {
 	 */
 	public static function post_delete($args, $prefix = '') {
 		$path = Files\Filesystem::normalizePath($args['path']);
-		$user = isset($args['user']) ? $args['user'] : \OC_User::getUser();
-
-		$view = new View('/' . $user . '/' . $prefix);
-		$absPath = Files\Filesystem::normalizePath($view->getAbsolutePath($path));
-		if (!isset(self::$deleteFileMapper[$absPath])) {
+		if (!isset(self::$deleteFileMapper[$path])) {
 			return;
 		}
-		$node = self::$deleteFileMapper[$absPath];
 
-		$preview = new Preview($user, $prefix, $node);
+		/** @var FileInfo $node */
+		$node = self::$deleteFileMapper[$path];
+		$preview = new Preview($node->getOwner()->getUID(), $prefix, $node);
 		$preview->deleteAllPreviews();
 	}
 

tests/lib/Files/MetaFilesTest.php

@@ -31,6 +31,7 @@
 use OC\Files\View;
 use OCA\Files_Versions\Hooks;
 use OCP\Files\Folder;
+use OCP\IImage;
 use Test\TestCase;
 use Test\Traits\UserTrait;
 
@@ -48,6 +49,12 @@ protected function tearDown() {
 		parent::tearDown();
 	}
 
+	/**
+	 * @throws \Exception
+	 * @throws \OCP\Files\ForbiddenException
+	 * @throws \OCP\Files\NotFoundException
+	 * @throws \OCP\Files\NotPermittedException
+	 */
 	public function testMetaInNodeAPI() {
 		// workaround: re-setup versions hooks
 		Hooks::connectHooks();
@@ -102,9 +109,10 @@ public function testMetaInNodeAPI() {
 		$this->assertEquals([], $metaNodeOfFile->getHeaders());
 		$this->assertEquals($file, $metaNodeOfFile->getContentDispositionFileName());
 		$this->assertEquals('text/plain', $metaNodeOfFile->getMimetype());
-		$this->assertEquals($info->getMTime(), $metaNodeOfFile->getMTime());
 		$this->assertInternalType('string', $metaNodeOfFile->getEtag());
 		$this->assertTrue(strlen($metaNodeOfFile->getEtag()) > 0);
+		$thumbnail = $metaNodeOfFile->getThumbnail([]);
+		$this->assertInstanceOf(IImage::class, $thumbnail);
 
 		/** @var MetaFileVersionNode $metaNodeOfFile */
 		$this->assertEquals('1234', $metaNodeOfFile->getContent());