ember-cli / broccoli-asset-rev

Broccoli plugin to add fingerprint checksums and CDN URLs to your assets
MIT License
86 stars 83 forks source link

Inconsistent Hashing (Take 2) #67

Closed nadnoslen closed 9 years ago

nadnoslen commented 9 years ago

Sorry for my previous commit and pull request that did not address tests. In my haste to get this working in my project I left the testing behind...my bad.

What Is Broke

What this pull request does is properly read the files in as a buffer so that they are hashed properly. Forcing (or defaulting to) an encoding of "UTF-8" will create inconsistent hashing on many binary files (e.g. png, jpg, etc.).

How Do We Know It Is Broke

Here's the best way to see what I mean; go to the head of the rickharrison/broccoli-asset-rev repo and execute the following commands:

$ md5 tests/fixtures/basic/input/images/sample.png 
MD5 (tests/fixtures/basic/input/images/sample.png) = 1f6b78f1b4667adc7e397f7bf94041ab

Notice the MD5 value is 1f6b78f1b4667adc7e397f7bf94041ab for the input file sample.png. In our output directory, that we are asserting against, the sample.png is expected to hash to b7caf246e908cd00d8011f370e3dd992. That is not correct as my command-line MD5 shows.

What Fixes The Problem

This was all a result of the incorrect hashing from the fs.readFileSync(..., {encoding: 'UTF-8'});. This pull request fixes this problem.

Cheers.

rickharrison commented 9 years ago

Can you squash into a single commit please

nadnoslen commented 9 years ago

Sorry about that. All in one commit now. Cheers.

rickharrison commented 9 years ago

Thanks for catching this. Much appreciated.