X Tutup
Skip to content

Save encrypted files in binary format#32110

Merged
PVince81 merged 1 commit intonextcloud:masterfrom
plumbeo:binary-encoding-4
May 5, 2022
Merged

Save encrypted files in binary format#32110
PVince81 merged 1 commit intonextcloud:masterfrom
plumbeo:binary-encoding-4

Conversation

@plumbeo
Copy link
Contributor

@plumbeo plumbeo commented Apr 24, 2022

Fixes #22156

Based, with modifications and adjustments where the code diverged, on owncloud/encryption#224 and
owncloud/core#38249.

Tested without seeing obvious problems on master with legacy encoding off and on and on a newly deployed 23.0.4 instance with encryption enabled and a few files uploaded beforehand to simulate an upgrade.

I fixed the existing unit tests that failed but didn’t add new ones to specifically exercise the new and the old format.

@plumbeo plumbeo force-pushed the binary-encoding-4 branch from ec5af88 to 838dbe9 Compare April 26, 2022 10:47
@plumbeo
Copy link
Contributor Author

plumbeo commented Apr 29, 2022

Sorry, I forgot to say that I addressed all the review comments.

@plumbeo plumbeo force-pushed the binary-encoding-4 branch from 75609d4 to e37b38c Compare May 3, 2022 14:04
@artonge artonge requested review from a team, PVince81 and come-nc and removed request for a team May 3, 2022 14:30
@artonge
Copy link
Contributor

artonge commented May 3, 2022

@plumbeo Could you fix the DCO ? Else, it looks good :)

I have pinged the server team for additional review.

@plumbeo plumbeo force-pushed the binary-encoding-4 branch from e37b38c to baea2f9 Compare May 3, 2022 14:48
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from a few comments, looks good.

@plumbeo plumbeo force-pushed the binary-encoding-4 branch from baea2f9 to 67b3ace Compare May 3, 2022 15:14
Default to the more space-efficient binary encoding for newly encrypted files
instead of the traditional base64 encoding, eliminating the 33% overhead.

The new option 'encryption.use_legacy_encoding' allows to force the legacy
encoding format if needed. Files encoded in the old format remain readable.

Based on owncloud/encryption#224 and
owncloud/core#38249 by karakayasemi.

Signed-off-by: plumbeo <plumbeo@users.noreply.github.com>
@plumbeo plumbeo force-pushed the binary-encoding-4 branch from 67b3ace to 1258cae Compare May 4, 2022 15:39
Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 code looks fine to me

@PVince81
Copy link
Member

PVince81 commented May 5, 2022

Tests:

  • download base64 file
  • download binary file
  • overwrite base64 file => new file is binary
  • overwrite binary file => still works
  • download versioned file that had base64 encoding

@PVince81 PVince81 merged commit fe24091 into nextcloud:master May 5, 2022
@welcome
Copy link

welcome bot commented May 5, 2022

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

@artonge
Copy link
Contributor

artonge commented May 5, 2022

Thanks a lot for the great work @plumbeo !!!

@plumbeo plumbeo deleted the binary-encoding-4 branch May 5, 2022 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Save space by storing encrypted files in raw binary, not in base64

6 participants

X Tutup