Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tests-only] [full-ci] Test quota improvements #39903

Closed
wants to merge 4 commits into from
Closed
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
37 changes: 33 additions & 4 deletions apps/dav/lib/Connector/Sabre/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -220,10 +220,7 @@ public function put($data) {
}

if ($result === false) {
$expected = -1;
if (isset($_SERVER['CONTENT_LENGTH'])) {
$expected = $_SERVER['CONTENT_LENGTH'];
}
$expected = $this->getExpectedContentLength($data);
throw new Exception('Error while copying file to target location (copied bytes: ' . $count . ', expected filesize: ' . $expected . ' )');
}

Expand Down Expand Up @@ -276,9 +273,21 @@ public function put($data) {
throw new Exception('Could not rename part file to final file');
}
} catch (ForbiddenException $ex) {
// try to revert the lock state so it can be cleanup properly
try {
$this->changeLock(ILockingProvider::LOCK_SHARED);
} catch (LockedException $e) {
$this->convertToSabreException($e);
}
throw new DAVForbiddenException($ex->getMessage(), $ex->getRetry());
} catch (\Exception $e) {
$partStorage->unlink($internalPartPath);
// try to revert the lock state so it can be cleanup properly
try {
$this->changeLock(ILockingProvider::LOCK_SHARED);
} catch (LockedException $e) {
$this->convertToSabreException($e);
}
$this->convertToSabreException($e);
}
}
Expand Down Expand Up @@ -309,6 +318,26 @@ public function put($data) {
return '"' . $this->info->getEtag() . '"';
}

/**
* Figure out the content length for the data. The data could be a resource or a string.
* The $_SERVER['CONTENT_LENGTH'] variable will be prioritized over anything if it's set
* Return -1 if we can't figure out the content length
*/
private function getExpectedContentLength($data) {
$expected = -1;
if (isset($_SERVER['CONTENT_LENGTH'])) {
$expected = $_SERVER['CONTENT_LENGTH'];
} elseif (\is_resource($data)) {
$stat = \fstat($data);
if ($stat !== false) {
$expected = $stat['size'];
}
} elseif (\is_string($data)) {
$expected = \strlen($data);
}
return $expected;
}

