robinmoisson
Decode() performance fix
Summary
codec.js/decode() may calculate encryptedDataHash multiple times. calculatedHMAC is based on encryptedDataHash which can be arbitrarily large. Recalculating this hash when the data hasn't changed is a performance bug. The fix is to calculate the hash once. While reviewing this change the use of recursion was replaced with explicit retries.
Fixes
Fixes #226
Details
- Calculate and reuse encryptedDataHash to avoid rehashing
- Unwind the use of recursion to simplify the code and improve readability
- function is actually shorter (LoC AND lines of text)
Testing
I manually tested node encrypt, node decrypt, and HTML wrapper decrypt. It's not clear to me exactly how I could test the backwards compatible password hashes. Additional testing is recommended.
Notes for Reviewers
Multiple hashing has been around a while, but it was not easy to address before PR #219. I'd say at that point the performance issue became a bug because it was fixable.
Probably variable names 'hashedPassword2' and 'hashedPassword3' could be improved. I assigned them in the order they were used, but:
const hashedPassword2 = await cryptoEngine.hashThirdRound(hashedPassword, salt);
let hashedPassword3 = await cryptoEngine.hashSecondRound(hashedPassword, salt);
May be confusing.
