Cleanup metadata and HttpURI usage

This commit is contained in:
Greg Wilkins 2016-08-07 17:03:42 +10:00
parent 25a9a35be0
commit 07f547782f
13 changed files with 186 additions and 122 deletions

View File

@ -40,7 +40,11 @@ public class HostPortHttpField extends HttpField
{
super(header,name,authority);
if (authority==null || authority.length()==0)
throw new IllegalArgumentException("No Authority");
{
_host="";
_port=0;
return;
}
try
{
if (authority.charAt(0)=='[')

View File

@ -126,7 +126,7 @@ public class HttpURI
public HttpURI(String uri)
{
_port=-1;
parse(State.START,uri,0,uri.length());
parse(State.START,uri);
}
/* ------------------------------------------------------------ */
@ -164,7 +164,7 @@ public class HttpURI
_host=host;
_port=port;
parse(State.PATH,pathQuery,0,pathQuery.length());
parse(State.PATH,pathQuery);
}
@ -173,7 +173,7 @@ public class HttpURI
{
clear();
_uri=uri;
parse(State.START,uri,0,uri.length());
parse(State.START,uri);
}
/* ------------------------------------------------------------ */
@ -190,7 +190,7 @@ public class HttpURI
if (HttpMethod.CONNECT.is(method))
_path=uri;
else
parse(uri.startsWith("/")?State.PATH:State.START,uri,0,uri.length());
parse(uri.startsWith("/")?State.PATH:State.START,uri);
}
/* ------------------------------------------------------------ */
@ -208,17 +208,18 @@ public class HttpURI
clear();
int end=offset+length;
_uri=uri.substring(offset,end);
parse(State.START,uri,offset,end);
parse(State.START,uri);
}
/* ------------------------------------------------------------ */
private void parse(State state, final String uri, final int offset, final int end)
private void parse(State state, final String uri)
{
boolean encoded=false;
int mark=offset;
int end=uri.length();
int mark=0;
int path_mark=0;
for (int i=offset; i<end; i++)
char last='/';
for (int i=0; i<end; i++)
{
char c=uri.charAt(i);
@ -251,6 +252,12 @@ public class HttpURI
state=State.ASTERISK;
break;
case '.':
path_mark=i;
state=State.PATH;
encoded=true;
break;
default:
mark=i;
if (_scheme==null)
@ -260,6 +267,7 @@ public class HttpURI
path_mark=i;
state=State.PATH;
}
break;
}
continue;
@ -328,6 +336,14 @@ public class HttpURI
path_mark=mark;
state=State.PATH;
break;
case '.':
// it is a path
encoded=true;
path_mark=mark;
state=State.PATH;
break;
default:
// it is a path
path_mark=mark;
@ -362,7 +378,7 @@ public class HttpURI
state=State.IPV6;
break;
}
continue;
break;
}
case IPV6:
@ -387,7 +403,7 @@ public class HttpURI
break;
}
continue;
break;
}
case PORT:
@ -407,9 +423,9 @@ public class HttpURI
path_mark=mark=i;
state=State.PATH;
}
continue;
break;
}
case PATH:
{
switch (c)
@ -431,9 +447,15 @@ public class HttpURI
case '%':
encoded=true;
break;
case '.':
if ('/'==last)
encoded=true;
}
continue;
break;
}
case PARAM:
{
@ -461,7 +483,7 @@ public class HttpURI
mark=i+1;
break;
}
continue;
break;
}
case QUERY:
@ -472,7 +494,7 @@ public class HttpURI
mark=i+1;
state=State.FRAGMENT;
}
continue;
break;
}
case ASTERISK:
@ -484,8 +506,10 @@ public class HttpURI
{
_fragment=uri.substring(mark,end);
i=end;
break;
}
}
last=c;
}
@ -579,7 +603,7 @@ public class HttpURI
public String getDecodedPath()
{
if (_decodedPath==null && _path!=null)
_decodedPath=URIUtil.decodePath(_path);
_decodedPath=URIUtil.canonicalPath(URIUtil.decodePath(_path));
return _decodedPath;
}
@ -722,7 +746,7 @@ public class HttpURI
_port=port;
_uri=null;
}
/* ------------------------------------------------------------ */
/**
* @param path the path
@ -734,6 +758,17 @@ public class HttpURI
_decodedPath=null;
}
/* ------------------------------------------------------------ */
/**
* @param path the decoded path
*/
public void setDecodedPath(String path)
{
_uri=null;
_path=URIUtil.encodePath(path);
_decodedPath=path;
}
/* ------------------------------------------------------------ */
public void setPathQuery(String path)
{
@ -743,7 +778,7 @@ public class HttpURI
_param=null;
_fragment=null;
if (path!=null)
parse(State.PATH,path,0,path.length());
parse(State.PATH,path);
}
/* ------------------------------------------------------------ */
@ -766,6 +801,12 @@ public class HttpURI
return _path;
return _path+"?"+_query;
}
/* ------------------------------------------------------------ */
public boolean hasAuthority()
{
return _host!=null;
}
/* ------------------------------------------------------------ */
public String getAuthority()

