Skip to content

Commit

Permalink
perf: Try to prevalidate on part file upload if rules are only of a c…
Browse files Browse the repository at this point in the history
…ertain kind

Signed-off-by: Joas Schilling <coding@schilljs.com>
  • Loading branch information
nickvergessen committed Aug 23, 2024
1 parent a2c1f1d commit c1f6375
Showing 1 changed file with 29 additions and 4 deletions.
33 changes: 29 additions & 4 deletions lib/Operation.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,16 @@ public function checkFileAccess(IStorage $storage, string $path, bool $isDir = f
return;
}

/**
* Optimization:
* When uploading part files we check all rules up front.
* If a rule matches that does not depend on:
* - FileMimeType
* - FileSystemTags
* we can already deny the upload and don't need to wait until the chunk assembly
*/
$partFileUpload = true; //$this->isPartFileUpload($path);

$this->nestingLevel++;

$filePath = $this->translatePath($storage, $path);
Expand All @@ -68,10 +78,17 @@ public function checkFileAccess(IStorage $storage, string $path, bool $isDir = f
$ruleMatcher->setEntitySubject($this->fileEntity, $node);
}
$ruleMatcher->setOperation($this);
$match = $ruleMatcher->getFlows();
$match = $ruleMatcher->getFlows(!$partFileUpload);

$this->nestingLevel--;

\OC::$server->getLogger()->error('json_encode(array_map(fn ($i) => get_class($i), $match))');

Check failure on line 85 in lib/Operation.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis

UndefinedClass

lib/Operation.php:85:3: UndefinedClass: Class, interface or enum named OC does not exist (see https://psalm.dev/019)
if ($partFileUpload) {
// [{"id":1,"class":"OCA\\FilesAccessControl\\Operation","name":"","checks":"[5,6,4]","operation":"deny","entity":"OCA\\WorkflowEngine\\Entity\\File","events":"[]","scope_type":0,"scope_actor_id":""}]
\OC::$server->getLogger()->error(json_encode($match));
// $match = array_filter($match, );
}

if (!empty($match)) {
$e = new \RuntimeException('Access denied for path ' . $path . ' that is ' . ($isDir ? '' : 'not ') . 'a directory and matches rules: ' . json_encode($match));
$this->logger->debug($e->getMessage(), ['exception' => $e]);
Expand All @@ -80,6 +97,10 @@ public function checkFileAccess(IStorage $storage, string $path, bool $isDir = f
}
}

protected function isPartFileUpload(string $path): bool {
return preg_match('/\.ocTransferId\d+\.part$/i', $path) === 1;
}

protected function isBlockablePath(IStorage $storage, string $path): bool {
if (property_exists($storage, 'mountPoint')) {
$hasMountPoint = $storage instanceof StorageWrapper;
Expand All @@ -103,9 +124,10 @@ protected function isBlockablePath(IStorage $storage, string $path): bool {
return false;
}

if (preg_match('/\.ocTransferId\d+\.part$/i', $path)) {
return false;
}
// FIXME check that uploads/ path does not break below
// if (preg_match('/\.ocTransferId\d+\.part$/i', $path)) {
// return false;
// }

// '', admin, 'files', 'path/to/file.txt'
$segment = explode('/', $fullPath, 4);
Expand All @@ -126,6 +148,9 @@ protected function isBlockablePath(IStorage $storage, string $path): bool {
* For thumbnails and versions we want to check the tags of the original file
*/
protected function translatePath(IStorage $storage, string $path): string {
// Cut of potential part file suffix
$path = preg_replace('/\.ocTransferId\d+\.part$/i', '', $path);

if (substr_count($path, '/') < 1) {
return $path;
}
Expand Down

0 comments on commit c1f6375

Please sign in to comment.