Skip to content

fix(fossid-webapp): Return empty PURL when artifact name is empty#11537

Open
nnobelis wants to merge 1 commit intooss-review-toolkit:mainfrom
boschglobal:nnobelis/fix_fossid_purl
Open

fix(fossid-webapp): Return empty PURL when artifact name is empty#11537
nnobelis wants to merge 1 commit intooss-review-toolkit:mainfrom
boschglobal:nnobelis/fix_fossid_purl

Conversation

@nnobelis
Copy link
Member

@nnobelis nnobelis commented Mar 5, 2026

The packageurl-jvm library throws MalformedPackageURLException when the name component is empty. This can happen for snippets with empty artifact names.
Return an empty string early when the name is blank.

This is a fixup for f1eadd8.

@nnobelis nnobelis requested a review from a team as a code owner March 5, 2026 12:58
@nnobelis nnobelis enabled auto-merge (rebase) March 5, 2026 12:58
@nnobelis nnobelis force-pushed the nnobelis/fix_fossid_purl branch from e44b210 to 8f3820a Compare March 5, 2026 13:23
.withVersion(snippet.version)
.build()
.canonicalize()
val snippetPurl = snippet.purl
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to inline this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Writing:

when {
              snippet.purl != null -> snippet.purl

Does not work because it returns String? (Kotlin cannot make a smart cast to String maybe ?)

Copy link
Member

Choose a reason for hiding this comment

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

Weird that smart-cast does not work in this case. Then I'd prefer

snippet.purl != null -> checkNotNull(snippet.purl)

@sschuberth
Copy link
Member

This is a fixup for 334810d.

Is this really a fixup for that commit? Because this change is specific to FossID.

@maennchen, should you commit 334810d maybe have been Pub-specific as well? In what context exactly did you come across the issue?

@codecov
Copy link

codecov bot commented Mar 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.97%. Comparing base (6159101) to head (5b1ebe1).

Additional details and impacted files
@@            Coverage Diff            @@
##               main   #11537   +/-   ##
=========================================
  Coverage     57.97%   57.97%           
  Complexity     1730     1730           
=========================================
  Files           349      349           
  Lines         12993    12993           
  Branches       1263     1263           
=========================================
  Hits           7533     7533           
  Misses         4995     4995           
  Partials        465      465           
Flag Coverage Δ
funTest-external-tools 14.18% <ø> (ø)
funTest-no-external-tools 30.87% <ø> (ø)
test-ubuntu-24.04 42.55% <ø> (ø)
test-windows-2025 42.53% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nnobelis nnobelis requested a review from sschuberth March 5, 2026 13:44
@nnobelis
Copy link
Member Author

nnobelis commented Mar 5, 2026

Is this really a fixup for that commit? Because this change is specific to FossID.

It's a fixup for c14f781 maybe?

@maennchen
Copy link
Contributor

@nnobelis f1eadd8 would make more sense.

@sschuberth The commit 334810d was specifically targeting a failing test in the Pub Plugin.

The fix here makes sense. It was already wrong before f1eadd8 since it would've created an invalid purl without a name. It would probably be good to add a test for this specific behavior.

The packageurl-jvm library throws MalformedPackageURLException when
the name component is empty. This can happen for snippets with empty
artifact names.
Return an empty string early when the name is blank.

This is a fixup for f1eadd8.

Signed-off-by: Nicolas Nobelis <nicolas.nobelis@bosch.com>
@nnobelis nnobelis force-pushed the nnobelis/fix_fossid_purl branch from 8f3820a to 5b1ebe1 Compare March 5, 2026 15:00
@sschuberth
Copy link
Member

sschuberth commented Mar 5, 2026

@sschuberth The commit 334810d was specifically targeting a failing test in the Pub Plugin.

@maennchen, so my point is, instead of

// Avoid a `MalformedPackageURLException` and behave like for `Identifier.EMPTY` in case of exotic packages like
// Pub SDK packages (e.g., sky_engine) where the name is not explicitly set.
if (name.isEmpty()) return ""

should we rather change something in the Pub analyzer so that such invalid PURLs are not being created in the first place?

@maennchen
Copy link
Contributor

@sschuberth I'm not a 100% sure anymore from the top of my head. But I think Pub is creating an Identifier with no name and not a purl directly. But fixing the Identifier name would for sure be good.

@sschuberth
Copy link
Member

But fixing the Identifier name would for sure be good.

See #11538.

Comment on lines +168 to +169
mappedSnippets shouldHaveSize 1
mappedSnippets.first().apply {
Copy link
Member

Choose a reason for hiding this comment

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

This can be combined as mappedSnippets.shouldBeSingleton {.

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