private function getPartFileBasePath($path) {
$partFileInStorage = \OC::$server->getConfig()->getSystemValue('part_file_in_storage', true);
if ($partFileInStorage) {
Expand Down
48 changes: 44 additions & 4 deletions apps/dav/lib/Connector/Sabre/QuotaPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use Sabre\DAV\Exception\InsufficientStorage;
use Sabre\DAV\Exception\ServiceUnavailable;
use Sabre\DAV\INode;
use Sabre\HTTP\RequestInterface;

/**
* This plugin check user quota and deny creating files when they exceeds the quota.
Expand Down Expand Up @@ -75,6 +76,15 @@ public function initialize(\Sabre\DAV\Server $server) {
$server->on('beforeWriteContent', [$this, 'handleBeforeWriteContent'], 10);
$server->on('beforeCreateFile', [$this, 'handleBeforeCreateFile'], 10);
$server->on('beforeMove', [$this, 'handleBeforeMove'], 10);

// QuotaPlugin is initialized during the processing of the "beforeMethod:*" event handling, which
// means that we can't listen on a "beforeMethod:*" event at this point because it's too late
// There is no "beforeCopy" event we can listen to neither.
// We'll assume the "beforeMethod:*" is being processed, so we'll process our "handleBeforeCopy"
// right away.
if ($server->httpRequest->getMethod() === 'COPY') {
$this->handleBeforeCopy($server->httpRequest);
}
}

/**
Expand All @@ -90,8 +100,12 @@ public function handleBeforeMove($source, $destination) {
$sourceNode = $this->server->tree->getNodeForPath($source);

// get target node for proper path conversion
$destinationSize = 0;
if ($this->server->tree->nodeExists($destination)) {
$targetNode = $this->server->tree->getNodeForPath($destination);
if ($targetNode instanceof Node) {
$destinationSize = $targetNode->getSize() ?? 0;
}
} else {
$dirname = \dirname($destination);
$dirname = $dirname === '.' ? '/' : $dirname;
Expand All @@ -112,7 +126,30 @@ public function handleBeforeMove($source, $destination) {
}
}
$path = $targetNode->getPath();
return $this->checkQuota($path, $sourceNode->getSize());
return $this->checkQuota($path, $sourceNode->getSize(), $destinationSize);
}

public function handleBeforeCopy(RequestInterface $request) {
if ($request->getMethod() !== 'COPY') {
return true;
}

$requestedPath = $request->getPath();
$sourceNode = $this->server->tree->getNodeForPath($requestedPath); // might throw a NotFound exception

$sourceSize = null;
if ($sourceNode instanceof Node) {
$sourceSize = $sourceNode->getSize();
}

$copyInfo = $this->server->getCopyAndMoveInfo($request); // it might throw exceptions
$path = $copyInfo['destination'];

$destinationSize = 0;
if ($copyInfo['destinationExists'] && $copyInfo['destinationNode'] instanceof Node) {
$destinationSize = $copyInfo['destinationNode']->getSize() ?? 0;
}
return $this->checkQuota($path, $sourceSize, $destinationSize);
}

/**
Expand All @@ -127,7 +164,9 @@ public function handleBeforeWriteContent($uri, $node, $data, $modified) {
if (!$node instanceof Node) {
return;
}
return $this->checkQuota($node->getPath());

$nodeSize = $node->getSize() ?? 0; // it might return null, so assume 0 in that case.
return $this->checkQuota($node->getPath(), null, $nodeSize);
}

/**
Expand All @@ -150,10 +189,11 @@ public function handleBeforeCreateFile($uri, $data, $parent, $modified) {
*
* @param string $path path of the user's home
* @param int $length size to check whether it fits
* @param int $extraSpace additional space granted, usually used when overwriting files
* @throws InsufficientStorage
* @return bool
*/
public function checkQuota($path, $length = null) {
public function checkQuota($path, $length = null, $extraSpace = 0) {
if ($length === null) {
$length = $this->getLength();
}
Expand All @@ -173,7 +213,7 @@ public function checkQuota($path, $length = null) {
$path = \rtrim($parentPath, '/') . '/' . $info['name'];
}
$freeSpace = $this->getFreeSpace($path);
if ($freeSpace !== FileInfo::SPACE_UNKNOWN && $freeSpace !== FileInfo::SPACE_UNLIMITED && $length > $freeSpace) {
if ($freeSpace !== FileInfo::SPACE_UNKNOWN && $freeSpace !== FileInfo::SPACE_UNLIMITED && $length > $freeSpace + $extraSpace) {
if (isset($chunkHandler)) {
$chunkHandler->cleanup();
}
Expand Down
3 changes: 2 additions & 1 deletion lib/private/Encryption/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
namespace OC\Encryption;

use OC\Cache\CappedMemoryCache;
use OC_Util;

class File implements \OCP\Encryption\IFile {

Expand Down Expand Up @@ -61,7 +62,7 @@ public function getAccessList($path) {
}

$ownerPath = \substr($ownerPath, \strlen('/files'));
$ownerPath = $this->util->stripPartialFileExtension($ownerPath);
$ownerPath = OC_Util::stripPartialFileExtension($ownerPath);

// first get the shares for the parent and cache the result so that we don't
// need to check all parents for every file
Expand Down
3 changes: 2 additions & 1 deletion lib/private/Encryption/Keys/Storage.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
use OCP\Encryption\Keys\IStorage;
use OCP\IUserSession;
use OC\User\NoUserException;
use OC_Util;

class Storage implements IStorage {

Expand Down Expand Up @@ -89,7 +90,7 @@ public function getUserKey($uid, $keyId, $encryptionModuleId) {
* @inheritdoc
*/
public function getFileKey($path, $keyId, $encryptionModuleId) {
$realFile = $this->util->stripPartialFileExtension($path);
$realFile = OC_Util::stripPartialFileExtension($path);
$keyDir = $this->getFileKeyDir($encryptionModuleId, $realFile);
$key = $this->getKey($keyDir . $keyId);

Expand Down
25 changes: 0 additions & 25 deletions lib/private/Encryption/Util.php
Original file line number Diff line number Diff line change
Expand Up @@ -271,31 +271,6 @@ public function getUidAndFilename($path) {
return [$ownerUid, $originalPath];
}

/**
* Remove .path extension from a file path
* @param string $path Path that may identify a .part file
* @return string File path without .part extension
* @note this is needed for reusing keys
*/
public function stripPartialFileExtension($path) {
$extension = \pathinfo($path, PATHINFO_EXTENSION);

if ($extension === 'part') {
$newLength = \strlen($path) - 5; // 5 = strlen(".part")
$fPath = \substr($path, 0, $newLength);

// if path also contains a transaction id, we remove it too
$extension = \pathinfo($fPath, PATHINFO_EXTENSION);
if (\substr($extension, 0, 12) === 'ocTransferId') { // 12 = strlen("ocTransferId")
$newLength = \strlen($fPath) - \strlen($extension) -1;
$fPath = \substr($fPath, 0, $newLength);
}
return $fPath;
} else {
return $path;
}
}

public function getUserWithAccessToMountPoint($users, $groups) {
$result = [];
if (\in_array('all', $users)) {
Expand Down
7 changes: 4 additions & 3 deletions lib/private/Files/Storage/Wrapper/Encryption.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
use OCP\Files\Storage;
use OCP\ILogger;
use OCP\Files\Cache\ICacheEntry;
use OC_Util;

class Encryption extends Wrapper {
use LocalTempFileTrait;
Expand Down Expand Up @@ -384,7 +385,7 @@ public function fopen($path, $mode) {

if ($this->util->isExcluded($fullPath) === false) {
$size = $unencryptedSize = 0;
$realFile = $this->util->stripPartialFileExtension($path);
$realFile = OC_Util::stripPartialFileExtension($path);
$targetExists = $this->file_exists($realFile) || $this->file_exists($path);
$targetIsEncrypted = false;
if ($targetExists) {
Expand Down Expand Up @@ -942,7 +943,7 @@ protected function readFirstBlock($path) {
protected function getHeaderSize($path) {
$headerSize = 0;
if (!\is_resource($path)) {
$realFile = $this->util->stripPartialFileExtension($path);
$realFile = OC_Util::stripPartialFileExtension($path);
if ($this->storage->file_exists($realFile)) {
$path = $realFile;
}
Expand Down Expand Up @@ -994,7 +995,7 @@ protected function getHeader($path) {
if (\is_resource($path)) {
$exists = false;
} else {
$realFile = $this->util->stripPartialFileExtension($path);
$realFile = OC_Util::stripPartialFileExtension($path);
$exists = $this->storage->file_exists($realFile);
if ($exists) {
$path = $realFile;
Expand Down
40 changes: 32 additions & 8 deletions lib/private/Files/Storage/Wrapper/Quota.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
namespace OC\Files\Storage\Wrapper;

use OCP\Files\Cache\ICacheEntry;
use OC\Files\Filesystem;
use OC_Util;

class Quota extends Wrapper {

Expand All @@ -39,13 +41,17 @@ class Quota extends Wrapper {
*/
protected $sizeRoot;

/** @var string $mountPoint */
protected $mountPoint;

/**
* @param array $parameters
*/
public function __construct($parameters) {
parent::__construct($parameters);
$this->quota = $parameters['quota'];
$this->sizeRoot = isset($parameters['root']) ? $parameters['root'] : '';
$this->mountPoint = $parameters['mountPoint'] ?? '';
}

/**
Expand Down Expand Up @@ -142,14 +148,32 @@ public function copy($source, $target) {
public function fopen($path, $mode) {
$source = $this->storage->fopen($path, $mode);

// don't apply quota for part files
if (!$this->isPartFile($path)) {
$free = $this->free_space('');
if ($source && $free >= 0 && $mode !== 'r' && $mode !== 'rb') {
// only apply quota for files, not metadata, trash or others
if (\strpos(\ltrim($path, '/'), 'files/') === 0) {
return \OC\Files\Stream\Quota::wrap($source, $free);
}
$used = \OCP\Files\FileInfo::SPACE_NOT_COMPUTED;
$free = $this->free_space('');
// if it's a .part file, check if we're trying to overwrite a file
if ($this->isPartFile($path)) {
$strippedPath = OC_Util::stripPartialFileExtension($path);
$used = $this->getSize($strippedPath);

$view = new \OC\Files\View();
$fullPath = Filesystem::normalizePath("{$this->mountPoint}/{$strippedPath}", true, true, true);

$fInfo = $view->getFileInfo($fullPath);
if ($fInfo && $fInfo->isShared()) {
$free = $view->free_space($fullPath);
$used = $fInfo->getSize();
}
}

if ($used >= 0) {
// if we're overwriting a file, add the size of that file to the available space
// so it's possible to overwrite in case the quota is limited
$free += $used;
}
if ($source && $free >= 0 && $mode !== 'r' && $mode !== 'rb') {
// only apply quota for files, not metadata, trash or others
if (\strpos(\ltrim($path, '/'), 'files/') === 0) {
return \OC\Files\Stream\Quota::wrap($source, $free);
}
}
return $source;
Expand Down
27 changes: 2 additions & 25 deletions lib/private/Files/Stream/Checksum.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

use Icewind\Streams\Wrapper;
use OC\Cache\CappedMemoryCache;
use OC_Util;

/**
* Computes the checksum of the wrapped stream. The checksum can be retrieved with
Expand Down Expand Up @@ -130,30 +131,6 @@ private function updateHashingContexts($data) {
}
}

/**
* Remove .part extension from a file path
* @param string $path Path that may identify a .part file
* @return string File path without .part extension
*/
private function stripPartialFileExtension($path) {
$extension = \pathinfo($path, PATHINFO_EXTENSION);

if ($extension === 'part') {
$newLength = \strlen($path) - 5; // 5 = strlen(".part")
$fPath = \substr($path, 0, $newLength);

// if path also contains a transaction id, we remove it too
$extension = \pathinfo($fPath, PATHINFO_EXTENSION);
if (\substr($extension, 0, 12) === 'ocTransferId') { // 12 = strlen("ocTransferId")
$newLength = \strlen($fPath) - \strlen($extension) -1;
$fPath = \substr($fPath, 0, $newLength);
}
return $fPath;
} else {
return $path;
}
}

/**
* Make checksums available for part files and the original file for which part file has been created
* @return bool
Expand All @@ -166,7 +143,7 @@ public function stream_close() {
// If current path belongs to part file, save checksum for original file
// As a result, call to getChecksums for original file (of this part file) will
// fetch checksum from cache
$originalFilePath = $this->stripPartialFileExtension($currentPath);
$originalFilePath = OC_Util::stripPartialFileExtension($currentPath);
if ($originalFilePath !== $currentPath) {
self::$checksums[$originalFilePath] = $checksum;
}
Expand Down
Loading