View File

@ -35,6 +35,23 @@ import org.junit.Test;
public class HttpURITest
{
@Test
public void testExample() throws Exception
{
HttpURI uri = new HttpURI("http://user:password@host:8888/ignored/../p%61th;ignored/info;param?query=value#fragment");
assertThat(uri.getScheme(),is("http"));
assertThat(uri.getUser(),is("user:password"));
assertThat(uri.getHost(),is("host"));
assertThat(uri.getPort(),is(8888));
assertThat(uri.getPath(),is("/ignored/../p%61th;ignored/info;param"));
assertThat(uri.getDecodedPath(),is("/path/info"));
assertThat(uri.getParam(),is("param"));
assertThat(uri.getQuery(),is("query=value"));
assertThat(uri.getFragment(),is("fragment"));
assertThat(uri.getAuthority(),is("host:8888"));
}
@Test
public void testInvalidAddress() throws Exception
{
@ -250,4 +267,39 @@ public class HttpURITest
assertEquals(uri.getAuthority(), "example.com:8888");
assertEquals(uri.getUser(), "user:password");
}
@Test
public void testCanonicalDecoded() throws Exception
{
HttpURI uri = new HttpURI("/path/.info");
assertEquals("/path/.info",uri.getDecodedPath());
uri = new HttpURI("/path/./info");
assertEquals("/path/info",uri.getDecodedPath());
uri = new HttpURI("/path/../info");
assertEquals("/info",uri.getDecodedPath());
uri = new HttpURI("/./path/info.");
assertEquals("/path/info.",uri.getDecodedPath());
uri = new HttpURI("./path/info/.");
assertEquals("path/info/",uri.getDecodedPath());
uri = new HttpURI("http://host/path/.info");
assertEquals("/path/.info",uri.getDecodedPath());
uri = new HttpURI("http://host/path/./info");
assertEquals("/path/info",uri.getDecodedPath());
uri = new HttpURI("http://host/path/../info");
assertEquals("/info",uri.getDecodedPath());
uri = new HttpURI("http://host/./path/info.");
assertEquals("/path/info.",uri.getDecodedPath());
uri = new HttpURI("http:./path/info/.");
assertEquals("path/info/",uri.getDecodedPath());
}
}

View File

@ -232,7 +232,8 @@ public class FormAuthenticator extends LoginAuthenticator
return; //this request is not for the same url as the original
//restore the original request's method on this request
if (LOG.isDebugEnabled()) LOG.debug("Restoring original method {} for {} with method {}", method, juri,httpRequest.getMethod());
if (LOG.isDebugEnabled())
LOG.debug("Restoring original method {} for {} with method {}", method, juri,httpRequest.getMethod());
Request base_request = Request.getBaseRequest(request);
base_request.setMethod(method);
}

View File

@ -24,7 +24,7 @@ import java.util.Locale;
import javax.servlet.http.Cookie;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.PathMap;
import org.eclipse.jetty.http.pathmap.PathMappings;
import org.eclipse.jetty.server.handler.StatisticsHandler;
import org.eclipse.jetty.util.DateCache;
import org.eclipse.jetty.util.annotation.ManagedAttribute;
@ -54,7 +54,7 @@ public abstract class AbstractNCSARequestLog extends AbstractLifeCycle implement
private String[] _ignorePaths;
private boolean _extended;
private transient PathMap<String> _ignorePathMap;
private PathMappings<String> _ignorePathMap;
private boolean _logLatency = false;
private boolean _logCookies = false;
private boolean _logServer = false;
@ -423,7 +423,7 @@ public abstract class AbstractNCSARequestLog extends AbstractLifeCycle implement
if (_ignorePaths != null && _ignorePaths.length > 0)
{
_ignorePathMap = new PathMap<>();
_ignorePathMap = new PathMappings<>();
for (int i = 0; i < _ignorePaths.length; i++)
_ignorePathMap.put(_ignorePaths[i], _ignorePaths[i]);
}

