Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/dav/appinfo/v1/webdav.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,6 @@
$event = new \OCP\SabrePluginEvent($server);
\OC::$server->getEventDispatcher()->dispatch('OCA\DAV\Connector\Sabre::authInit', $event);

\Sabre\DAV\Server::$streamMultiStatus = true;
// And off we go!
$server->exec();
5 changes: 4 additions & 1 deletion apps/dav/lib/Capabilities.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,14 @@ public function __construct(IConfig $config) {
}

public function getCapabilities() {
$cap = [
$cap = [
'dav' => [
'chunking' => '1.0',
'reports' => [
'search-files',
],
'propfind' => [
'depth_infinity' => $this->config->getSystemValue('dav.propfind.depth_infinity', true),
]
]
];
Expand Down
33 changes: 32 additions & 1 deletion apps/dav/lib/Connector/Sabre/FilesPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@
namespace OCA\DAV\Connector\Sabre;

use OC\AppFramework\Http\Request;
use OC\Files\Filesystem;
use OCA\DAV\Files\IProvidesAdditionalHeaders;
use OCA\DAV\Meta\MetaFile;
use OCP\Files\ForbiddenException;
use OCP\Files\StorageNotAvailableException;
use OCP\IConfig;
Expand All @@ -47,6 +47,7 @@
use Sabre\DAV\Tree;
use Sabre\HTTP\RequestInterface;
use Sabre\HTTP\ResponseInterface;
use OCA\DAV\Connector\Sabre\Node;

class FilesPlugin extends ServerPlugin {

Expand Down Expand Up @@ -167,6 +168,36 @@ public function initialize(\Sabre\DAV\Server $server) {
});
$this->server->on('beforeMove', [$this, 'checkMove']);
$this->server->on('validateTokens', [$this, 'validateTokens'], 0);
$this->server->on('method:PROPFIND', [$this, 'checkPropFind']);
}

/**
* Check if the file tree can be found and if it's traversable.
* Also check for read permissions if the requested resource is
* a share, since a share can be create-only.
*
* @param RequestInterface $request
*/
public function checkPropFind($request) {
$path = $request->getPath();
$node = $this->server->tree->getNodeForPath($path);

if ($node instanceof Node) {
/** @var Node $node */
'@phan-var Node $node';
$fileInfo = $node->getFileInfo();
list($storage, ) = Filesystem::resolvePath($fileInfo->getPath());

if ($storage->instanceOfStorage('\OCA\Files_Sharing\SharedStorage')) {
/** @var \OCA\Files_Sharing\SharedStorage $storage */
'@phan-var \OCA\Files_Sharing\SharedStorage $storage';
$hasReadPermission = ($storage->getShare()->getPermissions() & \OCP\Constants::PERMISSION_READ) > 0;
Comment thread
AlexAndBear marked this conversation as resolved.

if (!$hasReadPermission) {
throw new NotFound();
}
}
}
}

/**
Expand Down
12 changes: 12 additions & 0 deletions apps/dav/lib/Connector/Sabre/ServerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
use OCP\IUserSession;
use Sabre\DAV\Auth\Backend\BackendInterface;
use OCP\AppFramework\Utility\ITimeFactory;
use Sabre\DAV\Exception\PreconditionFailed;
use Sabre\HTTP\Request;

class ServerFactory {
/** @var IConfig */
Expand Down Expand Up @@ -133,6 +135,16 @@ public function createServer(
$server->addPlugin(new BrowserErrorPagePlugin());
}

$config = $this->config;

$server->on('beforeMethod:PROPFIND', function (Request $request) use ($config) {
$depthHeader = strtolower($request->getHeader('depth'));

if ($depthHeader === 'infinity' && !$config->getSystemValue('dav.propfind.depth_infinity', true)) {
throw new PreconditionFailed('Depth infinity not supported');
}
}, 0);

// wait with registering these until auth is handled and the filesystem is setup
$server->on('beforeMethod:*', function () use ($server, $objectTree, $viewCallBack) {
// ensure the skeleton is copied
Expand Down
11 changes: 11 additions & 0 deletions apps/dav/lib/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
use Sabre\CardDAV\VCFExportPlugin;
use Sabre\DAV\Auth\Plugin;
use Sabre\DAV\Exception;
use Sabre\HTTP\Request;

class Server {

Expand Down Expand Up @@ -226,6 +227,7 @@ public function __construct(IRequest $request, $baseUri) {
));

if ($this->isRequestForSubtree(['files', 'trash-bin', 'public-files'])) {
\Sabre\DAV\Server::$streamMultiStatus = true;
$this->server->addPlugin(new ViewOnlyPlugin(
OC::$server->getLogger()
));
Expand All @@ -236,6 +238,15 @@ public function __construct(IRequest $request, $baseUri) {
}

$this->server->addPlugin(new PreviewPlugin(OC::$server->getTimeFactory(), OC::$server->getPreviewManager()));

$this->server->on('beforeMethod:PROPFIND', function (Request $request) use ($config) {
$depthHeader = strtolower($request->getHeader('depth'));

if ($depthHeader === 'infinity' && !$config->getSystemValue('dav.propfind.depth_infinity', true)) {
throw new Exception\PreconditionFailed('Depth infinity not supported');
}
}, 0);

// wait with registering these until auth is handled and the filesystem is setup
$this->server->on('beforeMethod:*', function () use ($root) {
// custom properties plugin must be the last one
Expand Down
59 changes: 59 additions & 0 deletions apps/dav/tests/unit/CapabilitiesTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?php

namespace OCA\DAV\Tests\Unit;

use OCA\DAV\Capabilities;
use OCP\IConfig;
use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase;

/**
* @author Jan Ackermann <jackermann@owncloud.com>
* @author Jannik Stehle <jstehle@owncloud.com>
*
* @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/>
*
*/

class CapabilitiesTest extends TestCase {

/** @var IConfig| MockObject */
protected $config;

/** @var Capabilities */
protected $capabilities;

protected function setUp(): void {
parent::setUp();

$this->config = $this->createMock(IConfig::class);
$this->capabilities = new Capabilities($this->config);
}

public function testGetCapabilities() {
$this->config->expects($this->exactly(2))
->method('getSystemValue')
->withConsecutive(['dav.propfind.depth_infinity'], ['dav.enable.async'])
->willReturn(true, true);

$result = $this->capabilities->getCapabilities();
$this->assertEquals($result['dav']['chunking'], '1.0');
$this->assertEquals($result['dav']['reports'][0], 'search-files');
$this->assertEquals($result['dav']['trashbin'], '1.0');
$this->assertEquals($result['dav']['propfind']['depth_infinity'], true);
$this->assertEquals($result['async'], '1.0');
}
}
62 changes: 62 additions & 0 deletions apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,22 @@
*/
namespace OCA\DAV\Tests\unit\Connector\Sabre;

use OC\Files\Filesystem;
use OC\Files\Mount\MountPoint;
use OC\User\User;
use OCA\DAV\Connector\Sabre\Directory;
use OCA\DAV\Connector\Sabre\File;
use OCA\DAV\Connector\Sabre\FilesPlugin;
use OCP\Files\FileInfo;
use OCA\DAV\Connector\Sabre\Node;
use OCA\DAV\Meta\MetaFile;
use OCA\Files_Sharing\SharedStorage;
use OCP\Files\StorageNotAvailableException;
use OCP\IConfig;
use OCP\IRequest;
use OCP\Share\IShare;
use Sabre\DAV\Exception\NotFound;
use Sabre\DAV\ICollection;
use Sabre\DAV\PropFind;
use Sabre\DAV\PropPatch;
use Sabre\DAV\Server;
Expand All @@ -49,6 +55,8 @@
* This file is licensed under the Affero General Public License version 3 or
* later.
* See the COPYING-README file.
*
* @group DB
*/
class FilesPluginTest extends TestCase {
public const GETETAG_PROPERTYNAME = FilesPlugin::GETETAG_PROPERTYNAME;
Expand Down Expand Up @@ -627,4 +635,58 @@ public function testAdditionalHeaders() {

$this->plugin->httpGet($request, $response);
}

public function testCheckPropfindInvalidPath() {
$this->expectException(NotFound::class);

$this->plugin = new FilesPlugin(
$this->tree,
$this->config,
$this->request
);

$this->server->tree = new Tree($this->getMockBuilder(ICollection::class)->getMock());
$this->plugin->initialize($this->server);

$sabreRequest = $this->getMockBuilder(RequestInterface::class)->disableOriginalConstructor()->getMock();
$sabreRequest->method('getPath')->willReturn('/invalid/path');

$this->plugin->checkPropFind($sabreRequest);
}

public function testCheckPropfindInsufficientPermissions() {
$this->expectException(NotFound::class);

$this->plugin = new FilesPlugin(
$this->tree,
$this->config,
$this->request
);

$node = $this->createMock(Node::class);

$shareMock = $this->createMock(IShare::class);
$shareMock->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_CREATE);

$storage = $this->createMock(SharedStorage::class);
$storage->method('instanceOfStorage')->willReturn(true);
$storage->method('getShare')->willReturn($shareMock);
$mount = new MountPoint($storage, '/user2/files/sharedfolder', [[]], Filesystem::getLoader());
Filesystem::getMountManager()->addMount($mount);

$fileInfo = $this->createMock(FileInfo::class);
$fileInfo->method('getPath')->willReturn('user2/files/sharedfolder');

$node->method('getFileInfo')->willReturn($fileInfo);

$this->tree->method('getNodeForPath')->willReturn($node);
$this->server->tree = $this->tree;
$this->plugin->initialize($this->server);

$sabreRequest = $this->getMockBuilder(RequestInterface::class)->disableOriginalConstructor()->getMock();
$sabreRequest->method('getPath')->willReturn('user2/files/sharedfolder');

$this->plugin->checkPropFind($sabreRequest);
Filesystem::getMountManager()->removeMount('/user2/files/sharedfolder');
}
}
9 changes: 9 additions & 0 deletions changelog/unreleased/38583
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Change: Enable streaming for propfind requests

Propfind requests will now be streamed to reduce memory usage
with large responses.
Additionally, the new config `dav.propfind.depth_infinity` has been
added to tell clients whether `depth=infinity` is allowed for
propfind requests. It defaults to true.

https://github.com/owncloud/core/pull/38583
9 changes: 9 additions & 0 deletions config/config.sample.php
Original file line number Diff line number Diff line change
Expand Up @@ -1657,6 +1657,15 @@
*/
'dav.enable.async' => false,

/**
* Allow propfind depth infinity
*
* With this setting that defaults to true, propfind requests will now be streamed to reduce memory usage
* with large responses. It tells the clients whether `depth=infinity` is allowed for propfind requests.
* For details see: https://datatracker.ietf.org/doc/html/rfc4918#section-10.2
*/
'dav.propfind.depth_infinity' => true,

/**
* Show the grace period popup
* Decide whether show or not the grace period popup. There is no change in the
Expand Down
18 changes: 13 additions & 5 deletions tests/acceptance/features/apiWebdavOperations/propfind.feature
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,17 @@ Feature: PROPFIND
When user "Alice" requests "/remote.php/dav/files" with "PROPFIND" using basic auth
Then the HTTP status code should be "405"

Scenario: PROPFIND to "/remote.php/dav/files" with depth header
@skipOnOcV10.6 @skipOnOcV10.7 @skipOnOcV10.8.0
Scenario Outline: PROPFIND to "/remote.php/dav/files" with depth header
Given user "Alice" has been created with default attributes and without skeleton files
When user "Alice" requests "/remote.php/dav/files" with "PROPFIND" using basic auth and with headers
| header | value |
| depth | 0 |
Then the HTTP status code should be "207"
And the administrator has set depth_infinity_allowed to <depth_infinity_allowed>
When user "Alice" requests "/remote.php/dav/files/alice" with "PROPFIND" using basic auth and with headers
| header | value |
| depth | <depth> |
Then the HTTP status code should be "<http_status>"
Examples:
| depth_infinity_allowed | depth | http_status |
| 1 | 0 | 207 |
| 1 | infinity | 207 |
| 0 | 0 | 207 |
| 0 | infinity | 412 |
14 changes: 14 additions & 0 deletions tests/acceptance/features/bootstrap/OccContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,20 @@ public function theAdministratorHasSetTheDefaultFolderForReceivedSharesTo(string
);
}

/**
* @When the administrator has set depth_infinity_allowed to :depth_infinity_allowed
*
* @param int $depth_infinity_allowed
*
* @return void
*/
public function theAdministratorHasSetDepthInfinityAllowedTo($depth_infinity_allowed) {
$this->addSystemConfigKeyUsingTheOccCommand(
"dav.propfind.depth_infinity",
$depth_infinity_allowed
);
}

/**
* @Given the administrator has set the mail smtpmode to :smtpmode
*
Expand Down