Skip to content
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

Streams support #11

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

jeyries
Copy link

@jeyries jeyries commented Nov 16, 2022

This allows efficient processing of large files.

@CLAassistant
Copy link

CLAassistant commented Nov 16, 2022

CLA assistant check
All committers have signed the CLA.

@jeyries jeyries changed the title Added Streams support Streams support Nov 16, 2022
@tobihagemann
Copy link
Member

I hope there is no misunderstanding but I believe this is already implemented in this library. You'd like to encrypt/decrypt content via streams instead of URLs, right? Check this out:

func encryptContent(from cleartextStream: InputStream, to ciphertextStream: OutputStream, cleartextSize: Int?) throws {

func decryptContent(from ciphertextStream: InputStream, to cleartextStream: OutputStream, ciphertextSize: Int?) throws {

But it's not public yet. Should we just make it public? Or is your implementation kind of different?

@jeyries
Copy link
Author

jeyries commented Nov 16, 2022

Hello Tobias !

Thanks you for considering my pull request. Actually this is a more extensive support of the Streams than the two method encryptContent / decryptContent

For example you will need this if you want to decrypt then reencrypt a large file on the fly

that is without storing the full decrypted content on the memory or on the disk

@jeyries
Copy link
Author

jeyries commented Nov 16, 2022

Also, I take this opportunity to say thank you for Cryptomator, this is a great tool that I use every day :-)

@tobihagemann
Copy link
Member

I think I understand your PR now and conceptually, it makes sense to have input/output streams that decrypt/encrypt while streaming. But there will be no benefit for the cloud-access-swift library and therefore also not for the iOS app itself. However, it doesn't hurt to open up the API to give more flexibility.

Our goal is to have an unambiguous and concise API and content encryption/decryption is a major core to this library. I believe that your solution works but it creates code duplication and there are some implementation details that I don't agree with. To be honest, this would be a major change and would require some refactoring.

I'm unable to review this PR right now and I'll have to think about it a bit more. But thank you for providing your solution and kick-off the idea for streams support.

@jeyries
Copy link
Author

jeyries commented Nov 16, 2022

OK, I understand.
I will submit an other PR for just making public the part that I need for Stream support

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants