Skip to content

Use JSON.parse instead of safeEval - #71

Open
Adam-Clrk wants to merge 1 commit into
matheuss:masterfrom
Adam-Clrk:master
Open

Use JSON.parse instead of safeEval#71
Adam-Clrk wants to merge 1 commit into
matheuss:masterfrom
Adam-Clrk:master

Conversation

@Adam-Clrk

@Adam-Clrk Adam-Clrk commented Jun 23, 2018

Copy link
Copy Markdown

use of safeEval is considered a vulnerability

https://nodesecurity.io/advisories/337

@codecov-io

codecov-io commented Jun 23, 2018

Copy link
Copy Markdown

Codecov Report

Merging #71 into master will decrease coverage by 0.11%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #71      +/-   ##
=========================================
- Coverage   92.42%   92.3%   -0.12%     
=========================================
  Files           2       2              
  Lines          66      65       -1     
=========================================
- Hits           61      60       -1     
  Misses          5       5
Impacted Files Coverage Δ
index.js 90% <100%> (-0.2%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 777d7db...fd2d322. Read the comment docs.

@coveralls

coveralls commented Jun 23, 2018

Copy link
Copy Markdown

Coverage Status

Coverage increased (+1.4%) to 92.308% when pulling fd2d322 on Adam-Clrk:master into 777d7db on matheuss:master.

@alexis89x alexis89x left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jumped to the same conclusion after being warned bu GitHub that SafeEval is not secure.

@codenuts42 codenuts42 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes are done well.

@Norway174 Norway174 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it. And it works! Please approve of this.

@wadie wadie left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe someone should create a fork of this and keep it updated ?

@vitalets

Copy link
Copy Markdown

Maybe someone should create a fork of this and keep it updated ?

Done in vitalets#2.

@ArtanisTheOne ArtanisTheOne left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good

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.