Issue #5133 - Updates based on review

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
This commit is contained in:
Joakim Erdfelt 2020-08-26 12:19:22 -05:00
parent b35c4332b2
commit ccc863726b
No known key found for this signature in database
GPG Key ID: 2D0E1FB8FE4B68B4
9 changed files with 126 additions and 137 deletions

View File

@ -20,7 +20,6 @@ package org.eclipse.jetty.osgi.boot.internal.webapp;
import java.io.File;
import java.io.IOException;
import java.lang.reflect.Field;
import java.net.URL;
import java.util.ArrayList;
import java.util.Enumeration;
@ -264,6 +263,4 @@ public class OSGiWebappClassLoader extends WebAppClassLoader implements BundleRe
}
return true;
}
private static Field _contextField;
}

View File

@ -188,6 +188,7 @@ public class CachedContentFactory implements HttpContent.ContentFactory
if (_parent != null)
{
HttpContent httpContent = _parent.getContent(pathInContext, maxBufferSize);
if (httpContent != null)
return httpContent;
}
@ -209,7 +210,7 @@ public class CachedContentFactory implements HttpContent.ContentFactory
return (len > 0 && (_useFileMappedBuffer || (len < _maxCachedFileSize && len < _maxCacheSize)));
}
private HttpContent load(String pathInContext, Resource resource, int maxBufferSize)
private HttpContent load(String pathInContext, Resource resource, int maxBufferSize) throws IOException
{
if (resource == null || !resource.exists())
return null;
@ -233,8 +234,6 @@ public class CachedContentFactory implements HttpContent.ContentFactory
if (compressedContent == null || compressedContent.isValid())
{
compressedContent = null;
try
{
Resource compressedResource = _factory.getResource(compressedPathInContext);
if (compressedResource.exists() && compressedResource.lastModified() >= resource.lastModified() &&
compressedResource.length() < resource.length())
@ -248,12 +247,6 @@ public class CachedContentFactory implements HttpContent.ContentFactory
}
}
}
catch (IOException e)
{
if (LOG.isDebugEnabled())
LOG.debug("Unable to find compressed path in context: {}", compressedPathInContext, e);
}
}
if (compressedContent != null)
precompresssedContents.put(format, compressedContent);
}
@ -286,8 +279,6 @@ public class CachedContentFactory implements HttpContent.ContentFactory
if (compressedContent != null && compressedContent.isValid() && compressedContent.getResource().lastModified() >= resource.lastModified())
compressedContents.put(format, compressedContent);
try
{
// Is there a precompressed resource?
Resource compressedResource = _factory.getResource(compressedPathInContext);
if (compressedResource.exists() && compressedResource.lastModified() >= resource.lastModified() &&
@ -295,12 +286,6 @@ public class CachedContentFactory implements HttpContent.ContentFactory
compressedContents.put(format,
new ResourceHttpContent(compressedResource, _mimeTypes.getMimeByExtension(compressedPathInContext), maxBufferSize));
}
catch (IOException e)
{
if (LOG.isDebugEnabled())
LOG.debug("Unable to find compressed path in context: {}", compressedPathInContext, e);
}
}
if (!compressedContents.isEmpty())
return new ResourceHttpContent(resource, mt, maxBufferSize, compressedContents);
}

View File

@ -50,7 +50,7 @@ public class ResourceContentFactory implements ContentFactory
}
@Override
public HttpContent getContent(String pathInContext, int maxBufferSize)
public HttpContent getContent(String pathInContext, int maxBufferSize) throws IOException
{
try
{
@ -60,8 +60,16 @@ public class ResourceContentFactory implements ContentFactory
}
catch (Throwable t)
{
// Any error has potential to reveal fully qualified path
throw (InvalidPathException)new InvalidPathException(pathInContext, "Invalid PathInContext").initCause(t);
// There are many potential Exceptions that can reveal a fully qualified path.
// See Issue #2560 - Always wrap a Throwable here in an InvalidPathException
// that is limited to only the provided pathInContext.
// The cause (which might reveal a fully qualified path) is still available,
// on the Exception and the logging, but is not reported in normal error page situations.
// This specific exception also allows WebApps to specifically hook into a known / reliable
// Exception type for ErrorPageErrorHandling logic.
InvalidPathException saferException = new InvalidPathException(pathInContext, "Invalid PathInContext");
saferException.initCause(t);
throw saferException;
}
}

View File

@ -881,6 +881,6 @@ public class ResourceService
* @param pathInContext the path of the request
* @return The path of the matching welcome file in context or null.
*/
String getWelcomeFile(String pathInContext);
String getWelcomeFile(String pathInContext) throws IOException;
}
}

View File

