gnodet-bot commented on code in PR #28313:
URL: https://github.com/apache/camel/pull/28313#discussion_r4353211464
##########
core/camel-support/src/main/java/org/apache/camel/support/scan/DefaultPackageScanClassResolver.java:
##########
@@ -344,8 +344,12 @@ protected List<String> doLoadJarClassEntries(InputStream
stream, String urlPath)
*/
private void doLoadImplementationsInJar(
PackageScanFilter test, String parent, List<String> entries,
Set<Class<?>> classes) {
+ // only entries inside the parent package (or its sub packages)
qualify, not entries of a sibling
+ // package whose name merely starts with the same characters
(com/acme/routes vs com/acme/routesupport),
+ // which is also what the directory based scanning does
+ String prefix = parent == null || parent.isEmpty() ||
parent.endsWith("/") ? parent : parent + "/";
for (String entry : entries) {
- if (entry.startsWith(parent)) {
+ if (prefix == null || entry.startsWith(prefix)) {
addIfMatching(test, entry, classes);
Review Comment:
⚠️ **Unnecessary null guard introduces silent "scan everything" semantic**
The ternary sets `prefix = parent` (i.e. `null`) when `parent == null`, and
then `prefix == null || entry.startsWith(prefix)` passes **all** entries —
silently scanning the entire JAR. The original code would have thrown
`NullPointerException` on `entry.startsWith(null)`, which at least surfaced the
miscall.
In practice `parent` is never null here — this method is private, its only
call site is `loadImplementationsInJar`, and its `parent` argument always comes
from `find()` which produces `""` for the root package rather than `null`. So
the null branch is dead code. But it adds complexity that could mask a future
refactoring bug.
Simpler and safer — drop the null arm entirely:
```suggestion
// only entries inside the parent package (or its sub packages)
qualify, not entries of a sibling
// package whose name merely starts with the same characters
(com/acme/routes vs com/acme/routesupport),
// which is also what the directory based scanning does
String prefix = parent.isEmpty() || parent.endsWith("/") ? parent :
parent + "/";
for (String entry : entries) {
if (entry.startsWith(prefix)) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