View File

@ -127,11 +127,8 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque
break;
case HOST:
if (!_metadata.getURI().isAbsolute() && field instanceof HostPortHttpField)
{
HostPortHttpField hp = (HostPortHttpField)field;
_metadata.getURI().setAuthority(hp.getHost(), hp.getPort());
}
if (!(field instanceof HostPortHttpField))
field = new HostPortHttpField(value);
break;
case EXPECT:

View File

@ -171,7 +171,7 @@ public class Request implements HttpServletRequest
private final HttpInput _input;
private MetaData.Request _metaData;
private String _originalURI;
private String _originalUri;
private String _contextPath;
private String _servletPath;
@ -359,18 +359,19 @@ public class Request implements HttpServletRequest
private void extractQueryParameters()
{
MetaData.Request metadata = _metaData;
if (metadata==null || metadata.getURI() == null || !metadata.getURI().hasQuery())
HttpURI uri = metadata==null?null:metadata.getURI();
if (uri==null || !uri.hasQuery())
_queryParameters=NO_PARAMS;
else
{
_queryParameters = new MultiMap<>();
if (_queryEncoding == null)
metadata.getURI().decodeQueryTo(_queryParameters);
uri.decodeQueryTo(_queryParameters);
else
{
try
{
metadata.getURI().decodeQueryTo(_queryParameters, _queryEncoding);
uri.decodeQueryTo(_queryParameters, _queryEncoding);
}
catch (UnsupportedEncodingException e)
{
@ -951,7 +952,9 @@ public class Request implements HttpServletRequest
public String getMethod()
{
MetaData.Request metadata = _metaData;
return metadata==null?null:metadata.getMethod();
if (metadata!=null)
return metadata.getMethod();
return null;
}
/* ------------------------------------------------------------ */
@ -1253,7 +1256,7 @@ public class Request implements HttpServletRequest
public String getRequestURI()
{
MetaData.Request metadata = _metaData;
return (metadata==null)?null:metadata.getURI().getPath();
return metadata==null?null:metadata.getURI().getPath();
}
/* ------------------------------------------------------------ */
@ -1301,7 +1304,7 @@ public class Request implements HttpServletRequest
public String getScheme()
{
MetaData.Request metadata = _metaData;
String scheme=metadata==null?null:metadata.getURI().getScheme();
String scheme = metadata==null?null:metadata.getURI().getScheme();
return scheme==null?HttpScheme.HTTP.asString():scheme;
}
@ -1314,7 +1317,7 @@ public class Request implements HttpServletRequest
{
MetaData.Request metadata = _metaData;
String name = metadata==null?null:metadata.getURI().getHost();
// Return already determined host
if (name != null)
return name;
@ -1325,21 +1328,6 @@ public class Request implements HttpServletRequest
/* ------------------------------------------------------------ */
private String findServerName()
{
MetaData.Request metadata = _metaData;
// Return host from header field
HttpField host = metadata==null?null:metadata.getFields().getField(HttpHeader.HOST);
if (host!=null)
{
if (!(host instanceof HostPortHttpField) && host.getValue()!=null && !host.getValue().isEmpty())
host=new HostPortHttpField(host.getValue());
if (host instanceof HostPortHttpField)
{
HostPortHttpField authority = (HostPortHttpField)host;
metadata.getURI().setAuthority(authority.getHost(),authority.getPort());
return authority.getHost();
}
}
// Return host from connection
String name=getLocalName();
if (name != null)
@ -1383,19 +1371,6 @@ public class Request implements HttpServletRequest
/* ------------------------------------------------------------ */
private int findServerPort()
{
MetaData.Request metadata = _metaData;
// Return host from header field
HttpField host = metadata==null?null:metadata.getFields().getField(HttpHeader.HOST);
if (host!=null)
{
// TODO is this needed now?
HostPortHttpField authority = (host instanceof HostPortHttpField)
?((HostPortHttpField)host)
:new HostPortHttpField(host.getValue());
metadata.getURI().setAuthority(authority.getHost(),authority.getPort());
return authority.getPort();
}
// Return host from connection
if (_channel != null)
return getLocalPort();
@ -1537,8 +1512,9 @@ public class Request implements HttpServletRequest
*/
public String getOriginalURI()
{
return _originalURI;
return _originalUri;
}
/* ------------------------------------------------------------ */
/**
* @param uri the URI to set
@ -1546,7 +1522,8 @@ public class Request implements HttpServletRequest
public void setHttpURI(HttpURI uri)
{
MetaData.Request metadata = _metaData;
metadata.setURI(uri);
if (metadata!=null)
metadata.setURI(uri);
}
/* ------------------------------------------------------------ */
@ -1701,45 +1678,42 @@ public class Request implements HttpServletRequest
public void setMetaData(org.eclipse.jetty.http.MetaData.Request request)
{
_metaData=request;
_originalURI=_metaData.getURIString();
setMethod(request.getMethod());
HttpURI uri = request.getURI();
String path = uri.getDecodedPath();
String info;
if (path==null || path.length()==0)
_originalUri = uri.toString();
if (uri.getScheme()==null)
uri.setScheme("http");
if (!uri.hasAuthority())
{
if (uri.isAbsolute())
HttpField field = getHttpFields().getField(HttpHeader.HOST);
if (field instanceof HostPortHttpField)
{
path="/";
uri.setPath(path);
HostPortHttpField authority = (HostPortHttpField)field;
uri.setAuthority(authority.getHost(),authority.getPort());
}
else
}
String pathInfo = uri.getDecodedPath();
if (pathInfo==null || pathInfo.length()==0)
{
// If null path was not from an absolute http without a path
if (!request.getURI().isAbsolute() || uri.getPath()!=null)
{
setPathInfo("");
throw new BadMessageException(400,"Bad URI");
}
info=path;
}
else if (!path.startsWith("/"))
{
if (!"*".equals(path) && !HttpMethod.CONNECT.is(getMethod()))
{
setPathInfo(path);
throw new BadMessageException(400,"Bad URI");
}
info=path;
}
else
info = URIUtil.canonicalPath(path);// TODO should this be done prior to decoding???
if (info == null)
pathInfo="/";
uri.setDecodedPath(pathInfo);
}
else if (!(pathInfo.startsWith("/") || "*".equals(request.getURI().getPath()) || HttpMethod.CONNECT.is(getMethod())))
{
setPathInfo(path);
setPathInfo(pathInfo);
throw new BadMessageException(400,"Bad URI");
}
setPathInfo(info);
setPathInfo(pathInfo);
}
/* ------------------------------------------------------------ */
@ -1758,7 +1732,6 @@ public class Request implements HttpServletRequest
protected void recycle()
{
_metaData=null;
_originalURI=null;
if (_context != null)
throw new IllegalStateException("Request in context!");
@ -2015,21 +1988,19 @@ public class Request implements HttpServletRequest
/* ------------------------------------------------------------ */
/**
* @param method
* The method to set.
* @param method The method to set.
*/
public void setMethod(String method)
{
MetaData.Request metadata = _metaData;
if (metadata!=null)
metadata.setMethod(method);
metadata.setMethod(method);
}
/* ------------------------------------------------------------ */
public boolean isHead()
{
MetaData.Request metadata = _metaData;
return metadata!=null && HttpMethod.HEAD.is(metadata.getMethod());
return HttpMethod.HEAD.is(getMethod());
}
/* ------------------------------------------------------------ */
@ -2064,8 +2035,9 @@ public class Request implements HttpServletRequest
public void setQueryString(String queryString)
{
MetaData.Request metadata = _metaData;
if (metadata!=null)
metadata.getURI().setQuery(queryString);
HttpURI uri = metadata==null?null:metadata.getURI();
if (uri!=null)
uri.setQuery(queryString);
_queryEncoding = null; //assume utf-8
}
@ -2103,8 +2075,9 @@ public class Request implements HttpServletRequest
public void setURIPathQuery(String requestURI)
{
MetaData.Request metadata = _metaData;
if (metadata!=null)
metadata.getURI().setPathQuery(requestURI);
HttpURI uri = metadata==null?null:metadata.getURI();
if (uri!=null)
uri.setPathQuery(requestURI);
}
/* ------------------------------------------------------------ */
@ -2115,8 +2088,9 @@ public class Request implements HttpServletRequest
public void setScheme(String scheme)
{
MetaData.Request metadata = _metaData;
if (metadata!=null)
metadata.getURI().setScheme(scheme);
HttpURI uri = metadata==null?null:metadata.getURI();
if (uri!=null)
uri.setScheme(scheme);
}
/* ------------------------------------------------------------ */
@ -2129,8 +2103,9 @@ public class Request implements HttpServletRequest
public void setAuthority(String host,int port)
{
MetaData.Request metadata = _metaData;
if (metadata!=null)
metadata.getURI().setAuthority(host,port);
HttpURI uri = metadata==null?null:metadata.getURI();
if (uri!=null)
uri.setAuthority(host,port);
}
/* ------------------------------------------------------------ */

View File

@ -170,8 +170,7 @@ public class SecureRequestCustomizer implements HttpConfiguration.Customizer
SSLEngine sslEngine=sslConnection.getSSLEngine();
customize(sslEngine,request);
if (request.getHttpURI().getScheme()==null)
request.setScheme(HttpScheme.HTTPS.asString());
request.setScheme(HttpScheme.HTTPS.asString());
}
else if (endp instanceof ProxyConnectionFactory.ProxyEndPoint)
{

View File

@ -43,7 +43,6 @@ import org.eclipse.jetty.util.RolloverFileOutputStream;
* Details of the request and response are written to an output stream
* and the current thread name is updated with information that will link
* to the details in that output.
* @deprecated Use {@link DebugListener}
*/
public class DebugHandler extends HandlerWrapper implements Connection.Listener
{
@ -67,7 +66,7 @@ public class DebugHandler extends HandlerWrapper implements Connection.Listener
boolean retry=false;
String name=(String)request.getAttribute("org.eclipse.jetty.thread.name");
if (name == null)
name = old_name + ":" + baseRequest.getHttpURI();
name = old_name + ":" + baseRequest.getOriginalURI();
else
retry=true;

View File

@ -72,11 +72,9 @@ public class DebugHandlerTest
private URI serverURI;
private URI secureServerURI;
@SuppressWarnings("deprecation")
private DebugHandler debugHandler;
private ByteArrayOutputStream capturedLog;
@SuppressWarnings("deprecation")
@Before
public void startServer() throws Exception
{
@ -159,7 +157,7 @@ public class DebugHandlerTest
req.getString("/foo/bar?a=b");
String log = capturedLog.toString(StandardCharsets.UTF_8.name());
String expectedThreadName = String.format("//%s:%s/foo/bar?a=b",serverURI.getHost(),serverURI.getPort());
String expectedThreadName = ":/foo/bar?a=b";
assertThat("ThreadName", log, containsString(expectedThreadName));
// Look for bad/mangled/duplicated schemes
assertThat("ThreadName", log, not(containsString("http:"+expectedThreadName)));
@ -173,7 +171,7 @@ public class DebugHandlerTest
req.getString("/foo/bar?a=b");
String log = capturedLog.toString(StandardCharsets.UTF_8.name());
String expectedThreadName = String.format("https://%s:%s/foo/bar?a=b",secureServerURI.getHost(),secureServerURI.getPort());
String expectedThreadName = ":/foo/bar?a=b";
assertThat("ThreadName", log, containsString(expectedThreadName));
// Look for bad/mangled/duplicated schemes
assertThat("ThreadName", log, not(containsString("http:"+expectedThreadName)));

View File

@ -80,9 +80,7 @@ public class RequestLogTest
{
_connector.getResponse("GET /foo?data=1 HTTP/1.0\nhost: host:80\n\n");
String log = _log.exchange(null,5,TimeUnit.SECONDS);
// TODO should be without host (https://bugs.eclipse.org/bugs/show_bug.cgi?id=480276)
// assertThat(log,containsString("GET /foo?data=1 HTTP/1.0\" 200 "));
assertThat(log,containsString("GET //host:80/foo?data=1 HTTP/1.0\" 200 "));
assertThat(log,containsString("GET /foo?data=1 HTTP/1.0\" 200 "));
_connector.getResponse("GET //bad/foo?data=1 HTTP/1.0\n\n");
log = _log.exchange(null,5,TimeUnit.SECONDS);

View File

@ -258,8 +258,6 @@ public class SelectChannelServerSslTest extends HttpServerTestBase
// Read the response.
String response = readResponse(client);
System.err.println(response);
assertThat(response, containsString("HTTP/1.1 200 OK"));
assertThat(response, containsString("Hello world"));

View File

@ -345,8 +345,10 @@ public class URIUtil
char u=path.charAt(i+1);
if (u=='u')
{
// TODO this is wrong. This is a codepoint not a char
builder.append((char)(0xffff&TypeUtil.parseInt(path,i+2,4,16)));
int codepoint=0xffff&TypeUtil.parseInt(path,i+2,4,16);
char[] chars = Character.toChars(codepoint);
for (char ch:chars)
builder.append(ch);
i+=5;
}
else