isBinaryFile considers certain protobuf binary files as text

[darrnshn]

Expected behaviour

Protobuf binary files considered binary.

Actual behaviour

Protobuf binary files not considered binary.

Environment Details

• Karma version: v4.4.1
• Karma config:

module.exports = function(config) {
  config.files.push({
    pattern: 'test/**',
    watched: false,
    served: true,
    nocache: false,
    included: false,
  });
};

Steps to reproduce the behaviour

I don't have a small repro unfortunately, but the root cause is simple. isBinaryFile uses heuristics for detecting binary files. In our case, some of our protobuf files happened to be classified as text even though they are binary, so Karma calls buffer.toString(), which creates a bunch of unicode replacement characters that the protobuf parser can't interpret.

Could we add a way to override the result of isBinaryFile via karma files config? maybe something like:

module.exports = function(config) {
  config.files.push({
    pattern: 'test/**',
    watched: false,
    served: true,
    nocache: false,
    included: false,
    isBinary: true,
  });
};

[Russianze]

I think for this you need to write a plugin with the desired option, other methods are not available

[johnjbarton]

I think this happens because your "pb" files have a lot of valid ASCII characters so the classifier says "ok looks like extended ASCII".

Unfortunately the buffer.toString() occurs before any preprocessor call so no plugin can fix this easily.

[karmarunnerbot]

🎉 This issue has been resolved in version 5.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[larabr]

I stumbled upon a similar issue: I have a file that can easily be misclassified as non-binary, due to having valid ASCII chars, but it contains some exotic charsets too so it should not be converted to a string under the hood.

I found that the isBinary option is already set up as a Pattern field, but cannot be given under config.files, since it's not passed through as option here. Changing the code to pass through the option seems to work for my use case, and would avoid the need to write a custom middleware that does nothing more than reading the file. Could you add support for setting isBinary as part of the config, as originally suggested?