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

Fix info severity #18

Merged
merged 3 commits into from
Jul 9, 2018
Merged

Fix info severity #18

merged 3 commits into from
Jul 9, 2018

Conversation

mdix
Copy link
Contributor

@mdix mdix commented May 17, 2018

When the report contains vuln of type info, the detail & install reporters fail because ther's no info property in the severityColors object in utils.js. This issue was undiscovered because all fixtures had info: 0.

Use the following commits to reproduce locally with my fork
failing test
eb9d01b
implementation fixing it
4476b94

@welwood08
Copy link
Contributor

I was accidentally re-implementing a lot of what you've already done in several PRs here without checking first. So my time wasn't all wasted, I have a suggestion for this PR in particular. In lib/utils.js:

 function severityLabel (sev, withColor, bold) {
+  if (!(sev in severityColors)) return sev.charAt(0).toUpperCase() + sev.substr(1).toLowerCase()

This way it never crashes in the future if any unknown severity is added to the API's output.

@mdix mdix mentioned this pull request May 30, 2018
Copy link
Contributor

@zkat zkat left a comment

Choose a reason for hiding this comment

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

yay. This is a good find! Thanks for catching this <3

@zkat zkat merged commit 807db5a into npm:latest Jul 9, 2018
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.

3 participants