diff --git a/src/etc/testcases/taskdefs/manifestclasspath.xml b/src/etc/testcases/taskdefs/manifestclasspath.xml index 0e54c0ddf..a06806bf0 100644 --- a/src/etc/testcases/taskdefs/manifestclasspath.xml +++ b/src/etc/testcases/taskdefs/manifestclasspath.xml @@ -216,4 +216,22 @@ + + + + + + + + + + + + + + + + diff --git a/src/main/org/apache/tools/ant/taskdefs/ManifestClassPath.java b/src/main/org/apache/tools/ant/taskdefs/ManifestClassPath.java index 665dfdef2..0897993c1 100644 --- a/src/main/org/apache/tools/ant/taskdefs/ManifestClassPath.java +++ b/src/main/org/apache/tools/ant/taskdefs/ManifestClassPath.java @@ -66,70 +66,51 @@ public class ManifestClassPath extends Task { throw new BuildException("Missing nested !"); } + StringBuffer tooLongSb = new StringBuffer(); + for (int i = 0; i < maxParentLevels + 1; i++) { + tooLongSb.append("../"); + } + final String tooLongPrefix = tooLongSb.toString(); + // Normalize the reference directory (containing the jar) final FileUtils fileUtils = FileUtils.getFileUtils(); dir = fileUtils.normalize(dir.getAbsolutePath()); - // Create as many directory prefixes as parent levels to traverse, - // in addition to the reference directory itself - File currDir = dir; - String[] dirs = new String[maxParentLevels + 1]; - for (int i = 0; i < maxParentLevels + 1; ++i) { - dirs[i] = currDir.getAbsolutePath(); - if (!dirs[i].equals("" + File.separatorChar)) { - dirs[i] = dirs[i] + File.separatorChar; - } - currDir = currDir.getParentFile(); - if (currDir == null) { - maxParentLevels = i + 1; - break; - } - } - String[] elements = path.list(); StringBuffer buffer = new StringBuffer(); StringBuffer element = new StringBuffer(); for (int i = 0; i < elements.length; ++i) { // Normalize the current file File pathEntry = new File(elements[i]); - pathEntry = fileUtils.normalize(pathEntry.getAbsolutePath()); String fullPath = pathEntry.getAbsolutePath(); + pathEntry = fileUtils.normalize(fullPath); - // Find the longest prefix shared by the current file - // and the reference directory. String relPath = null; - for (int j = 0; j <= maxParentLevels && j < dirs.length; ++j) { - String dir = dirs[j]; - if (!fullPath.startsWith(dir)) { - continue; - } + String canonicalPath = null; + try { + relPath = FileUtils.getRelativePath(dir, pathEntry); - // We have a match! Add as many ../ as parent - // directory traversed to get the relative path - element.setLength(0); - for (int k = 0; k < j; ++k) { - element.append(".."); - element.append(File.separatorChar); + canonicalPath = pathEntry.getCanonicalPath(); + // getRelativePath always uses '/' as separator, adapt + if (File.separatorChar != '/') { + canonicalPath = + canonicalPath.replace(File.separatorChar, '/'); } - element.append(fullPath.substring(dir.length())); - relPath = element.toString(); - break; + } catch (Exception e) { + throw new BuildException("error trying to get the relative path" + + " from " + dir + " to " + fullPath, + e); } // No match, so bail out! - if (relPath == null) { + if (relPath.equals(canonicalPath) + || relPath.startsWith(tooLongPrefix)) { throw new BuildException( "No suitable relative path from " + dir + " to " + fullPath); } - // Manifest's ClassPath: attribute always uses forward - // slashes '/', and is space-separated. Ant will properly - // format it on 72 columns with proper line continuation - if (File.separatorChar != '/') { - relPath = relPath.replace(File.separatorChar, '/'); - } - if (pathEntry.isDirectory()) { + if (pathEntry.isDirectory() && !relPath.endsWith("/")) { relPath = relPath + '/'; } try { @@ -137,6 +118,9 @@ public class ManifestClassPath extends Task { } catch (UnsupportedEncodingException exc) { throw new BuildException(exc); } + // Manifest's ClassPath: attribute always uses forward + // slashes '/', and is space-separated. Ant will properly + // format it on 72 columns with proper line continuation buffer.append(relPath); buffer.append(' '); } @@ -175,6 +159,10 @@ public class ManifestClassPath extends Task { * @param levels the max level. Defaults to 2. */ public void setMaxParentLevels(int levels) { + if (levels < 0) { + throw new BuildException("maxParentLevels must not be a negative" + + " number"); + } this.maxParentLevels = levels; } diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/ManifestClassPathTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/ManifestClassPathTest.java index 08cdc8c0b..afeded712 100644 --- a/src/tests/junit/org/apache/tools/ant/taskdefs/ManifestClassPathTest.java +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/ManifestClassPathTest.java @@ -142,7 +142,7 @@ public class ManifestClassPathTest public void testInternationalGerman() { executeTarget("international-german"); expectLogContaining("run-two-jars", "beta alpha"); - + } public void testInternationalHebrew() { if (!Os.isFamily("windows")) { @@ -154,5 +154,32 @@ public class ManifestClassPathTest } + public void testSameWindowsDrive() { + if (!Os.isFamily("windows")) { + System.out.println("Test with drive letters only run on windows"); + } else { + executeTarget("testSameDrive"); + } + assertPropertyEquals("cp", "../a/b/x.jar"); + } + + public void testDifferentWindowsDrive() { + if (!Os.isFamily("windows")) { + System.out.println("Test with drive letters only run on windows"); + } else { + try { + new java.io.File("D:/").getCanonicalPath(); + } catch (java.io.IOException e) { + System.out.println("drive d: doesn't exist or is not ready," + + " skipping test"); + return; + } + + expectBuildExceptionContaining("testDifferentDrive", + "different drive", + "No suitable relative path from "); + assertPropertyUnset("cp"); + } + } } // END class ManifestClassPathTest