Skip to content

Commit 3d5335e

Browse files
cuppettclaude
cuppett
and
claude
committed
fix(encryption): Address review feedback and fix zero-byte size consistency
Apply come-nc's reviewer suggestions on Scanner.php: - Simplify cached unencrypted_size skip condition to explicit == 0 check - Remove redundant !isset guard on new-file path Fix remaining > 0 / !== 0 guards that treated zero as "no unencrypted_size": - FileInfo constructor: use encrypted flag for rawSize (getSize(false) path) - FileInfo::addSubEntry: use encrypted flag for sub-mount accumulation - Cache::calculateFolderSizeInner: use encrypted column instead of > 0 Add tests covering zero-byte encrypted file scenarios: - FileInfoTest: getSize(true/false) for zero-byte encrypted files - CacheTest: getUnencryptedSize and calculateFolderSize with encrypted=1,size=0 - EncryptionTest: verifyUnencryptedSize data provider cases for header-only files Assisted-by: ClaudeCode:claude-opus-4-6 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> Signed-off-by: Stephen Cuppett <[email protected]>
1 parent fe166c7 commit 3d5335e

6 files changed

Lines changed: 69 additions & 13 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,7 +1042,7 @@ protected function calculateFolderSizeInner(string $path, $entry = null, bool $i
10421042
$id = $entry['fileid'];
10431043

10441044
$query = $this->getQueryBuilder();
1045-
$query->select('size', 'unencrypted_size')
1045+
$query->select('size', 'unencrypted_size', 'encrypted')
10461046
->from('filecache')
10471047
->whereStorageId($this->getNumericStorageId())
10481048
->whereParent($id);
@@ -1062,7 +1062,7 @@ protected function calculateFolderSizeInner(string $path, $entry = null, bool $i
10621062
return Util::numericToNumber($row['unencrypted_size']);
10631063
}, $rows);
10641064
$unencryptedSizes = array_map(function (array $row) {
1065-
return Util::numericToNumber(($row['unencrypted_size'] > 0) ? $row['unencrypted_size'] : $row['size']);
1065+
return Util::numericToNumber($row['encrypted'] ? $row['unencrypted_size'] : $row['size']);
10661066
}, $rows);
10671067

10681068
$sum = array_sum($sizes);

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -184,12 +184,10 @@ public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData =
184184
}
185185
}
186186

187-
// Only skip updating unencrypted_size if both cached and new values are 0
188-
// This allows updating from incorrect cached 0 to correct non-zero value
189-
// while avoiding unnecessary updates when both are legitimately 0
187+
// Skip updating unencrypted_size only when both cached and new values are 0
190188
if (isset($cacheData['unencrypted_size'])
191189
&& $cacheData['unencrypted_size'] === 0
192-
&& (!isset($data['unencrypted_size']) || $data['unencrypted_size'] === 0)) {
190+
&& isset($data['unencrypted_size']) && $data['unencrypted_size'] === 0) {
193191
unset($data['unencrypted_size']);
194192
}
195193

