PING: RFR: [JDK-8216565] Specifying the same path creates a new directory in JFR.configure

[Yasumasa Suenaga]

Pushed: https://hg.openjdk.java.net/jdk/jdk/rev/046533575954

> While it doesn’t necessary need to be part of this fix, equals and compareTo should be consistent. As you noted in the offline discussion, the compareTo method of SafePath is dead code. So it would be best to remove the compareTo method.

I think we can change SafePath as below.
Currently SafePath has 2 field - path and text. They point same location.
I believe we can use Path::toRealPath to protect from malicious injection because it use FileSystem which is provided by JDK.

If it is ok, I will file it JBS and send review request.

-----------
diff -r 046533575954 src/jdk.jfr/share/classes/jdk/jfr/internal/SecuritySupport.java
--- a/src/jdk.jfr/share/classes/jdk/jfr/internal/SecuritySupport.java   Sat Jan 26 15:47:50 2019 +0900
+++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/SecuritySupport.java   Fri Sep 20 11:33:14 2019 +0900
@@ -150,14 +150,12 @@
       * a malicious provider.
       *
       */
-    public static final class SafePath {
+    public static final class SafePath implements Comparable<SafePath> {
          private final Path path;
-        private final String text;

          public SafePath(Path p) {
              // sanitize
-            text = p.toString();
-            path = Paths.get(text);
+            path = p.toRealPath().normalize();
          }

          public SafePath(String path) {
@@ -169,7 +167,7 @@
          }

          public String toString() {
-            return text;
+            return path.toString();
          }

          @Override
@@ -184,6 +182,11 @@
          public int hashCode() {
              return this.toPath().hashCode();
          }
+
+        @Override
+        public int compareTo(SafePath other) {
+            return path.compareTo(other.path);
+        }
      }

      private interface RunnableWithCheckedException {
-----------


Yasumasa


On 2019/09/20 9:50, Erik Gahlin wrote:
> Hi Chihiro,
> 
> I have tried your fix and it seems to work.
> 
> I’m on vacation, so if Yasumasa like to sponsor your fix, that’s fine for me.
> 
> Otherwise, I can do it at the end of next week.
> 
> While it doesn’t necessary need to be part of this fix, equals and compareTo should be consistent. As you noted in the offline discussion, the compareTo method of SafePath is dead code. So it would be best to remove the compareTo method.
> 
> Thanks
> Erik