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

Fix getState for rolling hashes #56

Merged
merged 1 commit into from
Mar 9, 2020
Merged

Fix getState for rolling hashes #56

merged 1 commit into from
Mar 9, 2020

Conversation

overlookmotel
Copy link
Contributor

This PR fixes a bug when calculating rolling hashes (i.e. get hash of data so far, and then continue to append more data).

One use case for rolling hashes is uploading large files to Google Drive via API. The file is transferred in chunks, and the server response after each chunk contains the MD5 hash of all data received up to that point. It's useful to be able to verify that hash on client and then continue appending data to the hash when sending following chunks.

More background on the use case: nodejs/node#29903

Test case:

// One shot
const hash1 = SparkMD5.hash('Hi there');
console.log('hash1:', hash1);

// Incremental
const spark2 = new SparkMD5();
spark2.append('Hi');
spark2.append(' there');
const hash2 = spark2.end(); 
console.log('hash2:', hash2);

// Rolling
const spark3 = new SparkMD5();
spark3.append('Hi');

const state = spark3.getState();
const hashSoFar = spark3.end();
spark3.setState(state);

spark3.append(' there');
const hash3 = spark3.end(); 
console.log('hash3:', hash3);

On current master, output is:

hash1: d9385462d3deff78c352ebb3f941ce12
hash2: d9385462d3deff78c352ebb3f941ce12
hash3: 00000000000000000000000000000000

With this patch:

hash1: d9385462d3deff78c352ebb3f941ce12
hash2: d9385462d3deff78c352ebb3f941ce12
hash3: d9385462d3deff78c352ebb3f941ce12

I assume this is not intended behavior.

The problem is caused by .getState() copying the ._hash array by reference into the state object. The values in that array are then mutated by further operations, so contents of state object get changed too. My solution is simply to clone the array.

@satazor satazor merged commit dcb2bc8 into satazor:master Mar 9, 2020
@satazor
Copy link
Owner

satazor commented Mar 9, 2020

Thanks!

@overlookmotel
Copy link
Contributor Author

Wow! That was fast!

Would you be able to release on NPM?

@satazor
Copy link
Owner

satazor commented Mar 9, 2020

Released as v3.0.1.

@overlookmotel
Copy link
Contributor Author

That was even faster! Huge thanks.

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.

2 participants