SONARJAVA-6769 Implement new rule S9342: Archive entries should not be empty

[romainbrenguier]

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.

[hashicorp-vault-sonar-prod]

SONARJAVA-6769

[nathsou]

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.

[nathsou]

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.

[nathsou]

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.

[nathsou]

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.

[nathsou]

Approving — found two false-negative gaps worth tracking, not blocking:

[nathsou]

putNextEntry/closeEntry correlation is scoped to a single BLOCK's immediate statement list, so the common try/finally idiom (putNextEntry() in the outer block, closeEntry() inside a nested finally/if block) is never analyzed, silently missing genuinely empty archive entries.

Example: zos.putNextEntry(new ZipEntry("empty.txt")); try { } finally { zos.closeEntry(); } creates a truly empty entry, but the rule never reports it — the nested block gets its own fresh, empty pendingEntries map in visitNode, and TrackedSymbolVisitor never treats CLOSE_ENTRY as clearing/using a tracked symbol.

[nathsou]

TrackedSymbolVisitor.visitNewClass clears a pending entry as soon as the tracked stream is passed to any constructor, even if the resulting wrapper's write method is never actually called.

Example: zos.putNextEntry(new ZipEntry("file.txt")); OutputStream w = new BufferedOutputStream(zos); zos.closeEntry(); (wrapper constructed but w.write(...) never invoked) produces a genuinely empty entry, yet this removes zos from pendingEntries merely because it appears as a constructor argument — a false negative for the exact bug the rule targets.

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
91.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

[gitar-bot]

Code Review ✅ Approved 2 resolved / 2 findings

Implements rule S9342 to detect empty archive entries in Zip and Jar output streams, addressing false positives on directory entries and wrapper streams. No issues found.

✅ 2 resolved

✅ Edge Case: False positive on intentional ZIP directory entries

📄 java-checks/src/main/java/org/sonar/java/checks/EmptyArchiveEntryCheck.java:85-92
ZIP/JAR directory entries are created intentionally with putNextEntry(new ZipEntry("dir/")) followed immediately by closeEntry() and no write() call — an empty entry is the correct and expected representation of a directory. The check flags this common, legitimate pattern as noncompliant. Consider excluding entries whose name ends with "/" (or otherwise detecting directory entries) and add a compliant test case covering directory entries.

✅ Edge Case: FP when stream content written via wrapper stream/writer

📄 java-checks/src/main/java/org/sonar/java/checks/EmptyArchiveEntryCheck.java:143-157
TrackedSymbolVisitor only clears a tracked entry when the stream is used in a method invocation (write(...) or passed as a method argument), but it never overrides visitNewClass. A very common pattern wraps the stream in a constructor — e.g. new PrintWriter(zos), new BufferedOutputStream(zos), new ObjectOutputStream(zos) — and writes through the wrapper before closeEntry(). Since zos is passed to a constructor (NewClassTree), not a method, the entry is never cleared and a false positive is raised. Override visitNewClass to also treat tracked symbols passed as constructor arguments as "used", and add a test case for wrapper-based writing.

Implementation Status ✅ 1 / 1 issues implemented

✅ SONARJAVA-6769 — 1 / 1 objectives

The PR implements the new rule S9342 regarding empty archive entries.

✅ 1 complete

• ✅ Implement new rule S9342: Archive entries should not be empty

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}