Base64 returning incorrect data.

[oros]

I'm still investigating further, but just wanted to share that when when using a PNG file as input, the output base64 data has been unusable for me. When compared with the output of something like this service, they are quite different and only the service ends up being usable. https://www.base64-image.de/

The file I used for comparison is: http://www.fnordware.com/superpng/pnggrad8rgb.png

I've found that I've actually had success by simply reading the file as binary data and converting that to base64 as described here: http://www.hacksparrow.com/base64-encoding-decoding-in-node-js.html

Specifically:

// function to encode file data to base64 encoded string
function base64_encode(file) {
    // read binary data
    var bitmap = fs.readFileSync(file);
    // convert binary data to base64 encoded string
    return new Buffer(bitmap).toString('base64');
}

Just wanted to share in case this was a common issue beyond my environment.

[oros]

Let me retry with the latest changes before digging too deep into this.

EDIT Yeah, my environment really doesn't like the base64 conversion for some reason. You can check out my fork (master branch) to see the usage of fs.readFileSync that I used to get the binary data for the raster images. It's a pretty basic swap out of code.

Again, I don't know why my environment is behaving this way, by all accounts your implementation is working for you, and should work for me (but for some reason it's not.)

[Prinzhorn]

You're not alone! I don't know when it broke, but I just realized my inlined images don't work anymore.

Which node version do you run? I'm on 5.9.1 and updated not too long ago.

[Prinzhorn]

The only test for valid base64 also fails when I clone the repo and make a clean install. I'll dive into it I guess.

Edit: it's definitely broken. Here's what I did:

• run clean docker image (https://hub.docker.com/_/node/) with node 5.10.1
• clone repo
• npm install
• npm test

Fails. Will now go through different node versions.

[Prinzhorn]

It passes with mochify 2.14.0

It fails with mochify 2.15.0 (https://github.com/mantoni/mochify.js#compatibility)

So my guess is that browserify 13 broke imgurify. The only thing that was changed was the buffer dependency (https://github.com/substack/node-browserify/blob/master/changelog.markdown#1300) which is a good candidate for breaking this.

[asbjornenge]

@Prinzhorn thanks for debugging this! I can confirm seeing the same as you, breaks with mochify @ 2.15.0. I suspect it can be related to the Buffer.concat but I haven't been able to find the bug yet. Please let me know if you figure something out :smile: :+1:

//cc @oros

[Prinzhorn]

So the buffer has the correct data, but there seem to be issues with multi-byte characters. When I use toString('utf8') it's the same but hex and base64 are different.

@asbjornenge it's unrelated to Buffer.concat. I've compared the buffers before concat (I'm just testing with a small 115byte png).

[Prinzhorn]

This is cumbersome when neither buffer nor through2 have changelogs...I have to stop for now.

[asbjornenge]

It also breaks for browserify 12.0.0, so seems broken for all browserify versions after 11.2.0.

[asbjornenge]

By changing this line in test/spec.js

var spinnerRaw = fs.readFileSync('./test/spinner.gif','base64')

to

var _spinnerRaw = fs.readFileSync('./test/spinner.gif','utf-8')
var spinnerRaw = new Buffer(_spinnerRaw).toString('base64')

I can make the test pass. Not sure what that means though... Maybe we should just go with @oros solution and read the file with fs instead of using the passed buffers. Did this solve broken images in the browser for you @oros ?

The downside of that is it will slow down bundling - perhaps considerably on a large codebase with lots of images. We could do it only for rasters...

[asbjornenge]

Simplified the code quite a bit :stuck_out_tongue_closed_eyes: I'm going to take 1 more shot at figuring out that buffer issue tomorrow. If I can't I'll release as it is now.

[asbjornenge]

Alright, no luck :disappointed: So I went ahead and released the rewrite as a new major version 2.0.0 :tada: I also opened and issue here in hope for some help debugging this. Hopefully we can push a 2.0.1 soon using the buffers instead of readFileSync :smile:

I'll leave both this issue and #3 open for now.