From 2f04b0f869bfae162b9cec23bdbee04043c43287 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 20 Apr 2017 11:38:28 +1000 Subject: [PATCH] Fix #1487 --- .../jetty/embedded/FastFileServer.java | 3 +- .../jetty/rewrite/handler/RedirectUtil.java | 2 +- .../eclipse/jetty/server/PushBuilderImpl.java | 2 +- .../org/eclipse/jetty/server/Request.java | 2 +- .../org/eclipse/jetty/server/Response.java | 2 +- .../java/org/eclipse/jetty/server/Server.java | 2 +- .../jetty/server/handler/ContextHandler.java | 4 +- .../jetty/server/handler/ResourceHandler.java | 2 +- .../eclipse/jetty/servlet/DefaultServlet.java | 13 +- .../jetty/servlet/DefaultServletTest.java | 27 +++- .../java/org/eclipse/jetty/util/URIUtil.java | 61 +++++++- .../jetty/util/resource/FileResource.java | 2 +- .../jetty/util/resource/PathResource.java | 2 +- .../eclipse/jetty/util/resource/Resource.java | 4 +- .../jetty/util/resource/URLResource.java | 2 +- .../org/eclipse/jetty/util/URIUtilTest.java | 145 +++++++++++++----- 16 files changed, 211 insertions(+), 64 deletions(-) diff --git a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/FastFileServer.java b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/FastFileServer.java index ac9b6d0b721..14e5ca46ac1 100644 --- a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/FastFileServer.java +++ b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/FastFileServer.java @@ -111,8 +111,7 @@ public class FastFileServer { if (!request.getPathInfo().endsWith(URIUtil.SLASH)) { - response.sendRedirect(response.encodeRedirectURL(URIUtil - .addPaths(request.getRequestURI(), URIUtil.SLASH))); + response.sendRedirect(response.encodeRedirectURL(request.getRequestURI()+URIUtil.SLASH)); return; } String listing = Resource.newResource(file).getListHTML( diff --git a/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectUtil.java b/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectUtil.java index b556fe64b8a..dd2363bea6e 100644 --- a/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectUtil.java +++ b/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectUtil.java @@ -54,7 +54,7 @@ public final class RedirectUtil // relative to request String path = request.getRequestURI(); String parent = (path.endsWith("/")) ? path : URIUtil.parentPath(path); - location = URIUtil.canonicalPath(URIUtil.addPaths(parent,location)); + location = URIUtil.canonicalPath(URIUtil.addEncodedPaths(parent,location)); if (!location.startsWith("/")) url.append('/'); } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/PushBuilderImpl.java b/jetty-server/src/main/java/org/eclipse/jetty/server/PushBuilderImpl.java index 4b332686122..12df9952710 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/PushBuilderImpl.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/PushBuilderImpl.java @@ -297,7 +297,7 @@ public class PushBuilderImpl implements PushBuilder _fields.add(HttpHeader.IF_MODIFIED_SINCE,_lastModified); } - HttpURI uri = HttpURI.createHttpURI(_request.getScheme(),_request.getServerName(),_request.getServerPort(),_path,param,query,null); + HttpURI uri = HttpURI.createHttpURI(_request.getScheme(),_request.getServerName(),_request.getServerPort(),path,param,query,null); MetaData.Request push = new MetaData.Request(_method,uri,_request.getHttpVersion(),_fields); if (LOG.isDebugEnabled()) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 417f5502c6e..8b5b145530a 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -1279,7 +1279,7 @@ public class Request implements HttpServletRequest relTo = relTo.substring(0,slash + 1); else relTo = "/"; - path = URIUtil.addPaths(URIUtil.encodePath(relTo),path); + path = URIUtil.addPaths(relTo,path); } return _context.getRequestDispatcher(path); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java index 24510b5671f..984209c3fff 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java @@ -648,7 +648,7 @@ public class Response implements HttpServletResponse // relative to request String path=_channel.getRequest().getRequestURI(); String parent=(path.endsWith("/"))?path:URIUtil.parentPath(path); - location=URIUtil.canonicalPath(URIUtil.addPaths(parent,location)); + location=URIUtil.canonicalPath(URIUtil.addEncodedPaths(parent,location)); if (!location.startsWith("/")) buf.append('/'); } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java index 71e5e49d4f2..6ff8e40ebca 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java @@ -563,7 +563,7 @@ public class Server extends HandlerWrapper implements Attributes // this is a dispatch with a path ServletContext context=event.getServletContext(); String query=baseRequest.getQueryString(); - baseRequest.setURIPathQuery(URIUtil.addPaths(context==null?null:URIUtil.encodePath(context.getContextPath()), path)); + baseRequest.setURIPathQuery(URIUtil.addEncodedPaths(context==null?null:URIUtil.encodePath(context.getContextPath()), path)); HttpURI uri = baseRequest.getHttpURI(); baseRequest.setPathInfo(uri.getDecodedPath()); if (uri.getQuery()!=null) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index 487e3339898..df66f624cd3 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -996,9 +996,9 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu // context request must end with / baseRequest.setHandled(true); if (baseRequest.getQueryString() != null) - response.sendRedirect(URIUtil.addPaths(baseRequest.getRequestURI(),URIUtil.SLASH) + "?" + baseRequest.getQueryString()); + response.sendRedirect(baseRequest.getRequestURI() + "/?" + baseRequest.getQueryString()); else - response.sendRedirect(URIUtil.addPaths(baseRequest.getRequestURI(),URIUtil.SLASH)); + response.sendRedirect(baseRequest.getRequestURI() + "/"); return false; } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java index 1185293f0fd..93776a1e80b 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java @@ -453,7 +453,7 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory boolean endsWithSlash=(pathInfo==null?request.getServletPath():pathInfo).endsWith(URIUtil.SLASH); if (!endsWithSlash) { - response.sendRedirect(response.encodeRedirectURL(URIUtil.addPaths(request.getRequestURI(),URIUtil.SLASH))); + response.sendRedirect(response.encodeRedirectURL(request.getRequestURI()+URIUtil.SLASH)); return; } diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java index 06d83a33a7f..38234b9db3c 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java @@ -490,9 +490,10 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory { String q=request.getQueryString(); pathInContext=pathInContext.substring(0,pathInContext.length()-1); + String uri = URIUtil.addPaths(_servletContext.getContextPath(),pathInContext); if (q!=null&&q.length()!=0) - pathInContext+="?"+q; - response.sendRedirect(response.encodeRedirectURL(URIUtil.addPaths(_servletContext.getContextPath(),pathInContext))); + uri+="?"+q; + response.sendRedirect(response.encodeRedirectURL(uri)); return; } @@ -578,11 +579,11 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory { // Redirect to the index response.setContentLength(0); + String uri=URIUtil.encodePath(URIUtil.addPaths( _servletContext.getContextPath(),welcome)); String q=request.getQueryString(); - if (q!=null&&q.length()!=0) - response.sendRedirect(response.encodeRedirectURL(URIUtil.addPaths( _servletContext.getContextPath(),welcome)+"?"+q)); - else - response.sendRedirect(response.encodeRedirectURL(URIUtil.addPaths( _servletContext.getContextPath(),welcome))); + if (q!=null&&!q.isEmpty()) + uri+="?"+q; + response.sendRedirect(response.encodeRedirectURL(uri)); } else { diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java index 6341ffd20c6..072f6c72ff1 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java @@ -411,7 +411,32 @@ public class DefaultServletTest response = connector.getResponses("GET /context/dir/ HTTP/1.0\r\n\r\n"); assertResponseContains("403", response); } - + + @Test + public void testWelcomeDirWithQuestion() throws Exception + { + testdir.ensureEmpty(); + File resBase = testdir.getPathFile("docroot").toFile(); + FS.ensureDirExists(resBase); + context.setBaseResource(Resource.newResource(resBase)); + + File dir = new File(resBase, "dir?"); + assertTrue(dir.mkdirs()); + File index = new File(dir, "index.html"); + createFile(index, "

