fix: remove the user directory only at the start of paths (in messages)

rdeltour · rdeltour · commit 5ee72e79333f · 2021-02-26T14:20:26.000+01:00
The previous implementation of the `removeWorkingDirectory` utility method was quite brutal and replaced all occurences of the "user.dir" system property in paths, even when found in the middle of the path. The new implementation only replaces the user directory when it occurs at the beginning of the path, and does nothing when the user directory is set to the root directory ("/"). Tests included. Fixes #1181
diff --git a/src/main/java/com/adobe/epubcheck/reporting/CheckerMetadata.java b/src/main/java/com/adobe/epubcheck/reporting/CheckerMetadata.java
@@ -34,8 +34,6 @@ class CheckerMetadata
   @JsonProperty
   private int nUsage = 0;
 
-  private final String workingDirectory  = System.getProperty("user.dir");
-
   public void setFileInfo(File epubFile)
   {
     this.path = PathUtil.removeWorkingDirectory(epubFile.getAbsolutePath());
diff --git a/src/main/java/com/adobe/epubcheck/util/PathUtil.java b/src/main/java/com/adobe/epubcheck/util/PathUtil.java
@@ -39,7 +39,6 @@
 // This class should probably be entirely refactored at some point 
 public class PathUtil
 {
-  static final String workingDirectory = System.getProperty("user.dir");
 
   private static final Pattern REGEX_URI_SCHEME = Pattern
       .compile("^\\p{Alpha}(\\p{Alnum}|\\.|\\+|-)*:");
@@ -151,7 +150,11 @@ public static String removeWorkingDirectory(String path)
     {
       return path;
     }
-    return path.replace(workingDirectory, ".");
+    String workingDirectory = System.getProperty("user.dir");
+    if ("/".equals(workingDirectory) || !path.startsWith(workingDirectory)) {
+      return path;
+    }
+    return ".".concat(path.substring(workingDirectory.length()));
   }
 
   public static String getFragment(String uri)
diff --git a/src/test/java/com/adobe/epubcheck/util/PathUtilTest.java b/src/test/java/com/adobe/epubcheck/util/PathUtilTest.java
@@ -6,7 +6,6 @@
 
 import org.junit.Test;
 
-
 public class PathUtilTest
 {
 
@@ -15,20 +14,19 @@ public void testIsRemoteNull()
   {
     PathUtil.isRemote(null);
   }
-  
+
   @Test
   public void testIsRemoteTrue()
   {
     assertTrue(PathUtil.isRemote("https://example.org"));
   }
-  
+
   @Test
   public void testIsRemoteFalse()
   {
     assertFalse(PathUtil.isRemote("OCF/path"));
   }
-  
-  
+
   @Test(expected = NullPointerException.class)
   public void testNormalizePathNull()
   {
@@ -131,7 +129,7 @@ public void testNormalizePathLeadingSlash()
     assertEquals("/foo", PathUtil.normalizePath("/./foo"));
     assertEquals("/../foo", PathUtil.normalizePath("/../foo"));
   }
-  
+
   @Test
   public void testNormalizePathAbsoluteURI()
   {
@@ -174,7 +172,7 @@ public void testRelativizeAbsoluteWithNullBaseIsReturnedAsIs()
   {
     assertEquals("https://foo", PathUtil.resolveRelativeReference(null, "https://foo"));
   }
-  
+
   @Test
   public void testRelativizeAbsoluteWithNonNullBaseIsReturnedAsIs()
   {
@@ -188,31 +186,35 @@ public void testRelativizeAbsoluteSchemes()
     assertEquals("https://foo?q#f", PathUtil.resolveRelativeReference(null, "https://foo?q#f"));
     assertEquals("data:foo", PathUtil.resolveRelativeReference(null, "data:foo"));
   }
-  
+
   @Test
   public void testRelativizeWithAbsoluteBase()
   {
-    assertEquals("https://example.org/foo", PathUtil.resolveRelativeReference("https://example.org/", "foo"));
+    assertEquals("https://example.org/foo",
+        PathUtil.resolveRelativeReference("https://example.org/", "foo"));
   }
-  
+
   @Test
   public void testRelativizeWithAbsoluteBaseAndFragment()
   {
-    assertEquals("https://example.org/foo", PathUtil.resolveRelativeReference("https://example.org/#bar", "foo"));
+    assertEquals("https://example.org/foo",
+        PathUtil.resolveRelativeReference("https://example.org/#bar", "foo"));
   }
 
   @Test
   public void testRelativizeWithAbsoluteBaseAndQuery()
   {
-    assertEquals("https://example.org/foo", PathUtil.resolveRelativeReference("https://example.org/?test#bar", "foo"));
+    assertEquals("https://example.org/foo",
+        PathUtil.resolveRelativeReference("https://example.org/?test#bar", "foo"));
   }
 
   @Test
   public void testRelativizeWithAbsoluteBaseIsNormalized()
   {
-    assertEquals("https://example.org/foo", PathUtil.resolveRelativeReference("https://example.org/foo/../bar", "bar/../foo"));
+    assertEquals("https://example.org/foo",
+        PathUtil.resolveRelativeReference("https://example.org/foo/../bar", "bar/../foo"));
   }
-  
+
   @Test
   public void testRelativizeWithRelBase()
   {
@@ -229,15 +231,15 @@ public void testRelativizeWithRelBaseIsNormalized()
     assertEquals("foo/foo/", PathUtil.resolveRelativeReference("foo/", "foo/"));
     assertEquals("bar/foo", PathUtil.resolveRelativeReference("foo/..", "bar/foo"));
   }
-  
+
   @Test
   public void testRelativizeFragment()
   {
     assertEquals("foo#bar", PathUtil.resolveRelativeReference("foo", "#bar"));
     assertEquals("foo/#bar", PathUtil.resolveRelativeReference("foo/", "#bar"));
     assertEquals("#bar", PathUtil.resolveRelativeReference(".", "#bar"));
   }
-  
+
   @Test
   public void testRelativizeDecodes()
   {
@@ -255,4 +257,23 @@ public void testRemoveAnchor()
     assertEquals(urlWithoutAnchor, PathUtil.removeFragment(urlWithoutAnchor));
   }
 
+  @Test
+  public void testRemoveWorkingDirectory()
+  {
+    String OLD_USER_DIR = System.getProperty("user.dir");
+
+    assertEquals(null, PathUtil.removeWorkingDirectory(null));
+    assertEquals("", PathUtil.removeWorkingDirectory(""));
+
+    System.setProperty("user.dir", "/user");
+    assertEquals("./epub", PathUtil.removeWorkingDirectory("/user/epub"));
+
+    assertEquals("/prefix/user/epub", PathUtil.removeWorkingDirectory("/prefix/user/epub"));
+
+    System.setProperty("user.dir", "/");
+    assertEquals("/dir/epub", PathUtil.removeWorkingDirectory("/dir/epub"));
+
+    System.setProperty("user.dir", OLD_USER_DIR);
+  }
+
 }

Original file line number	Diff line number	Diff line change
`@@ -34,8 +34,6 @@ class CheckerMetadata`
`34`	`34`	`@JsonProperty`
`35`	`35`	`private int nUsage = 0;`
`36`	`36`
`37`		`- private final String workingDirectory = System.getProperty("user.dir");`
`38`		`-`
`39`	`37`	`public void setFileInfo(File epubFile)`
`40`	`38`	`{`
`41`	`39`	`this.path = PathUtil.removeWorkingDirectory(epubFile.getAbsolutePath());`
Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,6 @@`
`39`	`39`	`// This class should probably be entirely refactored at some point`
`40`	`40`	`public class PathUtil`
`41`	`41`	`{`
`42`		`- static final String workingDirectory = System.getProperty("user.dir");`
`43`	`42`
`44`	`43`	`private static final Pattern REGEX_URI_SCHEME = Pattern`
`45`	`44`	`.compile("^\\p{Alpha}(\\p{Alnum}\|\\.\|\\+\|-)*:");`
`@@ -151,7 +150,11 @@ public static String removeWorkingDirectory(String path)`
`151`	`150`	`{`
`152`	`151`	`return path;`
`153`	`152`	`}`
`154`		`- return path.replace(workingDirectory, ".");`
	`153`	`+ String workingDirectory = System.getProperty("user.dir");`
	`154`	`+ if ("/".equals(workingDirectory) \|\| !path.startsWith(workingDirectory)) {`
	`155`	`+ return path;`
	`156`	`+ }`
	`157`	`+ return ".".concat(path.substring(workingDirectory.length()));`
`155`	`158`	`}`
`156`	`159`
`157`	`160`	`public static String getFragment(String uri)`
Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,6 @@`
`6`	`6`
`7`	`7`	`import org.junit.Test;`
`8`	`8`
`9`		`-`
`10`	`9`	`public class PathUtilTest`
`11`	`10`	`{`
`12`	`11`
`@@ -15,20 +14,19 @@ public void testIsRemoteNull()`
`15`	`14`	`{`
`16`	`15`	`PathUtil.isRemote(null);`
`17`	`16`	`}`
`18`		`-`
	`17`	`+`
`19`	`18`	`@Test`
`20`	`19`	`public void testIsRemoteTrue()`
`21`	`20`	`{`
`22`	`21`	`assertTrue(PathUtil.isRemote("https://example.org"));`
`23`	`22`	`}`
`24`		`-`
	`23`	`+`
`25`	`24`	`@Test`
`26`	`25`	`public void testIsRemoteFalse()`
`27`	`26`	`{`
`28`	`27`	`assertFalse(PathUtil.isRemote("OCF/path"));`
`29`	`28`	`}`
`30`		`-`
`31`		`-`
	`29`	`+`
`32`	`30`	`@Test(expected = NullPointerException.class)`
`33`	`31`	`public void testNormalizePathNull()`
`34`	`32`	`{`
`@@ -131,7 +129,7 @@ public void testNormalizePathLeadingSlash()`
`131`	`129`	`assertEquals("/foo", PathUtil.normalizePath("/./foo"));`
`132`	`130`	`assertEquals("/../foo", PathUtil.normalizePath("/../foo"));`
`133`	`131`	`}`
`134`		`-`
	`132`	`+`
`135`	`133`	`@Test`
`136`	`134`	`public void testNormalizePathAbsoluteURI()`
`137`	`135`	`{`
`@@ -174,7 +172,7 @@ public void testRelativizeAbsoluteWithNullBaseIsReturnedAsIs()`
`174`	`172`	`{`
`175`	`173`	`assertEquals("https://foo", PathUtil.resolveRelativeReference(null, "https://foo"));`
`176`	`174`	`}`
`177`		`-`
	`175`	`+`
`178`	`176`	`@Test`
`179`	`177`	`public void testRelativizeAbsoluteWithNonNullBaseIsReturnedAsIs()`
`180`	`178`	`{`
`@@ -188,31 +186,35 @@ public void testRelativizeAbsoluteSchemes()`
`188`	`186`	`assertEquals("https://foo?q#f", PathUtil.resolveRelativeReference(null, "https://foo?q#f"));`
`189`	`187`	`assertEquals("data:foo", PathUtil.resolveRelativeReference(null, "data:foo"));`
`190`	`188`	`}`
`191`		`-`
	`189`	`+`
`192`	`190`	`@Test`
`193`	`191`	`public void testRelativizeWithAbsoluteBase()`
`194`	`192`	`{`
`195`		`- assertEquals("https://example.org/foo", PathUtil.resolveRelativeReference("https://example.org/", "foo"));`
	`193`	`+ assertEquals("https://example.org/foo",`
	`194`	`+ PathUtil.resolveRelativeReference("https://example.org/", "foo"));`
`196`	`195`	`}`
`197`		`-`
	`196`	`+`
`198`	`197`	`@Test`
`199`	`198`	`public void testRelativizeWithAbsoluteBaseAndFragment()`
`200`	`199`	`{`
`201`		`- assertEquals("https://example.org/foo", PathUtil.resolveRelativeReference("https://example.org/#bar", "foo"));`
	`200`	`+ assertEquals("https://example.org/foo",`
	`201`	`+ PathUtil.resolveRelativeReference("https://example.org/#bar", "foo"));`
`202`	`202`	`}`
`203`	`203`
`204`	`204`	`@Test`
`205`	`205`	`public void testRelativizeWithAbsoluteBaseAndQuery()`
`206`	`206`	`{`
`207`		`- assertEquals("https://example.org/foo", PathUtil.resolveRelativeReference("https://example.org/?test#bar", "foo"));`
	`207`	`+ assertEquals("https://example.org/foo",`
	`208`	`+ PathUtil.resolveRelativeReference("https://example.org/?test#bar", "foo"));`
`208`	`209`	`}`
`209`	`210`
`210`	`211`	`@Test`
`211`	`212`	`public void testRelativizeWithAbsoluteBaseIsNormalized()`
`212`	`213`	`{`
`213`		`- assertEquals("https://example.org/foo", PathUtil.resolveRelativeReference("https://example.org/foo/../bar", "bar/../foo"));`
	`214`	`+ assertEquals("https://example.org/foo",`
	`215`	`+ PathUtil.resolveRelativeReference("https://example.org/foo/../bar", "bar/../foo"));`
`214`	`216`	`}`
`215`		`-`
	`217`	`+`
`216`	`218`	`@Test`
`217`	`219`	`public void testRelativizeWithRelBase()`
`218`	`220`	`{`
`@@ -229,15 +231,15 @@ public void testRelativizeWithRelBaseIsNormalized()`
`229`	`231`	`assertEquals("foo/foo/", PathUtil.resolveRelativeReference("foo/", "foo/"));`
`230`	`232`	`assertEquals("bar/foo", PathUtil.resolveRelativeReference("foo/..", "bar/foo"));`
`231`	`233`	`}`
`232`		`-`
	`234`	`+`
`233`	`235`	`@Test`
`234`	`236`	`public void testRelativizeFragment()`
`235`	`237`	`{`
`236`	`238`	`assertEquals("foo#bar", PathUtil.resolveRelativeReference("foo", "#bar"));`
`237`	`239`	`assertEquals("foo/#bar", PathUtil.resolveRelativeReference("foo/", "#bar"));`
`238`	`240`	`assertEquals("#bar", PathUtil.resolveRelativeReference(".", "#bar"));`
`239`	`241`	`}`
`240`		`-`
	`242`	`+`
`241`	`243`	`@Test`
`242`	`244`	`public void testRelativizeDecodes()`
`243`	`245`	`{`
`@@ -255,4 +257,23 @@ public void testRemoveAnchor()`
`255`	`257`	`assertEquals(urlWithoutAnchor, PathUtil.removeFragment(urlWithoutAnchor));`
`256`	`258`	`}`
`257`	`259`
	`260`	`+ @Test`
	`261`	`+ public void testRemoveWorkingDirectory()`
	`262`	`+ {`
	`263`	`+ String OLD_USER_DIR = System.getProperty("user.dir");`
	`264`	`+`
	`265`	`+ assertEquals(null, PathUtil.removeWorkingDirectory(null));`
	`266`	`+ assertEquals("", PathUtil.removeWorkingDirectory(""));`
	`267`	`+`
	`268`	`+ System.setProperty("user.dir", "/user");`
	`269`	`+ assertEquals("./epub", PathUtil.removeWorkingDirectory("/user/epub"));`
	`270`	`+`
	`271`	`+ assertEquals("/prefix/user/epub", PathUtil.removeWorkingDirectory("/prefix/user/epub"));`
	`272`	`+`
	`273`	`+ System.setProperty("user.dir", "/");`
	`274`	`+ assertEquals("/dir/epub", PathUtil.removeWorkingDirectory("/dir/epub"));`
	`275`	`+`
	`276`	`+ System.setProperty("user.dir", OLD_USER_DIR);`
	`277`	`+ }`
	`278`	`+`
`258`	`279`	`}`