kratos0718 commented on issue #138: URL: https://github.com/apache/maven-shared-jar/issues/138#issuecomment-5322955039
I tried to fix this and ended up concluding the NPE isn't reachable — posting the working rather than opening a PR for it. `remove(ROOT)` can only return `null` if no entry is classified as ROOT. But: 1. `analyzeMultiRelease()` runs only when `jarData.isMultiRelease()` is true. 2. `multiRelease` is set from `isAttributePresent(ATTR_MULTI_RELEASE)` — read off the manifest, so a manifest must exist. 3. An existing manifest means `META-INF/MANIFEST.MF` is an entry, and `getEntries()` is unfiltered (`JarData.getEntries()` returns the full list built in the `JarAnalyzer` constructor). 4. `ENTRY_FILTER_MULTI_RELEASE` is `^META-INF/versions/([1-9]\d*)/.*$`, which `META-INF/MANIFEST.MF` does not match, so it falls through to `ROOT`. So whenever this method runs there is always at least one ROOT entry — the manifest itself. I checked it rather than just reasoning about it: I built a JAR carrying `Multi-Release: true` and nothing but `META-INF/versions/9/` content, ran the analysis, and got no NPE. `getRootEntries()` came back non-empty, holding the manifest. My regression test failed on its own premise, which is what sent me back to re-read the classification. The way to actually reach it would be a JAR whose manifest is absent while `isMultiRelease()` is still true, and I can't see a path to that through `JarFile`. Happy to add the null guard anyway if you'd like it as defence-in-depth — but as it stands it would be unreachable code, so I'd rather ask first than send a PR for it. -- 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]
