Re: URL's fix

[Mark Wielaard]

Hi,

On Wed, 2003-10-29 at 19:29, Guilhem Lavaux wrote:

> I noticed some misbehaviour of classpath's URL compared to 
> Sun's JDK while merging these classes in kaffe's library. 
> Kaffe's regression test "URLTest" was failing nearly 
> completely.

I turned that kaffe test into a new Mauve test.

> BTW, there was an error when url of the type 
> "http://anonymous:address@hidden/"; was entered: URL reports 
> to detect "anonymous" as the host.

I also added that to the new Mauve test and saw that your patch indeed
fixes the parsing, but that we also had a bug in the toExternalForm() of
such URLs. Fix as follows:

2003-11-13  Guilhem Lavaux  <address@hidden>
            Mark Wielaard  <address@hidden>

        * java/net/URLStreamHandler (parseUrl): Fixed URL parsing
        ('@' should be checked to distinguish port from userinfo).
        (toExternalForm): Add @ userInfo if necessary.

> This patch fixes all these things.

It is easier when patches are split up into small parts.
That makes reviewing them much easier. Will review the rest of you patch
later.

Cheers,

Mark

Index: java/net/URLStreamHandler.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/net/URLStreamHandler.java,v
retrieving revision 1.19
diff -u -r1.19 URLStreamHandler.java
--- java/net/URLStreamHandler.java      19 Sep 2003 06:19:42 -0000      1.19
+++ java/net/URLStreamHandler.java      13 Nov 2003 22:51:54 -0000
@@ -129,11 +129,12 @@
     
     if (spec.regionMatches (start, "//", 0, 2))
       {
+       String genuineHost;
        int hostEnd;
-       int colon;
+       int colon, at_host;
 
        start += 2;
-       int slash = spec.indexOf('/', start);
+       int slash = spec.indexOf ('/', start);
        if (slash >= 0) 
          hostEnd = slash;
         else
@@ -141,24 +142,37 @@
 
        host = spec.substring (start, hostEnd);
        
+       // We first need a genuine host name (with userinfo).
+       // So we check for '@': if it's present check the port in the
+       // section after '@' in the other case check it in the full string.
+       // P.S.: We don't care having '@' at the beginning of the string.
+       if ((at_host = host.indexOf ('@')) >= 0)
+         genuineHost = host.substring (at_host);
+       else
+         genuineHost = host;
+
        // Look for optional port number.  It is valid for the non-port
        // part of the host name to be null (e.g. a URL "http://:80";).
        // TBD: JDK 1.2 in this case sets host to null rather than "";
        // this is undocumented and likely an unintended side effect in 1.2
        // so we'll be simple here and stick with "". Note that
        // "http://"; or "http:///"; produce a "" host in JDK 1.2.
-       if ((colon = host.indexOf(':')) >= 0)
+       if ((colon = genuineHost.indexOf (':')) >= 0)
          {
            try
              {
-               port = Integer.parseInt(host.substring(colon + 1));
+               port = Integer.parseInt (genuineHost.substring (colon + 1));
              }
            catch (NumberFormatException e)
              {
                ; // Ignore invalid port values; port is already set to u's
                  // port.
              }
-           host = host.substring(0, colon);
+           // Now we must cut the port number in the original string.
+           if (at_host >= 0)
+             host = host.substring (0, at_host + colon);
+           else
+             host = host.substring (0, colon);
          }
        file = null;
        start = hostEnd;
@@ -451,7 +465,7 @@
    */
   protected String toExternalForm(URL u)
   {
-    String protocol, host, file, ref;
+    String protocol, host, file, ref, user;
     int port;
 
     protocol = u.getProtocol();
@@ -465,6 +479,7 @@
     port = u.getPort();
     file = u.getFile();
     ref = u.getRef();
+    user = u.getUserInfo();
 
     // Guess a reasonable size for the string buffer so we have to resize
     // at most once.
@@ -478,7 +493,12 @@
       }
 
     if (host.length() != 0)
-      sb.append("//").append(host);
+      {
+       sb.append("//");
+       if (user != null && !"".equals(user))
+         sb.append(user).append('@');
+       sb.append(host);
+      }
 
     // Note that this produces different results from JDK 1.2 as JDK 1.2
     // ignores a non-default port if host is null or "".  That is inconsistent

signature.asc
Description: This is a digitally signed message part