471464 - Parsing issues with HttpURI

+ Updating expectations on HttpURIParseTest
+ Making results of new HttpURI(String) and new HttpURI(URI) consistent
+ Making results of HttpURI parsing consistent with java.net.URI
+ Making output of HttpURI.toString() and java.net.URI.toASCIIString()
  consistent with regards to ssp (scheme specific part) behavior

2 files changed, 105 insertions, 49 deletions

diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java
index 142d54a69b..0c79193850 100644
--- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java
+++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java
@@ -65,6 +65,9 @@ public class HttpURI
     ASTERISK};
 
     private String _scheme;
+    // used by toString for writing out scheme specific part properly
+    private boolean _hasAuthHostPortInSSP = false;
+    private String _user;
     private String _host;
     private int _port;
     private String _path;
@@ -108,6 +111,7 @@ public class HttpURI
         _scheme = scheme;
         _host = host;
         _port = port;
+        _hasAuthHostPortInSSP = (_host != null);
         _path = path;
         _param = param;
         _query = query;
@@ -135,11 +139,17 @@ public class HttpURI
         _scheme=uri.getScheme();
         _host=uri.getHost();
         _port=uri.getPort();
+        _hasAuthHostPortInSSP=uri.getRawSchemeSpecificPart().startsWith("//");
+        _user = uri.getUserInfo();
         _path=uri.getRawPath();
-        _decodedPath=uri.getPath();
-        int p=_path.lastIndexOf(';');
-        if (p>=0)
-            _param=_path.substring(p+1);
+        
+        _decodedPath = uri.getPath();
+        if (_decodedPath != null)
+        {
+            int p = _decodedPath.lastIndexOf(';');
+            if (p >= 0)
+                _param = _decodedPath.substring(p + 1);
+        }
         _query=uri.getRawQuery();
         _fragment=uri.getFragment();
         
