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.

Sign up for GitHub

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

Jump to bottom

Use JSON.parse instead of safeEval #71

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use JSON.parse instead of safeEval #71

wants to merge 1 commit into from

Conversation

Adam-Clrk
Copy link

@Adam-Clrk Adam-Clrk commented Jun 23, 2018 •
edited
Loading

use of safeEval is considered a vulnerability

https://nodesecurity.io/advisories/337

@codecov-io
Copy link

codecov-io commented Jun 23, 2018 •
edited
Loading

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
Copy link

coveralls commented Jun 23, 2018 •
edited
Loading

Coverage Status

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

alexis89x
alexis89x approved these changes Jul 19, 2018
View reviewed changes
Copy link

@alexis89x alexis89x left a comment

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.

vorwieger
vorwieger approved these changes Aug 29, 2018
View reviewed changes
Copy link

@vorwieger vorwieger left a comment

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.

@vorwieger vorwieger mentioned this pull request Aug 29, 2018
Open
Norway174
Norway174 approved these changes Sep 3, 2018
View reviewed changes
Copy link

@Norway174 Norway174 left a comment

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 approved these changes Sep 25, 2018
View reviewed changes
Copy link

@wadie wadie left a comment

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 vitalets mentioned this pull request Feb 13, 2019
Merged
@vitalets
Copy link

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

Done in vitalets#2.

ArtanisTheOne
ArtanisTheOne approved these changes Dec 23, 2020
View reviewed changes
Copy link

@ArtanisTheOne ArtanisTheOne left a comment

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
Reviewers

@ArtanisTheOne ArtanisTheOne ArtanisTheOne approved these changes

@remzh remzh remzh approved these changes

@alexis89x alexis89x alexis89x approved these changes

@dennismuench dennismuench dennismuench approved these changes

@vorwieger vorwieger vorwieger approved these changes

@Norway174 Norway174 Norway174 approved these changes

@wadie wadie wadie approved these changes

@gucong3000 gucong3000 gucong3000 approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.