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

add wrapper for external storage to ensure we don't get an mtime that is lower than we know it is #40499

Merged
merged 2 commits into from
Sep 21, 2023
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
28 changes: 10 additions & 18 deletions apps/files_external/lib/Config/ConfigAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

use OC\Files\Storage\FailedStorage;
use OC\Files\Storage\Wrapper\Availability;
use OC\Files\Storage\Wrapper\KnownMtime;
use OCA\Files_External\Lib\PersonalMount;
use OCA\Files_External\Lib\StorageConfig;
use OCA\Files_External\Service\UserGlobalStoragesService;
Expand All @@ -40,29 +41,17 @@
use OCP\Files\Storage\IStorageFactory;
use OCP\Files\StorageNotAvailableException;
use OCP\IUser;
use Psr\Clock\ClockInterface;

/**
* Make the old files_external config work with the new public mount config api
*/
class ConfigAdapter implements IMountProvider {

/** @var UserStoragesService */
private $userStoragesService;

/** @var UserGlobalStoragesService */
private $userGlobalStoragesService;

/**
* @param UserStoragesService $userStoragesService
* @param UserGlobalStoragesService $userGlobalStoragesService
*/
public function __construct(
UserStoragesService $userStoragesService,
UserGlobalStoragesService $userGlobalStoragesService
) {
$this->userStoragesService = $userStoragesService;
$this->userGlobalStoragesService = $userGlobalStoragesService;
}
private UserStoragesService $userStoragesService,
private UserGlobalStoragesService $userGlobalStoragesService,
private ClockInterface $clock,
) {}

/**
* Process storage ready for mounting
Expand All @@ -82,7 +71,7 @@
if (!is_subclass_of($objectClass, '\OCP\Files\ObjectStore\IObjectStore')) {
throw new \InvalidArgumentException('Invalid object store');
}
$storage->setBackendOption('objectstore', new $objectClass($objectStore));

Check failure on line 74 in apps/files_external/lib/Config/ConfigAdapter.php

View workflow job for this annotation

GitHub Actions / static-code-analysis-security

TaintedCallable

apps/files_external/lib/Config/ConfigAdapter.php:74:50: TaintedCallable: Detected tainted text (see https://psalm.dev/243)
}

$storage->getAuthMechanism()->manipulateStorageConfig($storage, $user);
Expand Down Expand Up @@ -155,7 +144,10 @@
$this->userStoragesService,
$storageConfig,
$storageConfig->getId(),
$storage,
new KnownMtime([
'storage' => $storage,
'clock' => $this->clock,
]),
'/' . $user->getUID() . '/files' . $storageConfig->getMountPoint(),
null,
$loader,
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -1346,6 +1346,7 @@
'OC\\Files\\Storage\\Wrapper\\EncodingDirectoryWrapper' => $baseDir . '/lib/private/Files/Storage/Wrapper/EncodingDirectoryWrapper.php',
'OC\\Files\\Storage\\Wrapper\\Encryption' => $baseDir . '/lib/private/Files/Storage/Wrapper/Encryption.php',
'OC\\Files\\Storage\\Wrapper\\Jail' => $baseDir . '/lib/private/Files/Storage/Wrapper/Jail.php',
'OC\\Files\\Storage\\Wrapper\\KnownMtime' => $baseDir . '/lib/private/Files/Storage/Wrapper/KnownMtime.php',
'OC\\Files\\Storage\\Wrapper\\PermissionsMask' => $baseDir . '/lib/private/Files/Storage/Wrapper/PermissionsMask.php',
'OC\\Files\\Storage\\Wrapper\\Quota' => $baseDir . '/lib/private/Files/Storage/Wrapper/Quota.php',
'OC\\Files\\Storage\\Wrapper\\Wrapper' => $baseDir . '/lib/private/Files/Storage/Wrapper/Wrapper.php',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -1379,6 +1379,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OC\\Files\\Storage\\Wrapper\\EncodingDirectoryWrapper' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/EncodingDirectoryWrapper.php',
'OC\\Files\\Storage\\Wrapper\\Encryption' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Encryption.php',
'OC\\Files\\Storage\\Wrapper\\Jail' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Jail.php',
'OC\\Files\\Storage\\Wrapper\\KnownMtime' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/KnownMtime.php',
'OC\\Files\\Storage\\Wrapper\\PermissionsMask' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/PermissionsMask.php',
'OC\\Files\\Storage\\Wrapper\\Quota' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Quota.php',
'OC\\Files\\Storage\\Wrapper\\Wrapper' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Wrapper.php',
Expand Down
142 changes: 142 additions & 0 deletions lib/private/Files/Storage/Wrapper/KnownMtime.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
<?php

namespace OC\Files\Storage\Wrapper;

use OCP\Cache\CappedMemoryCache;
use OCP\Files\Storage\IStorage;
use Psr\Clock\ClockInterface;

/**
* Wrapper that overwrites the mtime return by stat/getMetaData if the returned value
* is lower than when we last modified the file.
*
* This is useful because some storage servers can return an outdated mtime right after writes
*/
class KnownMtime extends Wrapper {
private CappedMemoryCache $knowMtimes;
private ClockInterface $clock;

public function __construct($arguments) {
parent::__construct($arguments);
$this->knowMtimes = new CappedMemoryCache();
$this->clock = $arguments['clock'];
}

public function file_put_contents($path, $data) {
$result = parent::file_put_contents($path, $data);
if ($result) {
$now = $this->clock->now()->getTimestamp();
$this->knowMtimes->set($path, $this->clock->now()->getTimestamp());
}
return $result;
}

public function stat($path) {
$stat = parent::stat($path);
if ($stat) {
$this->applyKnownMtime($path, $stat);
}
return $stat;
}

public function getMetaData($path) {
$stat = parent::getMetaData($path);
if ($stat) {
$this->applyKnownMtime($path, $stat);
}
return $stat;
}

private function applyKnownMtime(string $path, array &$stat) {
if (isset($stat['mtime'])) {
$knownMtime = $this->knowMtimes->get($path) ?? 0;
$stat['mtime'] = max($stat['mtime'], $knownMtime);
}
}

public function filemtime($path) {
$knownMtime = $this->knowMtimes->get($path) ?? 0;
return max(parent::filemtime($path), $knownMtime);
}

public function mkdir($path) {
$result = parent::mkdir($path);
if ($result) {
$this->knowMtimes->set($path, $this->clock->now()->getTimestamp());
}
return $result;
}

public function rmdir($path) {
$result = parent::rmdir($path);
if ($result) {
$this->knowMtimes->set($path, $this->clock->now()->getTimestamp());
}
return $result;
}

public function unlink($path) {
$result = parent::unlink($path);
if ($result) {
$this->knowMtimes->set($path, $this->clock->now()->getTimestamp());
}
return $result;
}

public function rename($source, $target) {
$result = parent::rename($source, $target);
if ($result) {
$this->knowMtimes->set($target, $this->clock->now()->getTimestamp());
$this->knowMtimes->set($source, $this->clock->now()->getTimestamp());
}
return $result;
}

public function copy($source, $target) {
$result = parent::copy($source, $target);
if ($result) {
$this->knowMtimes->set($target, $this->clock->now()->getTimestamp());
}
return $result;
}

public function fopen($path, $mode) {
$result = parent::fopen($path, $mode);
if ($result && $mode === 'w') {
$this->knowMtimes->set($path, $this->clock->now()->getTimestamp());
}
return $result;
}

public function touch($path, $mtime = null) {
$result = parent::touch($path, $mtime);
if ($result) {
$this->knowMtimes->set($path, $mtime ?? $this->clock->now()->getTimestamp());
}
return $result;
}

public function copyFromStorage(IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath) {
$result = parent::copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath);
if ($result) {
$this->knowMtimes->set($targetInternalPath, $this->clock->now()->getTimestamp());
}
return $result;
}

