google / osv-scanner

Vulnerability scanner written in Go which uses the data provided by https://osv.dev
https://google.github.io/osv-scanner/
Apache License 2.0
6.15k stars 347 forks source link

fix: only trim XML elements with no inner elements #1168

Closed cuixq closed 1 month ago

cuixq commented 1 month ago

This PR fixes two issues when reading/writing Maven manifest:

codecov-commenter commented 1 month ago

Codecov Report

Attention: Patch coverage is 33.33333% with 10 lines in your changes missing coverage. Please review.

Project coverage is 65.78%. Comparing base (8617d67) to head (6462956). Report is 5 commits behind head on main.

Files Patch % Lines
internal/manifest/maven.go 30.00% 4 Missing and 3 partials :warning:
internal/resolution/manifest/maven.go 40.00% 0 Missing and 3 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1168 +/- ## ========================================== + Coverage 65.65% 65.78% +0.13% ========================================== Files 164 165 +1 Lines 13750 13898 +148 ========================================== + Hits 9027 9143 +116 - Misses 4229 4251 +22 - Partials 494 504 +10 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

michaelkedar commented 1 month ago
  • when merging parents, does not check parent.groupId since Maven allows this to happen;

I don't think this is quite the correct behaviour. e.g. if you say your parent's groupId is org.foo but the parent pom says org.bar Maven throws an error.

The problem we are seeing is that the groupId is inherited from the parent if it is missing. So we'd need to add some logic somewhere like

if proj.GroupID == "" {
  proj.GroupID = parent.GroupID
}
cuixq commented 1 month ago

thanks for checking that @michaelkedar !