node-lintspaces icon indicating copy to clipboard operation
node-lintspaces copied to clipboard

[EditorConfig] Unrecognized values in charset property throw exceptions.

Open EricDunsworth opened this issue 5 years ago • 1 comments

Lintspaces doesn't claim to support .editorconfig's charset property, but appears to be reading it and attempting to use it in a way that can lead to exceptions.

An exception can occur if the charset property's value is unrecognized. Out of EditorConfig's 5 supported charsets, lintspaces seems to understand latin1, utf-8 and utf-16le, but errors out with utf-16be and utf-8-bom.

The same is also true for unset/invalid values. Btw I didn't cover the charset property in #47 since its exception would've prevented other warnings from appearing.

Would it be possible to either completely ignore the charset property or improve lintspace's handling of it?

Code/Output samples... The samples below demonstrate how lintspaces currently reacts to a charset = utf-8-bom property.

.editorconfig

root = true

[*]
charset = utf-8-bom

test.txt (BOM's presence doesn't matter)

test

check.js

var Validator = require('lintspaces');

var validator = new Validator({editorconfig: '.editorconfig'});
validator.validate('test.txt');

var results = validator.getInvalidFiles();

//Derived from https://stackoverflow.com/a/10729284
const util = require('util');

console.log(util.inspect(results, {showHidden: false, depth: null}));

Output when running node check.js in Windows:

internal/fs.js:21
    throw new Error(`Unknown encoding: ${encoding}`);
    ^

Error: Unknown encoding: utf-8-bom
    at assertEncoding (internal/fs.js:21:11)
    at getOptions (fs.js:80:5)
    at Object.fs.readFileSync (fs.js:549:13)
    at Validator._loadFile (C:\lintspaces-testing\node_modules\lintspaces\lib\Validator.js:123:18)
    at Validator.validate (C:\lintspaces-testing\node_modules\lintspaces\lib\Validator.js:66:8)
    at Object.<anonymous> (C:\lintspaces-testing\check.js:4:11)
    at Module._compile (module.js:652:30)
    at Object.Module._extensions..js (module.js:663:10)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)

Expected output of node check.js:

{}

EricDunsworth avatar Sep 14 '18 18:09 EricDunsworth

Hi @EricDunsworth, thank you for this detailed issue. This seems to be related to a unsupported encoding setting which is passed from editorconfig to readFileSync. I think lintspaces needs to check if the received encoding is a supported value for node's readFileSync(). I'm not clear about what will happen when lintspaces provides a fallback (utf-8-bom to utf-8) if the encoding isn't supported. I need to dive a bit deeper into it and will fix this problem in a future release. However, if you need a faster fix, I would be very happy to accept a PR to solve this.

schorfES avatar Sep 24 '18 12:09 schorfES