fix(encryption): Correctly report size for zero-byte encrypted files#60070
fix(encryption): Correctly report size for zero-byte encrypted files#60070cuppett wants to merge 4 commits into
Conversation
0a275c7 to
d7ea1f0
Compare
d7ea1f0 to
035098d
Compare
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
035098d to
52371dd
Compare
|
Rebase only. |
come-nc
left a comment
There was a problem hiding this comment.
Looks correct but some formatting remarks
12d49f8 to
e8cbeb2
Compare
3d5335e to
e6c0ee7
Compare
|
fwiw, rebased on master branch and force pushed. Found a couple more 0-size spots in FileInfo based on @come-nc review and added a few more tests. |
|
@come-nc @icewind1991 @CarlSchwan I think this ready for final review. Is there something I need to do for that check-ai-trailers test to pass? I have the trailers on the commits and applied the label it is hunting for (best I know how, appears a new thing)... the GH action itself looks more like a permissions issue than a structural fail with the PR. Thanks all for the patience here! |
e6c0ee7 to
9f2118f
Compare
|
Rebase only |
9f2118f to
8302e82
Compare
|
Rebase only update. |
Files with 0 bytes no longer incorrectly report as 8192 bytes. Widens unencryptedSize to ?int, fixes verifyUnencryptedSize to compare against header size instead of 0, and corrects Scanner to populate unencrypted_size on initial upload. Assisted-by: ClaudeCode:claude-opus-4-6 Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Stephen Cuppett <steve@cuppett.com>
…stency 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>
8302e82 to
b6fddab
Compare
miaulalala
left a comment
There was a problem hiding this comment.
One small nitpick:
tests/lib/Files/Cache/CacheTest.php::testCalculateFolderSizeWithEncryptedZeroByte doesn't actually exercise the fix. In Cache::calculateFolderSizeInner, the write-back is gated by:
$shouldWriteUnEncryptedSize = $unencryptedMax > 0 || $totalSize === 0 || ($entry['unencrypted_size'] ?? 0) > 0;
$unencryptedMax is derived from the raw unencrypted_size column, not the new encrypted-flag-aware calculation. In the test's scenario (one all-zero encrypted child), this gate evaluates false, so the folder's unencrypted_size is never written.
Not a blocker but an actual test for the new calculation path would be appreciated.
|
AI Policy is due to PR from fork, unrelated. |
…Byte Add a second encrypted child with unencrypted_size=100 so the write-back gate in calculateFolderSizeInner ($unencryptedMax > 0) actually opens. The original single zero-byte child left the gate closed, meaning the test passed against the DB default rather than the computed value. With two children the assertion distinguishes the fixed code (result: 100) from the old buggy code (result: 8292, falling back to on-disk size for the zero-byte child). Assisted-by: ClaudeCode:claude-opus-4-6 Signed-off-by: Stephen Cuppett <scuppett@redhat.com>
|
@miaulalala I added a test for that, thanks! |
|
/backport to stable34 |
|
/backport to stable33 |
Summary
Files with 0 bytes no longer incorrectly report as 8192 bytes. Widens unencryptedSize to ?int, fixes verifyUnencryptedSize to compare against header size instead of 0, and corrects Scanner to populate unencrypted_size on initial upload.
Updates for a couple edge cases and simplifies to get more consistent recording of the size in the cache and database.
Spawned from #57279
Checklist
3. to review, feature component)stable32)AI (if applicable)