Skip to content

Commit

Permalink
Show proper message if part file name too long
Browse files Browse the repository at this point in the history
  • Loading branch information
JammingBen committed Sep 6, 2021
1 parent ddb767c commit 2ebdb1a
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 1 deletion.
43 changes: 43 additions & 0 deletions apps/dav/lib/Connector/Sabre/Exception/FileNameTooLong.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php
/**
* @author Jannik Stehle <[email protected]>
* @author Jan Ackermann <[email protected]>
*
* @copyright Copyright (c) 2021, ownCloud GmbH
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/

namespace OCA\DAV\Connector\Sabre\Exception;

use Exception;

class FileNameTooLong extends \Sabre\DAV\Exception {
public function __construct($message = '', $code = 0, Exception $previous = null) {
if ($message === '') {
$message = 'File name is too long.';
}
parent::__construct($message, $code, $previous);
}

/**
* Returns the HTTP status code for this exception
*
* @return int
*/
public function getHTTPCode() {
return 400;
}
}
10 changes: 10 additions & 0 deletions apps/dav/lib/Connector/Sabre/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,15 @@
use OC\Files\Storage\Storage;
use OCA\DAV\Connector\Sabre\Exception\EntityTooLarge;
use OCA\DAV\Connector\Sabre\Exception\FileLocked;
use OCA\DAV\Connector\Sabre\Exception\FileNameTooLong;
use OCA\DAV\Connector\Sabre\Exception\Forbidden as DAVForbiddenException;
use OCA\DAV\Connector\Sabre\Exception\UnsupportedMediaType;
use OCA\DAV\Files\IFileNode;
use OCP\Encryption\Exceptions\GenericEncryptionException;
use OCP\Events\EventEmitterTrait;
use OCP\Files\EntityTooLargeException;
use OCP\Files\FileContentNotAllowedException;
use OCP\Files\FileNameTooLongException;
use OCP\Files\ForbiddenException;
use OCP\Files\InvalidContentException;
use OCP\Files\InvalidPathException;
Expand Down Expand Up @@ -191,6 +193,11 @@ public function put($data) {
throw new FileLocked($e->getMessage(), $e->getCode(), $e);
}

// check if the part file name is too long
if ($usePartFile) {
$partStorage->verifyPath($internalPartPath, \basename($internalPartPath));
}

$target = $partStorage->fopen($internalPartPath, 'wb');
if (!\is_resource($target)) {
\OCP\Util::writeLog('webdav', '\OC\Files\Filesystem::fopen() failed', \OCP\Util::ERROR);
Expand Down Expand Up @@ -672,6 +679,9 @@ private function convertToSabreException(\Exception $e) {
// the file content is not permitted
throw new UnsupportedMediaType($e->getMessage(), 0, $e);
}
if ($e instanceof FileNameTooLongException) {
throw new FileNameTooLong($e->getMessage(), 0, $e);
}
if ($e instanceof InvalidPathException) {
// the path for the file was not valid
// TODO: find proper http status code for this case
Expand Down
59 changes: 58 additions & 1 deletion apps/dav/tests/unit/Connector/Sabre/FileTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use OC\Files\Storage\Local;
use OC\Files\View;
use OCA\DAV\Connector\Sabre\Exception\FileLocked;
use OCA\DAV\Connector\Sabre\Exception\FileNameTooLong;
use OCA\DAV\Connector\Sabre\Exception\Forbidden;
use OCA\DAV\Connector\Sabre\File;
use OCP\Constants;
Expand Down Expand Up @@ -463,10 +464,20 @@ private function doPut($path, $viewRoot = null, \OC\AppFramework\Http\Request $r
return $result;
}

public function partFileInStorage() {
return [
[true], [false]
];
}

/**
* Test putting a single file
*
* @dataProvider partFileInStorage
*/
public function testPutSingleFile() {
public function testPutSingleFile($partFileInStorage) {
\OC::$server->getConfig()->setSystemValue('part_file_in_storage', $partFileInStorage);

$calledAfterEvent = [];
\OC::$server->getEventDispatcher()->addListener('file.aftercreate', function ($event) use (&$calledAfterEvent) {
$calledAfterEvent[] = 'file.aftercreate';
Expand All @@ -476,6 +487,7 @@ public function testPutSingleFile() {
$this->assertInstanceOf(GenericEvent::class, $calledAfterEvent[1]);
$this->assertArrayHasKey('path', $calledAfterEvent[1]);
$this->assertEquals('file.aftercreate', $calledAfterEvent[0]);
\OC::$server->getConfig()->setSystemValue('part_file_in_storage', true);
}

/**
Expand Down Expand Up @@ -1554,5 +1566,50 @@ public function testGetThrowsIfNoPermissionsAndFileNotExists() {
$file = new File($view, $info);

$file->get();
$file->get();
}

/**
* Test putting a file with which the generated part file name
* will exceed the file name character limit.
*/
public function testPutTooLongPartFileName() {
$path = 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren.txt';
$view = Filesystem::getView();
$viewRoot = '/' . $this->user . '/files';

$info = new FileInfo(
$viewRoot . '/' . \ltrim($path, '/'),
$this->getMockStorage(),
null,
['permissions' => Constants::PERMISSION_ALL],
null
);

/** @var File | \PHPUnit\Framework\MockObject\MockObject $file */
$file = $this->getMockBuilder(File::class)
->setConstructorArgs([$view, $info, null])
->setMethods(['header'])
->getMock();

list($storage) = $view->resolvePath("/$path");
$usePartFile = $storage->usePartFile();

if ($usePartFile) {
$this->expectException(FileNameTooLong::class);
}

$view->lockFile($path, ILockingProvider::LOCK_SHARED);
$file->put($this->getStream('test data'));
$view->unlockFile($path, ILockingProvider::LOCK_SHARED);

$this->assertFalse(
$this->isFileLocked($view, $path, ILockingProvider::LOCK_SHARED),
'File unlocked after put'
);
$this->assertFalse(
$this->isFileLocked($view, $path, ILockingProvider::LOCK_EXCLUSIVE),
'File unlocked after put'
);
}
}
4 changes: 4 additions & 0 deletions changelog/unreleased/39168
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Bugfix: Return proper error if part file name is too long

https://github.com/owncloud/core/pull/39168
https://github.com/owncloud/enterprise/issues/4692
3 changes: 3 additions & 0 deletions config/config.sample.php
Original file line number Diff line number Diff line change
Expand Up @@ -1498,6 +1498,9 @@
* same storage as the upload target. Setting this to false will store the part
* files in the root of the users folder which might be required to work with certain
* external storage setups that have limited rename capabilities.
*
* Note that setting this to false causes issues with the following apps: Encryption,
* Document classification, Anti-Virus and Ransomware Protection.
*/
'part_file_in_storage' => true,

Expand Down

0 comments on commit 2ebdb1a

Please sign in to comment.