SONAR-7788 Improve PathResolver::relativePath(File, File) when arguments

are not normalized

2 files changed, 81 insertions, 32 deletions

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/scan/filesystem/PathResolver.java b/sonar-plugin-api/src/main/java/org/sonar/api/scan/filesystem/PathResolver.java
index 555333fee7e..266abad42cb 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/scan/filesystem/PathResolver.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/scan/filesystem/PathResolver.java
@@ -20,16 +20,15 @@
 package org.sonar.api.scan.filesystem;
 
 import com.google.common.base.Joiner;
-import com.google.common.base.Preconditions;
-import org.sonar.api.batch.BatchSide;
-import org.sonar.api.utils.PathUtils;
-
-import javax.annotation.CheckForNull;
-
 import java.io.File;
+import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import javax.annotation.CheckForNull;
+import org.apache.commons.io.FilenameUtils;
+import org.sonar.api.batch.BatchSide;
+import org.sonar.api.utils.PathUtils;
 
 /**
  * @since 3.5
@@ -38,16 +37,7 @@ import java.util.List;
 public class PathResolver {
 
   public File relativeFile(File dir, String path) {
-    Preconditions.checkArgument(dir.isDirectory(), "Not a directory: " + dir.getAbsolutePath());
-    File file = new File(path);
-    if (!file.isAbsolute()) {
-      try {
-        file = new File(dir, path).getAbsoluteFile();
-      } catch (Exception e) {
-        throw new IllegalStateException("Fail to resolve path '" + path + "' relative to: " + dir.getAbsolutePath(), e);
-      }
-    }
-    return file;
+    return dir.toPath().resolve(path).normalize().toFile();
   }
 
   public List<File> relativeFiles(File dir, List<String> paths) {
@@ -58,6 +48,10 @@ public class PathResolver {
     return result;
   }
 
+  /**
+   * @deprecated since 6.0 was used when component keys were relative to source dirs
+   */
+  @Deprecated
   @CheckForNull
   public RelativePath relativePath(Collection<File> dirs, File file) {
     List<String> stack = new ArrayList<>();
@@ -73,19 +67,32 @@ public class PathResolver {
     return null;
   }
 
+  /**
+   * Similar to {@link Path#relativize(Path)} except that:
+   *   <ul>
+   *   <li>null is returned if file is not a child of dir
+   *   <li>the resulting path is converted to use Unix separators
+   *   </ul> 
+   * @since 6.0
+   */
   @CheckForNull
-  public String relativePath(File dir, File file) {
-    List<String> stack = new ArrayList<>();
-    String dirPath = PathUtils.canonicalPath(dir);
-    File cursor = file;
-    while (cursor != null) {
-      if (dirPath.equals(PathUtils.canonicalPath(cursor))) {
-        return Joiner.on("/").join(stack);
-      }
-      stack.add(0, cursor.getName());
-      cursor = cursor.getParentFile();
+  public String relativePath(Path dir, Path file) {
+    Path baseDir = dir.normalize();
+    Path path = file.normalize();
+    if (!path.startsWith(baseDir)) {
+      return null;
     }
-    return null;
+    try {
+      Path relativized = baseDir.relativize(path);
+      return FilenameUtils.separatorsToUnix(relativized.toString());
+    } catch (IllegalArgumentException e) {
+      return null;
+    }
+  }
+
+  @CheckForNull
+  public String relativePath(File dir, File file) {
+    return relativePath(dir.toPath(), file.toPath());
   }
 
   @CheckForNull
@@ -98,6 +105,10 @@ public class PathResolver {
     return null;
   }
 
+  /**
+   * @deprecated since 6.0 was used when component keys were relative to source dirs
+   */
+  @Deprecated
   public static final class RelativePath {
     private File dir;
     private String path;
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/scan/filesystem/PathResolverTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/scan/filesystem/PathResolverTest.java
index 1a29d917bb3..1693918ed82 100644
--- a/sonar-plugin-api/src/test/java/org/sonar/api/scan/filesystem/PathResolverTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/scan/filesystem/PathResolverTest.java
@@ -19,17 +19,18 @@
  */
 package org.sonar.api.scan.filesystem;
 
-import org.apache.commons.io.FilenameUtils;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
-
 import java.io.File;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.List;
+import org.apache.commons.io.FilenameUtils;
+import org.apache.commons.lang.SystemUtils;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assume.assumeTrue;
 
 public class PathResolverTest {
   @Rule
@@ -91,6 +92,35 @@ public class PathResolverTest {
   }
 
   @Test
+  public void relative_path_for_not_normalized_dir() throws IOException {
+    PathResolver resolver = new PathResolver();
+    File rootDir = temp.newFolder();
+    File file = new File(rootDir, "level1/../dir/file.c");
+
+    assertThat(resolver.relativePath(rootDir, file)).isEqualTo("dir/file.c");
+  }
+
+  @Test
+  public void relative_path_for_not_normalized_dir_sub_level() throws IOException {
+    PathResolver resolver = new PathResolver();
+    File rootDir = temp.newFolder();
+    File file = new File(rootDir, "level1/level2/../dir/file.c");
+
+    assertThat(resolver.relativePath(rootDir, file)).isEqualTo("level1/dir/file.c");
+  }
+
+  @Test
+  public void relative_path_for_case_insensitive_fs() throws IOException {
+    assumeTrue(SystemUtils.IS_OS_WINDOWS);
+    PathResolver resolver = new PathResolver();
+    File rootDir = temp.newFolder();
+    File baseDir = new File(rootDir, "level1");
+    File file = new File(baseDir, "../Level1/dir/file.c");
+
+    assertThat(resolver.relativePath(baseDir, file)).isEqualTo("dir/file.c");
+  }
+
+  @Test
   public void relative_path_from_multiple_dirs() throws IOException {
     PathResolver resolver = new PathResolver();
     File dir1 = temp.newFolder("D1");
@@ -143,6 +173,14 @@ public class PathResolverTest {
   }
 
   @Test
+  public void null_relative_path_when_file_is_not_in_dir2() throws IOException {
+    PathResolver resolver = new PathResolver();
+    File rootDir = temp.newFolder();
+
+    assertThat(resolver.relativePath(rootDir, new File(rootDir, "../Elsewhere.java"))).isNull();
+  }
+
+  @Test
   public void supportSymlink() {
     PathResolver resolver = new PathResolver();
     File rootDir = new File("src/test/resources/org/sonar/api/scan/filesystem/sample-with-symlink");