fix: guard against null LocalArtifactResult in DefaultNode.getRepository() - #13162
Conversation
…ory() Forward-port of hardening applied to the maven-4.0.x backport (PR apache#13159): - Use Optional.ofNullable(result).map(...) chain instead of result.getRepository() directly, protecting against third-party LRM implementations returning null - Strengthen tests with ArgumentCaptor to verify LocalArtifactRequest content - Add testGetRepositoryReturnsEmptyWhenLrmReturnsNull to cover the null-result path
gnodet-bot
left a comment
gnodet-bot
left a comment
There was a problem hiding this comment.
The fix is correct and the new test coverage is solid.
DefaultNode.java: Replacing Optional.ofNullable(result.getRepository()) with the Optional.ofNullable(result).map(...) chain is the right defensive move. LocalRepositoryManager.find() has no @Nonnull contract guarantee in the resolver API, so a third-party LRM returning null from find() would have NPE'd on the original code. The SimpleLocalRepositoryManager always returns non-null, so this is a safe guard for custom implementations.
DefaultNodeTest.java: The ArgumentCaptor additions properly verify that the LocalArtifactRequest is built with the correct artifact and repository list — previously any(LocalArtifactRequest.class) accepted anything silently. The new testGetRepositoryReturnsEmptyWhenLrmReturnsNull test explicitly covers the null-result path that the fix is protecting against.
Metadata: No milestone is set — this targets master, so milestone 4.1.0 (milestone 25) would be appropriate.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
2 participants
Forward-port of hardening applied during review of #13159 (maven-4.0.x backport).
Changes
DefaultNode.javaReplace
Optional.ofNullable(result.getRepository())with a properOptional.ofNullable(result).map(...)chain.LocalRepositoryManager.find()is not annotated@Nonnullin the resolver API contract — a third-party LRM implementation returningnullwould NPE on the original code. The standardSimpleLocalRepositoryManageralways returns a non-null result, so this is a defensive guard for custom implementations.DefaultNodeTest.javalrm.find()now useArgumentCaptor<LocalArtifactRequest>and assert that the captured request carries the correct artifact and repositories list. Previouslyany(LocalArtifactRequest.class)silently accepted any request.testGetRepositoryReturnsEmptyWhenLrmReturnsNullto explicitly cover the null-result path.