@ -18,9 +18,10 @@
package org.eclipse.jetty.server.handler;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Arrays;
import java.util.List;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
@ -36,6 +37,7 @@ import org.eclipse.jetty.server.ResourceContentFactory;
import org.eclipse.jetty.server.ResourceService;
import org.eclipse.jetty.server.ResourceService.WelcomeFactory;
import org.eclipse.jetty.server.handler.ContextHandler.Context;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.URIUtil;
import org.eclipse.jetty.util.resource.Resource;
import org.eclipse.jetty.util.resource.ResourceFactory;
@ -74,11 +76,11 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory,
{
}
});
_resourceService.setGzipEquivalentFileExtensions(new ArrayList<>(Collections.singletonList(".svgz")));
_resourceService.setGzipEquivalentFileExtensions(new ArrayList<>(Arrays.asList(new String[]{".svgz"})));
}
@Override
public String getWelcomeFile(String pathInContext)
public String getWelcomeFile(String pathInContext) throws IOException
{
if (_welcomes == null)
return null;
@ -86,19 +88,10 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory,
for (int i = 0; i < _welcomes.length; i++)
{
String welcomeInContext = URIUtil.addPaths(pathInContext, _welcomes[i]);
try
{
Resource welcome = getResource(welcomeInContext);
if (welcome.exists())
return welcomeInContext;
}
catch (IOException e)
{
// this happens on a critical failure of Resource
if (LOG.isDebugEnabled())
LOG.debug("Failed to resolve welcome file: {}", welcomeInContext);
}
}
// not found
return null;
}
@ -154,8 +147,16 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory,
if (LOG.isDebugEnabled())
LOG.debug("{} getResource({}): baseResource:{}", _context == null ? _baseResource : _context, path, _baseResource);
if (path != null && path.startsWith("/"))
if (StringUtil.isBlank(path))
{
throw new IllegalArgumentException("Path is blank");
}
if (!path.startsWith("/"))
{
throw new IllegalArgumentException("Path reference invalid: " + path);
}
Resource r = null;
if (_baseResource != null)
@ -167,7 +168,7 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory,
{
if (LOG.isDebugEnabled())
LOG.debug("Rejected alias resource={} alias={}", r, r.getAlias());
throw new IOException("Rejected (see debug logs)");
throw new IllegalStateException("Rejected alias reference: " + path);
}
}
else if (_context != null)
@ -180,11 +181,12 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory,
if ((r == null || !r.exists()) && path.endsWith("/jetty-dir.css"))
r = getStylesheet();
if (r != null)
return r;
if (r == null)
{
throw new FileNotFoundException("Resource: " + path);
}
throw new IOException("Unable to find Resource for " + path);
return r;
}
/**

View File

@ -25,7 +25,7 @@ import java.net.MalformedURLException;
import java.net.URI;
import java.nio.channels.ReadableByteChannel;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
@ -42,7 +42,7 @@ import org.eclipse.jetty.util.URIUtil;
*/
public class ResourceCollection extends Resource
{
private Resource[] _resources;
private List<Resource> _resources;
/**
* Instantiates an empty resource collection.
@ -51,7 +51,7 @@ public class ResourceCollection extends Resource
*/
public ResourceCollection()
{
_resources = new Resource[0];
_resources = new ArrayList<>();
}
/**
@ -61,7 +61,7 @@ public class ResourceCollection extends Resource
*/
public ResourceCollection(Resource... resources)
{
List<Resource> list = new ArrayList<>();
_resources = new ArrayList<>();
for (Resource r : resources)
{
if (r == null)
@ -70,15 +70,25 @@ public class ResourceCollection extends Resource
}
if (r instanceof ResourceCollection)
{
Collections.addAll(list, ((ResourceCollection)r).getResources());
_resources.addAll(((ResourceCollection)r).getResources());
}
else
{
assertResourceValid(r);
list.add(r);
_resources.add(r);
}
}
_resources = list.toArray(new Resource[0]);
}
/**
* Instantiates a new resource collection.
*
* @param resources the resources to be added to collection
*/
public ResourceCollection(Collection<Resource> resources)
{
_resources = new ArrayList<>();
_resources.addAll(resources);
}
/**
@ -88,14 +98,13 @@ public class ResourceCollection extends Resource
*/
public ResourceCollection(String[] resources)
{
_resources = new ArrayList<>();
if (resources == null || resources.length == 0)
{
_resources = null;
return;
}
ArrayList<Resource> res = new ArrayList<>();
try
{
for (String strResource : resources)
@ -106,16 +115,13 @@ public class ResourceCollection extends Resource
}
Resource resource = Resource.newResource(strResource);
assertResourceValid(resource);
res.add(resource);
_resources.add(resource);
}
if (res.isEmpty())
if (_resources.isEmpty())
{
_resources = null;
return;
throw new IllegalArgumentException("resources cannot be empty or null");
}
_resources = res.toArray(new Resource[0]);
}
catch (RuntimeException e)
{
@ -141,9 +147,9 @@ public class ResourceCollection extends Resource
/**
* Retrieves the resource collection's resources.
*
* @return the resource array
* @return the resource collection
*/
public Resource[] getResources()
public List<Resource> getResources()
{
return _resources;
}
@ -155,13 +161,13 @@ public class ResourceCollection extends Resource
*/
public void setResources(List<Resource> res)
{
_resources = new ArrayList<>();
if (res.isEmpty())
{
_resources = null;
return;
}
_resources = res.toArray(new Resource[0]);
_resources.addAll(res);
}
/**
@ -235,55 +241,48 @@ public class ResourceCollection extends Resource
return this;
}
Resource resource = null;
ArrayList<Resource> resources = null;
int i = 0;
for (; i < _resources.length; i++)
// Attempt a simple (single) Resource lookup that exists
for (Resource res : _resources)
{
resource = _resources[i].addPath(path);
if (resource.exists())
Resource fileResource = res.addPath(path);
if (fileResource.exists())
{
if (resource.isDirectory())
if (!fileResource.isDirectory())
{
break;
return fileResource;
}
return resource;
}
}
for (i++; i < _resources.length; i++)
// Create a list of potential resource for directories of this collection
ArrayList<Resource> potentialResources = null;
for (Resource res : _resources)
{
Resource r = _resources[i].addPath(path);
Resource r = res.addPath(path);
if (r.exists() && r.isDirectory())
{
if (resources == null)
if (potentialResources == null)
{
resources = new ArrayList<>();
potentialResources = new ArrayList<>();
}
if (resource != null)
potentialResources.add(r);
}
}
if (potentialResources == null || potentialResources.isEmpty())
{
resources.add(resource);
resource = null;
}
resources.add(r);
}
}
if (resource != null)
{
return resource;
}
if (resources != null)
{
return new ResourceCollection(resources.toArray(new Resource[0]));
}
throw new MalformedURLException("path does not result in Resource: " + path);
}
if (potentialResources.size() == 1)
{
return potentialResources.get(0);
}
return new ResourceCollection(potentialResources);
}
@Override
public boolean delete() throws SecurityException
{
@ -421,9 +420,8 @@ public class ResourceCollection extends Resource
{
Collections.addAll(set, r.list());
}
String[] result = set.toArray(new String[0]);
Arrays.sort(result);
return result;
return (String[])set.stream().sorted().toArray();
}
@Override
@ -449,9 +447,10 @@ public class ResourceCollection extends Resource
{
assertResourcesSet();
for (int r = _resources.length; r-- > 0; )
// Copy in reverse order
for (int r = _resources.size(); r-- > 0; )
{
_resources[r].copyTo(destination);
_resources.get(r).copyTo(destination);
}
}
@ -461,12 +460,12 @@ public class ResourceCollection extends Resource
@Override
public String toString()
{
if (_resources == null || _resources.length == 0)
if (_resources.isEmpty())
{
return "[]";
}
return String.valueOf(Arrays.asList(_resources));
return String.valueOf(_resources);
}
@Override
@ -478,7 +477,7 @@ public class ResourceCollection extends Resource
private void assertResourcesSet()
{
if (_resources == null || _resources.length == 0)
if (_resources == null || _resources.isEmpty())
{
throw new IllegalStateException("*resources* not set.");
}

View File

@ -36,7 +36,7 @@ public interface ResourceFactory
* </p>
*
* @param path The path to the resource
* @return The resource
* @return The resource, that might not actually exist (yet).
* @throws IOException if unable to create Resource
*/
Resource getResource(String path) throws IOException;

View File

@ -27,6 +27,7 @@ import java.net.URISyntaxException;
import java.net.URL;
import java.net.URLConnection;
import java.nio.channels.ReadableByteChannel;
import java.util.Objects;
import org.eclipse.jetty.util.URIUtil;
import org.eclipse.jetty.util.thread.AutoLock;
@ -287,12 +288,9 @@ public class URLResource extends Resource
*/
@Override
public Resource addPath(String path)
throws IOException, MalformedURLException
throws IOException
{
if (path == null)
{
throw new MalformedURLException("null path");
}
Objects.requireNonNull(path, "Path may not be null");
path = URIUtil.canonicalPath(path);

View File

@ -152,7 +152,7 @@ public class ResourceCollectionTest
assertThrows(IllegalStateException.class, () -> coll.setResources(new Resource[]{null, null, null}));
// Ensure not modified.
assertThat(coll.getResources().length, is(1));
assertThat(coll.getResources().size(), is(1));
}
private void assertThrowIllegalStateException(ResourceCollection coll)