bug 42275...handle failure of File.toURI() by printing the message (too noisy?) and falling back to our own code.

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

WHATSNEW

@@ -19,6 +19,10 @@ Fixed bugs:
 * Regression: Locator fails with URI encoding problem when spaces in path
   Bugzilla report 42222
 
+* Regression in Locator: running Ant off a network share does not work:
+  message "URI has authority component" appears
+  Bugzilla report 42275
+
 * Improvements in AntClassLoader Speed.
   Bugzilla report 42259
 
@@ -76,13 +80,14 @@ Fixed bugs:
  
 * -autoproxy turns Java1.5+ automatic proxy support on. Bugzilla 41904
 
-* handle null result of system getProperty(). Bugzilla 42334.
+* Handle null result of system getProperty(). Bugzilla 42334.
 
 * Regression: concat fixlastline="true" should not have applied to
   nested text, but did in Ant 1.7.0. Bugzilla 42369.
 
 * Regression: ant.version was not passed down in <subant>.
-  This worked in Ant1.6.5, but not in 1.7.0. Bugzilla bug 42263 
+  This worked in Ant1.6.5, but not in 1.7.0. Bugzilla bug 42263
+
 
 Other changes:
 --------------

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

@@ -32,6 +32,16 @@ import java.util.Locale;
  * The Locator is a utility class which is used to find certain items
  * in the environment.
  *
+ * It is used at boot time in the launcher, and cannot make use of any of Ant's other classes.
+ *
+ * This is a surprisingly brittle piece of code, and has had lots of bugs filed against it.
+ * {@link <a href="http://issues.apache.org/bugzilla/show_bug.cgi?id=42275">running ant off a network share can cause Ant to fail</a>}
+ * {@link <a href="http://issues.apache.org/bugzilla/show_bug.cgi?id=8031">use File.toURI().toURL().toExternalForm()</a>}
+ * {@link <a href="http://issues.apache.org/bugzilla/show_bug.cgi?id=42222">Locator implementation not encoding URI strings properly: spaces in paths</a>}
+ * It also breaks Eclipse 3.3 Betas
+ * {@link <a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=183283">Exception if installation path has spaces</a>}
+ *
+ * Be very careful when making changes to this class, as a break will upset a lot of people.
  * @since Ant 1.6
  */
 public final class Locator {
@@ -151,12 +161,28 @@ public final class Locator {
      * @since Ant 1.6
      */
     public static String fromURI(String uri) {
-        // #8031: first try Java 1.4.
+        // #buzilla8031: first try Java 1.4.
+        String result = null;
+        //result = fromUriJava14(uri);
+        if (result == null) {
+            result = fromURIJava13(uri);
+        }
+        return result;
+    }
+
+
+    /**
+     * Java1.4+ code to extract the path from the URI.
+     * @param uri
+     * @return null if a conversion was not possible
+     */
+    private static String fromUriJava14(String uri) {
         Class uriClazz = null;
         try {
             uriClazz = Class.forName("java.net.URI");
         } catch (ClassNotFoundException cnfe) {
             // Fine, Java 1.3 or earlier, do it by hand.
+            return null;
         }
         // Also check for properly formed URIs. Ant formerly recommended using
         // nonsense URIs such as "file:./foo.xml" in XML includes. You shouldn't
@@ -165,18 +191,22 @@ public final class Locator {
         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[] {encodeURI(uri)});
+                        = uriClazz.getMethod("create", new Class[]{String.class});
+                Object uriObj = createMethod.invoke(null, new Object[]{encodeURI(uri)});
                 java.lang.reflect.Constructor fileConst
-                    = File.class.getConstructor(new Class[] {uriClazz});
-                File f = (File) fileConst.newInstance(new Object[] {uriObj});
+                        = File.class.getConstructor(new Class[]{uriClazz});
+                File f = (File) fileConst.newInstance(new Object[]{uriObj});
                 //bug #42227 forgot to decode before returning
                 return decodeUri(f.getAbsolutePath());
             } catch (java.lang.reflect.InvocationTargetException e) {
                 Throwable e2 = e.getTargetException();
                 if (e2 instanceof IllegalArgumentException) {
                     // Bad URI, pass this on.
-                    throw new IllegalArgumentException("Bad URI "+uri+ ":"+e2.getMessage(),e2);
+                    // no, this is downgraded to a warning after various JRE bugs surfaced. Hand off
+                    // to our built in code on a failure
+                    //throw new IllegalArgumentException("Bad URI " + uri + ":" + e2.getMessage(), e2);
+                    e2.printStackTrace();
+
                 } else {
                     // Unexpected target exception? Should not happen.
                     e2.printStackTrace();
@@ -186,13 +216,14 @@ public final class Locator {
                 e.printStackTrace();
             }
         }
-        return fromURIJava13(uri);
+        return null;
     }
 
     /**
      * This is only public for testing purposes, so its use is strongly discouraged.
      * @param uri uri to expand
      * @return the decoded URI
+     * @since Ant1.7.1
      */
     public static String fromURIJava13(String uri) {
         // Fallback method for Java 1.3 or earlier.

src/tests/junit/org/apache/tools/ant/launch/LocatorTest.java

@@ -20,6 +20,9 @@ package org.apache.tools.ant.launch;
 
 import junit.framework.TestCase;
 
+import java.net.URISyntaxException;
+import java.io.File;
+
 /** Test the locator in the ant-launch JAR */
 
 public class LocatorTest extends TestCase {
@@ -40,19 +43,43 @@ public class LocatorTest extends TestCase {
     private String resolve(String uri) {
         String j14= Locator.fromURI(uri);
         String j13 = Locator.fromURIJava13(uri);
-        assertEquals(uri,j14,j13);
+        assertEquals("Different fromURI conversion.\nJava1.4="+j14+"\nJava1.3="+j13+"\n",
+                j14, j13);
         return j14;
     }
 
     private void resolveTo(String uri,String expectedResult) {
         String result = resolve(uri);
-        assertEquals(uri,expectedResult,result);
+        assertResolved(uri, expectedResult, result);
+    }
+
+    private void assertResolved(String uri, String expectedResult, String result) {
+        assertEquals("Expected "+uri+" to resolve to \n"+expectedResult+"\n but got\n"+result+"\n",
+                expectedResult,result);
+    }
+
+    /**
+     * This asserts that we can round trip the path to a URI and back again
+     * @param path
+     */
+    private void assertResolves(String path) throws Exception {
+        String asuri = new File(path).toURI().toASCIIString();
+        logURI(path +" => "+asuri);
+        resolveTo(asuri,path);
     }
 
     private void resolveTo13(String uri, String expectedResult) {
         String result = Locator.fromURIJava13(uri);
-        assertEquals(uri, expectedResult, result);
+        assertResolved(uri, expectedResult, result);
     }
+
+    private void logURI(String path) throws URISyntaxException{
+
+        String s = new File(path).toURI().toASCIIString();
+        System.out.println(path+" => "+s);
+
+    }
+
     /**
      * this isnt really a valid URI, except maybe in IE
      * @throws Exception
@@ -61,7 +88,7 @@ public class LocatorTest extends TestCase {
         resolveTo("file:\\\\PC03\\jclasses\\lib\\ant-1.7.0.jar","\\\\PC03\\jclasses\\lib\\ant-1.7.0.jar");
     }
 
-    public void testTripleForwardSlashNetworkURI_BugID_42275() throws Exception {
+    public void testTripleForwardSlashNetworkURI() throws Exception {
         resolveTo("file:///PC03/jclasses/lib/ant-1.7.0.jar", "///PC03/jclasses/lib/ant-1.7.0.jar");
     }
 
@@ -93,7 +120,13 @@ public class LocatorTest extends TestCase {
     }
 
 
+    public void testInternationalURI() throws Exception {
+        assertResolves("/L\\u00f6wenbrau/aus/M\\u00fcnchen");
+    }
 
+    public void testOddLowAsciiURI() throws Exception {
+        assertResolves("/hash#/ and /percent%");
+    }
 
 
 }