From 51d811d885e1a714e71de1421c44f31d9bea0364 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villafa=C3=B1ez?= Date: Fri, 12 Aug 2016 12:05:57 +0200 Subject: [PATCH 01/17] Fix file permissions for SMB (read-only folders will be writeable) (#25734) Read-only folders won't be deletable --- apps/files_external/lib/smb.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/apps/files_external/lib/smb.php b/apps/files_external/lib/smb.php index 59e074bb0240e..56504a36da863 100644 --- a/apps/files_external/lib/smb.php +++ b/apps/files_external/lib/smb.php @@ -362,6 +362,19 @@ public function isReadable($path) { } public function isUpdatable($path) { + try { + $info = $this->getFileInfo($path); + // following windows behaviour for read-only folders: they can be written into + // (https://support.microsoft.com/en-us/kb/326549 - "cause" section) + return !$info->isHidden() && (!$info->isReadOnly() || $this->is_dir($path)); + } catch (NotFoundException $e) { + return false; + } catch (ForbiddenException $e) { + return false; + } + } + + public function isDeletable($path) { try { $info = $this->getFileInfo($path); return !$info->isHidden() && !$info->isReadOnly(); From f39a2f4fefa45f005f17e83bad96aa21c7f67adb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 16 Aug 2016 10:50:32 +0200 Subject: [PATCH 02/17] add conditional smb trace logging for debug purposes --- apps/files_external/lib/smb.php | 261 ++++++++++++++++++++++++-------- 1 file changed, 199 insertions(+), 62 deletions(-) diff --git a/apps/files_external/lib/smb.php b/apps/files_external/lib/smb.php index 56504a36da863..c133521089eeb 100644 --- a/apps/files_external/lib/smb.php +++ b/apps/files_external/lib/smb.php @@ -42,6 +42,7 @@ use OC\Cache\CappedMemoryCache; use OC\Files\Filesystem; use OCP\Files\StorageNotAvailableException; +use OCP\Util; class SMB extends Common { /** @@ -65,6 +66,13 @@ class SMB extends Common { protected $statCache; public function __construct($params) { + $loggedParams = $params; + // remove password from log if it is set + if (!empty($loggedParams['password'])) { + $loggedParams['password'] = '***removed***'; + } + $this->log('enter: '.__FUNCTION__.'('.json_encode($loggedParams).')'); + if (isset($params['host']) && isset($params['user']) && isset($params['password']) && isset($params['share'])) { if (Server::NativeAvailable()) { $this->server = new NativeServer($params['host'], $params['user'], $params['password']); @@ -81,9 +89,12 @@ public function __construct($params) { $this->root .= '/'; } } else { - throw new \Exception('Invalid configuration'); + $ex = new \Exception('Invalid configuration'); + $this->leave(__FUNCTION__, $ex); + throw $ex; } $this->statCache = new CappedMemoryCache(); + $this->log('leave: '.__FUNCTION__.', getId:'.$this->getId()); } /** @@ -101,7 +112,9 @@ public function getId() { * @return string */ protected function buildPath($path) { - return Filesystem::normalizePath($this->root . '/' . $path); + $this->log('enter: '.__FUNCTION__."($path)"); + $result = Filesystem::normalizePath($this->root . '/' . $path, true, false, true); + return $this->leave(__FUNCTION__, $result); } /** @@ -110,15 +123,23 @@ protected function buildPath($path) { * @throws StorageNotAvailableException */ protected function getFileInfo($path) { + $this->log('enter: '.__FUNCTION__."($path)"); try { $path = $this->buildPath($path); if (!isset($this->statCache[$path])) { + $this->log("stat fetching '{$this->root}$path'"); $this->statCache[$path] = $this->share->stat($path); + } else { + $this->log("stat cache hit for '$path'"); } - return $this->statCache[$path]; + $result = $this->statCache[$path]; } catch (ConnectException $e) { - throw new StorageNotAvailableException($e->getMessage(), $e->getCode(), $e); + $ex = new StorageNotAvailableException( + $e->getMessage(), $e->getCode(), $e); + $this->leave(__FUNCTION__, $ex); + throw $ex; } + return $this->leave(__FUNCTION__, $result); } /** @@ -127,16 +148,20 @@ protected function getFileInfo($path) { * @throws StorageNotAvailableException */ protected function getFolderContents($path) { + $this->log('enter: '.__FUNCTION__."($path)"); try { $path = $this->buildPath($path); - $files = $this->share->dir($path); - foreach ($files as $file) { + $result = $this->share->dir($path); + foreach ($result as $file) { $this->statCache[$path . '/' . $file->getName()] = $file; } - return $files; } catch (ConnectException $e) { - throw new StorageNotAvailableException($e->getMessage(), $e->getCode(), $e); + $ex = new StorageNotAvailableException( + $e->getMessage(), $e->getCode(), $e); + $this->leave(__FUNCTION__, $ex); + throw $ex; } + return $this->leave(__FUNCTION__, $result); } /** @@ -155,30 +180,39 @@ protected function formatInfo($info) { * @return array */ public function stat($path) { - return $this->formatInfo($this->getFileInfo($path)); + $this->log('enter: '.__FUNCTION__."($path)"); + $result = $this->formatInfo($this->getFileInfo($path)); + return $this->leave(__FUNCTION__, $result); } /** * @param string $path * @return bool + * @throws StorageNotAvailableException */ public function unlink($path) { + $this->log('enter: '.__FUNCTION__."($path)"); + $result = false; try { if ($this->is_dir($path)) { - return $this->rmdir($path); + $result = $this->rmdir($path); } else { $path = $this->buildPath($path); unset($this->statCache[$path]); $this->share->del($path); - return true; + $result = true; } } catch (NotFoundException $e) { - return false; + $this->swallow(__FUNCTION__, $e); } catch (ForbiddenException $e) { - return false; + $this->swallow(__FUNCTION__, $e); } catch (ConnectException $e) { - throw new StorageNotAvailableException($e->getMessage(), $e->getCode(), $e); + $ex = new StorageNotAvailableException( + $e->getMessage(), $e->getCode(), $e); + $this->leave(__FUNCTION__, $ex); + throw $ex; } + return $this->leave(__FUNCTION__, $result); } /** @@ -189,37 +223,43 @@ public function unlink($path) { * @return bool */ public function hasUpdated($path, $time) { + $this->log('enter: '.__FUNCTION__."($path, $time)"); if (!$path and $this->root == '/') { // mtime doesn't work for shares, but giving the nature of the backend, // doing a full update is still just fast enough - return true; + $result = true; } else { $actualTime = $this->filemtime($path); - return $actualTime > $time; + $result = $actualTime > $time; } + return $this->leave(__FUNCTION__, $result); } /** * @param string $path * @param string $mode * @return resource + * @throws StorageNotAvailableException */ public function fopen($path, $mode) { + $this->log('enter: '.__FUNCTION__."($path, $mode)"); $fullPath = $this->buildPath($path); + $result = false; try { switch ($mode) { case 'r': case 'rb': - if (!$this->file_exists($path)) { - return false; + if ($this->file_exists($path)) { + $result = $this->share->read($fullPath); } - return $this->share->read($fullPath); + break; case 'w': case 'wb': $source = $this->share->write($fullPath); - return CallBackWrapper::wrap($source, null, null, function () use ($fullPath) { + $result = CallBackWrapper::wrap($source, null, null, function () use ($fullPath) { unset($this->statCache[$fullPath]); }); + break; case 'a': case 'ab': case 'r+': @@ -238,34 +278,39 @@ public function fopen($path, $mode) { } if ($this->file_exists($path)) { if (!$this->isUpdatable($path)) { - return false; + break; } $tmpFile = $this->getCachedFile($path); } else { if (!$this->isCreatable(dirname($path))) { - return false; + break; } $tmpFile = \OCP\Files::tmpFile($ext); } $source = fopen($tmpFile, $mode); $share = $this->share; - return CallbackWrapper::wrap($source, null, null, function () use ($tmpFile, $fullPath, $share) { + $result = CallbackWrapper::wrap($source, null, null, function () use ($tmpFile, $fullPath, $share) { unset($this->statCache[$fullPath]); $share->put($tmpFile, $fullPath); unlink($tmpFile); }); } - return false; } catch (NotFoundException $e) { - return false; + $this->swallow(__FUNCTION__, $e); } catch (ForbiddenException $e) { - return false; + $this->swallow(__FUNCTION__, $e); } catch (ConnectException $e) { - throw new StorageNotAvailableException($e->getMessage(), $e->getCode(), $e); + $ex = new StorageNotAvailableException( + $e->getMessage(), $e->getCode(), $e); + $this->leave(__FUNCTION__, $ex); + throw $ex; } + return $this->leave(__FUNCTION__, $result); } public function rmdir($path) { + $this->log('enter: '.__FUNCTION__."($path)"); + $result = false; try { $this->statCache = array(); $content = $this->share->dir($this->buildPath($path)); @@ -277,112 +322,149 @@ public function rmdir($path) { } } $this->share->rmdir($this->buildPath($path)); - return true; + $result = true; } catch (NotFoundException $e) { - return false; + $this->swallow(__FUNCTION__, $e); } catch (ForbiddenException $e) { - return false; + $this->swallow(__FUNCTION__, $e); } catch (ConnectException $e) { - throw new StorageNotAvailableException($e->getMessage(), $e->getCode(), $e); + $ex = new StorageNotAvailableException( + $e->getMessage(), $e->getCode(), $e); + $this->leave(__FUNCTION__, $ex); + throw $ex; } + return $this->leave(__FUNCTION__, $result); } public function touch($path, $time = null) { + $this->log('enter: '.__FUNCTION__."($path, $time)"); try { if (!$this->file_exists($path)) { $fh = $this->share->write($this->buildPath($path)); fclose($fh); - return true; + $result = true; + } else { + $result = false; } - return false; } catch (ConnectException $e) { - throw new StorageNotAvailableException($e->getMessage(), $e->getCode(), $e); + $ex = new StorageNotAvailableException( + $e->getMessage(), $e->getCode(), $e); + $this->leave(__FUNCTION__, $ex); + throw $ex; } + return $this->leave(__FUNCTION__, $result); } public function opendir($path) { + $this->log('enter: '.__FUNCTION__."($path)"); + $result = false; try { $files = $this->getFolderContents($path); + $names = array_map(function ($info) { + /** @var \Icewind\SMB\IFileInfo $info */ + return $info->getName(); + }, $files); + $result = IteratorDirectory::wrap($names); } catch (NotFoundException $e) { - return false; + $this->swallow(__FUNCTION__, $e); } catch (ForbiddenException $e) { - return false; + $this->swallow(__FUNCTION__, $e); } - $names = array_map(function ($info) { - /** @var \Icewind\SMB\IFileInfo $info */ - return $info->getName(); - }, $files); - return IteratorDirectory::wrap($names); + return $this->leave(__FUNCTION__, $result); } public function filetype($path) { + $this->log('enter: '.__FUNCTION__."($path)"); + $result = false; try { - return $this->getFileInfo($path)->isDirectory() ? 'dir' : 'file'; + $result = $this->getFileInfo($path)->isDirectory() ? 'dir' : 'file'; } catch (NotFoundException $e) { - return false; + $this->swallow(__FUNCTION__, $e); } catch (ForbiddenException $e) { - return false; + $this->swallow(__FUNCTION__, $e); } + return $this->leave(__FUNCTION__, $result); } public function mkdir($path) { + $this->log('enter: '.__FUNCTION__."($path)"); + $result = false; $path = $this->buildPath($path); try { $this->share->mkdir($path); - return true; + $result = true; } catch (ConnectException $e) { - throw new StorageNotAvailableException($e->getMessage(), $e->getCode(), $e); + $ex = new StorageNotAvailableException( + $e->getMessage(), $e->getCode(), $e); + $this->leave(__FUNCTION__, $ex); + throw $ex; } catch (Exception $e) { - return false; + $this->swallow(__FUNCTION__, $e); } + return $this->leave(__FUNCTION__, $result); } public function file_exists($path) { + $this->log('enter: '.__FUNCTION__."($path)"); + $result = false; try { $this->getFileInfo($path); - return true; + $result = true; } catch (NotFoundException $e) { - return false; + $this->swallow(__FUNCTION__, $e); } catch (ForbiddenException $e) { - return false; + $this->swallow(__FUNCTION__, $e); } catch (ConnectException $e) { - throw new StorageNotAvailableException($e->getMessage(), $e->getCode(), $e); + $ex = new StorageNotAvailableException( + $e->getMessage(), $e->getCode(), $e); + $this->leave(__FUNCTION__, $ex); + throw $ex; } + return $this->leave(__FUNCTION__, $result); } public function isReadable($path) { + $this->log('enter: '.__FUNCTION__."($path)"); + $result = false; try { $info = $this->getFileInfo($path); - return !$info->isHidden(); + $result = !$info->isHidden(); } catch (NotFoundException $e) { - return false; + $this->swallow(__FUNCTION__, $e); } catch (ForbiddenException $e) { - return false; + $this->swallow(__FUNCTION__, $e); } + return $this->leave(__FUNCTION__, $result); } public function isUpdatable($path) { + $this->log('enter: '.__FUNCTION__."($path)"); + $result = false; try { $info = $this->getFileInfo($path); // following windows behaviour for read-only folders: they can be written into // (https://support.microsoft.com/en-us/kb/326549 - "cause" section) - return !$info->isHidden() && (!$info->isReadOnly() || $this->is_dir($path)); + $result = !$info->isHidden() && (!$info->isReadOnly() || $this->is_dir($path)); } catch (NotFoundException $e) { - return false; + $this->swallow(__FUNCTION__, $e); } catch (ForbiddenException $e) { - return false; + $this->swallow(__FUNCTION__, $e); } + return $this->leave(__FUNCTION__, $result); } public function isDeletable($path) { + $this->log('enter: '.__FUNCTION__."($path)"); + $result = false; try { $info = $this->getFileInfo($path); - return !$info->isHidden() && !$info->isReadOnly(); + $result = !$info->isHidden() && !$info->isReadOnly(); } catch (NotFoundException $e) { - return false; + $this->swallow(__FUNCTION__, $e); } catch (ForbiddenException $e) { - return false; + $this->swallow(__FUNCTION__, $e); } + return $this->leave(__FUNCTION__, $result); } /** @@ -401,10 +483,65 @@ public static function checkDependencies() { * @return bool */ public function test() { + $this->log('enter: '.__FUNCTION__."()"); + $result = false; try { - return parent::test(); + $result = parent::test(); } catch (Exception $e) { - return false; + $this->swallow(__FUNCTION__, $e); + } + return $this->leave(__FUNCTION__, $result); + } + + + /** + * @param string $message + * @param int $level + * @param string $from + */ + private function log($message, $level = Util::DEBUG, $from = 'wnd') { + if (\OC::$server->getConfig()->getSystemValue('wnd.logging.enable', false) === true) { + Util::writeLog($from, $message, $level); + } + } + + /** + * if wnd.logging.enable is set to true in the config will log a leave line + * with the given function, the return value or the exception + * + * @param $function + * @param mixed $result an exception will be logged and then returned + * @return mixed + */ + private function leave($function, $result) { + if (\OC::$server->getConfig()->getSystemValue('wnd.logging.enable', false) === false) { + //don't bother building log strings + return $result; + } else if ($result === true) { + Util::writeLog('wnd', "leave: $function, return true", Util::DEBUG); + } else if ($result === false) { + Util::writeLog('wnd', "leave: $function, return false", Util::DEBUG); + } else if (is_string($result)) { + Util::writeLog('wnd', "leave: $function, return '$result'", Util::DEBUG); + } else if (is_resource($result)) { + Util::writeLog('wnd', "leave: $function, return resource", Util::DEBUG); + } else if ($result instanceof \Exception) { + Util::writeLog('wnd', "leave: $function, throw ".get_class($result) + .' - code: '.$result->getCode() + .' message: '.$result->getMessage() + .' trace: '.$result->getTraceAsString(), Util::DEBUG); + } else { + Util::writeLog('wnd', "leave: $function, return ".json_encode($result), Util::DEBUG); + } + return $result; + } + + private function swallow($function, \Exception $exception) { + if (\OC::$server->getConfig()->getSystemValue('wnd.logging.enable', false) === true) { + Util::writeLog('wnd', "$function swallowing ".get_class($exception) + .' - code: '.$exception->getCode() + .' message: '.$exception->getMessage() + .' trace: '.$exception->getTraceAsString(), Util::DEBUG); } } } From fde188ec84c4282fd4bbf0f2c4fad25bc72ab09d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 16 Aug 2016 14:55:08 +0200 Subject: [PATCH 03/17] stat for root of a share does not work --- apps/files_external/lib/smb.php | 49 ++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/apps/files_external/lib/smb.php b/apps/files_external/lib/smb.php index c133521089eeb..06e118b0bf22b 100644 --- a/apps/files_external/lib/smb.php +++ b/apps/files_external/lib/smb.php @@ -181,10 +181,57 @@ protected function formatInfo($info) { */ public function stat($path) { $this->log('enter: '.__FUNCTION__."($path)"); - $result = $this->formatInfo($this->getFileInfo($path)); + if ($this->remoteIsShare() && $this->isRootDir($path)) { //mtime doesn't work for shares + $result = [ + 'mtime' => $this->shareMTime(), + 'size' => 0, + 'type' => 'dir' + ]; + } else { + $result = $this->formatInfo($this->getFileInfo($path)); + } return $this->leave(__FUNCTION__, $result); } + /** + * get the best guess for the modification time of the share + * + * @return array the first element of the array is the calculated mtime for the folder, and + * the second is the list of readable files (the list contains the filename as key and the + * mtime for the file as value) + */ + private function shareMTime() { + $this->log('enter: '.__FUNCTION__, Util::DEBUG); + $files = $this->share->dir($this->root); + $result = 0; + foreach ($files as $fileInfo) { + if ($fileInfo->getMTime() > $result) { + $result = $fileInfo->getMTime(); + } + } + return $this->leave(__FUNCTION__, $result); + } + /** + * Check if the path is our root dir (not the smb one) + * + * @param string $path the path + * @return bool true if it's root, false if not + */ + private function isRootDir($path) { + $this->log('enter: '.__FUNCTION__."($path)", Util::DEBUG); + $result = $path === '' || $path === '/' || $path === '.'; + return $this->leave(__FUNCTION__, $result); + } + /** + * Check if our root points to a smb share + * + * @return bool true if our root points to a share false otherwise + */ + private function remoteIsShare() { + $this->log('enter: '.__FUNCTION__, Util::DEBUG); + $result = $this->share->getName() && (!$this->root || $this->root === '/'); + return $this->leave(__FUNCTION__, $result); + } /** * @param string $path * @return bool From a6882ee30860a5049b3c66659117c341b7dd9740 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 16 Aug 2016 15:07:41 +0200 Subject: [PATCH 04/17] don't list hidden files --- apps/files_external/lib/smb.php | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/apps/files_external/lib/smb.php b/apps/files_external/lib/smb.php index 06e118b0bf22b..a086601c17007 100644 --- a/apps/files_external/lib/smb.php +++ b/apps/files_external/lib/smb.php @@ -151,9 +151,18 @@ protected function getFolderContents($path) { $this->log('enter: '.__FUNCTION__."($path)"); try { $path = $this->buildPath($path); - $result = $this->share->dir($path); - foreach ($result as $file) { - $this->statCache[$path . '/' . $file->getName()] = $file; + $result = []; + $children = $this->share->dir($path); + foreach ($children as $fileInfo) { + // check if the file is readable before adding it to the list + // can't use "isReadable" function here, use smb internals instead + if ($fileInfo->isHidden()) { + $this->log("{$fileInfo->getName()} isn't readable, skipping", Util::DEBUG); + } else { + $result[] = $fileInfo; + //remember entry so we can answer file_exists and filetype without a full stat + $this->statCache[$path . '/' . $fileInfo->getName()] = $fileInfo; + } } } catch (ConnectException $e) { $ex = new StorageNotAvailableException( From 981065c21b7e7515ffba847bdb0cfa488716fa0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 16 Aug 2016 15:18:27 +0200 Subject: [PATCH 05/17] formatInfo should include type --- apps/files_external/lib/smb.php | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/apps/files_external/lib/smb.php b/apps/files_external/lib/smb.php index a086601c17007..65e09b6dc3dcc 100644 --- a/apps/files_external/lib/smb.php +++ b/apps/files_external/lib/smb.php @@ -178,10 +178,15 @@ protected function getFolderContents($path) { * @return array */ protected function formatInfo($info) { - return array( + $result = [ 'size' => $info->getSize(), - 'mtime' => $info->getMTime() - ); + 'mtime' => $info->getMTime(), + ]; + if ($info->isDirectory()) { + $result['type'] = 'dir'; + } else { + $result['type'] = 'file'; + } } /** From 0d47c1b326352051c145a7a17902a3061bfa2f8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 16 Aug 2016 15:22:45 +0200 Subject: [PATCH 06/17] overwrite target on rename --- apps/files_external/lib/smb.php | 34 +++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/apps/files_external/lib/smb.php b/apps/files_external/lib/smb.php index 65e09b6dc3dcc..8be9a803d5a91 100644 --- a/apps/files_external/lib/smb.php +++ b/apps/files_external/lib/smb.php @@ -31,6 +31,7 @@ namespace OC\Files\Storage; +use Icewind\SMB\Exception\AlreadyExistsException; use Icewind\SMB\Exception\ConnectException; use Icewind\SMB\Exception\Exception; use Icewind\SMB\Exception\ForbiddenException; @@ -189,6 +190,39 @@ protected function formatInfo($info) { } } + /** + * Rename the files + * + * @param string $source the old name of the path + * @param string $target the new name of the path + * @return bool true if the rename is successful, false otherwise + */ + public function rename($source, $target) { + $this->log("enter: rename('$source', '$target')", Util::DEBUG); + try { + $result = $this->share->rename($this->root . $source, $this->root . $target); + $this->removeFromCache($this->root . $source); + $this->removeFromCache($this->root . $target); + } catch (AlreadyExistsException $e) { + $this->unlink($target); + $result = $this->share->rename($this->root . $source, $this->root . $target); + $this->removeFromCache($this->root . $source); + $this->removeFromCache($this->root . $target); + $this->swallow(__FUNCTION__, $e); + } catch (\Exception $e) { + $this->swallow(__FUNCTION__, $e); + $result = false; + } + return $this->leave(__FUNCTION__, $result); + } + + private function removeFromCache($path) { + $path = trim($path, '/'); + // TODO The CappedCache does not really clear by prefix. It just clears all. + //$this->dirCache->clear($path); + $this->statCache->clear($path); + //$this->xattrCache->clear($path); + } /** * @param string $path * @return array From f0ddca0963b9ae4170a32d160668d519e32ccf14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 16 Aug 2016 15:37:29 +0200 Subject: [PATCH 07/17] use stat to determine has updated --- apps/files_external/lib/smb.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/files_external/lib/smb.php b/apps/files_external/lib/smb.php index 8be9a803d5a91..fc157ec194744 100644 --- a/apps/files_external/lib/smb.php +++ b/apps/files_external/lib/smb.php @@ -319,10 +319,10 @@ public function unlink($path) { */ public function hasUpdated($path, $time) { $this->log('enter: '.__FUNCTION__."($path, $time)"); - if (!$path and $this->root == '/') { - // mtime doesn't work for shares, but giving the nature of the backend, - // doing a full update is still just fast enough - $result = true; + $stat = $this->stat($path); + if ($stat === false) { + Util::writeLog('wnd', 'hasUpdated failed -> storage not available', Util::ERROR); + throw $this->leave(__FUNCTION__, new StorageNotAvailableException()); } else { $actualTime = $this->filemtime($path); $result = $actualTime > $time; From bd5ee22d8f549969e0673480c05043beb3cbe013 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 16 Aug 2016 15:43:55 +0200 Subject: [PATCH 08/17] stat now handles mtime for root shares, simplify hasUpdated --- apps/files_external/lib/smb.php | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/apps/files_external/lib/smb.php b/apps/files_external/lib/smb.php index fc157ec194744..9fabac1eed578 100644 --- a/apps/files_external/lib/smb.php +++ b/apps/files_external/lib/smb.php @@ -319,14 +319,8 @@ public function unlink($path) { */ public function hasUpdated($path, $time) { $this->log('enter: '.__FUNCTION__."($path, $time)"); - $stat = $this->stat($path); - if ($stat === false) { - Util::writeLog('wnd', 'hasUpdated failed -> storage not available', Util::ERROR); - throw $this->leave(__FUNCTION__, new StorageNotAvailableException()); - } else { - $actualTime = $this->filemtime($path); - $result = $actualTime > $time; - } + $actualTime = $this->filemtime($path); + $result = $actualTime > $time; return $this->leave(__FUNCTION__, $result); } From d86d5bfbb72f6be92237adaaa979bac6a8f9385e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 18 Aug 2016 15:06:23 +0200 Subject: [PATCH 09/17] move share check to getFileInfo --- apps/files_external/lib/smb.php | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/apps/files_external/lib/smb.php b/apps/files_external/lib/smb.php index 9fabac1eed578..7f34bd0cbf1ce 100644 --- a/apps/files_external/lib/smb.php +++ b/apps/files_external/lib/smb.php @@ -36,6 +36,7 @@ use Icewind\SMB\Exception\Exception; use Icewind\SMB\Exception\ForbiddenException; use Icewind\SMB\Exception\NotFoundException; +use Icewind\SMB\FileInfo; use Icewind\SMB\NativeServer; use Icewind\SMB\Server; use Icewind\Streams\CallbackWrapper; @@ -128,8 +129,12 @@ protected function getFileInfo($path) { try { $path = $this->buildPath($path); if (!isset($this->statCache[$path])) { - $this->log("stat fetching '{$this->root}$path'"); - $this->statCache[$path] = $this->share->stat($path); + if ($this->remoteIsShare() && $this->isRootDir($path)) { //mtime doesn't work for shares + $this->statCache[$path] = new FileInfo($path, '', 0, $this->shareMTime(), FileInfo::MODE_DIRECTORY); + } else { + $this->log("stat fetching '{$this->root}$path'"); + $this->statCache[$path] = $this->share->stat($path); + } } else { $this->log("stat cache hit for '$path'"); } @@ -229,24 +234,16 @@ private function removeFromCache($path) { */ public function stat($path) { $this->log('enter: '.__FUNCTION__."($path)"); - if ($this->remoteIsShare() && $this->isRootDir($path)) { //mtime doesn't work for shares - $result = [ - 'mtime' => $this->shareMTime(), - 'size' => 0, - 'type' => 'dir' - ]; - } else { - $result = $this->formatInfo($this->getFileInfo($path)); - } + $result = $this->formatInfo($this->getFileInfo($path)); return $this->leave(__FUNCTION__, $result); } /** * get the best guess for the modification time of the share + * NOTE: modification times do not bubble up the directory tree, basically + * we are just guessing a time * - * @return array the first element of the array is the calculated mtime for the folder, and - * the second is the list of readable files (the list contains the filename as key and the - * mtime for the file as value) + * @return int the calculated mtime for the folder */ private function shareMTime() { $this->log('enter: '.__FUNCTION__, Util::DEBUG); From e79d00f3265c3cf9dc994d5c24c394c7a5582bd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 18 Aug 2016 15:26:23 +0200 Subject: [PATCH 10/17] always try stat, fake if forbidden, actually return the fileinfo array --- apps/files_external/lib/smb.php | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/apps/files_external/lib/smb.php b/apps/files_external/lib/smb.php index 7f34bd0cbf1ce..d3ba2b0ab9f5d 100644 --- a/apps/files_external/lib/smb.php +++ b/apps/files_external/lib/smb.php @@ -123,28 +123,33 @@ protected function buildPath($path) { * @param string $path * @return \Icewind\SMB\IFileInfo * @throws StorageNotAvailableException + * @throws ForbiddenException */ protected function getFileInfo($path) { $this->log('enter: '.__FUNCTION__."($path)"); - try { $path = $this->buildPath($path); if (!isset($this->statCache[$path])) { - if ($this->remoteIsShare() && $this->isRootDir($path)) { //mtime doesn't work for shares - $this->statCache[$path] = new FileInfo($path, '', 0, $this->shareMTime(), FileInfo::MODE_DIRECTORY); - } else { + try { $this->log("stat fetching '{$this->root}$path'"); $this->statCache[$path] = $this->share->stat($path); + } catch (ConnectException $e) { + $ex = new StorageNotAvailableException( + $e->getMessage(), $e->getCode(), $e); + $this->leave(__FUNCTION__, $ex); + throw $ex; + } catch (ForbiddenException $e) { + if ($this->remoteIsShare() && $this->isRootDir($path)) { //mtime may not work for share root + $this->log("faking stat for forbidden '{$this->root}|$path'"); + $this->statCache[$path] = new FileInfo($path, '', 0, $this->shareMTime(), FileInfo::MODE_DIRECTORY); + } else { + $this->leave(__FUNCTION__, $e); + throw $e; + } } } else { $this->log("stat cache hit for '$path'"); } $result = $this->statCache[$path]; - } catch (ConnectException $e) { - $ex = new StorageNotAvailableException( - $e->getMessage(), $e->getCode(), $e); - $this->leave(__FUNCTION__, $ex); - throw $ex; - } return $this->leave(__FUNCTION__, $result); } @@ -193,6 +198,7 @@ protected function formatInfo($info) { } else { $result['type'] = 'file'; } + return $result; } /** @@ -617,7 +623,7 @@ private function leave($function, $result) { .' message: '.$result->getMessage() .' trace: '.$result->getTraceAsString(), Util::DEBUG); } else { - Util::writeLog('wnd', "leave: $function, return ".json_encode($result), Util::DEBUG); + Util::writeLog('wnd', "leave: $function, return ".print_r($result, true), Util::DEBUG); } return $result; } From 9c32080d131404df01fd671f966b4faaa97d0d1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 18 Aug 2016 17:28:12 +0200 Subject: [PATCH 11/17] share root is always readable --- apps/files_external/lib/smb.php | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/apps/files_external/lib/smb.php b/apps/files_external/lib/smb.php index d3ba2b0ab9f5d..2b53103f4f3f9 100644 --- a/apps/files_external/lib/smb.php +++ b/apps/files_external/lib/smb.php @@ -130,8 +130,18 @@ protected function getFileInfo($path) { $path = $this->buildPath($path); if (!isset($this->statCache[$path])) { try { - $this->log("stat fetching '{$this->root}$path'"); + $this->log("stat fetching '{$this->root}|$path'"); $this->statCache[$path] = $this->share->stat($path); + if ($this->remoteIsShare() && $this->isRootDir($path) && $this->statCache[$path]->isHidden()) { + $this->log(" stat for '{$this->root}|$path'"); + // make root never hidden, may happen when accessing a shared drive (mode is 22, archived and readonly - neither is true ... whatever) + if ($this->statCache[$path]->isReadOnly()) { + $mode = FileInfo::MODE_DIRECTORY & FileInfo::MODE_READONLY; + } else { + $mode = FileInfo::MODE_DIRECTORY; + } + $this->statCache[$path] = new FileInfo($path, '', 0, $this->statCache[$path]->getMTime(), $mode); + } } catch (ConnectException $e) { $ex = new StorageNotAvailableException( $e->getMessage(), $e->getCode(), $e); @@ -623,7 +633,7 @@ private function leave($function, $result) { .' message: '.$result->getMessage() .' trace: '.$result->getTraceAsString(), Util::DEBUG); } else { - Util::writeLog('wnd', "leave: $function, return ".print_r($result, true), Util::DEBUG); + Util::writeLog('wnd', "leave: $function, return ".json_encode($result, true), Util::DEBUG); } return $result; } From acecb36a585d955ef1b4bbd3a8015430b1044162 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 22 Aug 2016 14:55:50 +0200 Subject: [PATCH 12/17] do not overwrite stat cache with plain array --- apps/files_external/lib/smb.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_external/lib/smb.php b/apps/files_external/lib/smb.php index 2b53103f4f3f9..6a9662355da39 100644 --- a/apps/files_external/lib/smb.php +++ b/apps/files_external/lib/smb.php @@ -414,7 +414,7 @@ public function rmdir($path) { $this->log('enter: '.__FUNCTION__."($path)"); $result = false; try { - $this->statCache = array(); + $this->removeFromCache($path); $content = $this->share->dir($this->buildPath($path)); foreach ($content as $file) { if ($file->isDirectory()) { From a5a222db5422167637adba737c48c999664dd4a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 23 Aug 2016 12:19:30 +0200 Subject: [PATCH 13/17] fix test execution due to connection limit --- apps/files_external/lib/smb.php | 15 ++++++++++----- apps/files_external/tests/backends/smb.php | 4 +++- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/apps/files_external/lib/smb.php b/apps/files_external/lib/smb.php index 6a9662355da39..6b310aec93770 100644 --- a/apps/files_external/lib/smb.php +++ b/apps/files_external/lib/smb.php @@ -130,10 +130,10 @@ protected function getFileInfo($path) { $path = $this->buildPath($path); if (!isset($this->statCache[$path])) { try { - $this->log("stat fetching '{$this->root}|$path'"); + $this->log("stat fetching '$path'"); $this->statCache[$path] = $this->share->stat($path); if ($this->remoteIsShare() && $this->isRootDir($path) && $this->statCache[$path]->isHidden()) { - $this->log(" stat for '{$this->root}|$path'"); + $this->log(" stat for '$path'"); // make root never hidden, may happen when accessing a shared drive (mode is 22, archived and readonly - neither is true ... whatever) if ($this->statCache[$path]->isReadOnly()) { $mode = FileInfo::MODE_DIRECTORY & FileInfo::MODE_READONLY; @@ -493,8 +493,7 @@ public function mkdir($path) { $result = false; $path = $this->buildPath($path); try { - $this->share->mkdir($path); - $result = true; + $result = $this->share->mkdir($path); } catch (ConnectException $e) { $ex = new StorageNotAvailableException( $e->getMessage(), $e->getCode(), $e); @@ -595,7 +594,6 @@ public function test() { return $this->leave(__FUNCTION__, $result); } - /** * @param string $message * @param int $level @@ -646,4 +644,11 @@ private function swallow($function, \Exception $exception) { .' trace: '.$exception->getTraceAsString(), Util::DEBUG); } } + + /** + * immediately close / free connection + */ + public function __destruct() { + unset($this->share); + } } diff --git a/apps/files_external/tests/backends/smb.php b/apps/files_external/tests/backends/smb.php index 49afcea8a5600..06174b42188d6 100644 --- a/apps/files_external/tests/backends/smb.php +++ b/apps/files_external/tests/backends/smb.php @@ -48,12 +48,14 @@ protected function setUp() { } $config['root'] .= $id; //make sure we have an new empty folder to work in $this->instance = new \OC\Files\Storage\SMB($config); - $this->instance->mkdir('/'); + $this->assertTrue($this->instance->mkdir('/')); } protected function tearDown() { if ($this->instance) { $this->instance->rmdir(''); + // force disconnect of the client + unset($this->instance); } parent::tearDown(); From 271c500089908ff7b7eef71b383bcc15bad8b408 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 24 Aug 2016 10:03:40 +0200 Subject: [PATCH 14/17] add dir based stat fallback --- apps/files_external/lib/smb.php | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/apps/files_external/lib/smb.php b/apps/files_external/lib/smb.php index 6b310aec93770..15cada6d9c494 100644 --- a/apps/files_external/lib/smb.php +++ b/apps/files_external/lib/smb.php @@ -45,6 +45,7 @@ use OC\Files\Filesystem; use OCP\Files\StorageNotAvailableException; use OCP\Util; +use Sabre\DAV\Exception\NotFound; class SMB extends Common { /** @@ -131,7 +132,22 @@ protected function getFileInfo($path) { if (!isset($this->statCache[$path])) { try { $this->log("stat fetching '$path'"); - $this->statCache[$path] = $this->share->stat($path); + try { + $this->statCache[$path] = $this->share->stat($path); + } catch (NotFoundException $e) { + $this->log("stat for '$path' failed, trying to read parent dir"); + $infos = $this->share->dir(dirname($path)); + foreach ($infos as $fileInfo) { + if ($fileInfo->getName() === basename($path)) { + $this->statCache[$path] = $fileInfo; + break; + } + } + if (empty($this->statCache[$path])) { + $this->leave(__FUNCTION__, $e); + throw $e; + } + } if ($this->remoteIsShare() && $this->isRootDir($path) && $this->statCache[$path]->isHidden()) { $this->log(" stat for '$path'"); // make root never hidden, may happen when accessing a shared drive (mode is 22, archived and readonly - neither is true ... whatever) From d371aaa07f76b229f3a7d3ee215c9bef82557545 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 24 Aug 2016 12:45:18 +0200 Subject: [PATCH 15/17] fix return values for smbclient backend see https://github.com/icewind1991/SMB/commit/6e544f617cc271e34d41ea28dd7714a9b1ff75f1 --- apps/files_external/3rdparty/icewind/smb/src/Share.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_external/3rdparty/icewind/smb/src/Share.php b/apps/files_external/3rdparty/icewind/smb/src/Share.php index 694bd30bd0df6..fe253a94c2f06 100644 --- a/apps/files_external/3rdparty/icewind/smb/src/Share.php +++ b/apps/files_external/3rdparty/icewind/smb/src/Share.php @@ -370,7 +370,7 @@ protected function execute($command) { * @return bool */ protected function parseOutput($lines, $path = '') { - $this->parser->checkForError($lines, $path); + return $this->parser->checkForError($lines, $path); } /** From 0b7c8d37b380d306c96098ec831952ad207196b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 24 Aug 2016 13:07:44 +0200 Subject: [PATCH 16/17] allow unhiding root folder in subfolder of share --- apps/files_external/lib/smb.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/files_external/lib/smb.php b/apps/files_external/lib/smb.php index 15cada6d9c494..e4db4d9a0fae3 100644 --- a/apps/files_external/lib/smb.php +++ b/apps/files_external/lib/smb.php @@ -125,6 +125,7 @@ protected function buildPath($path) { * @return \Icewind\SMB\IFileInfo * @throws StorageNotAvailableException * @throws ForbiddenException + * @throws NotFoundException */ protected function getFileInfo($path) { $this->log('enter: '.__FUNCTION__."($path)"); @@ -148,8 +149,8 @@ protected function getFileInfo($path) { throw $e; } } - if ($this->remoteIsShare() && $this->isRootDir($path) && $this->statCache[$path]->isHidden()) { - $this->log(" stat for '$path'"); + if ($this->isRootDir($path) && $this->statCache[$path]->isHidden()) { + $this->log("unhiding stat for '$path'"); // make root never hidden, may happen when accessing a shared drive (mode is 22, archived and readonly - neither is true ... whatever) if ($this->statCache[$path]->isReadOnly()) { $mode = FileInfo::MODE_DIRECTORY & FileInfo::MODE_READONLY; @@ -165,7 +166,7 @@ protected function getFileInfo($path) { throw $ex; } catch (ForbiddenException $e) { if ($this->remoteIsShare() && $this->isRootDir($path)) { //mtime may not work for share root - $this->log("faking stat for forbidden '{$this->root}|$path'"); + $this->log("faking stat for forbidden '$path'"); $this->statCache[$path] = new FileInfo($path, '', 0, $this->shareMTime(), FileInfo::MODE_DIRECTORY); } else { $this->leave(__FUNCTION__, $e); From f1de1178feae20838a81d3439fe2207a688a3d44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 24 Aug 2016 14:12:40 +0200 Subject: [PATCH 17/17] trust libsmbclient on stat, only do dir fallback for smbclient --- apps/files_external/lib/smb.php | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/apps/files_external/lib/smb.php b/apps/files_external/lib/smb.php index e4db4d9a0fae3..f9e677c52a880 100644 --- a/apps/files_external/lib/smb.php +++ b/apps/files_external/lib/smb.php @@ -39,13 +39,13 @@ use Icewind\SMB\FileInfo; use Icewind\SMB\NativeServer; use Icewind\SMB\Server; +use Icewind\SMB\Share; use Icewind\Streams\CallbackWrapper; use Icewind\Streams\IteratorDirectory; use OC\Cache\CappedMemoryCache; use OC\Files\Filesystem; use OCP\Files\StorageNotAvailableException; use OCP\Util; -use Sabre\DAV\Exception\NotFound; class SMB extends Common { /** @@ -136,15 +136,22 @@ protected function getFileInfo($path) { try { $this->statCache[$path] = $this->share->stat($path); } catch (NotFoundException $e) { - $this->log("stat for '$path' failed, trying to read parent dir"); - $infos = $this->share->dir(dirname($path)); - foreach ($infos as $fileInfo) { - if ($fileInfo->getName() === basename($path)) { - $this->statCache[$path] = $fileInfo; - break; + if ($this->share instanceof Share) { + // smbclient may have problems with the allinfo cmd + $this->log("stat for '$path' failed, trying to read parent dir"); + $infos = $this->share->dir(dirname($path)); + foreach ($infos as $fileInfo) { + if ($fileInfo->getName() === basename($path)) { + $this->statCache[$path] = $fileInfo; + break; + } } - } - if (empty($this->statCache[$path])) { + if (empty($this->statCache[$path])) { + $this->leave(__FUNCTION__, $e); + throw $e; + } + } else { + // trust the results of libsmb $this->leave(__FUNCTION__, $e); throw $e; }