more systematic handling of URIs, delegation of work to the JDK

for toURI and fromURI
patches prepared by Jesse Glick, Bugzilla 8031


git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@357131 13f79535-47bb-0310-9956-ffa450edef68

docs/faq.html

@@ -1390,7 +1390,7 @@ while(<STDIN>) {
 <?xml version="1.0"?>
 
 <!DOCTYPE project [
-    <!ENTITY common SYSTEM "file:./common.xml">
+       <!ENTITY common SYSTEM "common.xml">
 ]>
 
 <project name="test" default="test" basedir=".">
@@ -1407,10 +1407,13 @@ while(<STDIN>) {
 </pre>
                         <p>will literally include the contents of <code>common.xml</code> where
         you've placed the <code>&amp;common;</code> entity.</p>
+                        <p>(The filename <code>common.xml</code> in this example is resolved
+        relative to the containing XML file by the XML parser. You may also use
+        an absolute <code>file:</code> protocol URI.)</p>
                         <p>In combination with a DTD, this would look like this:</p>
                         <pre class="code">
-&lt;!DOCTYPE project PUBLIC &quot;-//ANT//DTD project//EN&quot; &quot;file:./ant.dtd&quot; [
-   &lt;!ENTITY include SYSTEM &quot;file:./header.xml&quot;&gt;
+&lt;!DOCTYPE project PUBLIC &quot;-//ANT//DTD project//EN&quot; &quot;ant.dtd&quot; [
+   &lt;!ENTITY include SYSTEM &quot;header.xml&quot;&gt;
 ]&gt;
 </pre>
                         <p>Starting with Ant 1.6, there is a new

src/main/org/apache/tools/ant/AntClassLoader.java

@@ -1217,8 +1217,7 @@ public class AntClassLoader extends ClassLoader implements SubBuildListener {
 
         if (sealedString != null && sealedString.equalsIgnoreCase("true")) {
             try {
-                // XXX should be using FileUtils!
-                sealBase = new URL("file:" + container.getPath());
+                sealBase = new URL(FileUtils.getFileUtils().toURI(container.getAbsolutePath()));
             } catch (MalformedURLException e) {
                 // ignore
             }

src/main/org/apache/tools/ant/launch/Locator.java

@@ -130,8 +130,8 @@ public final class Locator {
      *
      * <p>Will be an absolute path if the given URI is absolute.</p>
      *
-     * <p>Swallows '%' that are not followed by two characters,
-     * doesn't deal with non-ASCII characters.</p>
+     * <p>Prior to Java 1.4,
+     * swallows '%' that are not followed by two characters.</p>
      *
      * @see <a href="http://www.w3.org/TR/xml11/#dt-sysid">dt-sysid</a>
      * which makes some mention of how
@@ -139,9 +139,45 @@ public final class Locator {
      *
      * @param uri the URI designating a file in the local filesystem.
      * @return the local file system path for the file.
+     * @throws IllegalArgumentException if the URI is malformed or not a legal file: URL
      * @since Ant 1.6
      */
     public static String fromURI(String uri) {
+        // #8031: first try Java 1.4.
+        Class uriClazz = null;
+        try {
+            uriClazz = Class.forName("java.net.URI");
+        } catch (ClassNotFoundException cnfe) {
+            // Fine, Java 1.3 or earlier, do it by hand.
+        }
+        // Also check for properly formed URIs. Ant formerly recommended using
+        // nonsense URIs such as "file:./foo.xml" in XML includes. You shouldn't
+        // do that (just "foo.xml" is correct) but for compatibility we special-case
+        // things when the path is not absolute, and fall back to the old parsing behavior.
+        if (uriClazz != null && uri.startsWith("file:/")) {
+            try {
+                java.lang.reflect.Method createMethod = uriClazz.getMethod("create", new Class[] {String.class});
+                Object uriObj = createMethod.invoke(null, new Object[] {uri});
+                java.lang.reflect.Constructor fileConst = File.class.getConstructor(new Class[] {uriClazz});
+                File f = (File)fileConst.newInstance(new Object[] {uriObj});
+                return f.getAbsolutePath();
+            } catch (java.lang.reflect.InvocationTargetException e) {
+                Throwable e2 = e.getTargetException();
+                if (e2 instanceof IllegalArgumentException) {
+                    // Bad URI, pass this on.
+                    throw (IllegalArgumentException)e2;
+                } else {
+                    // Unexpected target exception? Should not happen.
+                    e2.printStackTrace();
+                }
+            } catch (Exception e) {
+                // Reflection problems? Should not happen, debug.
+                e.printStackTrace();
+            }
+        }
+
+        // Fallback method for Java 1.3 or earlier.
+
         URL url = null;
         try {
             url = new URL(uri);

src/main/org/apache/tools/ant/taskdefs/optional/TraXLiaison.java

@@ -47,6 +47,7 @@ import org.apache.tools.ant.taskdefs.XSLTProcess;
 import org.apache.tools.ant.taskdefs.XSLTLogger;
 import org.apache.tools.ant.taskdefs.XSLTLoggerAware;
 import org.apache.tools.ant.types.XMLCatalog;
+import org.apache.tools.ant.util.FileUtils;
 import org.apache.tools.ant.util.JAXPUtils;
 import org.xml.sax.EntityResolver;
 import org.xml.sax.InputSource;
@@ -401,8 +402,8 @@ public class TraXLiaison implements XSLTLiaison2, ErrorListener, XSLTLoggerAware
             String systemid = locator.getSystemId();
             if (systemid != null) {
                 String url = systemid;
-                if (url.startsWith("file:///")) {
-                    url = url.substring(8);
+                if (url.startsWith("file:")) {
+                    url = FileUtils.newFileUtils().fromURI(url);
                 }
                 msg.append(url);
             } else {

src/main/org/apache/tools/ant/taskdefs/optional/depend/Depend.java

@@ -35,6 +35,7 @@ import org.apache.tools.ant.taskdefs.rmic.DefaultRmicAdapter;
 import org.apache.tools.ant.taskdefs.rmic.WLRmic;
 import org.apache.tools.ant.types.Path;
 import org.apache.tools.ant.types.Reference;
+import org.apache.tools.ant.util.FileUtils;
 import org.apache.tools.ant.util.depend.DependencyAnalyzer;
 
 /**
@@ -393,14 +394,15 @@ public class Depend extends MatchingTask {
                             if (classURL != null) {
                                 if (classURL.getProtocol().equals("jar")) {
                                     String jarFilePath = classURL.getFile();
+                                    int classMarker = jarFilePath.indexOf('!');
+                                    jarFilePath = jarFilePath.substring(0, classMarker);
                                     if (jarFilePath.startsWith("file:")) {
-                                        int classMarker = jarFilePath.indexOf('!');
-                                        jarFilePath = jarFilePath.substring(5, classMarker);
+                                        classpathFileObject = new File(FileUtils.getFileUtils().fromURI(jarFilePath));
+                                    } else {
+                                        throw new IOException("Bizarre nested path in jar: protocol: " + jarFilePath);
                                     }
-                                    classpathFileObject = new File(jarFilePath);
                                 } else if (classURL.getProtocol().equals("file")) {
-                                    String classFilePath = classURL.getFile();
-                                    classpathFileObject = new File(classFilePath);
+                                    classpathFileObject = new File(FileUtils.getFileUtils().fromURI(classURL.toExternalForm()));
                                 }
                                 log("Class " + className
                                     + " depends on " + classpathFileObject
@@ -544,9 +546,9 @@ public class Depend extends MatchingTask {
 
     /**
      * test for being an RMI stub
-     * @param affectedClass
-     * @param className
-     * @return
+     * @param affectedClass  class being tested
+     * @param className      possible origin of the RMI stub
+     * @return whether the class affectedClass is a RMI stub
      */
     private boolean isRmiStub(String affectedClass, String className) {
         return isStub(affectedClass, className, DefaultRmicAdapter.RMI_STUB_SUFFIX)

src/main/org/apache/tools/ant/taskdefs/optional/junit/XMLResultAggregator.java

@@ -33,6 +33,7 @@ import org.apache.tools.ant.Project;
 import org.apache.tools.ant.Task;
 import org.apache.tools.ant.types.FileSet;
 import org.apache.tools.ant.util.DOMElementWriter;
+import org.apache.tools.ant.util.FileUtils;
 import org.apache.tools.ant.util.StringUtils;
 import org.apache.tools.ant.util.FileUtils;
 import org.w3c.dom.Document;
@@ -246,7 +247,7 @@ public class XMLResultAggregator extends Task implements XMLConstants {
                 log("Parsing file: '" + file + "'", Project.MSG_VERBOSE);
                 if(file.length()>0) {
                     Document testsuiteDoc
-                            = builder.parse("file:///" + file.getAbsolutePath());
+                            = builder.parse(FileUtils.getFileUtils().toURI(files[i].getAbsolutePath()));
                     Element elem = testsuiteDoc.getDocumentElement();
                     // make sure that this is REALLY a testsuite.
                     if (TESTSUITE.equals(elem.getNodeName())) {

src/main/org/apache/tools/ant/taskdefs/optional/sitraka/CovReport.java

@@ -399,7 +399,7 @@ public class CovReport extends CovBase {
                 transformer.setOutputProperty(OutputKeys.INDENT, "yes");
                 transformer.setOutputProperty(OutputKeys.METHOD, "xml");
                 Source src = new DOMSource(doc);
-                Result res = new StreamResult("file:///" + tofile.toString());
+                Result res = new StreamResult(tofile);
                 transformer.transform(src, res);
             } catch (Exception e) {
                 throw new BuildException("Error while performing enhanced XML "

src/main/org/apache/tools/ant/util/FileUtils.java

@@ -1006,6 +1006,25 @@ public class FileUtils {
      * @since Ant 1.6
      */
     public String toURI(String path) {
+        // #8031: first try Java 1.4.
+        Class uriClazz = null;
+        try {
+            uriClazz = Class.forName("java.net.URI");
+        } catch (ClassNotFoundException e) {
+            // OK, Java 1.3.
+        }
+        if (uriClazz != null) {
+            try {
+                File f = new File(path).getAbsoluteFile();
+                java.lang.reflect.Method toURIMethod = File.class.getMethod("toURI", new Class[0]);
+                Object uriObj = toURIMethod.invoke(f, new Object[] {});
+                java.lang.reflect.Method toASCIIStringMethod = uriClazz.getMethod("toASCIIString", new Class[0]);
+                return (String) toASCIIStringMethod.invoke(uriObj, new Object[] {});
+            } catch (Exception e) {
+                // Reflection problems? Should not happen, debug.
+                e.printStackTrace();
+            }
+        }
         boolean isDir = new File(path).isDirectory();
 
         StringBuffer sb = new StringBuffer("file:");

src/testcases/org/apache/tools/ant/util/FileUtilsTest.java

@@ -452,45 +452,66 @@ public class FileUtilsTest extends TestCase {
             dosRoot = "";
         }
         if (Os.isFamily("dos")) {
-            assertEquals("file:///C:/foo", FILE_UTILS.toURI("c:\\foo"));
+            assertEquals("file:/c:/foo", removeExtraneousAuthority(FILE_UTILS.toURI("c:\\foo")));
         }
         if (Os.isFamily("netware")) {
-            assertEquals("file:///SYS:/foo", FILE_UTILS.toURI("sys:\\foo"));
+            assertEquals("file:/SYS:/foo", removeExtraneousAuthority(FILE_UTILS.toURI("sys:\\foo")));
+        }
+        if (File.pathSeparatorChar == '/') {
+            assertEquals("file:/foo", removeExtraneousAuthority(FILE_UTILS.toURI("/foo")));
+            assertTrue("file: URIs must name absolute paths", FILE_UTILS.toURI("./foo").startsWith("file:/"));
+            assertTrue(FILE_UTILS.toURI("./foo").endsWith("/foo"));
+            assertEquals("file:/" + dosRoot + "foo%20bar", removeExtraneousAuthority(FILE_UTILS.toURI("/foo bar")));
+            assertEquals("file:/" + dosRoot + "foo%23bar", removeExtraneousAuthority(FILE_UTILS.toURI("/foo#bar")));
+        } else if (File.pathSeparatorChar == '\\') {
+            assertEquals("file:/" + dosRoot + "foo", removeExtraneousAuthority(FILE_UTILS.toURI("\\foo")));
+            assertTrue("file: URIs must name absolute paths", FILE_UTILS.toURI(".\\foo").startsWith("file:/"));
+            assertTrue(FILE_UTILS.toURI(".\\foo").endsWith("/foo"));
+            assertEquals("file:/" + dosRoot + "foo%20bar", removeExtraneousAuthority(FILE_UTILS.toURI("\\foo bar")));
+            assertEquals("file:/" + dosRoot + "foo%23bar", removeExtraneousAuthority(FILE_UTILS.toURI("\\foo#bar")));
         }
-        assertEquals("file:///" + dosRoot + "foo", FILE_UTILS.toURI("/foo"));
-        /* May fail if the directory ${user.dir}/foo/ exists
-         * (and anyway is the tested behavior actually desirable?):
-        assertEquals("file:./foo",  fu.toURI("./foo"));
-         */
-        assertEquals("file:///" + dosRoot + "foo", FILE_UTILS.toURI("\\foo"));
-        /* See above:
-        assertEquals("file:./foo",  fu.toURI(".\\foo"));
-         */
-        assertEquals("file:///" + dosRoot + "foo%20bar", FILE_UTILS.toURI("/foo bar"));
-        assertEquals("file:///" + dosRoot + "foo%20bar", FILE_UTILS.toURI("\\foo bar"));
-        assertEquals("file:///" + dosRoot + "foo%23bar", FILE_UTILS.toURI("/foo#bar"));
-        assertEquals("file:///" + dosRoot + "foo%23bar", FILE_UTILS.toURI("\\foo#bar"));
         // a test with ant for germans
-        // i would expect here %E4NT ???
-        // anyway, this is the fix for the bug 37348wh
-        assertEquals("file:///" + dosRoot + "%C3%A4nt", FILE_UTILS.toURI("/\u00E4nt"));
+        // the escaped character used for the test is the "a umlaut"
+        // this is the fix for the bug 37348
+        assertEquals("file:/" + dosRoot + "%C3%A4nt", removeExtraneousAuthority(FILE_UTILS.toURI("/\u00E4nt")));
     }
 
+    /**
+     * Authority field is unnecessary, but harmless, in file: URIs.
+     * Java 1.4 does not produce it when using File.toURI.
+     */
+    private static String removeExtraneousAuthority(String uri) {
+        String prefix = "file:///";
+        if (uri.startsWith(prefix)) {
+            return "file:/" + uri.substring(prefix.length());
+        } else {
+            return uri;
+        }
+    }
     /**
      * test fromUri
      */
     public void testFromURI() {
+        String dosRoot = null;
+        if (Os.isFamily("dos") || Os.isFamily("netware")) {
+            dosRoot = Character.toUpperCase(
+                System.getProperty("user.dir").charAt(0)) + ":";
+        }
+        else
+        {
+            dosRoot = "";
+        }
         if (Os.isFamily("netware")) {
             assertEqualsIgnoreDriveCase("SYS:\\foo", FILE_UTILS.fromURI("file:///sys:/foo"));
         }
         if (Os.isFamily("dos")) {
             assertEqualsIgnoreDriveCase("C:\\foo", FILE_UTILS.fromURI("file:///c:/foo"));
         }
-        assertEqualsIgnoreDriveCase(File.separator + "foo", FILE_UTILS.fromURI("file:///foo"));
+        assertEqualsIgnoreDriveCase(dosRoot + File.separator + "foo", FILE_UTILS.fromURI("file:///foo"));
         assertEquals("." + File.separator + "foo",
                      FILE_UTILS.fromURI("file:./foo"));
-        assertEquals(File.separator + "foo bar", FILE_UTILS.fromURI("file:///foo%20bar"));
-        assertEquals(File.separator + "foo#bar", FILE_UTILS.fromURI("file:///foo%23bar"));
+        assertEquals(dosRoot + File.separator + "foo bar", FILE_UTILS.fromURI("file:///foo%20bar"));
+        assertEquals(dosRoot + File.separator + "foo#bar", FILE_UTILS.fromURI("file:///foo%23bar"));
     }
 
     public void testModificationTests() {

src/testcases/org/apache/tools/ant/util/JAXPUtilsTest.java

@@ -36,7 +36,7 @@ public class JAXPUtilsTest extends TestCase {
             file = new File("/user/local/bin");
         }
         String systemid = JAXPUtils.getSystemId(file);
-        assertTrue("SystemIDs should start by file:///", systemid.startsWith("file:///"));
+        assertTrue("SystemIDs should start by file:/", systemid.startsWith("file:/"));
         assertTrue("SystemIDs should not start with file:////", !systemid.startsWith("file:////"));
     }
 }

xdocs/faq.xml

@@ -1067,7 +1067,7 @@ while(<STDIN>) {
 <?xml version="1.0"?>
 
 <!DOCTYPE project [
-    <!ENTITY common SYSTEM "file:./common.xml">
+       <!ENTITY common SYSTEM "common.xml">
 ]>
 
 <project name="test" default="test" basedir=".">
@@ -1086,11 +1086,15 @@ while(<STDIN>) {
         <p>will literally include the contents of <code>common.xml</code> where
         you&apos;ve placed the <code>&amp;common;</code> entity.</p>
 
+        <p>(The filename <code>common.xml</code> in this example is resolved
+        relative to the containing XML file by the XML parser. You may also use
+        an absolute <code>file:</code> protocol URI.)</p>
+
         <p>In combination with a DTD, this would look like this:</p>
 
         <source><![CDATA[
-<!DOCTYPE project PUBLIC "-//ANT//DTD project//EN" "file:./ant.dtd" [
-   <!ENTITY include SYSTEM "file:./header.xml">
+<!DOCTYPE project PUBLIC "-//ANT//DTD project//EN" "ant.dtd" [
+   <!ENTITY include SYSTEM "header.xml">
 ]>
 ]]></source>