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

Added 'hashAlgorithm' options to use another algorithm for hashing file content other than md5 #245

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

remtori
Copy link

@remtori remtori commented Sep 2, 2020

Like the title said, this PR added 'hashAlgorithm' options to specify other hash algorithm than md5.

In my use case, i need sha1 content hash instead of md5 so i will use its like:

const fileUpload = require('express-fileupload');

// omited

app.use(fileUpload({ 
    useTempFiles: true,
    hashAlgorithm: 'sha1',
});

app.post('/upload', (req, res) => {
    console.log('File content sha1 hash: ' + req.files.file.sha1);
    res.send('Ok');
});

@@ -50,7 +50,7 @@ module.exports = (options, fileUploadOptions = {}) => {
tempFilePath: options.tempFilePath,
truncated: options.truncated,
mimetype: options.mimetype,
md5: options.hash,
[fileUploadOptions.hashAlgorithm || 'md5']: options.hash,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couldn't we just use hash here as the object key?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but that a breaking change require a major version bump.
doing its this way make it backward compatiable with existing code expecting the md5 field

@Cerothen
Copy link

I'd be a big fan if this PR got merged and released as I too would like to use a hash algorithm that is less likely to have a collision.

As noted in another PR the test that failed is also failing in the master build and isn't related to the changes. #253 (comment)

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