fix(config): close resource streams - #55
Federico2014 wants to merge 2 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 6 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="plugins/src/main/java/common/org/tron/plugins/utils/FileUtils.java">
<violation number="1" location="plugins/src/main/java/common/org/tron/plugins/utils/FileUtils.java:174">
P2: `copyTree` walks with `FileVisitOption.FOLLOW_LINKS`, and `copy` then uses `Files.copy`, which also follows links by default. A symlink inside a database directory that points outside the source tree copies/traverses content from outside the intended subtree, and a symlink cycle makes `Files.walk` throw `FileSystemLoopException`. Because `copyDatabases`/`copyDir` wrap any `IOException` in a `RuntimeException`, a single cycle aborts the entire multi-directory copy and leaves a partial destination in the backup/archive path. This behavior carried over from the pre-PR code, but the new shared `copyTree` is the right place to harden it: walk without following links (`Files.walk(Paths.get(src.toString(), dir))`), or switch to `Files.walkFileTree` with explicit `visitFile`/cycle detection if symlinked directories inside the tree are intentional.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| private static void copyTree(Path src, Path dest, String dir) throws IOException { | ||
| try (Stream<Path> paths = Files.walk( | ||
| Paths.get(src.toString(), dir), FileVisitOption.FOLLOW_LINKS)) { |
There was a problem hiding this comment.
P2: copyTree walks with FileVisitOption.FOLLOW_LINKS, and copy then uses Files.copy, which also follows links by default. A symlink inside a database directory that points outside the source tree copies/traverses content from outside the intended subtree, and a symlink cycle makes Files.walk throw FileSystemLoopException. Because copyDatabases/copyDir wrap any IOException in a RuntimeException, a single cycle aborts the entire multi-directory copy and leaves a partial destination in the backup/archive path. This behavior carried over from the pre-PR code, but the new shared copyTree is the right place to harden it: walk without following links (Files.walk(Paths.get(src.toString(), dir))), or switch to Files.walkFileTree with explicit visitFile/cycle detection if symlinked directories inside the tree are intentional.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At plugins/src/main/java/common/org/tron/plugins/utils/FileUtils.java, line 174:
<comment>`copyTree` walks with `FileVisitOption.FOLLOW_LINKS`, and `copy` then uses `Files.copy`, which also follows links by default. A symlink inside a database directory that points outside the source tree copies/traverses content from outside the intended subtree, and a symlink cycle makes `Files.walk` throw `FileSystemLoopException`. Because `copyDatabases`/`copyDir` wrap any `IOException` in a `RuntimeException`, a single cycle aborts the entire multi-directory copy and leaves a partial destination in the backup/archive path. This behavior carried over from the pre-PR code, but the new shared `copyTree` is the right place to harden it: walk without following links (`Files.walk(Paths.get(src.toString(), dir))`), or switch to `Files.walkFileTree` with explicit `visitFile`/cycle detection if symlinked directories inside the tree are intentional.</comment>
<file context>
@@ -138,43 +138,44 @@ public static boolean isSymbolicLink(File file) throws IOException {
+
+ private static void copyTree(Path src, Path dest, String dir) throws IOException {
+ try (Stream<Path> paths = Files.walk(
+ Paths.get(src.toString(), dir), FileVisitOption.FOLLOW_LINKS)) {
+ paths.forEach(source -> copy(source, dest.resolve(src.relativize(source))));
+ }
</file context>
| Paths.get(src.toString(), dir), FileVisitOption.FOLLOW_LINKS)) { | |
| try (Stream<Path> paths = Files.walk(Paths.get(src.toString(), dir))) { |
1 participant
What does this PR do?
Closes streams used for Git metadata and zk parameter loading, avoids opening a stream just to locate configuration resources, and closes directory-walk streams in plugin copy utilities and affected tests. Missing zk parameter resources now produce an explicit error.
Why are these changes required?
Unclosed streams can retain file or directory handles during repeated operations and cause resource exhaustion or platform-dependent failures.
This PR has been tested by:
Follow up
None.
Extra details
Fixes resource-stream handling on
release_v4.8.3. SkipsGetTransactionByIdSolidityServletTestbecause the release branch already replaced its file-based response handling, and preserves the updated error-response assertion inBroadcastServletTest.