Skip to content
Open
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
14 changes: 3 additions & 11 deletions lib/private/Files/Storage/Wrapper/DirPermissionsMask.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,7 @@ public function getPermissions($path): int {

#[\Override]
public function rename($source, $target): bool {
$isPartialUploadFile = dirname($source) === dirname($target)
&& strpos($source, '.ocTransferId') > 0;

if (
!($isPartialUploadFile
|| $this->isUpdatable($source)
|| $this->isDeletable($source)
)
) {
if (!($this->isPartialFileMove($source, $target) || $this->isUpdatable($source) || $this->isDeletable($source))) {
return false;
}

Expand Down Expand Up @@ -151,10 +143,10 @@ public function copy($source, $target): bool {
#[\Override]
public function touch($path, $mtime = null): bool {
if ($this->checkPath($path)) {
return parent::touch($path);
return parent::touch($path, $mtime);
}

return $this->storage->touch($path);
return $this->storage->touch($path, $mtime);
}

#[\Override]
Expand Down
20 changes: 13 additions & 7 deletions lib/private/Files/Storage/Wrapper/PermissionsMask.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
namespace OC\Files\Storage\Wrapper;

use OC\Files\Cache\Wrapper\CachePermissionsMask;
use OC\Files\Storage\Storage;
use OCP\Constants;
use OCP\Files\Cache\ICache;
use OCP\Files\Cache\IScanner;
Expand Down Expand Up @@ -70,13 +69,13 @@ public function getPermissions(string $path): int {

#[\Override]
public function rename(string $source, string $target): bool {
//This is a rename of the transfer file to the original file
$sourceDir = dirname($source);
$targetDir = dirname($target);
if (($sourceDir === $targetDir || $sourceDir === dirname($targetDir)) && strpos($source, '.ocTransferId') > 0) {
return $this->checkMask(Constants::PERMISSION_CREATE) && parent::rename($source, $target);
$required = Constants::PERMISSION_UPDATE;
if ($this->isPartialFileMove($source, $target) && !$this->file_exists($target)) {
// This is a move/rename of a partial file to the final file
$required = Constants::PERMISSION_CREATE;
}
return $this->checkMask(Constants::PERMISSION_UPDATE) && parent::rename($source, $target);

return $this->checkMask($required) && parent::rename($source, $target);
}

#[\Override]
Expand Down Expand Up @@ -158,4 +157,11 @@ public function getDirectoryContent(string $directory): \Traversable {
yield $data;
}
}

/**
* If source and target are compatible with a rename/move operation of a partial file into its final location
*/
protected function isPartialFileMove(string $source, string $target): bool {
return str_starts_with(dirname($target), dirname($source)) && preg_match('/\.ocTransferId\d+\.part$/i', $source) === 1;
}
}
142 changes: 142 additions & 0 deletions tests/lib/Files/Storage/Wrapper/DirPermissionsMaskTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace Test\Files\Storage\Wrapper;

use OC\Files\Storage\Temporary;
use OC\Files\Storage\Wrapper\DirPermissionsMask;
use OCP\Constants;
use PHPUnit\Framework\Attributes\DataProvider;

#[\PHPUnit\Framework\Attributes\Group('DB')]
class DirPermissionsMaskTest extends \Test\Files\Storage\Storage {
// Same mask path the public share wrapper uses for home storages (publicremote.php)
private const MASKED_PATH = 'files';

private Temporary $sourceStorage;

#[\Override]
protected function setUp(): void {
parent::setUp();
$this->sourceStorage = new Temporary([]);
$this->instance = $this->getMaskedStorage(Constants::PERMISSION_ALL);
}

#[\Override]
protected function tearDown(): void {
$this->sourceStorage->cleanUp();
parent::tearDown();
}

protected function getMaskedStorage(int $mask): DirPermissionsMask {
return new DirPermissionsMask([
'storage' => $this->sourceStorage,
'mask' => $mask,
'path' => self::MASKED_PATH,
]);
}

public static function dataRenamePartFile(): array {
// Upload-only public link: no UPDATE, no DELETE
$uploadOnly = Constants::PERMISSION_READ | Constants::PERMISSION_CREATE;
$partialFileName = 'x.ocTransferId123.part';
return [
'partial file in same directory' => [
'mask' => $uploadOnly,
'sourcePath' => 'files/drop/' . $partialFileName,
'targetPath' => 'files/drop/x',
'shouldSucceed' => true,
],
'partial file in share root, target nested (part_file_in_storage=false)' => [
'mask' => $uploadOnly,
'sourcePath' => 'files/drop/' . $partialFileName,
'targetPath' => 'files/drop/a/b/x',
'shouldSucceed' => true,
],
'partial file onto existing target without update permission' => [
'mask' => $uploadOnly,
'sourcePath' => 'files/drop/' . $partialFileName,
'targetPath' => 'files/drop/a/x',
'shouldSucceed' => false,
'targetExisting' => true,
],
'partial file onto existing target with update permission' => [
'mask' => $uploadOnly | Constants::PERMISSION_UPDATE,
'sourcePath' => 'files/drop/' . $partialFileName,
'targetPath' => 'files/drop/a/x',
'shouldSucceed' => true,
'targetExisting' => true,
],
'partial file without create permission' => [
'mask' => Constants::PERMISSION_READ,
'sourcePath' => 'files/drop/' . $partialFileName,
'targetPath' => 'files/drop/x',
'shouldSucceed' => false,
],
'transfer id without .part extension' => [
'mask' => $uploadOnly,
'sourcePath' => 'files/drop/foo.ocTransferId123',
'targetPath' => 'files/drop/a/x',
'shouldSucceed' => false,
],
'regular file without update or delete permission' => [
'mask' => $uploadOnly,
'sourcePath' => 'files/drop/foo',
'targetPath' => 'files/drop/x',
'shouldSucceed' => false,
],
'outside masked path the mask does not apply' => [
'mask' => Constants::PERMISSION_READ,
'sourcePath' => 'cache/foo',
'targetPath' => 'cache/bar',
'shouldSucceed' => true,
],
];
}

#[DataProvider(methodName: 'dataRenamePartFile')]
public function testRenamePartFile(int $mask, string $sourcePath, string $targetPath, bool $shouldSucceed, bool $targetExisting = false): void {
// Set up the tree on the unmasked storage so the mask doesn't interfere with the fixtures
foreach ([dirname($sourcePath), dirname($targetPath)] as $dir) {
$path = '';
foreach (explode('/', $dir) as $part) {
$path .= $part . '/';
if (!$this->sourceStorage->is_dir($path)) {
$this->assertTrue($this->sourceStorage->mkdir($path));
}
}
}
$this->assertNotFalse($this->sourceStorage->file_put_contents($sourcePath, 'new'));
if ($targetExisting) {
$this->assertNotFalse($this->sourceStorage->file_put_contents($targetPath, 'existing'));
}

$storage = $this->getMaskedStorage($mask);

$this->assertSame($shouldSucceed, $storage->rename($sourcePath, $targetPath), 'Renaming should ' . ($shouldSucceed ? '' : 'not ') . 'succeed');
if ($shouldSucceed) {
$this->assertFalse($this->sourceStorage->file_exists($sourcePath));
$this->assertSame('new', $this->sourceStorage->file_get_contents($targetPath));
} else {
$this->assertTrue($this->sourceStorage->file_exists($sourcePath));
if ($targetExisting) {
$this->assertSame('existing', $this->sourceStorage->file_get_contents($targetPath));
if ($shouldSucceed) {
$this->assertFalse($this->sourceStorage->file_exists($sourcePath));
$this->assertSame('new', $this->sourceStorage->file_get_contents($targetPath));
} else {
$this->assertTrue($this->sourceStorage->file_exists($sourcePath));
if ($targetExisting) {
$this->assertSame('existing', $this->sourceStorage->file_get_contents($targetPath));
}
}
}
}
}
}
55 changes: 55 additions & 0 deletions tests/lib/Files/Storage/Wrapper/PermissionsMaskTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use OC\Files\Storage\Wrapper\Wrapper;
use OCP\Constants;
use OCP\Files\Cache\IScanner;
use PHPUnit\Framework\Attributes\DataProvider;

#[\PHPUnit\Framework\Attributes\Group('DB')]
class PermissionsMaskTest extends \Test\Files\Storage\Storage {
Expand Down Expand Up @@ -47,6 +48,60 @@ public function testMkdirNoCreate(): void {
$this->assertFalse($storage->file_exists('foo'));
}

public static function dataRenamePartFile(): array {
$partialFileName = 'x.ocTransferId123.part';
return [
'renaming partial file in same directory' => [
'sourcePath' => $partialFileName,
'targetPath' => 'x',
'shouldSucceed' => true,
],
'renaming partial file for file uploaded in parent (part_file_in_storage=false) ' => [
'sourcePath' => '/A/' . $partialFileName,
'targetPath' => '/A/Share/x',
'shouldSucceed' => true,
],
'renaming partial file for file uploaded in ancestor (part_file_in_storage=false) ' => [
'sourcePath' => '/A/' . $partialFileName,
'targetPath' => '/A/B/C/Share/x',
'shouldSucceed' => true,
],
'renaming non-partial file in parent' => [
'sourcePath' => '/A/foo.ocTransferId123',
'targetPath' => '/A/Share/x',
'shouldSucceed' => false,
],
'renaming partial file into existing file' => [
'sourcePath' => '/A/' . $partialFileName,
'targetPath' => '/A/Share/x',
'shouldSucceed' => false,
'targetExisting' => true,
]
];
}
#[DataProvider(methodName: 'dataRenamePartFile')]
public function testRenamePartFile(string $sourcePath, string $targetPath, bool $shouldSucceed, bool $targetExisting = false): void {
$storage = $this->getMaskedStorage(Constants::PERMISSION_ALL - Constants::PERMISSION_UPDATE);
$directories = array_unique([ ...explode('/', dirname($sourcePath)), ...explode('/', dirname($targetPath)) ]);
$path = '';
foreach ($directories as $part) {
if (in_array($part, ['', '.'], true)) {
continue;
}
$path .= $part . '/';
$this->assertTrue($storage->mkdir($path));
}

$this->assertNotFalse($this->sourceStorage->file_put_contents($sourcePath, 'bar'));
$this->assertTrue($this->sourceStorage->file_exists($sourcePath));
if ($targetExisting) {
$this->assertNotFalse($this->sourceStorage->file_put_contents($targetPath, 'existing'));
$this->assertTrue($this->sourceStorage->file_exists($targetPath));
}

$this->assertEquals($shouldSucceed, $storage->rename($sourcePath, $targetPath), 'Renaming should ' . ($shouldSucceed ? '' : 'not ') . 'succeed');
}

public function testRmdirNoDelete(): void {
$storage = $this->getMaskedStorage(Constants::PERMISSION_ALL - Constants::PERMISSION_DELETE);
$this->assertTrue($storage->mkdir('foo'));
Expand Down
Loading