Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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
This commit is contained in:
Joakim Erdfelt 2015-06-30 17:37:40 -07:00
parent f3fe4331c4
commit e7d733bda0
2 changed files with 105 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

75
jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java
View File

@ -65,6 +65,9 @@ public class HttpURI
ASTERISK}; ASTERISK};
private String _scheme; private String _scheme;
// used by toString for writing out scheme specific part properly
private boolean _hasAuthHostPortInSSP = false;
private String _user;
private String _host; private String _host;
private int _port; private int _port;
private String _path; private String _path;
@ -108,6 +111,7 @@ public class HttpURI
_scheme = scheme; _scheme = scheme;
_host = host; _host = host;
_port = port; _port = port;
_hasAuthHostPortInSSP = (_host != null);
_path = path; _path = path;
_param = param; _param = param;
_query = query; _query = query;
@ -135,11 +139,17 @@ public class HttpURI
_scheme=uri.getScheme(); _scheme=uri.getScheme();
_host=uri.getHost(); _host=uri.getHost();
_port=uri.getPort(); _port=uri.getPort();
_hasAuthHostPortInSSP=uri.getRawSchemeSpecificPart().startsWith("//");
_user = uri.getUserInfo();
_path=uri.getRawPath(); _path=uri.getRawPath();
_decodedPath=uri.getPath();
int p=_path.lastIndexOf(';'); _decodedPath = uri.getPath();
if (p>=0) if (_decodedPath != null)
_param=_path.substring(p+1); {
int p = _decodedPath.lastIndexOf(';');
if (p >= 0)
_param = _decodedPath.substring(p + 1);
}
_query=uri.getRawQuery(); _query=uri.getRawQuery();
_fragment=uri.getFragment(); _fragment=uri.getFragment();
@ -180,8 +190,8 @@ public class HttpURI
private void parse(State state, final String uri, final int offset, final int end) private void parse(State state, final String uri, final int offset, final int end)
{ {
boolean encoded=false; boolean encoded=false;
int m=offset; int m=offset; // mark?
int p=0; int p=0; // position?
for (int i=offset; i<end; i++) for (int i=offset; i<end; i++)
{ {
@ -194,14 +204,28 @@ public class HttpURI
switch(c) switch(c)
{ {
case '/': 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; p=m=i;
state=State.PATH; state = State.PATH;
break; break;
case ';': case ';':
m=i+1; m=i+1;
state=State.PARAM; state=State.PARAM;
break; break;
case '?': case '?':
// assume empty path (if seen at start)
_path = "";
m=i+1; m=i+1;
state=State.QUERY; state=State.QUERY;
break; break;
@ -249,7 +273,7 @@ public class HttpURI
case ';': case ';':
{ {
// must have been in a path // must have been in a path
p=m; m=i+1;
state=State.PARAM; state=State.PARAM;
break; break;
} }
@ -258,6 +282,7 @@ public class HttpURI
{ {
// must have been in a path // must have been in a path
_path=uri.substring(m,i); _path=uri.substring(m,i);
m=i+1;
state=State.QUERY; state=State.QUERY;
break; break;
} }
@ -283,10 +308,16 @@ public class HttpURI
switch(c) switch(c)
{ {
case '/': case '/':
_hasAuthHostPortInSSP = true;
m=i+1; m=i+1;
state=State.HOST; state=State.HOST;
break; break;
case '@':
_user=uri.substring(m,i);
m=i+1;
break;
case ';': case ';':
case '?': case '?':
case '#': case '#':
@ -309,20 +340,22 @@ public class HttpURI
{ {
case '/': case '/':
{ {
_host=uri.substring(m,i); if (i > m)
_host = uri.substring(m,i);
p=m=i; p=m=i;
state=State.PATH; state=State.PATH;
break; break;
} }
case ':': case ':':
{ {
_host=uri.substring(m,i); if (i > m)
_host=uri.substring(m,i);
m=i+1; m=i+1;
state=State.PORT; state=State.PORT;
break; break;
} }
case '@': case '@':
// ignore user _user=uri.substring(m,i);
m=i+1; m=i+1;
break; break;
@ -482,7 +515,8 @@ public class HttpURI
break; break;
case HOST: case HOST:
_host=uri.substring(m,end); if(end>m)
_host=uri.substring(m,end);
break; break;
case IPV6: case IPV6:
@ -541,6 +575,11 @@ public class HttpURI
} }
/* ------------------------------------------------------------ */ /* ------------------------------------------------------------ */
/**
* The parsed Path.
*
* @return the path as parsed, or null if undefined
*/
public String getPath() public String getPath()
{ {
return _path; return _path;
@ -637,8 +676,15 @@ public class HttpURI
if (_scheme!=null) if (_scheme!=null)
out.append(_scheme).append(':'); out.append(_scheme).append(':');
if (_host!=null) if(_hasAuthHostPortInSSP)
out.append("//").append(_host); out.append("//");
if (_host != null)
{
if (_user != null)
out.append(_user).append('@');
out.append(_host);
}
if (_port>0) if (_port>0)
out.append(':').append(_port); out.append(':').append(_port);
@ -686,6 +732,7 @@ public class HttpURI
{ {
_host=host; _host=host;
_port=port; _port=port;
_hasAuthHostPortInSSP = (_host != null);
_uri=null; _uri=null;
} }

79
jetty-http/src/test/java/org/eclipse/jetty/http/HttpURIParseTest.java
View File

@ -28,7 +28,6 @@ import java.net.URISyntaxException;
import java.util.Arrays; import java.util.Arrays;
import java.util.List; import java.util.List;
import org.junit.Ignore;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.junit.runners.Parameterized; import org.junit.runners.Parameterized;
@ -39,8 +38,6 @@ import org.junit.runners.Parameterized.Parameters;
@RunWith(Parameterized.class) @RunWith(Parameterized.class)
public class HttpURIParseTest 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}") @Parameters(name="{0}")
public static List<String[]> data() public static List<String[]> data()
{ {
@ -73,11 +70,10 @@ public class HttpURIParseTest
// Protocol Less (aka scheme-less) URIs // Protocol Less (aka scheme-less) URIs
// FIXME (these have host and port) {"//host/path/info",null,"host",null,"/path/info",null,null,null},
{"//host/path/info",null,null,null,"//host/path/info",null,null,null}, {"//user@host/path/info",null,"host",null,"/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,"host","8080","/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,"host","8080","/path/info",null,null,null},
{"//host:8080/path/info",null,null,null,"//host:8080/path/info",null,null,null},
// Host Less // Host Less
@ -92,14 +88,12 @@ public class HttpURIParseTest
// Everything and the kitchen sink // 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"}, {"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"}, {"xxxxx://user@host:8080/path/info;param?query#fragment","xxxxx","host","8080","/path/info;param","param","query","fragment"},
// No host, parameter with no content // No host, parameter with no content
// FIXME (no host, should result in null for host, not empty string) {"http:///;?#","http",null,null,"/;","","",""},
{"http:///;?#","http","",null,"/;","","",""},
// Path with query that has no value // Path with query that has no value
@ -111,20 +105,18 @@ public class HttpURIParseTest
// Scheme-less, with host and port (overlapping with path) // Scheme-less, with host and port (overlapping with path)
// FIXME (this has host and port) {"//host:8080//",null,"host","8080","//",null,null,null},
{"//host:8080//",null,null,null,"//host:8080//",null,null,null},
// File reference // File reference
// FIXME (no host, should result in null for host, not empty string) {"file:///path/info","file",null,null,"/path/info",null,null,null},
{"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/","http","localhost",null,"/",null,null,null},
{"http://localhost:8080/", "http", "localhost","8080","/", 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) // 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}, {"http://user@[2001:db8::1]:8080/","http","[2001:db8::1]","8080","/",null,null,null},
// Simple IPv6 host no port (default path) // Simple IPv6 host no port (default path)
@ -162,8 +153,7 @@ public class HttpURIParseTest
// Scheme-less IPv6, host with port (default path) // Scheme-less IPv6, host with port (default path)
// FIXME (this has host and port) {"//[2001:db8::1]:8080/",null,"[2001:db8::1]","8080","/",null,null,null},
{"//[2001:db8::1]:8080/",null,null,null,"//[2001:db8::1]:8080/",null,null,null},
// Interpreted as relative path of "*" (no host/port/scheme/query/fragment) // 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}, {"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}, {"/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}, {"/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}, {"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;q1=v1","q1=v1","q2=v2",null},
// Path-less, query only (seen from JSP/JSTL and <c:url> use // 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,"q1=v1&q2=v2",null}
{"?q1=v1&q2=v2",null,null,null,null,null,"q1=v1&q2=v2",null}
}; };
return Arrays.asList(tests); return Arrays.asList(tests);
@ -213,18 +202,38 @@ public class HttpURIParseTest
{ {
HttpURI httpUri = new HttpURI(input); HttpURI httpUri = new HttpURI(input);
assertThat("[" + input + "] .scheme",httpUri.getScheme(),is(scheme)); try
assertThat("[" + input + "] .host",httpUri.getHost(),is(host)); {
assertThat("[" + input + "] .port",httpUri.getPort(),is(port == null ? -1 : Integer.parseInt(port))); new URI(input);
assertThat("[" + input + "] .path",httpUri.getPath(),is(path)); // URI is valid (per java.net.URI parsing)
assertThat("[" + input + "] .param",httpUri.getParam(),is(param));
assertThat("[" + input + "] .query",httpUri.getQuery(),is(query)); // Test case sanity check
assertThat("[" + input + "] .fragment",httpUri.getFragment(),is(fragment)); assertThat("[" + input + "] expected path (test case) cannot be null",path,notNullValue());
assertThat("[" + input + "] .toString",httpUri.toString(),is(input));
// 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 @Test
@Ignore("There are many examples of inconsistent results from .testParseString()")
public void testParseURI() throws Exception public void testParseURI() throws Exception
{ {
URI javaUri = null; URI javaUri = null;
@ -248,11 +257,11 @@ public class HttpURIParseTest
assertThat("[" + input + "] .param",httpUri.getParam(),is(param)); assertThat("[" + input + "] .param",httpUri.getParam(),is(param));
assertThat("[" + input + "] .query",httpUri.getQuery(),is(query)); assertThat("[" + input + "] .query",httpUri.getQuery(),is(query));
assertThat("[" + input + "] .fragment",httpUri.getFragment(),is(fragment)); assertThat("[" + input + "] .fragment",httpUri.getFragment(),is(fragment));
assertThat("[" + input + "] .toString",httpUri.toString(),is(input)); assertThat("[" + input + "] .toString",httpUri.toString(),is(input));
} }
@Test @Test
@Ignore("There are many examples of inconsistent results from .testParseString()")
public void testCompareToJavaNetURI() throws Exception public void testCompareToJavaNetURI() throws Exception
{ {
URI javaUri = null; URI javaUri = null;