mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Throw exception on `getPath` if file does not exist 

Currently the `getPath` methods returned `NULL` in case when a file with the specified ID does not exist. This however mandates that developers are checking for the `NULL` case and if they do not the door for bugs with all kind of impact is widely opened.

This is especially harmful if used in context with Views where the final result is limited based on the result of `getPath`, if `getPath` returns `NULL` PHP type juggles this to an empty string resulting in all possible kind of bugs.

While one could argue that this is a misusage of the API the fact is that it is very often misused and an exception will trigger an immediate stop of execution as well as log this behaviour and show a pretty error page.

I also adjusted some usages where I believe that we need to catch these errors, in most cases this is though simply an error that should hard-fail.
This commit is contained in:
Lukas Reschke 2015-06-15 14:10:10 +02:00
parent eb10e3abc2
commit 13e817e901
14 changed files with 121 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
apps/files_sharing/api/server2server.php
View File

@ -25,6 +25,7 @@
namespace OCA\Files_Sharing\API; namespace OCA\Files_Sharing\API;
use OCA\Files_Sharing\Activity; use OCA\Files_Sharing\Activity;
use OCP\Files\NotFoundException;
class Server2Server { class Server2Server {
@ -264,7 +265,11 @@ class Server2Server {
private function getFile($user, $fileSource) { private function getFile($user, $fileSource) {
\OC_Util::setupFS($user); \OC_Util::setupFS($user);
$file = \OC\Files\Filesystem::getPath($fileSource); try {
$file = \OC\Files\Filesystem::getPath($fileSource);
} catch (NotFoundException $e) {
$file = null;
}
$args = \OC\Files\Filesystem::is_dir($file) ? array('dir' => $file) : array('dir' => dirname($file), 'scrollto' => $file); $args = \OC\Files\Filesystem::is_dir($file) ? array('dir' => $file) : array('dir' => dirname($file), 'scrollto' => $file);
$link = \OCP\Util::linkToAbsolute('files', 'index.php', $args); $link = \OCP\Util::linkToAbsolute('files', 'index.php', $args);

3
apps/files_sharing/lib/controllers/sharecontroller.php
View File

@ -333,8 +333,7 @@ class ShareController extends Controller {
OC_Util::tearDownFS(); OC_Util::tearDownFS();
OC_Util::setupFS($rootLinkItem['uid_owner']); OC_Util::setupFS($rootLinkItem['uid_owner']);
$path = Filesystem::getPath($linkItem['file_source']); $path = Filesystem::getPath($linkItem['file_source']);
if(Filesystem::isReadable($path)) {
if(!empty($path) && Filesystem::isReadable($path)) {
return $path; return $path;
} }
} }

8
apps/files_sharing/lib/helper.php
View File

@ -28,6 +28,8 @@
*/ */
namespace OCA\Files_Sharing; namespace OCA\Files_Sharing;
use OCP\Files\NotFoundException;
class Helper { class Helper {
public static function registerHooks() { public static function registerHooks() {
@ -48,6 +50,7 @@ class Helper {
* @param string $token string share token * @param string $token string share token
* @param string $relativePath optional path relative to the share * @param string $relativePath optional path relative to the share
* @param string $password optional password * @param string $password optional password
* @return array
*/ */
public static function setupFromToken($token, $relativePath = null, $password = null) { public static function setupFromToken($token, $relativePath = null, $password = null) {
\OC_User::setIncognitoMode(true); \OC_User::setIncognitoMode(true);
@ -71,10 +74,11 @@ class Helper {
\OCP\JSON::checkUserExists($rootLinkItem['uid_owner']); \OCP\JSON::checkUserExists($rootLinkItem['uid_owner']);
\OC_Util::tearDownFS(); \OC_Util::tearDownFS();
\OC_Util::setupFS($rootLinkItem['uid_owner']); \OC_Util::setupFS($rootLinkItem['uid_owner']);
$path = \OC\Files\Filesystem::getPath($linkItem['file_source']);
} }
if ($path === null) { try {
$path = \OC\Files\Filesystem::getPath($linkItem['file_source']);
} catch (NotFoundException $e) {
\OCP\Util::writeLog('share', 'could not resolve linkItem', \OCP\Util::DEBUG); \OCP\Util::writeLog('share', 'could not resolve linkItem', \OCP\Util::DEBUG);
\OC_Response::setStatus(404); \OC_Response::setStatus(404);
\OCP\JSON::error(array('success' => false)); \OCP\JSON::error(array('success' => false));

12
apps/files_sharing/lib/propagation/recipientpropagator.php
View File

@ -25,6 +25,7 @@ namespace OCA\Files_Sharing\Propagation;
use OC\Files\Cache\ChangePropagator; use OC\Files\Cache\ChangePropagator;
use OC\Files\View; use OC\Files\View;
use OC\Share\Share; use OC\Share\Share;
use OCP\Files\NotFoundException;
/** /**
* Propagate etags for share recipients * Propagate etags for share recipients
@ -128,6 +129,9 @@ class RecipientPropagator {
protected $propagatingIds = []; protected $propagatingIds = [];
/**
* @param int $id
*/
public function propagateById($id) { public function propagateById($id) {
if (isset($this->propagatingIds[$id])) { if (isset($this->propagatingIds[$id])) {
return; return;
@ -142,7 +146,13 @@ class RecipientPropagator {
if ($share['share_with'] === $this->userId) { if ($share['share_with'] === $this->userId) {
$user = $share['uid_owner']; $user = $share['uid_owner'];
$view = new View('/' . $user . '/files'); $view = new View('/' . $user . '/files');
$path = $view->getPath($share['file_source']);
try {
$path = $view->getPath($share['file_source']);
} catch (NotFoundException $e) {
$path = null;
}
$watcher = new ChangeWatcher($view, $this->manager->getSharePropagator($user)); $watcher = new ChangeWatcher($view, $this->manager->getSharePropagator($user));
$watcher->writeHook(['path' => $path]); $watcher->writeHook(['path' => $path]);
} }

14
apps/files_sharing/lib/share/file.php
View File

@ -40,15 +40,16 @@ class OC_Share_Backend_File implements OCP\Share_Backend_File_Dependent {
private $path; private $path;
public function isValidSource($itemSource, $uidOwner) { public function isValidSource($itemSource, $uidOwner) {
$path = \OC\Files\Filesystem::getPath($itemSource); try {
if ($path) { $path = \OC\Files\Filesystem::getPath($itemSource);
// FIXME: attributes should not be set here, // FIXME: attributes should not be set here,
// keeping this pattern for now to avoid unexpected // keeping this pattern for now to avoid unexpected
// regressions // regressions
$this->path = \OC\Files\Filesystem::normalizePath(basename($path)); $this->path = \OC\Files\Filesystem::normalizePath(basename($path));
return true; return true;
} catch (\OCP\Files\NotFoundException $e) {
return false;
} }
return false;
} }
public function getFilePath($itemSource, $uidOwner) { public function getFilePath($itemSource, $uidOwner) {
@ -57,12 +58,13 @@ class OC_Share_Backend_File implements OCP\Share_Backend_File_Dependent {
$this->path = null; $this->path = null;
return $path; return $path;
} else { } else {
$path = \OC\Files\Filesystem::getPath($itemSource); try {
if ($path) { $path = \OC\Files\Filesystem::getPath($itemSource);
return $path; return $path;
} catch (\OCP\Files\NotFoundException $e) {
return false;
} }
} }
return false;
} }
/** /**

6
apps/files_sharing/tests/controller/sharecontroller.php
View File

@ -199,8 +199,7 @@ class ShareControllerTest extends \Test\TestCase {
} }
/** /**
* @expectedException \Exception * @expectedException \OCP\Files\NotFoundException
* @expectedExceptionMessage No file found belonging to file.
*/ */
public function testShowShareWithDeletedFile() { public function testShowShareWithDeletedFile() {
$this->container['UserManager']->expects($this->once()) $this->container['UserManager']->expects($this->once())
@ -216,8 +215,7 @@ class ShareControllerTest extends \Test\TestCase {
} }
/** /**
* @expectedException \Exception * @expectedException \OCP\Files\NotFoundException
* @expectedExceptionMessage No file found belonging to file.
*/ */
public function testDownloadShareWithDeletedFile() { public function testDownloadShareWithDeletedFile() {
$this->container['UserManager']->expects($this->once()) $this->container['UserManager']->expects($this->once())

50
apps/files_sharing/tests/etagpropagation.php
View File

@ -173,7 +173,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsChanged($users, 'sub1/sub2'); $this->assertEtagsChanged($users, 'sub1/sub2');
} }
private function assertAllUnchaged() { private function assertAllUnchanged() {
$users = [self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, $users = [self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]; self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4];
$this->assertEtagsNotChanged($users); $this->assertEtagsNotChanged($users);
@ -186,7 +186,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, $this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
self::TEST_FILES_SHARING_API_USER3]); self::TEST_FILES_SHARING_API_USER3]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testOwnerWritesToSingleFileShare() { public function testOwnerWritesToSingleFileShare() {
@ -195,7 +195,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsNotChanged([self::TEST_FILES_SHARING_API_USER4, self::TEST_FILES_SHARING_API_USER3]); $this->assertEtagsNotChanged([self::TEST_FILES_SHARING_API_USER4, self::TEST_FILES_SHARING_API_USER3]);
$this->assertEtagsChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2]); $this->assertEtagsChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testOwnerWritesToShareWithReshare() { public function testOwnerWritesToShareWithReshare() {
@ -204,7 +204,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, $this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]); self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testOwnerRenameInShare() { public function testOwnerRenameInShare() {
@ -214,7 +214,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, $this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
self::TEST_FILES_SHARING_API_USER3]); self::TEST_FILES_SHARING_API_USER3]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testOwnerRenameInReShare() { public function testOwnerRenameInReShare() {
@ -223,7 +223,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, $this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]); self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testOwnerRenameIntoReShare() { public function testOwnerRenameIntoReShare() {
@ -232,7 +232,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, $this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]); self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testOwnerRenameOutOfReShare() { public function testOwnerRenameOutOfReShare() {
@ -241,7 +241,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, $this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]); self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testOwnerDeleteInShare() { public function testOwnerDeleteInShare() {
@ -251,7 +251,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, $this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
self::TEST_FILES_SHARING_API_USER3]); self::TEST_FILES_SHARING_API_USER3]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testOwnerDeleteInReShare() { public function testOwnerDeleteInReShare() {
@ -260,7 +260,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, $this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]); self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testOwnerUnshares() { public function testOwnerUnshares() {
@ -283,7 +283,7 @@ class EtagPropagation extends TestCase {
self::TEST_FILES_SHARING_API_USER4, self::TEST_FILES_SHARING_API_USER4,
]); ]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testRecipientUnsharesFromSelf() { public function testRecipientUnsharesFromSelf() {
@ -298,7 +298,7 @@ class EtagPropagation extends TestCase {
self::TEST_FILES_SHARING_API_USER4, self::TEST_FILES_SHARING_API_USER4,
]); ]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testRecipientWritesToShare() { public function testRecipientWritesToShare() {
@ -308,7 +308,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, $this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
self::TEST_FILES_SHARING_API_USER3]); self::TEST_FILES_SHARING_API_USER3]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testRecipientWritesToReshare() { public function testRecipientWritesToReshare() {
@ -317,7 +317,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, $this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]); self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testRecipientWritesToOtherRecipientsReshare() { public function testRecipientWritesToOtherRecipientsReshare() {
@ -326,7 +326,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, $this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]); self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testRecipientRenameInShare() { public function testRecipientRenameInShare() {
@ -336,7 +336,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, $this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
self::TEST_FILES_SHARING_API_USER3]); self::TEST_FILES_SHARING_API_USER3]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testRecipientRenameInReShare() { public function testRecipientRenameInReShare() {
@ -345,7 +345,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, $this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]); self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testRecipientRenameResharedFolder() { public function testRecipientRenameResharedFolder() {
@ -356,7 +356,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsChanged([self::TEST_FILES_SHARING_API_USER2], 'sub1'); $this->assertEtagsChanged([self::TEST_FILES_SHARING_API_USER2], 'sub1');
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testRecipientDeleteInShare() { public function testRecipientDeleteInShare() {
@ -366,7 +366,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, $this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
self::TEST_FILES_SHARING_API_USER3]); self::TEST_FILES_SHARING_API_USER3]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testRecipientDeleteInReShare() { public function testRecipientDeleteInReShare() {
@ -375,7 +375,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, $this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]); self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testReshareRecipientWritesToReshare() { public function testReshareRecipientWritesToReshare() {
@ -384,7 +384,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, $this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]); self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testReshareRecipientRenameInReShare() { public function testReshareRecipientRenameInReShare() {
@ -393,7 +393,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, $this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]); self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testReshareRecipientDeleteInReShare() { public function testReshareRecipientDeleteInReShare() {
@ -402,7 +402,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, $this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]); self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testRecipientUploadInDirectReshare() { public function testRecipientUploadInDirectReshare() {
@ -411,7 +411,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsNotChanged([self::TEST_FILES_SHARING_API_USER3]); $this->assertEtagsNotChanged([self::TEST_FILES_SHARING_API_USER3]);
$this->assertEtagsChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, self::TEST_FILES_SHARING_API_USER4]); $this->assertEtagsChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
public function testEtagChangeOnPermissionsChange() { public function testEtagChangeOnPermissionsChange() {
@ -424,6 +424,6 @@ class EtagPropagation extends TestCase {
$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER2, self::TEST_FILES_SHARING_API_USER4]); $this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER2, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged(); $this->assertAllUnchanged();
} }
} }

14
apps/files_trashbin/lib/trashbin.php
View File

@ -41,6 +41,7 @@ use OC\Files\Filesystem;
use OC\Files\View; use OC\Files\View;
use OCA\Files_Trashbin\AppInfo\Application; use OCA\Files_Trashbin\AppInfo\Application;
use OCA\Files_Trashbin\Command\Expire; use OCA\Files_Trashbin\Command\Expire;
use OCP\Files\NotFoundException;
class Trashbin { class Trashbin {
@ -64,15 +65,24 @@ class Trashbin {
self::getUidAndFilename($params['path']); self::getUidAndFilename($params['path']);
} }
/**
* @param string $filename
* @return array
* @throws \OC\User\NoUserException
*/
public static function getUidAndFilename($filename) { public static function getUidAndFilename($filename) {
$uid = \OC\Files\Filesystem::getOwner($filename); $uid = \OC\Files\Filesystem::getOwner($filename);
\OC\Files\Filesystem::initMountPoints($uid); \OC\Files\Filesystem::initMountPoints($uid);
if ($uid != \OCP\User::getUser()) { if ($uid != \OCP\User::getUser()) {
$info = \OC\Files\Filesystem::getFileInfo($filename); $info = \OC\Files\Filesystem::getFileInfo($filename);
$ownerView = new \OC\Files\View('/' . $uid . '/files'); $ownerView = new \OC\Files\View('/' . $uid . '/files');
$filename = $ownerView->getPath($info['fileid']); try {
$filename = $ownerView->getPath($info['fileid']);
} catch (NotFoundException $e) {
$filename = null;
}
} }
return array($uid, $filename); return [$uid, $filename];
} }
/** /**

14
apps/files_versions/lib/storage.php
View File

@ -44,6 +44,7 @@ namespace OCA\Files_Versions;
use OCA\Files_Versions\AppInfo\Application; use OCA\Files_Versions\AppInfo\Application;
use OCA\Files_Versions\Command\Expire; use OCA\Files_Versions\Command\Expire;
use OCP\Lock\ILockingProvider; use OCP\Lock\ILockingProvider;
use OCP\Files\NotFoundException;
class Storage { class Storage {
@ -74,15 +75,24 @@ class Storage {
/** @var \OCA\Files_Versions\AppInfo\Application */ /** @var \OCA\Files_Versions\AppInfo\Application */
private static $application; private static $application;
/**
* @param string $filename
* @return array
* @throws \OC\User\NoUserException
*/
public static function getUidAndFilename($filename) { public static function getUidAndFilename($filename) {
$uid = \OC\Files\Filesystem::getOwner($filename); $uid = \OC\Files\Filesystem::getOwner($filename);
\OC\Files\Filesystem::initMountPoints($uid); \OC\Files\Filesystem::initMountPoints($uid);
if ( $uid != \OCP\User::getUser() ) { if ( $uid != \OCP\User::getUser() ) {
$info = \OC\Files\Filesystem::getFileInfo($filename); $info = \OC\Files\Filesystem::getFileInfo($filename);
$ownerView = new \OC\Files\View('/'.$uid.'/files'); $ownerView = new \OC\Files\View('/'.$uid.'/files');
$filename = $ownerView->getPath($info['fileid']); try {
$filename = $ownerView->getPath($info['fileid']);
} catch (NotFoundException $e) {
$filename = null;
}
} }
return array($uid, $filename); return [$uid, $filename];
} }
/** /**

7
apps/files_versions/tests/versions.php
View File

@ -759,7 +759,11 @@ class Test_Files_Versioning extends \Test\TestCase {
); );
} }
private function createAndCheckVersions($view, $path) { /**
* @param \OC\Files\View $view
* @param string $path
*/
private function createAndCheckVersions(\OC\Files\View $view, $path) {
$view->file_put_contents($path, 'test file'); $view->file_put_contents($path, 'test file');
$view->file_put_contents($path, 'version 1'); $view->file_put_contents($path, 'version 1');
$view->file_put_contents($path, 'version 2'); $view->file_put_contents($path, 'version 2');
@ -782,7 +786,6 @@ class Test_Files_Versioning extends \Test\TestCase {
/** /**
* @param string $user * @param string $user
* @param bool $create * @param bool $create
* @param bool $password
*/ */
public static function loginHelper($user, $create = false) { public static function loginHelper($user, $create = false) {

4
lib/private/files/filesystem.php
View File

@ -62,6 +62,7 @@ use OC\Cache\File;
use OC\Files\Config\MountProviderCollection; use OC\Files\Config\MountProviderCollection;
use OC\Files\Storage\StorageFactory; use OC\Files\Storage\StorageFactory;
use OCP\Files\Config\IMountProvider; use OCP\Files\Config\IMountProvider;
use OCP\Files\NotFoundException;
use OCP\IUserManager; use OCP\IUserManager;
class Filesystem { class Filesystem {
@ -855,7 +856,7 @@ class Filesystem {
* @param string $path * @param string $path
* @param boolean $includeMountPoints whether to add mountpoint sizes, * @param boolean $includeMountPoints whether to add mountpoint sizes,
* defaults to true * defaults to true
* @return \OC\Files\FileInfo * @return \OC\Files\FileInfo|bool False if file does not exist
*/ */
public static function getFileInfo($path, $includeMountPoints = true) { public static function getFileInfo($path, $includeMountPoints = true) {
return self::$defaultInstance->getFileInfo($path, $includeMountPoints); return self::$defaultInstance->getFileInfo($path, $includeMountPoints);
@ -891,6 +892,7 @@ class Filesystem {
* Note that the resulting path is not guaranteed to be unique for the id, multiple paths can point to the same file * Note that the resulting path is not guaranteed to be unique for the id, multiple paths can point to the same file
* *
* @param int $id * @param int $id
* @throws NotFoundException
* @return string * @return string
*/ */
public static function getPath($id) { public static function getPath($id) {

16
lib/private/files/view.php
View File

@ -48,6 +48,7 @@ use OC\Files\Mount\MoveableMount;
use OCP\Files\FileNameTooLongException; use OCP\Files\FileNameTooLongException;
use OCP\Files\InvalidCharacterInPathException; use OCP\Files\InvalidCharacterInPathException;
use OCP\Files\InvalidPathException; use OCP\Files\InvalidPathException;
use OCP\Files\NotFoundException;
use OCP\Files\ReservedWordException; use OCP\Files\ReservedWordException;
use OCP\Lock\ILockingProvider; use OCP\Lock\ILockingProvider;
use OCP\Lock\LockedException; use OCP\Lock\LockedException;
@ -533,6 +534,7 @@ class View {
* @param string $path * @param string $path
* @param mixed $data * @param mixed $data
* @return bool|mixed * @return bool|mixed
* @throws \Exception
*/ */
public function file_put_contents($path, $data) { public function file_put_contents($path, $data) {
if (is_resource($data)) { //not having to deal with streams in file_put_contents makes life easier if (is_resource($data)) { //not having to deal with streams in file_put_contents makes life easier
@ -989,12 +991,13 @@ class View {
* @param array $hooks (optional) * @param array $hooks (optional)
* @param mixed $extraParam (optional) * @param mixed $extraParam (optional)
* @return mixed * @return mixed
* @throws \Exception
* *
* This method takes requests for basic filesystem functions (e.g. reading & writing * This method takes requests for basic filesystem functions (e.g. reading & writing
* files), processes hooks and proxies, sanitises paths, and finally passes them on to * files), processes hooks and proxies, sanitises paths, and finally passes them on to
* \OC\Files\Storage\Storage for delegation to a storage backend for execution * \OC\Files\Storage\Storage for delegation to a storage backend for execution
*/ */
private function basicOperation($operation, $path, $hooks = array(), $extraParam = null) { private function basicOperation($operation, $path, $hooks = [], $extraParam = null) {
$postFix = (substr($path, -1, 1) === '/') ? '/' : ''; $postFix = (substr($path, -1, 1) === '/') ? '/' : '';
$absolutePath = Filesystem::normalizePath($this->getAbsolutePath($path)); $absolutePath = Filesystem::normalizePath($this->getAbsolutePath($path));
if (Filesystem::isValidPath($path) if (Filesystem::isValidPath($path)
@ -1166,7 +1169,7 @@ class View {
* @param boolean|string $includeMountPoints true to add mountpoint sizes, * @param boolean|string $includeMountPoints true to add mountpoint sizes,
* 'ext' to add only ext storage mount point sizes. Defaults to true. * 'ext' to add only ext storage mount point sizes. Defaults to true.
* defaults to true * defaults to true
* @return \OC\Files\FileInfo|false * @return \OC\Files\FileInfo|bool False if file does not exist
*/ */
public function getFileInfo($path, $includeMountPoints = true) { public function getFileInfo($path, $includeMountPoints = true) {
$this->assertPathLength($path); $this->assertPathLength($path);
@ -1563,7 +1566,8 @@ class View {
* Note that the resulting path is not guarantied to be unique for the id, multiple paths can point to the same file * Note that the resulting path is not guarantied to be unique for the id, multiple paths can point to the same file
* *
* @param int $id * @param int $id
* @return string|null * @throws NotFoundException
* @return string
*/ */
public function getPath($id) { public function getPath($id) {
$id = (int)$id; $id = (int)$id;
@ -1588,9 +1592,13 @@ class View {
} }
} }
} }
return null; throw new NotFoundException(sprintf('File with id "%s" has not been found.', $id));
} }
/**
* @param string $path
* @throws InvalidPathException
*/
private function assertPathLength($path) { private function assertPathLength($path) {
$maxLen = min(PHP_MAXPATHLEN, 4000); $maxLen = min(PHP_MAXPATHLEN, 4000);
// Check for the string length - performed using isset() instead of strlen() // Check for the string length - performed using isset() instead of strlen()

4
lib/private/hook.php
View File

@ -78,12 +78,12 @@ class OC_Hook{
* @param string $signalName name of signal * @param string $signalName name of signal
* @param mixed $params default: array() array with additional data * @param mixed $params default: array() array with additional data
* @return bool true if slots exists or false if not * @return bool true if slots exists or false if not
* * @throws \OC\ServerNotAvailableException
* Emits a signal. To get data from the slot use references! * Emits a signal. To get data from the slot use references!
* *
* TODO: write example * TODO: write example
*/ */
static public function emit($signalClass, $signalName, $params = array()) { static public function emit($signalClass, $signalName, $params = []) {
// Return false if no hook handlers are listening to this // Return false if no hook handlers are listening to this
// emitting class // emitting class

16
tests/lib/files/view.php
View File

@ -207,15 +207,31 @@ class View extends \Test\TestCase {
$rootView = new \OC\Files\View(''); $rootView = new \OC\Files\View('');
$cachedData = $rootView->getFileInfo('/foo.txt'); $cachedData = $rootView->getFileInfo('/foo.txt');
/** @var int $id1 */
$id1 = $cachedData['fileid']; $id1 = $cachedData['fileid'];
$this->assertEquals('/foo.txt', $rootView->getPath($id1)); $this->assertEquals('/foo.txt', $rootView->getPath($id1));
$cachedData = $rootView->getFileInfo('/substorage/foo.txt'); $cachedData = $rootView->getFileInfo('/substorage/foo.txt');
/** @var int $id2 */
$id2 = $cachedData['fileid']; $id2 = $cachedData['fileid'];
$this->assertEquals('/substorage/foo.txt', $rootView->getPath($id2)); $this->assertEquals('/substorage/foo.txt', $rootView->getPath($id2));
$folderView = new \OC\Files\View('/substorage'); $folderView = new \OC\Files\View('/substorage');
$this->assertEquals('/foo.txt', $folderView->getPath($id2)); $this->assertEquals('/foo.txt', $folderView->getPath($id2));
}
/**
* @expectedException \OCP\Files\NotFoundException
*/
function testGetPathNotExisting() {
$storage1 = $this->getTestStorage();
\OC\Files\Filesystem::mount($storage1, [], '/');
$rootView = new \OC\Files\View('');
$cachedData = $rootView->getFileInfo('/foo.txt');
/** @var int $id1 */
$id1 = $cachedData['fileid'];
$folderView = new \OC\Files\View('/substorage');
$this->assertNull($folderView->getPath($id1)); $this->assertNull($folderView->getPath($id1));
} }