{"thread":{"id":"20545","subject":"[JGIT PATCH 1/1] Fix for Repository.stripWorkDir when using partial paths","startedAt":"2009-08-12T00:48:39Z","lastAt":"2009-08-19T12:50:59Z","messageCount":4,"participants":["Adam W. Hawks","Shawn O. Pearce","Robin Rosenberg","Jonas Fonseca"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"120342","messageId":"4A821167.6030107@writeme.com","threadId":"20545","inReplyTo":null,"subject":"[JGIT PATCH 1/1] Fix for Repository.stripWorkDir when using partial paths","fromName":"Adam W. Hawks","fromEmail":"awhawks@writeme.com","sentAt":"2009-08-12T00:48:39Z","receivedAt":"2009-08-12T00:48:39Z","isPatch":true,"sender":{"key":"awhawks@writeme.com","avatar":null},"body":"\n>From ef993e633cdcb1dddda5e71db1b62306df7ce83f Mon Sep 17 00:00:00 2001\nDate: Tue, 11 Aug 2009 20:02:56 -0400\n\nWhen you call stripWorkDir with a relative path\nyou can get a string out of bounds error.\n\nThis change fixes that problem by using the absolute paths\nof the file instead of its relative name.\n\nSigned-off-by: Adam W. Hawks <awhawks@writeme.com>\n---\n .../src/org/spearce/jgit/lib/Repository.java       |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\nindex 468cf4c..a68817b 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n@@ -1036,7 +1036,7 @@ public static boolean isValidRefName(final String refName) {\n \t * @return normalized repository relative path\n \t */\n \tpublic static String stripWorkDir(File wd, File f) {\n-\t\tString relName = f.getPath().substring(wd.getPath().length() + 1);\n+\t\tString relName = f.getAbsolutePath().substring(wd.getPath().length() + 1);\n \t\trelName = relName.replace(File.separatorChar, '/');\n \t\treturn relName;\n \t}\n-- \n1.6.0.2\n"},{"id":"120407","messageId":"20090812142918.GB1033@spearce.org","threadId":"20545","inReplyTo":"4A821167.6030107@writeme.com","subject":"Re: [JGIT PATCH 1/1] Fix for Repository.stripWorkDir when using partial paths","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-08-12T14:29:18Z","receivedAt":"2009-08-12T14:29:18Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"\"Adam W. Hawks\" <awhawks@writeme.com> wrote:\n> When you call stripWorkDir with a relative path\n> you can get a string out of bounds error.\n> \n> This change fixes that problem by using the absolute paths\n> of the file instead of its relative name.\n\nExcept it made the existing test suite fail, badly.  I'm counting\n7 errors and 28 test failures as a result of applying this patch.\n \n> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n> index 468cf4c..a68817b 100644\n> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n> @@ -1036,7 +1036,7 @@ public static boolean isValidRefName(final String refName) {\n>  \t * @return normalized repository relative path\n>  \t */\n>  \tpublic static String stripWorkDir(File wd, File f) {\n> -\t\tString relName = f.getPath().substring(wd.getPath().length() + 1);\n> +\t\tString relName = f.getAbsolutePath().substring(wd.getPath().length() + 1);\n>  \t\trelName = relName.replace(File.separatorChar, '/');\n>  \t\treturn relName;\n>  \t}\n\n-- \nShawn.\n"},{"id":"120443","messageId":"200908122147.52530.robin.rosenberg.lists@dewire.com","threadId":"20545","inReplyTo":"4A821167.6030107@writeme.com","subject":"Re: [JGIT PATCH 1/1] Fix for Repository.stripWorkDir when using partial paths","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2009-08-12T19:47:52Z","receivedAt":"2009-08-12T19:47:52Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"onsdag 12 augusti 2009 02:48:39 skrev \"Adam W. Hawks\" <awhawks@writeme.com>:\n> \n> From ef993e633cdcb1dddda5e71db1b62306df7ce83f Mon Sep 17 00:00:00 2001\n> Date: Tue, 11 Aug 2009 20:02:56 -0400\n> \n> When you call stripWorkDir with a relative path\n> you can get a string out of bounds error.\n> \n> This change fixes that problem by using the absolute paths\n> of the file instead of its relative name.\n> \n> Signed-off-by: Adam W. Hawks <awhawks@writeme.com>\n> ---\n>  .../src/org/spearce/jgit/lib/Repository.java       |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n> \n> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n> index 468cf4c..a68817b 100644\n> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n> @@ -1036,7 +1036,7 @@ public static boolean isValidRefName(final String refName) {\n>  \t * @return normalized repository relative path\n>  \t */\n>  \tpublic static String stripWorkDir(File wd, File f) {\n> -\t\tString relName = f.getPath().substring(wd.getPath().length() + 1);\n> +\t\tString relName = f.getAbsolutePath().substring(wd.getPath().length() + 1);\n>  \t\trelName = relName.replace(File.separatorChar, '/');\n>  \t\treturn relName;\n>  \t}\n\nWhy not convert both paths? A trickier issue is that getAbsolutePath is very slow when\nthe path is not absolute. I don't think we will always need to normalize in order to\nfix this. A few unit tests to show the cases solved would help.\n\n-- robin\n"},{"id":"121246","messageId":"1250686259-15301-1-git-send-email-fonseca@diku.dk","threadId":"20545","inReplyTo":"200908122147.52530.robin.rosenberg.lists@dewire.com","subject":"[PATCH JGIT] Make Repository.stripWorkDir more robust","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2009-08-19T12:50:59Z","receivedAt":"2009-08-19T12:50:59Z","isPatch":true,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"Repository.stripWorkDir was assuming too much about its File arguments,\nnamely that the given file was always a decendant of the workdir.\nFuthermore, it did not \"normalize\" paths of relative files, but simpy\nrelied on the path returned by File.getPath().\n\nThe new behavior is to fall-back to using File.getAbsolutePath() if the\npath returned by File.getPath() cannot be normalized. Test the new\nbehavior against mix of relative and absolute paths given as arguments.\nFixes problem with string out of bound exception when the path of the\ngiven file is shorter than the workdir, usually meaning it is not a\ndecendant of the workdir.\n\nReported-by: Adam W. Hawks <awhawks@writeme.com>\nSigned-off-by: Jonas Fonseca <fonseca@diku.dk>\n---\n\n On Wed, Aug 12, 2009 at 15:47, Robin Rosenberg<robin.rosenberg.lists@dewire.com> wrote:\n > Why not convert both paths? A trickier issue is that getAbsolutePath is very slow when\n > the path is not absolute. I don't think we will always need to normalize in order to\n > fix this. A few unit tests to show the cases solved would help.\n\n Something like this? I am myself interested in fixing the string out of bound\n exception, which makes this method unusable for me.\n\n .../tst/org/spearce/jgit/lib/T0003_Basic.java      |   25 ++++++++++++++++\n .../src/org/spearce/jgit/lib/Repository.java       |   31 ++++++++++++++-----\n 2 files changed, 48 insertions(+), 8 deletions(-)\n\ndiff --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\nindex 3660b45..c2b1b91 100644\n--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0003_Basic.java\n+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/T0003_Basic.java\n@@ -545,6 +545,31 @@ public void test029_mapObject() throws IOException {\n \t\tassertEquals(Commit.class, db.mapObject(ObjectId.fromString(\"540a36d136cf413e4b064c2b0e0a4db60f77feab\"), null).getClass());\n \t\tassertEquals(Tree.class, db.mapObject(ObjectId.fromString(\"aabf2ffaec9b497f0950352b3e582d73035c2035\"), null).getClass());\n \t\tassertEquals(Tag.class, db.mapObject(ObjectId.fromString(\"17768080a2318cd89bba4c8b87834401e2095703\"), null).getClass());\n+\t}\n+\n+\tpublic void test30_stripWorkDir() {\n+\t\tFile relCwd = new File(\".\");\n+\t\tFile absCwd = relCwd.getAbsoluteFile();\n+\t\tFile absBase = new File(new File(absCwd, \"repo\"), \"workdir\");\n+\t\tFile relBase = new File(new File(relCwd, \"repo\"), \"workdir\");\n+\t\tassertEquals(absBase.getAbsolutePath(), relBase.getAbsolutePath());\n+\n+\t\tFile relBaseFile = new File(new File(relBase, \"other\"), \"module.c\");\n+\t\tFile absBaseFile = new File(new File(absBase, \"other\"), \"module.c\");\n+\t\tassertEquals(\"other/module.c\", Repository.stripWorkDir(relBase, relBaseFile));\n+\t\tassertEquals(\"other/module.c\", Repository.stripWorkDir(relBase, absBaseFile));\n+\t\tassertEquals(\"other/module.c\", Repository.stripWorkDir(absBase, relBaseFile));\n+\t\tassertEquals(\"other/module.c\", Repository.stripWorkDir(absBase, absBaseFile));\n+\n+\t\tFile relNonFile = new File(new File(relCwd, \"not-repo\"), \".gitignore\");\n+\t\tFile absNonFile = new File(new File(absCwd, \"not-repo\"), \".gitignore\");\n+\t\tassertEquals(\"\", Repository.stripWorkDir(relBase, relNonFile));\n+\t\tassertEquals(\"\", Repository.stripWorkDir(absBase, absNonFile));\n+\n+\t\tassertEquals(\"\", Repository.stripWorkDir(db.getWorkDir(), db.getWorkDir()));\n+\n+\t\tFile file = new File(new File(db.getWorkDir(), \"subdir\"), \"File.java\");\n+\t\tassertEquals(\"subdir/File.java\", Repository.stripWorkDir(db.getWorkDir(), file));\n \n \t}\n }\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\nindex d6be9bf..46b7804 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n@@ -729,7 +729,7 @@ else if (item.equals(\"\")) {\n \t\t\t\t\t}\n \t\t\t\t}\n \t\t\t\tif (time != null)\n-\t\t\t\t\tthrow new RevisionSyntaxException(\"reflogs not yet supported by revision parser yet\", revstr);\n+\t\t\t\t\tthrow new RevisionSyntaxException(\"reflogs not yet supported by revision parser\", revstr);\n \t\t\t\ti = m - 1;\n \t\t\t\tbreak;\n \t\t\tdefault:\n@@ -1029,15 +1029,30 @@ public static boolean isValidRefName(final String refName) {\n \t}\n \n \t/**\n-\t * Strip work dir and return normalized repository path\n+\t * Strip work dir and return normalized repository path.\n \t *\n-\t * @param wd Work dir\n-\t * @param f File whose path shall be stripped of its workdir\n-\t * @return normalized repository relative path\n+\t * @param workDir Work dir\n+\t * @param file File whose path shall be stripped of its workdir\n+\t * @return normalized repository relative path or the empty\n+\t *         string if the file is not relative to the work directory.\n \t */\n-\tpublic static String stripWorkDir(File wd, File f) {\n-\t\tString relName = f.getPath().substring(wd.getPath().length() + 1);\n-\t\trelName = relName.replace(File.separatorChar, '/');\n+\tpublic static String stripWorkDir(File workDir, File file) {\n+\t\tfinal String filePath = file.getPath();\n+\t\tfinal String workDirPath = workDir.getPath();\n+\n+\t\tif (filePath.length() <= workDirPath.length() ||\n+\t\t    filePath.charAt(workDirPath.length()) != File.separatorChar ||\n+\t\t    !filePath.startsWith(workDirPath)) {\n+\t\t\tFile absWd = workDir.isAbsolute() ? workDir : workDir.getAbsoluteFile();\n+\t\t\tFile absFile = file.isAbsolute() ? file : file.getAbsoluteFile();\n+\t\t\tif (absWd == workDir && absFile == file)\n+\t\t\t\treturn \"\";\n+\t\t\treturn stripWorkDir(absWd, absFile);\n+\t\t}\n+\n+\t\tString relName = filePath.substring(workDirPath.length() + 1);\n+\t\tif (File.separatorChar != '/')\n+\t\t\trelName = relName.replace(File.separatorChar, '/');\n \t\treturn relName;\n \t}\n \n-- \n1.6.4.rc3.195.g2b05f\n"}]}