Fix for Repository.stripWorkDir when using partial paths
PATCH 1/1 Fix for Repository.stripWorkDir when using partial paths
Quoted 2 lines
From
ef993e633cdcb1dddda5e71db1b62306df7ce83fMon Sep 17 00:00:00 2001
Date: Tue, 11 Aug 2009 20:02:56 -0400
When you call stripWorkDir with a relative path
you can get a string out of bounds error.
This change fixes that problem by using the absolute paths of the file instead of its relative name.
--- .../src/org/spearce/jgit/lib/Repository.java | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java +1 -1
@@ -1036,7 +1036,7 @@ public static boolean isValidRefName(final String refName) { * @return normalized repository relative path */ public static String stripWorkDir(File wd, File f) { - String relName = f.getPath().substring(wd.getPath().length() + 1); + String relName = f.getAbsolutePath().substring(wd.getPath().length() + 1); relName = relName.replace(File.separatorChar, '/'); return relName; }
Signature · 1 line
1.6.0.2
Quoted 5 lines · Adam W. Hawks
When you call stripWorkDir with a relative path
you can get a string out of bounds error.This change fixes that problem by using the absolute paths of the file instead of its relative name.
Except it made the existing test suite fail, badly. I'm counting 7 errors and 28 test failures as a result of applying this patch.
Quoted 13 lines
org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java +1
-1@@ -1036,7 +1036,7 @@ public static boolean isValidRefName(final String refName) { * @return normalized repository relative path */ public static String stripWorkDir(File wd, File f) { - String relName = f.getPath().substring(wd.getPath().length() + 1); + String relName = f.getAbsolutePath().substring(wd.getPath().length() + 1); relName = relName.replace(File.separatorChar, '/'); return relName; }
Signature · 1 line
Shawn.
onsdag 12 augusti 2009 02:48:39 skrev "Adam W. Hawks" <[email protected]>:
Quoted 20 lines
From
ef993e633cdcb1dddda5e71db1b62306df7ce83fMon Sep 17 00:00:00 2001
Date: Tue, 11 Aug 2009 20:02:56 -0400When you call stripWorkDir with a relative path
you can get a string out of bounds error.This change fixes that problem by using the absolute paths of the file instead of its relative name.
Signed-off-by Adam W. Hawks <[email protected]>--- .../src/org/spearce/jgit/lib/Repository.java | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java +0
-0
org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java public static boolean isValidRefName(final String refName) { · line 1042
Quoted 8 lines
org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java +1
-1@@ -1036,7 +1036,7 @@ public static boolean isValidRefName(final String refName) { * @return normalized repository relative path */ public static String stripWorkDir(File wd, File f) { - String relName = f.getPath().substring(wd.getPath().length() + 1); + String relName = f.getAbsolutePath().substring(wd.getPath().length() + 1); relName = relName.replace(File.separatorChar, '/'); return relName; }
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.
-- robin
PATCH 1/1 Make Repository.stripWorkDir more robust
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.
---
Quoted 3 lines · Robin Rosenberg
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(-)
org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0003_Basic.java +25 -0
@@ -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)); } }
org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java +23 -8
@@ -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; }
Signature · 1 line
1.6.4.rc3.195.g2b05f