wbrawner/server
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #16957 from owncloud/throw-exception-if-file-does-not-exist

Browse source 
Throw exception on `getPath` if file does not exist
This commit is contained in:
Lukas Reschke 2015-10-25 18:35:57 +01:00
commit add164ef2d
14 changed files with 121 additions and 54 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
apps/files_sharing/api/server2server.php
View file

@ -25,6 +25,7 @@
namespace OCA\Files_Sharing\API;
use OCA\Files_Sharing\Activity;
use OCP\Files\NotFoundException;
class Server2Server {
@ -264,7 +265,11 @@ class Server2Server {
private function getFile($user, $fileSource) {
\OC_Util::setupFS($user);
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);
$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::setupFS($rootLinkItem['uid_owner']);
$path = Filesystem::getPath($linkItem['file_source']);
if(!empty($path) && Filesystem::isReadable($path)) {
if(Filesystem::isReadable($path)) {
return $path;
}
}

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

@ -28,6 +28,8 @@
*/
namespace OCA\Files_Sharing;
use OCP\Files\NotFoundException;
class Helper {
public static function registerHooks() {
@ -48,6 +50,7 @@ class Helper {
* @param string $token string share token
* @param string $relativePath optional path relative to the share
* @param string $password optional password
* @return array
*/
public static function setupFromToken($token, $relativePath = null, $password = null) {
\OC_User::setIncognitoMode(true);
@ -71,10 +74,11 @@ class Helper {
\OCP\JSON::checkUserExists($rootLinkItem['uid_owner']);
\OC_Util::tearDownFS();
\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);
\OC_Response::setStatus(404);
\OCP\JSON::error(array('success' => false));

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

12
apps/files_sharing/lib/share/file.php
View file

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

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

@ -199,8 +199,7 @@ class ShareControllerTest extends \Test\TestCase {
}
/**
* @expectedException \Exception
* @expectedExceptionMessage No file found belonging to file.
* @expectedException \OCP\Files\NotFoundException
*/
public function testShowShareWithDeletedFile() {
$this->container['UserManager']->expects($this->once())
@ -216,8 +215,7 @@ class ShareControllerTest extends \Test\TestCase {
}
/**
* @expectedException \Exception
* @expectedExceptionMessage No file found belonging to file.
* @expectedException \OCP\Files\NotFoundException
*/
public function testDownloadShareWithDeletedFile() {
$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');
}
private function assertAllUnchaged() {
private function assertAllUnchanged() {
$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];
$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,
self::TEST_FILES_SHARING_API_USER3]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
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->assertEtagsChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
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,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
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,
self::TEST_FILES_SHARING_API_USER3]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
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,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
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,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
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,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
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,
self::TEST_FILES_SHARING_API_USER3]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
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,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
public function testOwnerUnshares() {
@ -283,7 +283,7 @@ class EtagPropagation extends TestCase {
self::TEST_FILES_SHARING_API_USER4,
]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
public function testRecipientUnsharesFromSelf() {
@ -298,7 +298,7 @@ class EtagPropagation extends TestCase {
self::TEST_FILES_SHARING_API_USER4,
]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
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,
self::TEST_FILES_SHARING_API_USER3]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
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,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
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,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
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,
self::TEST_FILES_SHARING_API_USER3]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
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,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
public function testRecipientRenameResharedFolder() {
@ -356,7 +356,7 @@ class EtagPropagation extends TestCase {
$this->assertEtagsChanged([self::TEST_FILES_SHARING_API_USER2], 'sub1');
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
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,
self::TEST_FILES_SHARING_API_USER3]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
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,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
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,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
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,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
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,
self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
$this->assertAllUnchaged();
$this->assertAllUnchanged();
}
public function testRecipientUploadInDirectReshare() {
@ -411,7 +411,7 @@ class EtagPropagation extends TestCase {
$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->assertAllUnchaged();
$this->assertAllUnchanged();
}
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->assertAllUnchaged();
$this->assertAllUnchanged();
}
}

12
apps/files_trashbin/lib/trashbin.php
View file

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

12
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\Command\Expire;
use OCP\Lock\ILockingProvider;
use OCP\Files\NotFoundException;
class Storage {
@ -74,15 +75,24 @@ class Storage {
/** @var \OCA\Files_Versions\AppInfo\Application */
private static $application;
/**
* @param string $filename
* @return array
* @throws \OC\User\NoUserException
*/
public static function getUidAndFilename($filename) {
$uid = \OC\Files\Filesystem::getOwner($filename);
\OC\Files\Filesystem::initMountPoints($uid);
if ( $uid != \OCP\User::getUser() ) {
$info = \OC\Files\Filesystem::getFileInfo($filename);
$ownerView = new \OC\Files\View('/'.$uid.'/files');
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, 'version 1');
$view->file_put_contents($path, 'version 2');
@ -782,7 +786,6 @@ class Test_Files_Versioning extends \Test\TestCase {
/**
* @param string $user
* @param bool $create
* @param bool $password
*/
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\Storage\StorageFactory;
use OCP\Files\Config\IMountProvider;
use OCP\Files\NotFoundException;
use OCP\IUserManager;
class Filesystem {
@ -855,7 +856,7 @@ class Filesystem {
* @param string $path
* @param boolean $includeMountPoints whether to add mountpoint sizes,
* 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) {
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
*
* @param int $id
* @throws NotFoundException
* @return string
*/
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\InvalidCharacterInPathException;
use OCP\Files\InvalidPathException;
use OCP\Files\NotFoundException;
use OCP\Files\ReservedWordException;
use OCP\Lock\ILockingProvider;
use OCP\Lock\LockedException;
@ -533,6 +534,7 @@ class View {
* @param string $path
* @param mixed $data
* @return bool|mixed
* @throws \Exception
*/
public function file_put_contents($path, $data) {
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 mixed $extraParam (optional)
* @return mixed
* @throws \Exception
*
* 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
* \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) === '/') ? '/' : '';
$absolutePath = Filesystem::normalizePath($this->getAbsolutePath($path));
if (Filesystem::isValidPath($path)
@ -1166,7 +1169,7 @@ class View {
* @param boolean|string $includeMountPoints true to add mountpoint sizes,
* 'ext' to add only ext storage mount point sizes. 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) {
$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
*
* @param int $id
* @return string|null
* @throws NotFoundException
* @return string
*/
public function getPath($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) {
$maxLen = min(PHP_MAXPATHLEN, 4000);
// 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 mixed $params default: array() array with additional data
* @return bool true if slots exists or false if not
*
* @throws \OC\ServerNotAvailableException
* Emits a signal. To get data from the slot use references!
*
* 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
// emitting class

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

@ -207,15 +207,31 @@ class View extends \Test\TestCase {
$rootView = new \OC\Files\View('');
$cachedData = $rootView->getFileInfo('/foo.txt');
/** @var int $id1 */
$id1 = $cachedData['fileid'];
$this->assertEquals('/foo.txt', $rootView->getPath($id1));
$cachedData = $rootView->getFileInfo('/substorage/foo.txt');
/** @var int $id2 */
$id2 = $cachedData['fileid'];
$this->assertEquals('/substorage/foo.txt', $rootView->getPath($id2));
$folderView = new \OC\Files\View('/substorage');
$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));
}