Hello Index

"); + + ServletHolder defholder = context.addServlet(DefaultServlet.class, "/"); + defholder.setInitParameter("dirAllowed", "false"); + defholder.setInitParameter("redirectWelcome", "true"); + defholder.setInitParameter("welcomeServlets", "false"); + defholder.setInitParameter("gzip", "false"); + + String response = connector.getResponse("GET /context/dir%3F HTTP/1.0\r\n\r\n"); + assertResponseContains("Location: http://0.0.0.0/context/dir%3F/", response); + + response = connector.getResponse("GET /context/dir%3F/ HTTP/1.0\r\n\r\n"); + assertResponseContains("Location: http://0.0.0.0/context/dir%3F/index.html", response); + } @Test diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java index 1b21dcf7c4f..3d3671ebd11 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java @@ -472,14 +472,14 @@ public class URIUtil /* ------------------------------------------------------------ */ - /** Add two URI path segments. + /** Add two encoded URI path segments. * Handles null and empty paths, path and query params (eg ?a=b or * ;JSESSIONID=xxx) and avoids duplicate '/' * @param p1 URI path segment (should be encoded) * @param p2 URI path segment (should be encoded) * @return Legally combined path segments. */ - public static String addPaths(String p1, String p2) + public static String addEncodedPaths(String p1, String p2) { if (p1==null || p1.length()==0) { @@ -524,7 +524,54 @@ public class URIUtil return buf.toString(); } - + + /* ------------------------------------------------------------ */ + /** Add two Decoded URI path segments. + * Handles null and empty paths. Path and query params (eg ?a=b or + * ;JSESSIONID=xxx) are not handled + * @param p1 URI path segment (should be decoded) + * @param p2 URI path segment (should be decoded) + * @return Legally combined path segments. + */ + public static String addPaths(String p1, String p2) + { + if (p1==null || p1.length()==0) + { + if (p1!=null && p2==null) + return p1; + return p2; + } + if (p2==null || p2.length()==0) + return p1; + + boolean p1EndsWithSlash = p1.endsWith(SLASH); + boolean p2StartsWithSlash = p2.startsWith(SLASH); + + if (p1EndsWithSlash && p2StartsWithSlash) + { + if (p2.length()==1) + return p1; + if (p1.length()==1) + return p2; + } + + StringBuilder buf = new StringBuilder(p1.length()+p2.length()+2); + buf.append(p1); + + if (p1.endsWith(SLASH)) + { + if (p2.startsWith(SLASH)) + buf.setLength(buf.length()-1); + } + else + { + if (!p2.startsWith(SLASH)) + buf.append(SLASH); + } + buf.append(p2); + + return buf.toString(); + } /* ------------------------------------------------------------ */ /** Return the parent Path. * Treat a URI like a directory path and return the parent directory. @@ -916,7 +963,12 @@ public class URIUtil return equalsIgnoreEncodings(uriA.getPath(),uriB.getPath()); } - public static URI addDecodedPath(URI uri, String path) + /** + * @param uri A URI to add the path to + * @param path A decoded path element + * @return URI with path added. + */ + public static URI addPath(URI uri, String path) { String base = uri.toASCIIString(); StringBuilder buf = new StringBuilder(base.length()+path.length()*3); @@ -924,7 +976,6 @@ public class URIUtil if (buf.charAt(base.length()-1)!='/') buf.append('/'); - byte[] bytes=null; int offset=path.charAt(0)=='/'?1:0; encodePath(buf,path,offset); diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileResource.java index 6deda09ef26..868402f2555 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileResource.java @@ -169,7 +169,7 @@ public class FileResource extends Resource if (base.isDirectory()) { // treat all paths being added as relative - uri=new URI(URIUtil.addPaths(base.toURI().toASCIIString(),encoded)); + uri=new URI(URIUtil.addEncodedPaths(base.toURI().toASCIIString(),encoded)); } else { diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java index 54fdd56dc56..968847a8bc0 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java @@ -213,7 +213,7 @@ public class PathResource extends Resource this.path = parent.path.getFileSystem().getPath(parent.path.toString(), childPath); if (isDirectory() &&!childPath.endsWith("/")) childPath+="/"; - this.uri = URIUtil.addDecodedPath(parent.uri,childPath); + this.uri = URIUtil.addPath(parent.uri,childPath); this.alias = checkAliasPath(); } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java index 195fa839f15..a00837edf6f 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java @@ -566,7 +566,7 @@ public abstract class Resource implements ResourceFactory, Closeable if (parent) { buf.append("Parent Directory\n"); } @@ -579,7 +579,7 @@ public abstract class Resource implements ResourceFactory, Closeable Resource item = addPath(ls[i]); buf.append("\n