webpack-contrib / file-loader

File Loader
MIT License
1.86k stars 255 forks source link

chore(deps): update #369

Closed alexander-akait closed 4 years ago

alexander-akait commented 4 years ago

BREAKING CHANGE: use md4 by default for hashing

This PR contains a:

Motivation / Use-Case

Use md4 by default for perf reasons

Breaking Changes

Yes

Additional Info

No

codecov[bot] commented 4 years ago

Codecov Report

Merging #369 into master will not change coverage by %. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #369   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines           29        29           
  Branches        13        12    -1     
=========================================
  Hits            29        29           
Impacted Files Coverage Δ
src/index.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e1fe27c...c69953c. Read the comment docs.

jens-duttke commented 4 years ago

May I ask why the default has been switched to MD4? Referring Wikipedia, the risk of a collision is much higher than for MD5 hashes.

I assume it can't be the speed on modern computers.

alexander-akait commented 4 years ago

@jens-duttke for perf reasons, webpack also uses md4 for hashing

risk of a collision

No, not in out cases

mrlubos commented 4 years ago

@jens-duttke for perf reasons, webpack also uses md4 for hashing

risk of a collision

No, not in out cases

Can you explain why please?

krysopath commented 4 years ago

when you retire an outdated hash algo due security concerns, then this security concern would be also the best reason to switch to sha256 algo or better immeditaly. In contrast, I cant see the reason for md4 at all.

btw, md5 is deprecated by experts since Dobbertin, 1996

And wrt md4, quote wikipedia:

MD5 is one in a series of message digest algorithms designed by Professor Ronald Rivest of MIT (Rivest, 1992). When analytic work indicated that MD5's predecessor MD4 was likely to be insecure, Rivest designed MD5 in 1991 as a secure replacement. (Hans Dobbertin did indeed later find weaknesses in MD4.)

Will CRC-1 be the next perf enhancement for webpack?

odensc commented 4 years ago

I'd assume what @evilebottnawi meant is that since the hash is used for caching purposes (not cryptographic purposes) it doesn't matter if there's a high collision risk, because the consequences in the event of a collision are close to nil.

However a non-cryptographic hash function such as xxHash would be much better suited to this use-case than an obsolete cryptographic hashing function, and would most likely be even faster.

It would be nice to get some insight into the reasoning behind this change from the maintainer themselves.

alexander-akait commented 4 years ago

I'd assume what @evilebottnawi meant is that since the hash is used for caching purposes (not cryptographic purposes)

Yes

However a non-cryptographic hash function such as xxHash would be much better suited to this use-case than an obsolete cryptographic hashing function, and would most likely be even faster.

yes, but we can't use it because it is require library compilation and can incompatibility with some platforms

Why md4?

So using md4 is best solution right now

tomaszs commented 3 years ago

This change is marked as breaking change. How to adjust file-loader to comply with this change?

digilist commented 3 years ago

@tomaszs I do not know the intention of the library authors for sure, but I guess they chose to bump the major version, because the build result will be different and so all caches will be invalidated. So I think there is no change necessary and you can simply update (this is what I did).