Issue 1100 decrypt atts with kotlin by DenBond7 · Pull Request #1225 · FlowCrypt/flowcrypt-android

DenBond7 · 2021-05-12T05:49:34Z

This PR added an ability to decrypt files of any size using streams

close#1100 // if this PR closes an issue

Tests(delete all except exactly one):

Tests added or updated

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

addresses the issue it closes (if any)
code is readable and understandable
is accompanied with tests, or tests are not needed
is free of vulnerabilities

…restriction for encrypted attachments. Refactored code.| #1100

…1100

tomholub · 2021-05-12T07:50:46Z

@IvanPizhenko do you have ability to actually build and run the app to try things?

DenBond7 · 2021-05-12T07:55:56Z

@tomholub It's a draft.(not completed yet) I've added the base realization and now I'm on the adding tests stage.

… data.| #1100

DenBond7 · 2021-05-14T05:40:34Z

it's ready for a review

IvanPizhenko

Some minor changes and questions.

IvanPizhenko · 2021-05-16T19:42:08Z

+ }
+
+ enum class DecryptionErrorType {
+ KEY_MISMATCH,


I think we must already have such enum in the PgpMsg. Can it be used here?

Currently, we have the following:
PgpArmor - is responsible for armor source PgpDecrypt - is responsible for decrypting things PgpEncrypt - is responsible for encrypting things PgpKey - is responsible for handling keys PgpMsg - is responsible for processing PGP messages PgpPwd - is responsible for passphrases
In my opinion, it's more transparent to store the decryption error types in PgpDecrypt. Because you work on #1057, #1223 I've decided don't remove that code to prevent conflicts. We can clean code in a separate issue.

It's not obvious decision, I suggest to move it now. I am ok with the fact it may require then to resolve some conflicts.

tomholub · 2021-05-18T14:10:10Z

We could do a release after this lands

IvanPizhenko

Please move DecryptionErrorType now

IvanPizhenko · 2021-05-19T06:05:57Z

+ }
+
+ enum class DecryptionErrorType {
+ KEY_MISMATCH,


It's not obvious decision, I suggest to move it now. I am ok with the fact it may require then to resolve some conflicts.

tomholub

Looks good

tomholub · 2021-05-19T07:56:16Z

+ e.message?.contains("exception decrypting session info") == true
+ || e.message?.contains("encoded length out of range") == true
+ || e.message?.contains("Exception recovering session info") == true
+ || e.message?.contains("No suitable decryption key") == true -> {
+ DecryptionException(DecryptionErrorType.KEY_MISMATCH, e)
+ }


Is there issue for this too?

Not sure. I think we can improve that in another PR too. I'll create an issue for that.

DenBond7 added 4 commits May 11, 2021 10:29

Added an ability to decrypt attachments with PGPainless. Removed 3Mb …
d8d9c88
…restriction for encrypted attachments. Refactored code.| #1100

Added PgpDecrypt.|#1100
ed2141e

Added handling some errors.|#1100
044ea69

Modified PgpDecrypt.decrypt() to return OpenPgpMetadata as result.|#…
e5b81d0
…1100

DenBond7 added the PR submitted PR is submitted for this issue label May 12, 2021

DenBond7 added this to the 1.1.8 milestone May 12, 2021

DenBond7 self-assigned this May 12, 2021

DenBond7 added 3 commits May 12, 2021 09:02

Fixed wrongly added code.|#1100
1380df4

Improved streams usage in AttachmentDownloadManagerService.|#1100
047b2d5

Reverted back changes in PgpMsg.|#1100
7087c61

tomholub requested a review from IvanPizhenko May 12, 2021 07:50

DenBond7 added 6 commits May 12, 2021 10:57

Added PgpDecryptTest(not completed).|#1100
884d228

Improved TemporaryFolder.createFileWithGivenSize to use really random…
3f084d2
… data.| #1100

Added PgpDecryptTest.testDecryptionErrorWrongPassphrase().|#1100
303c93c

Modified tests.|#1100
0b297c1

Added using a regex pattern to detect encrypted attachments.|#1100
0ff9243

Removed unused resources.|#1100
a2bdc4c

DenBond7 marked this pull request as ready for review May 13, 2021 13:12

DenBond7 requested a review from seisvelas as a code owner May 13, 2021 13:12

IvanPizhenko suggested changes May 16, 2021
View reviewed changes

Fixed "EOF" issue.|#1100
32dc11b

DenBond7 requested a review from IvanPizhenko May 17, 2021 12:30

IvanPizhenko suggested changes May 19, 2021
View reviewed changes

Refactored code.|#1100
3abe008

DenBond7 requested a review from IvanPizhenko May 19, 2021 07:26

IvanPizhenko previously approved these changes May 19, 2021
View reviewed changes

Fixed PgpMsgTest after refactoring.|#1100
6c5dd1c

DenBond7 dismissed IvanPizhenko’s stale review via 6c5dd1cMay 19, 2021 07:54

tomholub approved these changes May 19, 2021
View reviewed changes

This was referenced May 19, 2021
Use WrongPassphraseException instead of the own realization #1237
Closed
Improve handling DecryptionErrorType.KEY_MISMATCH #1238
Closed

DenBond7 merged commit f89c7d4 into masterMay 19, 2021

DenBond7 deleted the issue_1100_decrypt_atts_with_kotlin branch May 19, 2021 11:28

Uh oh!

Conversation

DenBond7 commented May 12, 2021• edited by IvanPizhenko Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

To be filled by reviewers

Uh oh!

tomholub commented May 12, 2021

Uh oh!

DenBond7 commented May 12, 2021

Uh oh!

DenBond7 commented May 14, 2021

Uh oh!

IvanPizhenko left a comment

Choose a reason for hiding this comment

Uh oh!

IvanPizhenkoMay 16, 2021

Choose a reason for hiding this comment

Uh oh!

DenBond7May 17, 2021

Choose a reason for hiding this comment

Uh oh!

IvanPizhenkoMay 19, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tomholub commented May 18, 2021

Uh oh!

IvanPizhenko left a comment

Choose a reason for hiding this comment

Uh oh!

IvanPizhenkoMay 19, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tomholub left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tomholubMay 19, 2021

Choose a reason for hiding this comment

Uh oh!

DenBond7May 19, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

DenBond7 commented May 12, 2021•
edited by IvanPizhenko
Loading

tomholub left a comment •
edited
Loading