Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Defend against various internal errors with object notation #129

Merged
merged 1 commit into from Oct 8, 2014
Merged

Defend against various internal errors with object notation #129

merged 1 commit into from Oct 8, 2014

Conversation

oliversalzburg
Copy link
Contributor

These changes simply allow us to bail out of the whole method when an inappropriate input is detected. Otherwise, the functions might cause errors like Cannot call method 'hasOwnProperty' of undefined.
The above case can happen when, for example, there's a syntax error in a locale .json file.

Returning a noop when the locale wasn't found, will ultimately result in the locale being written. So, if there was an error in the JSON file (trailing comma, string not closed, …), the whole file will be overwritten. Personally, I consider this a benefit, because it indicates to me that there was a problem with the locale file. If you have no copies of the file (in VCS or wherever), it would probably suck.

To prevent all of this happening, it might be wise to check locales[locale] after it was read (This is already happening, but the final read of defaultLocale is never checked). For reference: https://github.com/oliversalzburg/i18n-node/blob/0711eb7591c6b961cb1bc927ae1dcbf148b29752/i18n.js#L438

These changes simply allow us to bail out of the whole method when an inappropriate input is detected. Otherwise, the functions might cause errors like `Cannot call method 'hasOwnProperty' of undefined`.
The above case can happen when, for example, there's a syntax error in a locale .json file.
mashpie added a commit that referenced this pull request Oct 8, 2014
Defend against various internal errors with object notation
@mashpie mashpie merged commit 5311e67 into mashpie:master Oct 8, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants