fix: incorrect usage of Hash.digest #347
Conversation
Hash.digest expect an encoding value as parameter(latin1 | hex | base64). The absolute path of the file was passed to Hash.digest which is incorrect. To generate a hash based on the pathname we have to call Hash.update first. The desired encoding format is then passed to Hash.digest. This remove the need to call toString method.
Codecov Report
@@ Coverage Diff @@
## master #347 +/- ##
=======================================
Coverage 69.37% 69.37%
=======================================
Files 12 12
Lines 382 382
=======================================
Hits 265 265
Misses 117 117
Continue to review full report at Codecov.
|
|
@guybedford It looks like hashing/caching was added in #162 but does that mean it never working properly? |
|
So yes, this never worked... Thanks so much for the fix! Effectively we have always been outputting the The main important thing is that the directory name is random enough that multiple runs of We could entirely use a random generator here too, which would probably be even less likely to cause the conflict above. The other concern over fully random directories, was if the process was interrupted before it can clean up, then we leave behind the temporary folder, so there is a risk of file system bloat which was trying to be avoided by at least having a predictable hash generation based on the name. Hope that explains somewhat the constraints in play here, and why the issue hasn't been critical. |
|
I'm glad it help. Keep up the great work ! |
Hey,
Hash.digest expect an encoding value as parameter(latin1 | hex | base64).
The absolute path of the file was passed to
Hash.digestwhich is incorrect. To generate a hashbased on the pathname we have to call
Hash.updatefirst.The desired encoding format is then passed to
Hash.digest. This remove the need to calltoStringmethod.