-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Increment required only when encryption is enabled #28820
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
Conversation
b3c48db to
3f7dacf
Compare
|
One reason why we still set the encryption wrapper even when encryption is disabled is mostly because we want to be able to detect formerly encrypted files and avoid sending these back. So if we want to add such bypass we might need to at least put it after the encryption check, which might be when reading the header. |
3f7dacf to
6b62edf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made alterations at the getHeader method. The intention here was to pass encryption module wrapped in array if encryption is enabled. And this value is later used https://github.com/owncloud/core/pull/28820/files#diff-938b56891f3079e628cfda1c331911b7R381 to call different storage's fopen. I hope this is the same header which is mentioned at #28820 (comment)
ec8f695 to
dd13bd0
Compare
|
I see #28780 fixed here. |
dd13bd0 to
a710404
Compare
PVince81
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 code looks fine.
As discussed it would be good to adjust the trashbin tests to verify that the restored file's contents matches before deletion, and have this test run for encryption too (some encryption tests are disabled with trashbin)
Please also retest versions: restoring or downloading older versions of a file must still work on home and external storage
a710404 to
1485064
Compare
Increment to files in filecache is required only when encryption is enabled. Else use the version as it is. Signed-off-by: Sujith H <[email protected]>
1485064 to
98680b0
Compare
SergioBertolinSG
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Integration tests part is ok
|
Verified that transfer ownership works with this change. Verified that upon deletion of the file after transfer, made some update, the versions are restored. |
|
@sharidas please backport to stable10 |
|
Backport : #28880 |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Increment to files in filecache is required only when
encryption is enabled. Else use the version as it is.
Signed-off-by: Sujith H [email protected]
Description
When encryption is not enabled then no need to go through each and every step in encryption wrapper's fopen. Instead we can get another storage wrapper's fopen handler.
Related Issue
#28780
Motivation and Context
This would help resolve the exceptions thrown in the logs.
How Has This Been Tested?
testunder shared folder.testtestfolder from the trashAlso I have noted that during verification on my local machine the storage passes to local storage from the encryption wrapper ( when no encryption is enabled ).
Screenshots (if appropriate):
Types of changes
Checklist: