This is the mail archive of the java-patches@gcc.gnu.org mailing list for the Java project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: URLStreamHandler.parseURL() (was RE: Patch: more protocol fun in URL)


Hi Jeroen,

>To be honest I didn't look too closely at it originally. Sending bz2
>attachments is not a good way to get my attention ;-)
>
>However, I just looked at the test case and it works fine on IKVM (both
>on Linux and on Windows). Also, since file comes from the passed in URL,
>can't we assume that it already is in the correct format?
>
>So, as it stands, I'd like to see more evidence that this patch is
>correct, before committing it.

I'll try to make this email attachment-free :)

If I apply the following patch to the original URLStreamHandler libgcj
sources (like I said, I don't know to what extent these are the
same as the Classpath sources):

-----------------------------------------------------------------------------
Index: java/net/URLStreamHandler.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/net/URLStreamHandler.java,v
retrieving revision 1.29
diff -u -2 -r1.29 URLStreamHandler.java
--- java/net/URLStreamHandler.java	5 May 2004 08:51:04 -0000	1.29
+++ java/net/URLStreamHandler.java	5 Jul 2004 14:48:04 -0000
@@ -131,4 +131,12 @@
     String query = null;
     
+    if (spec.indexOf("hello.txt") != -1)
+      {
+	System.out.println("file: "+file);
+	System.out.println("spec: "+spec);
+	System.out.println("start: "+start);
+	System.out.println("end: "+end);
+      }
+    
     // On Windows we need to change \ to / for file URLs
     if (url.getProtocol().equals("file"))
@@ -206,4 +214,13 @@
 	int lastSlash = file.lastIndexOf('/');
 
+	if (spec.indexOf("hello.txt") != -1)
+	{
+	  System.out.println("lastSlash: "+lastSlash);
+	  if (lastSlash == -1)
+	    {
+	      System.out.println("I'm about to blow....");
+	    }
+	}
+
 	file =
 	  file.substring(0, lastSlash) + '/' + spec.substring(start, end);
-----------------------------------------------------------------------------

...and then run it with this program on Win32:

-----------------------------------------------------------------------------
import java.io.*;

public class ResourceAsStream
{
public static void main(String[] args)
{
    try
    {
        InputStream istr =
            ResourceAsStream.class.getResourceAsStream("/hello.txt");
        BufferedReader rdr =
            new BufferedReader(new InputStreamReader(istr));
        String strLine;
        while ((strLine = rdr.readLine()) != null)
            System.out.println(strLine);
    }
    catch (Exception e)
    {
        e.printStackTrace();
    }
}
}
-----------------------------------------------------------------------------

...I get this output:

-----------------------------------------------------------------------------
file: .\
spec: hello.txt
start: 0
end: 9
lastSlash: -1
I'm about to blow....
java.lang.StringIndexOutOfBoundsException
   at 0x0041547e (Unknown Source)
   ...
-----------------------------------------------------------------------------

If you look at the differences between revisions 1.28 and 1.27 in the libgcj
sources, you'll see that this incorrect behavior can be attributed to this
portion of your patch:

-----------------------------------------------------------------------------

@@ -193,18 +203,10 @@
     else if (start < end)
       {
 // Context is available, but only override it if there is a new file.
-char sepChar = '/';
-int lastSlash = file.lastIndexOf(sepChar);
-if (lastSlash < 0 && File.separatorChar != sepChar
-    && url.getProtocol().equals("file"))
-  {
-    // On Windows, even '\' is allowed in a "file" URL.
-    sepChar = File.separatorChar;
-    lastSlash = file.lastIndexOf(sepChar);
-  }
+int lastSlash = file.lastIndexOf('/');

 file =
-  file.substring(0, lastSlash) + sepChar + spec.substring(start, end);
+  file.substring(0, lastSlash) + '/' + spec.substring(start, end);

 if (url.getProtocol().equals("file"))
   {
-----------------------------------------------------------------------------

You have removed this block:

-if (lastSlash < 0 && File.separatorChar != sepChar
-    && url.getProtocol().equals("file"))
-  {
-    // On Windows, even '\' is allowed in a "file" URL.
-    sepChar = File.separatorChar;
-    lastSlash = file.lastIndexOf(sepChar);
-  }

...and have replaced it with this:

+int lastSlash = file.lastIndexOf('/');

...but nowhere in your patch have you changed the
original contents of the file variable, which contains
this backslash.

-- Mohan
http://www.thisiscool.com/
http://www.animalsong.org/





Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]