Skip to content

fix(projects): prevent data corruption and crashes in attachment usage inheritance#3773

Open
Aman-Cool wants to merge 1 commit intoeclipse-sw360:mainfrom
Aman-Cool:fix/attachment-usages-inheritance
Open

fix(projects): prevent data corruption and crashes in attachment usage inheritance#3773
Aman-Cool wants to merge 1 commit intoeclipse-sw360:mainfrom
Aman-Cool:fix/attachment-usages-inheritance

Conversation

@Aman-Cool
Copy link
Contributor

Description

Fixes three critical bugs in attachment usage inheritance that cause data corruption, service crashes, and compliance violations. When INHERIT_ATTACHMENT_USAGES is enabled, projects with component-level attachments write corrupted data to CouchDB, deleted sub-projects cause NullPointerException, and circular project references trigger StackOverflowError that kills service threads.

The primary bug: parseAttachmentUsages() incorrectly assumes all attachment owners are releases. Thrift's Source union can also be components or projects, but calling getReleaseId() on a non-release Source silently returns null. This null gets written to CouchDB as Source.releaseId(null), corrupting AttachmentUsage documents and breaking downstream license report generation.

Related Issue

N/A

Changes Made

1. Null owner corruption fix (parseAttachmentUsages - primary bug)

  • Added isSetReleaseId() check before accessing release ID from Thrift Source union
  • Skip attachment usages with non-release owners instead of writing null IDs to CouchDB
  • Prevents corruption of compliance-critical AttachmentUsage documents

2. NPE fix (buildProjectPaths - deleted sub-projects)

  • Added null guard for repository.get() result
  • Handles deleted or inaccessible sub-project references gracefully

3. StackOverflowError fix (buildProjectPaths - circular references)

  • Added cycle detection with Set<String> visited parameter
  • Follows pattern from getReleaseIdsOfProjectTree() in same file
  • Prevents infinite recursion on circular project links

File changed: backend/common/src/main/java/org/eclipse/sw360/datahandler/db/ProjectDatabaseHandler.java (~15 lines)

Testing

  • ✅ Manual test performed with component-level attachments (non-release Source owners)
  • ✅ Verified no null release IDs written to CouchDB
  • ✅ Tested with deleted sub-project references - no NPE
  • ✅ Tested circular project links - no StackOverflowError
  • ⭕ Unit test (existing tests cover this code path)

Impact

  • Data integrity: Prevents silent corruption of AttachmentUsage documents with null release IDs
  • Compliance: License disclosure reports complete successfully instead of crashing or omitting obligations
  • Stability: Deleted sub-projects no longer cause NPE during inheritance
  • Availability: Circular project graphs no longer kill service threads with StackOverflowError
  • Trust: Maintains OSS compliance accuracy for projects using attachment usage inheritance
  • No breaking changes to API or data model

Checklist

  • ✅ Code follows project style guidelines
  • ✅ All existing tests pass
  • ✅ Signed-off-by added to commit (git commit -s)
  • ✅ Commit message follows conventional format: fix(projects): prevent data corruption and crashes in attachment usage inheritance
  • ✅ No new files added (existing file modified)

Additional Notes

This commit fixes three related bugs in the attachment usage inheritance feature:

Primary bug: Thrift Source union type safety issue - Apache Thrift's Java code generation allows calling getReleaseId() on a Source union whose active field is componentId or projectId, silently returning null instead of throwing an error. The correct API (isSetReleaseId()) exists but wasn't used. This demonstrates a key challenge the Thrift migration project aims to solve - with sealed interfaces or discriminated unions in pure Java/Spring, this would be a compile-time error.

Secondary bugs: Missing defensive checks - both the null guard and cycle detection follow patterns already established in getReleaseIdsOfProjectTree() in the same file, but were omitted in the new code.

Production impact: Silent data corruption in compliance-critical license documents, service crashes from NPE, and thread pool exhaustion from StackOverflowError.

@Aman-Cool
Copy link
Contributor Author

@GMishx @amritkv, Fixed three bugs in attachment usage inheritance:

  1. parseAttachmentUsages was writing null release IDs to CouchDB when processing
    component-level attachments. The Thrift Source union can be a component, but
    getReleaseId() returns null silently instead of erroring. Added the isSetReleaseId()
    check that was missing.

  2. buildProjectPaths crashed with NPE when a sub-project was deleted - added null guard.

  3. Circular project links caused StackOverflowError and killed service threads - added
    cycle detection like getReleaseIdsOfProjectTree() already has.

Let me know if you need any changes.

@GMishx GMishx added needs code review needs general test This is general testing, meaning that there is no org specific issue to check for labels Mar 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs code review needs general test This is general testing, meaning that there is no org specific issue to check for

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants