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: crash when analyzing node package with a local dependency #5235

Conversation

rturner-edjuster
Copy link
Contributor

Fixes Issue #1947

Description of Change

When a package has a local dependency, the package-lock.json file contains an entry without a version number, with "link": "true". This lack of version attribute causes the NodePackageAnalyzer class to crash.

These entries should be skipped instead as they cannot be processed.

Have test cases been added to cover the new functionality?

yes

Issue jeremylong#1947

When a package has a local dependency, the package-lock.json file
contains an entry without a version number, with "link": "true". This
lack of version attribute causes the NodePackageAnalyzer class to crash.

These entries should be skipped instead as they cannot be processed.
@boring-cyborg boring-cyborg bot added core changes to core tests test cases labels Jan 6, 2023
@rturner-edjuster rturner-edjuster changed the title fix:crash when analyzing node package with a local dependency fix: crash when analyzing node package with a local dependency Jan 6, 2023
@rturner-edjuster
Copy link
Contributor Author

@jeremylong Let me know if there is anything else I can do on this? (like update to merge in latest master, etc).

@jeremylong
Copy link
Owner

Thanks! The change is good as is - I'm just debating on the release schedule. I'll likely put in one more release before 8.0.0.

@jeremylong jeremylong added this to the 8.0.0 milestone Jan 10, 2023
@jeremylong jeremylong merged commit 66e583b into jeremylong:main Jan 10, 2023
@rturner-edjuster rturner-edjuster deleted the bugfix/issue-1947-fix-crash-for-local-packages branch January 10, 2023 17:30
@rturner-edjuster
Copy link
Contributor Author

@jeremylong I would consider this for a patch -- it's pretty debilitating. We've had to completely disable use of DependencyCheck until it's resolved (largely in combination with the need for the fix for the field length + data issue that you released a fix for recently).

@MenschNestor
Copy link

Seconded regarding the patch release – we were caught between a rock and a hard place because we needed the 7.4.4 fix for using DependencyCheck at all and then had to disable the Node package analyzer or we would have had no dependency check at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core changes to core tests test cases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants