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(pom): compare ArtifactIDs for base and parent pom's when relativePath field is used #267

Merged

Conversation

DmitriyLewen
Copy link
Collaborator

@DmitriyLewen DmitriyLewen commented Oct 26, 2023

Description

We need to compare ArtifactIDs for base and parent pom's when relativePath == ../pom.xml is used.
it is needed to avoid cases when we get incorrect parent or get infinity loop (if ../pom.xml is scanned pom.xml).

We can inherit GroupID. Also Version can contains property.
So we need to compare only ArtifactID.

New test contains simple example - https://github.com/DmitriyLewen/go-dep-parser/blob/7d2c865da8912f9f3ea475cc94da61c819dcae0d/pkg/java/pom/parse_test.go#L987-L999
Example from real life - aquasecurity/trivy/discussions/5445

Related Issues

@DmitriyLewen DmitriyLewen self-assigned this Oct 26, 2023
@DmitriyLewen DmitriyLewen requested a review from knqyf263 October 26, 2023 10:01
@DmitriyLewen DmitriyLewen marked this pull request as ready for review October 26, 2023 10:02
@@ -510,6 +510,13 @@ func (p *parser) tryRelativePath(parentArtifact artifact, currentPath, relativeP
return nil, err
}

// We can inherit GroupID from parent (requires p.analyze function to get GroupID)
// Version can contain a property (requires p.analyze function to get version)
// But ArtifactID must be the same for parent and base poms
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you mean? The parent artifact id and the base artifact id won't match.
Do you mean, "The artifact id of the parent pom in the base pom and the actual artifact id in the parent pom must match."?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right!

But i see that this comment may be confusing.
I updated comment in cedc6ce :
Take a look, please.

@knqyf263 knqyf263 merged commit 4548cca into aquasecurity:main Oct 30, 2023
1 check passed
@DmitriyLewen DmitriyLewen deleted the fix/infinity-loop-java-parent-pom branch October 30, 2023 05:07
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.

2 participants