[PATCH JGIT] Make Repository.stripWorkDir more robust

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Repository.stripWorkDir was assuming too much about its File arguments,
namely that the given file was always a decendant of the workdir.
Futhermore, it did not "normalize" paths of relative files, but simpy
relied on the path returned by File.getPath().

The new behavior is to fall-back to using File.getAbsolutePath() if the
path returned by File.getPath() cannot be normalized. Test the new
behavior against mix of relative and absolute paths given as arguments.
Fixes problem with string out of bound exception when the path of the
given file is shorter than the workdir, usually meaning it is not a
decendant of the workdir.

Reported-by: Adam W. Hawks <awhawks@xxxxxxxxxxx>
Signed-off-by: Jonas Fonseca <fonseca@xxxxxxx>
---

 On Wed, Aug 12, 2009 at 15:47, Robin Rosenberg<robin.rosenberg.lists@xxxxxxxxxx> wrote:
 > Why not convert both paths? A trickier issue is that getAbsolutePath is very slow when
 > the path is not absolute. I don't think we will always need to normalize in order to
 > fix this. A few unit tests to show the cases solved would help.

 Something like this? I am myself interested in fixing the string out of bound
 exception, which makes this method unusable for me.

 .../tst/org/spearce/jgit/lib/T0003_Basic.java      |   25 ++++++++++++++++
 .../src/org/spearce/jgit/lib/Repository.java       |   31 ++++++++++++++-----
 2 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0003_Basic.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0003_Basic.java
index 3660b45..c2b1b91 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0003_Basic.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0003_Basic.java
@@ -545,6 +545,31 @@ public void test029_mapObject() throws IOException {
 		assertEquals(Commit.class, db.mapObject(ObjectId.fromString("540a36d136cf413e4b064c2b0e0a4db60f77feab"), null).getClass());
 		assertEquals(Tree.class, db.mapObject(ObjectId.fromString("aabf2ffaec9b497f0950352b3e582d73035c2035"), null).getClass());
 		assertEquals(Tag.class, db.mapObject(ObjectId.fromString("17768080a2318cd89bba4c8b87834401e2095703"), null).getClass());
+	}
+
+	public void test30_stripWorkDir() {
+		File relCwd = new File(".");
+		File absCwd = relCwd.getAbsoluteFile();
+		File absBase = new File(new File(absCwd, "repo"), "workdir");
+		File relBase = new File(new File(relCwd, "repo"), "workdir");
+		assertEquals(absBase.getAbsolutePath(), relBase.getAbsolutePath());
+
+		File relBaseFile = new File(new File(relBase, "other"), "module.c");
+		File absBaseFile = new File(new File(absBase, "other"), "module.c");
+		assertEquals("other/module.c", Repository.stripWorkDir(relBase, relBaseFile));
+		assertEquals("other/module.c", Repository.stripWorkDir(relBase, absBaseFile));
+		assertEquals("other/module.c", Repository.stripWorkDir(absBase, relBaseFile));
+		assertEquals("other/module.c", Repository.stripWorkDir(absBase, absBaseFile));
+
+		File relNonFile = new File(new File(relCwd, "not-repo"), ".gitignore");
+		File absNonFile = new File(new File(absCwd, "not-repo"), ".gitignore");
+		assertEquals("", Repository.stripWorkDir(relBase, relNonFile));
+		assertEquals("", Repository.stripWorkDir(absBase, absNonFile));
+
+		assertEquals("", Repository.stripWorkDir(db.getWorkDir(), db.getWorkDir()));
+
+		File file = new File(new File(db.getWorkDir(), "subdir"), "File.java");
+		assertEquals("subdir/File.java", Repository.stripWorkDir(db.getWorkDir(), file));
 
 	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
index d6be9bf..46b7804 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -729,7 +729,7 @@ else if (item.equals("")) {
 					}
 				}
 				if (time != null)
-					throw new RevisionSyntaxException("reflogs not yet supported by revision parser yet", revstr);
+					throw new RevisionSyntaxException("reflogs not yet supported by revision parser", revstr);
 				i = m - 1;
 				break;
 			default:
@@ -1029,15 +1029,30 @@ public static boolean isValidRefName(final String refName) {
 	}
 
 	/**
-	 * Strip work dir and return normalized repository path
+	 * Strip work dir and return normalized repository path.
 	 *
-	 * @param wd Work dir
-	 * @param f File whose path shall be stripped of its workdir
-	 * @return normalized repository relative path
+	 * @param workDir Work dir
+	 * @param file File whose path shall be stripped of its workdir
+	 * @return normalized repository relative path or the empty
+	 *         string if the file is not relative to the work directory.
 	 */
-	public static String stripWorkDir(File wd, File f) {
-		String relName = f.getPath().substring(wd.getPath().length() + 1);
-		relName = relName.replace(File.separatorChar, '/');
+	public static String stripWorkDir(File workDir, File file) {
+		final String filePath = file.getPath();
+		final String workDirPath = workDir.getPath();
+
+		if (filePath.length() <= workDirPath.length() ||
+		    filePath.charAt(workDirPath.length()) != File.separatorChar ||
+		    !filePath.startsWith(workDirPath)) {
+			File absWd = workDir.isAbsolute() ? workDir : workDir.getAbsoluteFile();
+			File absFile = file.isAbsolute() ? file : file.getAbsoluteFile();
+			if (absWd == workDir && absFile == file)
+				return "";
+			return stripWorkDir(absWd, absFile);
+		}
+
+		String relName = filePath.substring(workDirPath.length() + 1);
+		if (File.separatorChar != '/')
+			relName = relName.replace(File.separatorChar, '/');
 		return relName;
 	}
 
-- 
1.6.4.rc3.195.g2b05f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]