SONARJAVA-6769 Implement new rule S9342: Archive entries should not be empty - #5938
SONARJAVA-6769 Implement new rule S9342: Archive entries should not be empty#5938romainbrenguier wants to merge 3 commits into
Conversation
Detect empty archive entries where closeEntry() is called on a ZipOutputStream or JarOutputStream after putNextEntry() without any intervening write() call, which creates useless empty entries in the archive.
romainbrenguier
changed the title
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
…tor wrapping - Fix false positive when stream is passed to a helper method as argument (e.g., writeContent(zos)) by checking arguments when receiver is null - Fix false positive on ZIP directory entries (names ending with "/") - Fix false positive when stream is wrapped in a constructor (e.g., new PrintWriter(zos)) by adding visitNewClass to TrackedSymbolVisitor - Skip analysis when semantic model is unavailable to avoid false positives Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Code Review ✅ Approved 2 resolved / 2 findingsImplements the new S9342 rule to detect empty archive entries, while correctly resolving false positives on intentional ZIP directory entries and wrapper streams. No issues found. ✅ 2 resolved✅ Edge Case: False positive on intentional ZIP directory entries
✅ Edge Case: FP when stream content written via wrapper stream/writer
Implementation Status ✅ 1 / 1 issues implemented✅ SONARJAVA-6769 — 1 / 1 objectivesThe PR successfully implements the new rule S9342: Archive entries should not be empty, along with comprehensive test cases and rule metadata. ✅ 1 complete
OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
nathsou
left a comment
nathsou
left a comment
There was a problem hiding this comment.
I found three verified false-positive paths in S9342. The focused submitted test passes (mvn -pl java-checks -Dtest=EmptyArchiveEntryCheckTest test), but each case below produces an unexpected issue when analyzed with the new check.
|
|
||
| private void handleMethodInvocation(MethodInvocationTree mit, Map<Symbol, MethodInvocationTree> pendingEntries) { | ||
| Symbol receiver = getReceiverSymbol(mit); | ||
| if (receiver == null) { |
There was a problem hiding this comment.
Argument escape handling only runs when the invocation has no receiver. Consequently, this.writeContent(stream) (or helper.writeContent(stream)) leaves the entry pending and closeEntry() is reported even though the helper writes content. Apply the conservative argument clearing for other non-rule invocations too, and add a qualified-helper regression case.
| return null; | ||
| } | ||
|
|
||
| private static boolean isDirectoryEntry(MethodInvocationTree putNextEntry) { |
There was a problem hiding this comment.
The directory exemption only recognizes an inline new ZipEntry("dir/"). A common equivalent—constructing ZipEntry directory = new ZipEntry("dir/") first and passing directory to putNextEntry()—is reported as an empty file. Either recognize the entry value through the local symbol or conservatively avoid reporting when directory status cannot be established; add this form to the sample.
| reportIssue(ExpressionUtils.methodName(mit), "Write content to this archive entry; it is empty.", | ||
| Collections.singletonList(new JavaFileScannerContext.Location("Entry opened here", ExpressionUtils.methodName(putNextEntry))), null); | ||
| } | ||
| } else if (WRITE.matches(mit)) { |
There was a problem hiding this comment.
Pending entries are keyed only by the ZipOutputStream symbol. If an OutputStream writer = new BufferedOutputStream(stream) is created before putNextEntry(), writer.write(...) removes writer rather than stream, so a subsequent stream.closeEntry() is falsely reported. Track aliases/wrappers or conservatively clear the wrapped stream’s state, including for wrappers created before an entry opens.
2 participants
Detect empty archive entries where closeEntry() is called on a ZipOutputStream or JarOutputStream after putNextEntry() without any intervening write() call, which creates useless empty entries in the archive.