jeresig / i18n-node-2

Lightweight simple translation module for node.js / express.js with dynamic json storage. Uses common __('...') syntax in app and templates.
MIT License
507 stars 79 forks source link

Locale file overwritten on parsing error #39

Closed ec1980 closed 9 years ago

ec1980 commented 9 years ago

If there is a json parsing error when i18n tries to open a locale file, it will create a new file and overwrite any existing one. Needless to say, it is not a good idea if you have spent a lot of time to create a file and then lose it simply because of a parsing error, which happened to me. I suggest you add fs.existsSync(localeFile) right above this.writeFile(locale) so that it will ONLY write file if localeFile doesn't exist.

        try {
            var localeFile = fs.readFileSync(file);

            try {
                // parsing filecontents to locales[locale]
                this.initLocale(locale, JSON.parse(localeFile));

            } catch (e) {
                console.error('unable to parse locales from file (maybe ' + file +
                    ' is empty or invalid json?): ', e);
            }

        } catch (e) {
            // unable to read, so intialize that file
            // locales[locale] are already set in memory, so no extra read required
            // or locales[locale] are empty, which initializes an empty locale.json file
            // @ec1980: Do not overwrite existing locale files
            if (!fs.existsSync(localeFile))
                this.writeFile(locale);
        }

BTW, if you are scratching your head trying to find out why your perfectly okay json file causes unexpected token parsing error, check that the file is not encoded with UTF-8 BOM, which is the default format on Windows. You can disable it in Visual Studio -> File -> Advanced Save Options -> Encoding -> Unicode(UTF-8 without signature).

ec1980 commented 9 years ago

Upon further review, the above fix didn't work because writeFile was called elsewhere when in the dev mode. I suggest adding fs.existsSync check inside writeFile. Basically, you should never overwrite an existing locale file even if the dev mode.

rshov commented 9 years ago

I've run into this as well. A simple comma after the last entry in the json file will cause the entire file to be wiped out.

hrant-ian commented 9 years ago

I can confirm this. Has this been fixed yet or is there a way to protect the files? I'm using the koa-i18n module, but that one just wraps this one.

ec1980 commented 9 years ago

It has been a while so I may not get this right. However, I recall writeFile is called in multiple places so the fix you have based on my original post may not cover all cases. My final fix was to replace this line in writeFile: fs.renameSync(tmp, target); with this:

            if (fs.existsSync(this.locateFile(locale))) {
                console.warn('%s created as the original locale file may contain errors.', tmp);
            } else {
                fs.renameSync(tmp, target);
            }

renameSync should only be called if the target file doesn't exist.