imbajin commented on code in PR #2918:
URL:
https://github.com/apache/incubator-hugegraph/pull/2918#discussion_r2618200638
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/util/CompressUtil.java:
##########
@@ -173,6 +173,21 @@ private static Path zipSlipProtect(ArchiveEntry entry,
Path targetDir)
return normalizePath;
}
+ private static Path zipSlipProtect(ZipEntry entry, Path targetDir)
Review Comment:
⚠️ **Code Quality: Redundant zipSlipProtect Method**
The new `zipSlipProtect(ZipEntry, Path)` method duplicates the existing
`zipSlipProtect(ArchiveEntry, Path)` method's logic. Both perform identical
path traversal validation.
**Recommendation:** Extract common logic to avoid duplication:
```suggestion
private static Path zipSlipProtect(String entryName, Path targetDir)
throws IOException {
Path targetDirResolved = targetDir.resolve(entryName);
Path normalizePath = targetDirResolved.normalize();
if (!normalizePath.startsWith(targetDir.normalize())) {
throw new IOException(String.format("Bad entry: %s", entryName));
}
return normalizePath;
}
private static Path zipSlipProtect(ArchiveEntry entry, Path targetDir)
throws IOException {
return zipSlipProtect(entry.getName(), targetDir);
}
private static Path zipSlipProtect(ZipEntry entry, Path targetDir)
throws IOException {
return zipSlipProtect(entry.getName(), targetDir);
}
```
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/util/CompressUtil.java:
##########
@@ -220,9 +235,7 @@ public static void decompressZip(String sourceFile, String
outputDir,
ZipInputStream zis = new ZipInputStream(bis)) {
ZipEntry entry;
while ((entry = zis.getNextEntry()) != null) {
- String fileName = entry.getName();
- File entryFile = new File(Paths.get(outputDir, fileName)
- .toString());
+ File entryFile = new File(zipSlipProtect(entry,
Paths.get(outputDir)).toString());
Review Comment:
✅ **Good: Zip Slip Protection Applied**
Good security improvement! Applying `zipSlipProtect()` to the ZipEntry path
prevents directory traversal attacks during ZIP decompression.
Minor style suggestion for consistency with existing code pattern:
```suggestion
Path targetPath = zipSlipProtect(entry,
Paths.get(outputDir));
File entryFile = new File(targetPath.toString());
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