public function moveFromStorage(IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath) {
$result = parent::moveFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath);
if ($result) {
$this->knowMtimes->set($targetInternalPath, $this->clock->now()->getTimestamp());
}
return $result;
}

public function writeStream(string $path, $stream, int $size = null): int {
$result = parent::writeStream($path, $stream, $size);
if ($result) {
$this->knowMtimes->set($path, $this->clock->now()->getTimestamp());
}
return $result;
}
}
70 changes: 70 additions & 0 deletions tests/lib/Files/Storage/Wrapper/KnownMtimeTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
<?php
/**
* Copyright (c) 2014 Robin Appelman <icewind@owncloud.com>
* This file is licensed under the Affero General Public License version 3 or
* later.
* See the COPYING-README file.
*/

namespace lib\Files\Storage\Wrapper;

use OC\Files\Storage\Temporary;
use OC\Files\Storage\Wrapper\KnownMtime;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Clock\ClockInterface;
use Test\Files\Storage\Storage;

/**
* @group DB
*/
class KnownMtimeTest extends Storage {
/** @var Temporary */
private $sourceStorage;

/** @var ClockInterface|MockObject */
private $clock;
private int $fakeTime = 0;

protected function setUp(): void {
parent::setUp();
$this->fakeTime = 0;
$this->sourceStorage = new Temporary([]);
$this->clock = $this->createMock(ClockInterface::class);
$this->clock->method('now')->willReturnCallback(function () {
if ($this->fakeTime) {
return new \DateTimeImmutable("@{$this->fakeTime}");
} else {
return new \DateTimeImmutable();
}
});
$this->instance = $this->getWrappedStorage();
}

protected function tearDown(): void {
$this->sourceStorage->cleanUp();
parent::tearDown();
}

protected function getWrappedStorage() {
return new KnownMtime([
'storage' => $this->sourceStorage,
'clock' => $this->clock,
]);
}

public function testNewerKnownMtime() {
$future = time() + 1000;
$this->fakeTime = $future;

$this->instance->file_put_contents('foo.txt', 'bar');

// fuzzy match since the clock might have ticked
$this->assertLessThan(2, abs(time() - $this->sourceStorage->filemtime('foo.txt')));
$this->assertEquals($this->sourceStorage->filemtime('foo.txt'), $this->sourceStorage->stat('foo.txt')['mtime']);
$this->assertEquals($this->sourceStorage->filemtime('foo.txt'), $this->sourceStorage->getMetaData('foo.txt')['mtime']);

$this->assertEquals($future, $this->instance->filemtime('foo.txt'));
$this->assertEquals($future, $this->instance->stat('foo.txt')['mtime']);
$this->assertEquals($future, $this->instance->getMetaData('foo.txt')['mtime']);
}
}
Loading