Allow resolve to return non-existent resources (#11476)

Fix #11411 by allowing non-existent resources to be returned from resolve
Co-authored-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Co-authored-by: Ludovic Orban <lorban@bitronix.be>
This commit is contained in:
Greg Wilkins 2024-03-21 05:49:50 -07:00 committed by GitHub
parent ef87cee33d
commit 3d49cd3b3c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 175 additions and 51 deletions

View File

@ -27,6 +27,7 @@ import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.util.component.AbstractLifeCycle;
import org.eclipse.jetty.util.component.LifeCycle;
import org.eclipse.jetty.util.resource.Resource;
import org.eclipse.jetty.util.resource.Resources;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -158,6 +159,11 @@ public class AllowedResourceAliasChecker extends AbstractLifeCycle implements Al
protected boolean check(String pathInContext, Resource resource)
{
// If there is a single Path available, check it
Path path = resource.getPath();
if (path != null && Files.exists(path))
return check(pathInContext, path);
// Allow any aliases (symlinks, 8.3, casing, etc.) so long as
// the resulting real file is allowed.
for (Resource r : resource)
@ -186,7 +192,7 @@ public class AllowedResourceAliasChecker extends AbstractLifeCycle implements Al
for (String protectedTarget : _protected)
{
Resource p = _baseResource.resolve(protectedTarget);
if (p == null)
if (Resources.missing(p))
continue;
for (Resource r : p)
{

View File

@ -67,10 +67,11 @@ public class SymlinkAllowedResourceAliasChecker extends AllowedResourceAliasChec
// Add the segment to the path and realURI.
segmentPath.append("/").append(segment);
Resource fromBase = _baseResource.resolve(segmentPath.toString());
for (Resource r : fromBase)
{
Path p = r.getPath();
// If there is a single path, check it
Path p = fromBase.getPath();
if (p != null)
{
// If the ancestor of the alias is a symlink, then check if the real URI is protected, otherwise allow.
// This allows symlinks like /other->/WEB-INF and /external->/var/lib/docroot
// This does not allow symlinks like /WeB-InF->/var/lib/other
@ -80,10 +81,28 @@ public class SymlinkAllowedResourceAliasChecker extends AllowedResourceAliasChec
// If the ancestor is not allowed then do not allow.
if (!isAllowed(p))
return false;
// TODO as we are building the realURI of the resource, it would be possible to
// re-check that against security constraints.
}
else
{
// otherwise check all possibles
for (Resource r : fromBase)
{
p = r.getPath();
// If the ancestor of the alias is a symlink, then check if the real URI is protected, otherwise allow.
// This allows symlinks like /other->/WEB-INF and /external->/var/lib/docroot
// This does not allow symlinks like /WeB-InF->/var/lib/other
if (Files.isSymbolicLink(p))
return !getContextHandler().isProtectedTarget(segmentPath.toString());
// If the ancestor is not allowed then do not allow.
if (!isAllowed(p))
return false;
}
}
// TODO as we are building the realURI of the resource, it would be possible to
// re-check that against security constraints.
}
}
catch (Throwable t)

View File

@ -81,6 +81,8 @@ public class Jetty
{
try
{
if (StringUtil.isBlank(timestamp))
return "unknown";
long epochMillis = Long.parseLong(timestamp);
return Instant.ofEpochMilli(epochMillis).toString();
}

View File

@ -29,7 +29,6 @@ import java.util.regex.Pattern;
import java.util.stream.Stream;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.URIUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -411,17 +410,11 @@ public class AttributeNormalizer
{
case "WAR", "WAR.path" ->
{
Resource r = baseResource.resolve(suffix);
if (r == null)
return prefix + URIUtil.addPaths(baseResource.iterator().next().getPath().toString(), suffix);
return prefix + r.getPath();
return prefix + baseResource.resolve(suffix).getPath();
}
case "WAR.uri" ->
{
Resource r = baseResource.resolve(suffix);
if (r == null)
return prefix + URIUtil.addPaths(baseResource.iterator().next().getURI().toString(), suffix);
return prefix + r.getURI();
return prefix + baseResource.resolve(suffix).getURI();
}
}

View File

@ -153,20 +153,23 @@ public class CombinedResource extends Resource
// Attempt a simple (single) Resource lookup that exists
Resource resolved = null;
Resource notFound = null;
for (Resource res : _resources)
{
resolved = res.resolve(subUriPath);
if (Resources.missing(resolved))
continue; // skip, doesn't exist
if (!resolved.isDirectory())
if (!Resources.missing(resolved) && !resolved.isDirectory())
return resolved; // Return simple (non-directory) Resource
if (Resources.missing(resolved) && notFound == null)
notFound = resolved;
if (resources == null)
resources = new ArrayList<>();
resources.add(resolved);
}
if (resources == null)
return resolved; // This will not exist
return notFound; // This will not exist
if (resources.size() == 1)
return resources.get(0);
@ -177,13 +180,28 @@ public class CombinedResource extends Resource
@Override
public boolean exists()
{
return _resources.stream().anyMatch(Resource::exists);
for (Resource r : _resources)
if (r.exists())
return true;
return false;
}
@Override
public Path getPath()
{
return null;
int exists = 0;
Path path = null;
for (Resource r : _resources)
{
if (r.exists() && exists++ == 0)
path = r.getPath();
}
return switch (exists)
{
case 0 -> _resources.get(0).getPath();
case 1 -> path;
default -> null;
};
}
@Override
@ -213,7 +231,19 @@ public class CombinedResource extends Resource
@Override
public URI getURI()
{
return null;
int exists = 0;
URI uri = null;
for (Resource r : _resources)
{
if (r.exists() && exists++ == 0)
uri = r.getURI();
}
return switch (exists)
{
case 0 -> _resources.get(0).getURI();
case 1 -> uri;
default -> null;
};
}
@Override
@ -289,6 +319,8 @@ public class CombinedResource extends Resource
Collection<Resource> all = getAllResources();
for (Resource r : all)
{
if (!r.exists())
continue;
Path relative = getPathTo(r);
Path pathTo = Objects.equals(relative.getFileSystem(), destination.getFileSystem())
? destination.resolve(relative)
@ -335,6 +367,37 @@ public class CombinedResource extends Resource
return Objects.hash(_resources);
}
@Override
public boolean isAlias()
{
for (Resource r : _resources)
{
if (r.isAlias())
return true;
}
return false;
}
@Override
public URI getRealURI()
{
if (!isAlias())
return getURI();
int exists = 0;
URI uri = null;
for (Resource r : _resources)
{
if (r.exists() && exists++ == 0)
uri = r.getRealURI();
}
return switch (exists)
{
case 0 -> _resources.get(0).getRealURI();
case 1 -> uri;
default -> null;
};
}
/**
* @return the list of resources
*/
@ -375,6 +438,8 @@ public class CombinedResource extends Resource
// return true it's relative location to the first matching resource.
for (Resource r : _resources)
{
if (!r.exists())
continue;
Path path = r.getPath();
if (otherPath.startsWith(path))
return path.relativize(otherPath);
@ -387,8 +452,14 @@ public class CombinedResource extends Resource
Path relative = null;
loop : for (Resource o : other)
{
if (!o.exists())
continue;
for (Resource r : _resources)
{
if (!r.exists())
continue;
if (o.getPath().startsWith(r.getPath()))
{
Path rel = r.getPath().relativize(o.getPath());

View File

@ -292,10 +292,7 @@ public class PathResource extends Resource
URI uri = getURI();
URI resolvedUri = URIUtil.addPath(uri, subUriPath);
Path path = Paths.get(resolvedUri);
if (Files.exists(path))
return newResource(path, resolvedUri);
return null;
return newResource(path, resolvedUri);
}
/**

View File

@ -14,8 +14,6 @@
package org.eclipse.jetty.util.resource;
import java.net.URI;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
public class PathResourceFactory implements ResourceFactory
@ -23,9 +21,6 @@ public class PathResourceFactory implements ResourceFactory
@Override
public Resource newResource(URI uri)
{
Path path = Paths.get(uri.normalize());
if (!Files.exists(path))
return null;
return new PathResource(path, uri, false);
return new PathResource(Paths.get(uri.normalize()), uri, false);
}
}

View File

@ -268,7 +268,8 @@ public abstract class Resource implements Iterable<Resource>
* Resolve an existing Resource.
*
* @param subUriPath the encoded subUriPath
* @return an existing Resource representing the requested subUriPath, or null if resource does not exist.
* @return a Resource representing the requested subUriPath, which may not {@link #exists() exist},
* or null if the resource cannot exist.
* @throws IllegalArgumentException if subUriPath is invalid
*/
public abstract Resource resolve(String subUriPath);
@ -303,6 +304,9 @@ public abstract class Resource implements Iterable<Resource>
public void copyTo(Path destination)
throws IOException
{
if (!exists())
throw new IOException("Resource does not exist: " + getFileName());
Path src = getPath();
if (src == null)
{

View File

@ -49,7 +49,6 @@ import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
@ExtendWith(WorkDirExtension.class)
public class CombinedResourceTest
@ -118,7 +117,7 @@ public class CombinedResourceTest
assertThat(relative, containsInAnyOrder(expected));
Resource unk = rc.resolve("unknown");
assertNull(unk);
assertFalse(unk.exists());
assertEquals(getContent(rc, "1.txt"), "1 - one");
assertEquals(getContent(rc, "2.txt"), "2 - two");

View File

@ -406,7 +406,7 @@ public class PathResourceTest
// Resolve to name, but different case
testText = archiveResource.resolve("/TEST.TXT");
assertNull(testText);
assertFalse(testText.exists());
// Resolve using path navigation
testText = archiveResource.resolve("/foo/../test.txt");
@ -464,9 +464,9 @@ public class PathResourceTest
// Resolve file to name, but different case
testText = archiveResource.resolve("/dir/TEST.TXT");
assertNull(testText);
assertFalse(testText.exists());
testText = archiveResource.resolve("/DIR/test.txt");
assertNull(testText);
assertFalse(testText.exists());
// Resolve file using path navigation
testText = archiveResource.resolve("/foo/../dir/test.txt");
@ -480,7 +480,7 @@ public class PathResourceTest
// Resolve file using extension-less directory
testText = archiveResource.resolve("/dir./test.txt");
assertNull(testText);
assertFalse(testText.exists());
// Resolve directory to name, no slash
Resource dirResource = archiveResource.resolve("/dir");

View File

@ -21,6 +21,7 @@ import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.time.Instant;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Supplier;
@ -266,7 +267,7 @@ public class ResourceTest
if (data.exists)
assertThat("Exists: " + res.getName(), res.exists(), equalTo(data.exists));
else
assertNull(res);
assertFalse(res.exists());
}
@ParameterizedTest
@ -299,10 +300,16 @@ public class ResourceTest
Assumptions.assumeTrue(resource != null);
Path targetDir = workDir.getEmptyPathDir();
resource.copyTo(targetDir);
Path targetToTest = resource.isDirectory() ? targetDir : targetDir.resolve(resource.getFileName());
assertResourceSameAsPath(resource, targetToTest);
if (Resources.exists(resource))
{
resource.copyTo(targetDir);
Path targetToTest = resource.isDirectory() ? targetDir : targetDir.resolve(resource.getFileName());
assertResourceSameAsPath(resource, targetToTest);
}
else
{
assertThrows(IOException.class, () -> resource.copyTo(targetDir));
}
}
@ParameterizedTest
@ -317,9 +324,40 @@ public class ResourceTest
String filename = resource.getFileName();
Path targetDir = workDir.getEmptyPathDir();
Path targetFile = targetDir.resolve(filename);
resource.copyTo(targetFile);
if (Resources.exists(resource))
{
resource.copyTo(targetFile);
assertResourceSameAsPath(resource, targetFile);
}
else
{
assertThrows(IOException.class, () -> resource.copyTo(targetFile));
}
}
assertResourceSameAsPath(resource, targetFile);
@Test
public void testNonExistentResource()
{
Path nonExistentFile = workDir.getPathFile("does-not-exists");
Resource resource = resourceFactory.newResource(nonExistentFile);
assertFalse(resource.exists());
assertThrows(IOException.class, () -> resource.copyTo(workDir.getEmptyPathDir()));
assertTrue(resource.list().isEmpty());
assertFalse(resource.contains(resourceFactory.newResource(workDir.getPath())));
assertEquals("does-not-exists", resource.getFileName());
assertFalse(resource.isReadable());
assertEquals(nonExistentFile, resource.getPath());
assertEquals(Instant.EPOCH, resource.lastModified());
assertEquals(0L, resource.length());
assertThrows(IOException.class, resource::newInputStream);
assertThrows(IOException.class, resource::newReadableByteChannel);
assertEquals(nonExistentFile.toUri(), resource.getURI());
assertFalse(resource.isAlias());
assertNull(resource.getRealURI());
assertNotNull(resource.getName());
Resource subResource = resource.resolve("does-not-exist-too");
assertFalse(subResource.exists());
assertEquals(nonExistentFile.resolve("does-not-exist-too"), subResource.getPath());
}
@Test
@ -430,7 +468,7 @@ public class ResourceTest
Path dir = workDir.getEmptyPathDir().resolve("foo/bar");
// at this point we have a directory reference that does not exist
Resource resource = resourceFactory.newResource(dir);
assertNull(resource);
assertFalse(resource.exists());
}
@Test
@ -442,7 +480,7 @@ public class ResourceTest
// at this point we have a file reference that does not exist
assertFalse(Files.exists(file));
Resource resource = resourceFactory.newResource(file);
assertNull(resource);
assertFalse(resource.exists());
}
@Test

View File

@ -1364,7 +1364,7 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
// If a WAR file is mounted, or is extracted to a temp directory,
// then the first entry of the resource base must be the WAR file.
Resource resource = WebAppContext.this.getResource(path);
if (resource == null)
if (Resources.missing(resource))
return null;
for (Resource r: resource)

View File

@ -1429,7 +1429,7 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
// If a WAR file is mounted, or is extracted to a temp directory,
// then the first entry of the resource base must be the WAR file.
Resource resource = WebAppContext.this.getResource(path);
if (resource == null)
if (Resources.missing(resource))
return null;
for (Resource r: resource)