@@ -180,8 +190,8 @@ public class HttpURI
     private void parse(State state, final String uri, final int offset, final int end)
     {
         boolean encoded=false;
-        int m=offset;
-        int p=0;
+        int m=offset; // mark?
+        int p=0;      // position?
         
         for (int i=offset; i<end; i++)
         {
@@ -194,14 +204,28 @@ public class HttpURI
                     switch(c)
                     {
                         case '/':
+                            if (i + 1 < end)
+                            {
+                                c = uri.charAt(i+1);
+                                m = i;
+                                if (c == '/')
+                                {
+                                    _hasAuthHostPortInSSP = true;
+                                    state = State.HOST_OR_PATH;
+                                    break;
+                                }
+                            }
+                            
                             p=m=i;
-                            state=State.PATH;
+                            state = State.PATH;
                             break;
                         case ';':
                             m=i+1;
                             state=State.PARAM;
                             break;
                         case '?':
+                            // assume empty path (if seen at start)
+                            _path = "";
                             m=i+1;
                             state=State.QUERY;
                             break;
@@ -249,7 +273,7 @@ public class HttpURI
                         case ';':
                         {
                             // must have been in a path 
-                            p=m;
+                            m=i+1;
                             state=State.PARAM;
                             break;
                         }
@@ -258,6 +282,7 @@ public class HttpURI
                         {
                             // must have been in a path 
                             _path=uri.substring(m,i);
+                            m=i+1;
                             state=State.QUERY;
                             break;
                         }
@@ -283,10 +308,16 @@ public class HttpURI
                     switch(c)
                     {
                         case '/':
+                            _hasAuthHostPortInSSP = true;
                             m=i+1;
                             state=State.HOST;
                             break;
                             
+                        case '@':
+                            _user=uri.substring(m,i);
+                            m=i+1;
+                            break;
+                            
                         case ';':
                         case '?':
                         case '#':
@@ -309,20 +340,22 @@ public class HttpURI
                     {
                         case '/':
                         {
-                            _host=uri.substring(m,i);
+                            if (i > m)
+                                _host = uri.substring(m,i);
                             p=m=i;
                             state=State.PATH;
                             break;
                         }
                         case ':':
                         {
-                            _host=uri.substring(m,i);
+                            if (i > m)
+                                _host=uri.substring(m,i);
                             m=i+1;
                             state=State.PORT;
                             break;
                         }
                         case '@':
-                            // ignore user
+                            _user=uri.substring(m,i);
                             m=i+1;
                             break;
                             
@@ -482,7 +515,8 @@ public class HttpURI
                 break;
                 
             case HOST:
-                _host=uri.substring(m,end);
+                if(end>m)
+                    _host=uri.substring(m,end);
                 break;
                 
             case IPV6:
@@ -541,6 +575,11 @@ public class HttpURI
     }
 
     /* ------------------------------------------------------------ */
+    /**
+     * The parsed Path.
+     * 
+     * @return the path as parsed, or null if undefined
+     */
     public String getPath()
     {
         return _path;
@@ -637,8 +676,15 @@ public class HttpURI
             if (_scheme!=null)
                 out.append(_scheme).append(':');
             
-            if (_host!=null)
-                out.append("//").append(_host);
+            if(_hasAuthHostPortInSSP)
+                out.append("//");
+            
+            if (_host != null)
+            {
+                if (_user != null)
+                    out.append(_user).append('@');
+                out.append(_host);
+            }
             
             if (_port>0)
                 out.append(':').append(_port);
@@ -686,6 +732,7 @@ public class HttpURI
     {
         _host=host;
         _port=port;
+        _hasAuthHostPortInSSP = (_host != null);
         _uri=null;
     }
 
diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURIParseTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURIParseTest.java
index 4b42bf436a..ad21864978 100644
--- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURIParseTest.java
+++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURIParseTest.java
@@ -28,7 +28,6 @@ import java.net.URISyntaxException;
 import java.util.Arrays;
 import java.util.List;
 
-import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
@@ -39,8 +38,6 @@ import org.junit.runners.Parameterized.Parameters;
 @RunWith(Parameterized.class)
 public class HttpURIParseTest
 {
-    public static int INPUT=0,SCHEME=1,HOST=2,PORT=3,PATH=4,PARAM=5,QUERY=6,FRAGMENT=7;
-
     @Parameters(name="{0}")
     public static List<String[]> data()
     {
@@ -73,11 +70,10 @@ public class HttpURIParseTest
         
         // Protocol Less (aka scheme-less) URIs
         
-        // FIXME (these have host and port)
-        {"//host/path/info",null,null,null,"//host/path/info",null,null,null},
-        {"//user@host/path/info",null,null,null,"//user@host/path/info",null,null,null},
-        {"//user@host:8080/path/info",null,null,null,"//user@host:8080/path/info",null,null,null},
-        {"//host:8080/path/info",null,null,null,"//host:8080/path/info",null,null,null},
+        {"//host/path/info",null,"host",null,"/path/info",null,null,null},
+        {"//user@host/path/info",null,"host",null,"/path/info",null,null,null},
+        {"//user@host:8080/path/info",null,"host","8080","/path/info",null,null,null},
+        {"//host:8080/path/info",null,"host","8080","/path/info",null,null,null},
         
         // Host Less 
         
@@ -92,14 +88,12 @@ public class HttpURIParseTest
         
         // Everything and the kitchen sink
         
-        // FIXME ("user@" authentication information is lost during parse and toString)
         {"http://user@host:8080/path/info;param?query#fragment","http","host","8080","/path/info;param","param","query","fragment"},
         {"xxxxx://user@host:8080/path/info;param?query#fragment","xxxxx","host","8080","/path/info;param","param","query","fragment"},
         
         // No host, parameter with no content
         
-        // FIXME (no host, should result in null for host, not empty string)
-        {"http:///;?#","http","",null,"/;","","",""},
+        {"http:///;?#","http",null,null,"/;","","",""},
         
         // Path with query that has no value
         
@@ -111,20 +105,18 @@ public class HttpURIParseTest
         
         // Scheme-less, with host and port (overlapping with path)
         
-        // FIXME (this has host and port)
-        {"//host:8080//",null,null,null,"//host:8080//",null,null,null},
+        {"//host:8080//",null,"host","8080","//",null,null,null},
         
         // File reference
         
-        // FIXME (no host, should result in null for host, not empty string)
-        {"file:///path/info","file","",null,"/path/info",null,null,null},
+        {"file:///path/info","file",null,null,"/path/info",null,null,null},
         {"file:/path/info","file",null,null,"/path/info",null,null,null},
         
-        // Without Authority (this is a bad URI according to spec) 
+        // Bad URI (no scheme, no host, no path) 
         
-        {"//",null,null,null,"//",null,null,null},
+        {"//",null,null,null,null,null,null,null},
         
-        // Simple Localhost references
+        // Simple localhost references
         
         {"http://localhost/","http","localhost",null,"/",null,null,null},
         {"http://localhost:8080/", "http", "localhost","8080","/", null, null,null},
@@ -153,7 +145,6 @@ public class HttpURIParseTest
         
         // IPv6 authenticated host with port (default path)
         
-        // FIXME ("user@" authentication information is lost during parse and toString)
         {"http://user@[2001:db8::1]:8080/","http","[2001:db8::1]","8080","/",null,null,null},
         
         // Simple IPv6 host no port (default path)
@@ -162,8 +153,7 @@ public class HttpURIParseTest
         
         // Scheme-less IPv6, host with port (default path)
         
-        // FIXME (this has host and port)
-        {"//[2001:db8::1]:8080/",null,null,null,"//[2001:db8::1]:8080/",null,null,null},
+        {"//[2001:db8::1]:8080/",null,"[2001:db8::1]","8080","/",null,null,null},
         
         // Interpreted as relative path of "*" (no host/port/scheme/query/fragment)
         
@@ -173,12 +163,11 @@ public class HttpURIParseTest
         {"http://host:8080/path/info?q1=v1&q2=v2","http","host","8080","/path/info",null,"q1=v1&q2=v2",null},
         {"/path/info?q1=v1&q2=v2",null,null,null,"/path/info",null,"q1=v1&q2=v2",null},
         {"/info?q1=v1&q2=v2",null,null,null,"/info",null,"q1=v1&q2=v2",null},
-        // FIXME (Bad Path/Query results) {"info?q1=v1&q2=v2",null,null,null,"info",null,"q1=v1&q2=v2",null},
-        // FIXME (StringIndexOutOfBoundsException) {"info;q1=v1?q2=v2",null,null,null,"info;q1=v1",null,"q2=v2",null},
+        {"info?q1=v1&q2=v2",null,null,null,"info",null,"q1=v1&q2=v2",null},
+        {"info;q1=v1?q2=v2",null,null,null,"info;q1=v1","q1=v1","q2=v2",null},
         
         // Path-less, query only (seen from JSP/JSTL and <c:url> use
-        // FIXME (path should be null in parse(URI) version)
-        {"?q1=v1&q2=v2",null,null,null,null,null,"q1=v1&q2=v2",null}
+        {"?q1=v1&q2=v2",null,null,null,"",null,"q1=v1&q2=v2",null}
         };
         
         return Arrays.asList(tests);
@@ -213,18 +202,38 @@ public class HttpURIParseTest
     {
         HttpURI httpUri = new HttpURI(input);
         
-        assertThat("[" + input + "] .scheme",httpUri.getScheme(),is(scheme));
-        assertThat("[" + input + "] .host",httpUri.getHost(),is(host));
-        assertThat("[" + input + "] .port",httpUri.getPort(),is(port == null ? -1 : Integer.parseInt(port)));
-        assertThat("[" + input + "] .path",httpUri.getPath(),is(path));
-        assertThat("[" + input + "] .param",httpUri.getParam(),is(param));
-        assertThat("[" + input + "] .query",httpUri.getQuery(),is(query));
-        assertThat("[" + input + "] .fragment",httpUri.getFragment(),is(fragment));
-        assertThat("[" + input + "] .toString",httpUri.toString(),is(input));
+        try
+        {
+            new URI(input);
+            // URI is valid (per java.net.URI parsing)
+            
+            // Test case sanity check
+            assertThat("[" + input + "] expected path (test case) cannot be null",path,notNullValue());
+
+            // Assert expectations
+            assertThat("[" + input + "] .scheme",httpUri.getScheme(),is(scheme));
+            assertThat("[" + input + "] .host",httpUri.getHost(),is(host));
+            assertThat("[" + input + "] .port",httpUri.getPort(),is(port == null ? -1 : Integer.parseInt(port)));
+            assertThat("[" + input + "] .path",httpUri.getPath(),is(path));
+            assertThat("[" + input + "] .param",httpUri.getParam(),is(param));
+            assertThat("[" + input + "] .query",httpUri.getQuery(),is(query));
+            assertThat("[" + input + "] .fragment",httpUri.getFragment(),is(fragment));
+            assertThat("[" + input + "] .toString",httpUri.toString(),is(input));
+        }
+        catch (URISyntaxException e)
+        {
+            // Assert HttpURI values for invalid URI (such as "//")
+            assertThat("[" + input + "] .scheme",httpUri.getScheme(),is(nullValue()));
+            assertThat("[" + input + "] .host",httpUri.getHost(),is(nullValue()));
+            assertThat("[" + input + "] .port",httpUri.getPort(),is(-1));
+            assertThat("[" + input + "] .path",httpUri.getPath(),is(nullValue()));
+            assertThat("[" + input + "] .param",httpUri.getParam(),is(nullValue()));
+            assertThat("[" + input + "] .query",httpUri.getQuery(),is(nullValue()));
+            assertThat("[" + input + "] .fragment",httpUri.getFragment(),is(nullValue()));
+        }
     }
     
     @Test
-    @Ignore("There are many examples of inconsistent results from .testParseString()")
     public void testParseURI() throws Exception
     {
         URI javaUri = null;
@@ -248,11 +257,11 @@ public class HttpURIParseTest
         assertThat("[" + input + "] .param",httpUri.getParam(),is(param));
         assertThat("[" + input + "] .query",httpUri.getQuery(),is(query));
         assertThat("[" + input + "] .fragment",httpUri.getFragment(),is(fragment));
+        
         assertThat("[" + input + "] .toString",httpUri.toString(),is(input));
     }
     
     @Test
-    @Ignore("There are many examples of inconsistent results from .testParseString()")
     public void testCompareToJavaNetURI() throws Exception
     {
         URI javaUri = null;