From 91e3597ca7e444e6b81e19d6615336b516cbd3f3 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 29 Feb 2024 23:53:16 +0100 Subject: [PATCH] Support files opened via `Files#newBufferedWriter` (#161) Co-authored-by: Basil Crow --- pom.xml | 11 +++++++ .../kohsuke/file_leak_detector/AgentMain.java | 21 +++++++++++++ .../kohsuke/file_leak_detector/Listener.java | 12 ++++++++ .../instrumented/FileDemo.java | 30 +++++++++++++++++++ 4 files changed, 74 insertions(+) diff --git a/pom.xml b/pom.xml index 1343c9b..4856ce8 100644 --- a/pom.xml +++ b/pom.xml @@ -43,6 +43,17 @@ args4j 2.33 + + com.github.spotbugs + spotbugs-annotations + true + + + com.google.code.findbugs + jsr305 + + + org.ow2.asm asm diff --git a/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java b/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java index 3a84303..fe1e983 100644 --- a/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java +++ b/src/main/java/org/kohsuke/file_leak_detector/AgentMain.java @@ -2,6 +2,7 @@ import java.io.BufferedReader; import java.io.File; +import java.io.FileDescriptor; import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.FileOutputStream; @@ -146,6 +147,7 @@ public void run() { Files.class); addIfFound(classes, "sun.nio.ch.SocketChannelImpl"); + addIfFound(classes, "sun.nio.ch.FileChannelImpl"); addIfFound(classes, "java.net.AbstractPlainSocketImpl"); addIfFound(classes, "java.net.PlainSocketImpl"); addIfFound(classes, "sun.nio.fs.UnixDirectoryStream"); @@ -370,6 +372,25 @@ static List createSpec() { "(Ljava/nio/channels/spi/SelectorProvider;Ljava/io/FileDescriptor;Ljava/net/InetSocketAddress;)V"), new OpenSocketInterceptor("", "(Ljava/nio/channels/spi/SelectorProvider;)V"), new CloseInterceptor("kill"))); + spec.add(new ClassTransformSpec( + "sun/nio/ch/FileChannelImpl", + new ReturnFromStaticMethodInterceptor( + "open", + "(Ljava/io/FileDescriptor;Ljava/lang/String;ZZZLjava/io/Closeable;)Ljava/nio/channels/FileChannel;", + 4, + "openFileString", + Object.class, + FileDescriptor.class, + String.class), + // this is for java 11/17 - which use Object instead of Closeable as the last parameter + new ReturnFromStaticMethodInterceptor( + "open", + "(Ljava/io/FileDescriptor;Ljava/lang/String;ZZZLjava/lang/Object;)Ljava/nio/channels/FileChannel;", + 4, + "openFileString", + Object.class, + FileDescriptor.class, + String.class))); return spec; } diff --git a/src/main/java/org/kohsuke/file_leak_detector/Listener.java b/src/main/java/org/kohsuke/file_leak_detector/Listener.java index b8d6b87..bc048c9 100644 --- a/src/main/java/org/kohsuke/file_leak_detector/Listener.java +++ b/src/main/java/org/kohsuke/file_leak_detector/Listener.java @@ -1,6 +1,8 @@ package org.kohsuke.file_leak_detector; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.File; +import java.io.FileDescriptor; import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.OutputStream; @@ -21,6 +23,7 @@ import java.nio.charset.Charset; import java.nio.file.DirectoryStream; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Date; import java.util.LinkedHashMap; @@ -343,6 +346,15 @@ public static synchronized void open(Object _this, Path p) { } } + @SuppressFBWarnings( + value = "PATH_TRAVERSAL_IN", + justification = "path comes from sun.nio.fs.UnixChannelFactory.newFileChannel(int, sun.nio.fs.UnixPath, " + + "java.lang.String, java.util.Set, int). At this point, the path " + + "is not controlled by the user.") + public static synchronized void openFileString(Object _this, FileDescriptor fileDescriptor, String path) { + open(_this, Paths.get(path)); + } + /** * Called when a pipe is opened, e.g. via SelectorProvider * diff --git a/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java b/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java index 1aae663..7dedd99 100644 --- a/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java +++ b/src/test/java/org/kohsuke/file_leak_detector/instrumented/FileDemo.java @@ -8,6 +8,8 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import java.io.BufferedReader; +import java.io.BufferedWriter; import java.io.File; import java.io.FileInputStream; import java.io.IOException; @@ -135,6 +137,34 @@ public void openCloseFile() throws Exception { assertThat(traceOutput, containsString("Closed " + tempFile)); } + @Test + public void openCloseFilesBufferedWriter() throws Exception { + try (BufferedWriter writer = Files.newBufferedWriter(tempFile.toPath())) { + assertNotNull(writer); + assertNotNull("No file record for file=" + tempFile + " found", findPathRecord(tempFile.toPath())); + assertThat("Did not have the expected type of 'marker' object: " + obj, obj, instanceOf(FileChannel.class)); + } + assertNull("File record for file=" + tempFile + " not removed", findPathRecord(tempFile.toPath())); + + String traceOutput = output.toString(); + assertThat(traceOutput, containsString("Opened " + tempFile)); + assertThat(traceOutput, containsString("Closed " + tempFile)); + } + + @Test + public void openCloseFilesBufferedReader() throws Exception { + try (BufferedReader reader = Files.newBufferedReader(tempFile.toPath())) { + assertNotNull(reader); + assertNotNull("No file record for file=" + tempFile + " found", findPathRecord(tempFile.toPath())); + assertThat("Did not have the expected type of 'marker' object: " + obj, obj, instanceOf(FileChannel.class)); + } + assertNull("File record for file=" + tempFile + " not removed", findPathRecord(tempFile.toPath())); + + String traceOutput = output.toString(); + assertThat(traceOutput, containsString("Opened " + tempFile)); + assertThat(traceOutput, containsString("Closed " + tempFile)); + } + @Test public void openCloseFileChannel() throws Exception { try (FileChannel fileChannel = FileChannel.open(tempFile.toPath(), StandardOpenOption.APPEND)) {