Skip to content

fix: handle null package.json name when parsing and matching #1877

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

Merged
merged 3 commits into from
Jun 18, 2025

Conversation

rlmestre
Copy link
Contributor

Closes #1876

@rlmestre rlmestre requested a review from prabhu as a code owner June 17, 2025 19:56
const match = (typeof name === "object" ? name.name || "" : name || "").match(
nameRegExp,
);
const name = (typeof name === "object" && name) ? name.name || "" : name || "";
Copy link
Collaborator

Choose a reason for hiding this comment

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

chatgpt is suggesting like this. wdyt?

// ensure we end up with a string, whether name is an object or a primitive
const processedName = typeof name === 'object'
  ? name.name ?? ''
  : name ?? '';

// use exec if you only need the first match (avoids oddities with global regexes)
const match = nameRegExp.exec(processedName);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would have the same issue of not handling nulls properly. typeof name === 'object' is true, so when it lands in the first branch of the ternary, name.name errors like it does now. Alternatively, if the project doesn't need to support older runtimes and/or TS downlevels enough, we can do just name?.name

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah, looks like we cannot reuse the variable name here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

don't know why I am vibe coding. how about this one?

// normalize to a string, safely handling null/undefined and objects
const safeName = String(name?.name ?? name ?? '');

// if you only need the first match, exec is a bit more predictable than match
const match = nameRegExp.exec(safeName);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I skipped the String() conversion because the chain should handle nullish values and fall back to '' if needed

Copy link
Collaborator

@prabhu prabhu left a comment

Choose a reason for hiding this comment

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

Thank you!

@prabhu
Copy link
Collaborator

prabhu commented Jun 17, 2025

Is there a unit test or repo test we could add?

@prabhu
Copy link
Collaborator

prabhu commented Jun 18, 2025

@rlmestre any thoughts about unit tests and the proposed shorter fix?

@rlmestre rlmestre force-pushed the fix/handle-malformed-package-json branch from e1e8457 to bccdf53 Compare June 18, 2025 12:41
@rlmestre
Copy link
Contributor Author

Added tests and simplified the code

@prabhu
Copy link
Collaborator

prabhu commented Jun 18, 2025

@rlmestre could you kindly rebase? Apologies, we had some major refactoring in other PRs.

rlmestre added 3 commits June 18, 2025 09:36
Signed-off-by: Rafael Mestre <277805+rlmestre@users.noreply.github.com>
Signed-off-by: Rafael Mestre <277805+rlmestre@users.noreply.github.com>
Signed-off-by: Rafael Mestre <277805+rlmestre@users.noreply.github.com>
@rlmestre rlmestre force-pushed the fix/handle-malformed-package-json branch from bccdf53 to 4b2d41b Compare June 18, 2025 13:37
@prabhu prabhu merged commit 15e4734 into CycloneDX:master Jun 18, 2025
72 checks passed
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.

package.json with missing name property unhandled in some scenarios
2 participants