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
Empty string encryption #21
Comments
Yup, this is come after 2.4.0 update with the assumption that empty string is not subject to encryption. I could agree to disable empty string check. As the goals of this library is to be simple, I don't see an optional configuration is coming. Beside that snippet you provide, empty string check happens 3 times here. simple-crypto-js/src/SimpleCrypto.ts Line 51 in 0ccd426
simple-crypto-js/src/SimpleCrypto.ts Lines 172 to 174 in 0ccd426
simple-crypto-js/src/SimpleCrypto.ts Lines 206 to 208 in 0ccd426
You may, if you wish, provide a PR to remove empty string check. |
@danang-id Thank you! PR coming shortly. 😁 |
@danang-id As I began working on this PR i noticed that this snippet you linked me: simple-crypto-js/src/SimpleCrypto.ts Lines 172 to 174 in 0ccd426
This snippet isn't validating whether a plain string is empty, but whether the cypher text is empty. I don't think I should remove this, correct? |
Merged PR #22 |
@TransmissionsDev The empty string check of simple-crypto-js/src/SimpleCrypto.ts Lines 176 to 178 in 0ccd426
The empty string check on |
@danang-id Thanks for all your hard work, and thanks for being so quick to respond. Closing this PR as no further work needs to be done. 👍 |
The line here prevents us from encrypting an empty string, even though it was possible before 2.4.0?
simple-crypto-js/src/SimpleCrypto.ts
Line 51 in 0ccd426
I understand the reasoning behind this (as to prevent silly errors by developers), but this actually complicates my use case of doing E2EE, as live editing forms where users may clear a form, will throw an error.
Is there anyway I could make a PR that would disable this protection or at least add an an optional parameter to disable it?
The text was updated successfully, but these errors were encountered: