8162989: jshell tool: /edit with external editor leaks files in /tmp
authorrfield
Mon, 11 Dec 2017 21:56:34 -0800
changeset 48286 745ea7d5039a
parent 48285 7e8a0c4ee95e
child 48287 e53948132278
8162989: jshell tool: /edit with external editor leaks files in /tmp Reviewed-by: jlahoda
src/jdk.internal.ed/share/classes/jdk/internal/editor/external/ExternalEditor.java
test/langtools/jdk/jshell/CustomEditor.java
test/langtools/jdk/jshell/ExternalEditorTest.java
--- a/src/jdk.internal.ed/share/classes/jdk/internal/editor/external/ExternalEditor.java	Mon Dec 11 15:17:03 2017 -0800
+++ b/src/jdk.internal.ed/share/classes/jdk/internal/editor/external/ExternalEditor.java	Mon Dec 11 21:56:34 2017 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -29,10 +29,13 @@
 import java.nio.charset.Charset;
 import java.nio.file.ClosedWatchServiceException;
 import java.nio.file.FileSystems;
+import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
 import java.nio.file.WatchKey;
 import java.nio.file.WatchService;
+import java.nio.file.attribute.BasicFileAttributes;
 import java.util.Arrays;
 import java.util.Scanner;
 import java.util.function.Consumer;
@@ -104,6 +107,8 @@
             launch(cmd);
         } catch (IOException ex) {
             errorHandler.accept(ex.getMessage());
+        } finally {
+            deleteDirectory();
         }
     }
 
@@ -189,4 +194,31 @@
             errorHandler.accept("Failure in read edit file: " + ex.getMessage());
         }
     }
+
+    private void deleteDirectory() {
+        try {
+            Files.walkFileTree(dir, new SimpleFileVisitor<Path>() {
+                @Override
+                public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
+                        throws IOException {
+                    Files.delete(file);
+                    return FileVisitResult.CONTINUE;
+                }
+
+                @Override
+                public FileVisitResult postVisitDirectory(Path directory, IOException fail)
+                        throws IOException {
+                    if (fail == null) {
+                        Files.delete(directory);
+                        return FileVisitResult.CONTINUE;
+                    }
+                    throw fail;
+                }
+            });
+        } catch (IOException exc) {
+            // ignore: The end-user will not want to see this, it is in a temp
+            // directory so it will go away eventually, and tests verify that
+            // the deletion is occurring.
+        }
+    }
 }
--- a/test/langtools/jdk/jshell/CustomEditor.java	Mon Dec 11 15:17:03 2017 -0800
+++ b/test/langtools/jdk/jshell/CustomEditor.java	Mon Dec 11 21:56:34 2017 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017. Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -39,6 +39,7 @@
     public static final int SOURCE_CODE = 0;
     public static final int GET_SOURCE_CODE = 1;
     public static final int REMOVE_CODE = 2;
+    public static final int GET_FILENAME = 3;
 
     public static final int EXIT_CODE = -1;
     public static final int ACCEPT_CODE = -2;
@@ -68,10 +69,7 @@
                     return;
                 }
                 case GET_SOURCE_CODE: {
-                    byte[] bytes = source.getBytes(StandardCharsets.UTF_8);
-                    output.writeInt(bytes.length);
-                    output.write(bytes);
-                    output.flush();
+                    writeString(output, source);
                     break;
                 }
                 case REMOVE_CODE: {
@@ -79,6 +77,10 @@
                     Files.delete(path);
                     break;
                 }
+                case GET_FILENAME: {
+                    writeString(output, path.toString());
+                    break;
+                }
                 case CANCEL_CODE: {
                     return;
                 }
@@ -97,6 +99,13 @@
         }
     }
 
+    private void writeString(DataOutputStream output, String s) throws IOException {
+        byte[] bytes = s.getBytes(StandardCharsets.UTF_8);
+        output.writeInt(bytes.length);
+        output.write(bytes);
+        output.flush();
+    }
+
     public static void main(String[] args) throws IOException {
         if (args.length != 2) {
             System.err.println("Usage: port file");
--- a/test/langtools/jdk/jshell/ExternalEditorTest.java	Mon Dec 11 15:17:03 2017 -0800
+++ b/test/langtools/jdk/jshell/ExternalEditorTest.java	Mon Dec 11 21:56:34 2017 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -24,7 +24,7 @@
 /*
  * @test
  * @summary Testing external editor.
- * @bug 8143955 8080843 8163816 8143006 8169828 8171130
+ * @bug 8143955 8080843 8163816 8143006 8169828 8171130 8162989
  * @modules jdk.jshell/jdk.internal.jshell.tool
  * @build ReplToolTesting CustomEditor EditorTestBase
  * @run testng ExternalEditorTest
@@ -52,6 +52,8 @@
 import org.testng.annotations.Test;
 
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
 import static org.testng.Assert.fail;
 
 public class ExternalEditorTest extends EditorTestBase {
@@ -76,15 +78,7 @@
 
     @Override
     public String getSource() {
-        try {
-            outputStream.writeInt(CustomEditor.GET_SOURCE_CODE);
-            int length = inputStream.readInt();
-            byte[] bytes = new byte[length];
-            inputStream.readFully(bytes);
-            return new String(bytes, StandardCharsets.UTF_8);
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
-        }
+        return readString(CustomEditor.GET_SOURCE_CODE);
     }
 
     private void sendCode(int code) {
@@ -112,6 +106,22 @@
         sendCode(CustomEditor.CANCEL_CODE);
     }
 
+    protected String getFilename() {
+        return readString(CustomEditor.GET_FILENAME);
+    }
+
+    private String readString(int code) {
+        try {
+            outputStream.writeInt(code);
+            int length = inputStream.readInt();
+            byte[] bytes = new byte[length];
+            inputStream.readFully(bytes);
+            return new String(bytes, StandardCharsets.UTF_8);
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
+    }
+
     @Override
     public void testEditor(boolean defaultStartup, String[] args, ReplTest... tests) {
         ReplTest[] t = new ReplTest[tests.length + 1];
@@ -143,6 +153,22 @@
         );
     }
 
+    @Test
+    public void testTempFileDeleted() {
+        String[] fna = new String[1];
+        testEditor(
+                a -> assertVariable(a, "int", "a", "0", "0"),
+                a -> assertEditOutput(a, "/ed 1", "a ==> 10", () -> {
+                    fna[0] = getFilename();
+                    assertTrue(Files.exists(Paths.get(fna[0])), "Test set-up failed: " + fna[0]);
+                    writeSource("\n\n\nint a = 10;\n\n\n");
+                    exit();
+                }),
+               a -> assertCommand(a, "if (true) {} else {}", "")
+        );
+        assertFalse(Files.exists(Paths.get(fna[0])), "File not deleted: " + fna[0]);
+    }
+
     private static boolean isWindows() {
         return System.getProperty("os.name").startsWith("Windows");
     }