Skip to content

Commit ed4e235

Browse files
committed
fix(files): carry the unencrypted size over when copying a cache entry
The size of an encrypted file lives in `unencrypted_size`, and both CacheEntry::getUnencryptedSize() and FileInfo::getSize() prefer it over `size`. Cache::copyFromCache() copied the `encrypted` mark without it, so the copy of an encrypted file was marked encrypted with an unencrypted size of 0 and reported as empty - in the web UI, to clients and for quota - until something rescanned it. Copy the unencrypted size alongside the encrypted version, and reset it when the mark is dropped for a target that is not encrypted, where the size is read from `size`. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
1 parent ee3e6d0 commit ed4e235

4 files changed

Lines changed: 101 additions & 4 deletions

File tree

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
9+
namespace OCA\encryption\tests;
10+
11+
use OC\Files\Storage\Temporary;
12+
use OC\Files\View;
13+
use OCA\Encryption\KeyManager;
14+
use OCP\Server;
15+
use Test\TestCase;
16+
use Test\Traits\EncryptionTrait;
17+
use Test\Traits\MountProviderTrait;
18+
use Test\Traits\UserTrait;
19+
20+
/**
21+
* The size of an encrypted file is kept in `unencrypted_size`, which every reader
22+
* prefers over `size`. A copy has to carry it over, otherwise the copy is reported
23+
* as empty.
24+
*/
25+
#[\PHPUnit\Framework\Attributes\Group(name: 'DB')]
26+
class CopySizeTest extends TestCase {
27+
use MountProviderTrait;
28+
use EncryptionTrait;
29+
use UserTrait;
30+
31+
private function setUpView(): View {
32+
Server::get(KeyManager::class)->validateMasterKey();
33+
Server::get(KeyManager::class)->validateShareKey();
34+
$this->createUser('test1', 'test2');
35+
$this->setupForUser('test1', 'test2');
36+
$this->registerMount('test1', new Temporary(), '/test1/files/other');
37+
$this->loginWithEncryption('test1');
38+
39+
return new View('/test1/files');
40+
}
41+
42+
public function testCopyKeepsTheSize(): void {
43+
$view = $this->setUpView();
44+
45+
$view->file_put_contents('source.bin', str_repeat('a', 20000));
46+
$this->assertTrue($view->copy('source.bin', 'target.bin'));
47+
48+
$this->assertEquals(20000, $view->getFileInfo('target.bin')->getSize());
49+
}
50+
51+
public function testCopyToAnotherStorageKeepsTheSize(): void {
52+
$view = $this->setUpView();
53+
54+
$view->file_put_contents('source.bin', str_repeat('a', 20000));
55+
$this->assertTrue($view->copy('source.bin', 'other/target.bin'));
56+
57+
$this->assertEquals(20000, $view->getFileInfo('other/target.bin')->getSize());
58+
}
59+
60+
public function testCopyOfAFolderKeepsTheSizes(): void {
61+
$view = $this->setUpView();
62+
63+
$view->mkdir('source');
64+
$view->file_put_contents('source/file.bin', str_repeat('a', 20000));
65+
$this->assertTrue($view->copy('source', 'target'));
66+
67+
$this->assertEquals(20000, $view->getFileInfo('target/file.bin')->getSize());
68+
$this->assertEquals(20000, $view->getFileInfo('target')->getSize());
69+
}
70+
}

‎build/integration/encryption_features/encryption.feature‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ Feature: encryption
2626
And User "user0" adds a file of 20000 bytes to "/big.bin"
2727
When User "user0" copies file "/big.bin" to "/copy.bin"
2828
Then the HTTP status code should be "201"
29+
And File "/copy.bin" should have prop "d:getcontentlength" equal to "20000"
2930
When Downloading file "/copy.bin"
3031
Then the HTTP status code should be "200"
3132

‎lib/private/Files/Cache/Cache.php‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1281,6 +1281,8 @@ public function copyFromCache(ICache $sourceCache, ICacheEntry $sourceEntry, str
12811281
// normalizeData() prefers 'encryptedVersion' over 'encrypted' when both are
12821282
// set, so it has to be cleared too or the mark above gets ignored
12831283
unset($data['encryptedVersion']);
1284+
// without the `encrypted` mark the size of the copy is read from `size`
1285+
$data['unencrypted_size'] = 0;
12841286
} elseif (isset($data['encryptedVersion'])) {
12851287
// The storage re-encrypts the content it writes to the target, so the target
12861288
// is at its own version - the one recorded for it while it was written - and
@@ -1323,8 +1325,14 @@ private function cacheEntryToArray(ICacheEntry $entry): array {
13231325
$data['permissions'] = $entry['scan_permissions'];
13241326
}
13251327

1326-
if ($entry->isEncrypted() && isset($entry['encryptedVersion'])) {
1327-
$data['encryptedVersion'] = $entry['encryptedVersion'];
1328+
if ($entry->isEncrypted()) {
1329+
// the size of an encrypted file is stored in its own column, which every
1330+
// reader prefers over `size`, so the copy is reported as empty without it
1331+
$data['unencrypted_size'] = $entry->getUnencryptedSize();
1332+
1333+
if (isset($entry['encryptedVersion'])) {
1334+
$data['encryptedVersion'] = $entry['encryptedVersion'];
1335+
}
13281336
}
13291337

13301338
return $data;

‎tests/lib/Files/Cache/CacheTest.php‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -726,10 +726,25 @@ public function testCopyFromCachePreservesEncryptedVersion(): void {
726726
$this->assertSame(3, $targetEntry['encryptedVersion']);
727727
}
728728

729+
public function testCopyFromCachePreservesUnencryptedSize(): void {
730+
$data = [
731+
'size' => 128, 'mtime' => 50, 'mimetype' => 'foo/bar',
732+
'encrypted' => true, 'encryptedVersion' => 3, 'unencrypted_size' => 100,
733+
];
734+
$this->cache->put('source', $data);
735+
$sourceEntry = $this->cache->get('source');
736+
$this->assertEquals(100, $sourceEntry->getUnencryptedSize());
737+
738+
$this->cache->copyFromCache($this->cache, $sourceEntry, 'target');
739+
740+
$targetEntry = $this->cache->get('target');
741+
$this->assertEquals(100, $targetEntry->getUnencryptedSize());
742+
}
743+
729744
public function testCopyFromCacheClearsEncryptedVersionWhenCopyingToNonEncryptedStorage(): void {
730745
$data = [
731-
'size' => 100, 'mtime' => 50, 'mimetype' => 'foo/bar',
732-
'encrypted' => true, 'encryptedVersion' => 3,
746+
'size' => 128, 'mtime' => 50, 'mimetype' => 'foo/bar',
747+
'encrypted' => true, 'encryptedVersion' => 3, 'unencrypted_size' => 100,
733748
];
734749
$this->cache2->put('source', $data);
735750
$sourceEntry = $this->cache2->get('source');
@@ -751,6 +766,9 @@ public function testCopyFromCacheClearsEncryptedVersionWhenCopyingToNonEncrypted
751766
$targetEntry = $targetCache->get('target');
752767
$this->assertFalse($targetEntry->isEncrypted());
753768
$this->assertSame(0, $targetEntry['encryptedVersion']);
769+
// the target is not marked as encrypted, so its size is read from `size`
770+
$this->assertEquals(0, $targetEntry['unencrypted_size']);
771+
$this->assertEquals(128, $targetEntry->getSize());
754772
}
755773

756774
public function testGetIncomplete(): void {

0 commit comments

Comments
 (0)