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

Safer DOMStorage.js #2188

Merged
merged 2 commits into from Oct 3, 2017

Conversation

robertbryer
Copy link
Contributor

Don't trust the contents of local storage to be uncorrupted, assume parseInt can throw, parseFloat can throw, extend the try-catch around them.

@nicosang
Copy link
Contributor

Hi @robertbryer,
according to parseInt and ParseFloat documentation, thoses functions don't throw errors.....but JSON.parse can indeed throw an error. could you, please, update the DOMStorage unit test with a mock localStorage?
I have an other question why do you move the bracket in this line?
const obj = JSON.parse(localStorage.getItem(key) || {});

Thanks,
Nico

@robertbryer
Copy link
Contributor Author

Sorry, I'm getting confused here - I don't have the || {} in my branch!

@epiclabsDASH
Copy link
Contributor

@robertbryer, just to check with you. If you are working on adding localStorageMock and unit tests related with it, as suggested by @nicosang, please, merge your branch with development one. There are some recent changes in development branch related with enforcing coding rules for tests and that could break development branch compilation after merging this PR.

@epiclabsDASH epiclabsDASH merged commit ef49d9d into Dash-Industry-Forum:development Oct 3, 2017
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

3 participants