@@ -207,10 +205,8 @@ public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData =
207205
$data['etag_changed'] = true;
208206
}
209207
} else {
210-
// For new files, preserve unencrypted_size only when the file is encrypted
211-
// and the key is present; otherwise clear it so it won't be written stale.
212-
if (!isset($data['encrypted']) || !$data['encrypted']
213-
|| !isset($data['unencrypted_size'])) {
208+
// For new files, only preserve unencrypted_size when the file is encrypted
209+
if (!isset($data['encrypted']) || !$data['encrypted']) {
214210
unset($data['unencrypted_size']);
215211
}
216212
$newData = $data;

‎lib/private/Files/FileInfo.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public function __construct(
5959
private ?IUser $owner = null,
6060
) {
6161
$this->mount = $mount;
62-
if (isset($this->data['unencrypted_size']) && $this->data['unencrypted_size'] !== 0) {
62+
if (($this->data['encrypted'] ?? false) && isset($this->data['unencrypted_size'])) {
6363
$this->rawSize = $this->data['unencrypted_size'];
6464
} else {
6565
$this->rawSize = $this->data['size'] ?? 0;
@@ -356,7 +356,7 @@ public function addSubEntry($data, $entryPath) {
356356
if (!$data) {
357357
return;
358358
}
359-
$hasUnencryptedSize = isset($data['unencrypted_size']) && $data['unencrypted_size'] > 0;
359+
$hasUnencryptedSize = !empty($data['encrypted']) && isset($data['unencrypted_size']);
360360
if ($hasUnencryptedSize) {
361361
$subSize = $data['unencrypted_size'];
362362
} else {

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,16 @@ public function testCacheEntryGetters(): void {
153153
$this->assertEquals($entry->getUnencryptedSize(), 100);
154154
}
155155

156+
public function testGetUnencryptedSizeEncryptedZeroByte(): void {
157+
$file1 = 'encrypted_zero';
158+
$this->cache->put($file1, ['size' => 8192, 'mtime' => 50, 'mimetype' => 'application/octet-stream', 'encrypted' => 1, 'unencrypted_size' => 0]);
159+
$entry = $this->cache->get($file1);
160+
161+
// getUnencryptedSize() must return 0 (the true plaintext size), not 8192 (the encrypted on-disk size)
162+
$this->assertEquals(0, $entry->getUnencryptedSize());
163+
$this->assertTrue($entry->isEncrypted());
164+
}
165+
156166
public function testPartial(): void {
157167
$file1 = 'foo';
158168

@@ -292,6 +302,28 @@ public function testEncryptedFolder(): void {
292302
$this->assertFalse($this->cache->inCache('folder/bar'));
293303
}
294304

305+
public function testCalculateFolderSizeWithEncryptedZeroByte(): void {
306+
$folder = 'enc_folder';
307+
$this->cache->put($folder, ['size' => -1, 'mtime' => 20, 'mimetype' => ICacheEntry::DIRECTORY_MIMETYPE]);
308+
309+
// Child: a zero-byte encrypted file — on-disk size is 8192 (header), but plaintext is 0
310+
$child = $folder . '/empty.enc';
311+
$this->cache->put($child, [
312+
'size' => 8192,
313+
'mtime' => 20,
314+
'mimetype' => 'application/octet-stream',
315+
'encrypted' => 1,
316+
'unencrypted_size' => 0,
317+
]);
318+
319+
$folderSize = $this->cache->calculateFolderSize($folder);
320+
321+
// The folder's unencrypted_size must reflect the true plaintext total (0), not the encrypted on-disk size (8192)
322+
$entry = $this->cache->get($folder);
323+
$this->assertEquals(0, $entry['unencrypted_size'], 'Folder unencrypted_size should be 0 for zero-byte encrypted children');
324+
$this->assertEquals(8192, $entry['size'], 'Folder size should equal encrypted on-disk size');
325+
}
326+
295327
public function testRootFolderSizeForNonHomeStorage(): void {
296328
$dir1 = 'knownsize';
297329
$dir2 = 'unknownsize';

‎tests/lib/Files/FileInfoTest.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,30 @@ protected function setUp(): void {
2929
$this->config = $this->getMockBuilder(IConfig::class)->getMock();
3030
}
3131

32+
private function makeFileInfo(array $data): FileInfo {
33+
$storage = new Temporary();
34+
return new FileInfo('', $storage, '', $data, new MountPoint($storage, '/foo/files'));
35+
}
36+
37+
public function testGetSizeEncryptedZeroByte(): void {
38+
$info = $this->makeFileInfo(['encrypted' => true, 'size' => 8192, 'unencrypted_size' => 0]);
39+
// Both paths must report the true plaintext size (0), not the on-disk encrypted size (8192)
40+
$this->assertSame(0, $info->getSize(true));
41+
$this->assertSame(0, $info->getSize(false));
42+
}
43+
44+
public function testGetSizeEncryptedNonZero(): void {
45+
$info = $this->makeFileInfo(['encrypted' => true, 'size' => 16384, 'unencrypted_size' => 5000]);
46+
$this->assertSame(5000, $info->getSize(true));
47+
$this->assertSame(5000, $info->getSize(false));
48+
}
49+
50+
public function testGetSizeNonEncrypted(): void {
51+
$info = $this->makeFileInfo(['encrypted' => false, 'size' => 100]);
52+
$this->assertSame(100, $info->getSize(true));
53+
$this->assertSame(100, $info->getSize(false));
54+
}
55+
3256
public function testIsMountedHomeStorage(): void {
3357
$user = $this->createMock(IUser::class);
3458
$user->method('getUID')

‎tests/lib/Files/Storage/Wrapper/EncryptionTest.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,11 @@ public static function dataTestVerifyUnencryptedSize(): array {
386386
[120, 80, false, 80],
387387
[120, 120, false, 80],
388388
[120, -1, false, 80],
389-
[120, -1, true, -1]
389+
[120, -1, true, -1],
390+
// Zero-byte encrypted file: on-disk size equals header only (8192) — should NOT recalculate
391+
[8192, 0, false, 0],
392+
// Encrypted file with content beyond header but unencrypted_size=0 — SHOULD recalculate
393+
[16384, 0, false, 80],
390394
];
391395
}
392396

0 commit comments

Comments
 (0)