Skip to content

Commit 2d6a291

Browse files
cuppettclaude
authored andcommitted
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) <noreply@anthropic.com> Signed-off-by: Stephen Cuppett <scuppett@redhat.com>
1 parent 0e164d5 commit 2d6a291

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
@@ -1043,7 +1043,7 @@ protected function calculateFolderSizeInner(string $path, $entry = null, bool $i
10431043
$id = $entry['fileid'];
10441044

10451045
$query = $this->getQueryBuilder();
1046-
$query->select('size', 'unencrypted_size')
1046+
$query->select('size', 'unencrypted_size', 'encrypted')
10471047
->from('filecache')
10481048
->whereStorageId($this->getNumericStorageId())
10491049
->whereParent($id);
@@ -1063,7 +1063,7 @@ protected function calculateFolderSizeInner(string $path, $entry = null, bool $i
10631063
return Util::numericToNumber($row['unencrypted_size']);
10641064
}, $rows);
10651065
$unencryptedSizes = array_map(function (array $row) {
1066-
return Util::numericToNumber(($row['unencrypted_size'] > 0) ? $row['unencrypted_size'] : $row['size']);
1066+
return Util::numericToNumber($row['encrypted'] ? $row['unencrypted_size'] : $row['size']);
10671067
}, $rows);
10681068

10691069
$sum = array_sum($sizes);

lib/private/Files/Cache/Scanner.php

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

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

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

lib/private/Files/FileInfo.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public function __construct(
5858
private ?IUser $owner = null,
5959
) {
6060
$this->mount = $mount;
61-
if (isset($this->data['unencrypted_size']) && $this->data['unencrypted_size'] !== 0) {
61+
if (($this->data['encrypted'] ?? false) && isset($this->data['unencrypted_size'])) {
6262
$this->rawSize = $this->data['unencrypted_size'];
6363
} else {
6464
$this->rawSize = $this->data['size'] ?? 0;
@@ -355,7 +355,7 @@ public function addSubEntry($data, $entryPath) {
355355
if (!$data) {
356356
return;
357357
}
358-
$hasUnencryptedSize = isset($data['unencrypted_size']) && $data['unencrypted_size'] > 0;
358+
$hasUnencryptedSize = !empty($data['encrypted']) && isset($data['unencrypted_size']);
359359
if ($hasUnencryptedSize) {
360360
$subSize = $data['unencrypted_size'];
361361
} 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
@@ -387,7 +387,11 @@ public static function dataTestVerifyUnencryptedSize(): array {
387387
[120, 80, false, 80],
388388
[120, 120, false, 80],
389389
[120, -1, false, 80],
390-
[120, -1, true, -1]
390+
[120, -1, true, -1],
391+
// Zero-byte encrypted file: on-disk size equals header only (8192) — should NOT recalculate
392+
[8192, 0, false, 0],
393+
// Encrypted file with content beyond header but unencrypted_size=0 — SHOULD recalculate
394+
[16384, 0, false, 80],
391395
];
392396
}
393397

0 commit comments

Comments
 (0)