Skip to content

Commit

Permalink
Remove usages of URIUtil.toURI and URIUtil.split (#11577)
Browse files Browse the repository at this point in the history
* Issue #11567 - fix relative path for resourceBase set in DefaultServlet

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

* Remove the ResourceFactory adaption from between baseResource and a HttpContent.Factory

* Remove toURI usage

* ensure ee9 DefaultServlet encodes the pathInContext

* Moved toURI functionality to ResourceFactory

* use context baseResource if baseResource not set

* removed usages of URIUtil.split

* fixed javadoc

* updates from review

* updates from review

* updates from review

* updates from review

* fixed resource leak in test

* fixed resource leak in test

* updates from review

* Fixes to URIUtil for Windows (#11585)

* Fixes for Windows
* Remove test that is not needed

* inlined resolveOrNew

---------

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Co-authored-by: Lachlan Roberts <lachlan@webtide.com>
Co-authored-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
  • Loading branch information
3 people authored Mar 28, 2024
1 parent 89c41b2 commit 9b6944e
Show file tree
Hide file tree
Showing 32 changed files with 570 additions and 253 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@
*/
public class ResourceHttpContentFactory implements HttpContent.Factory
{
private final ResourceFactory _factory;
private final Resource _baseResource;
private final MimeTypes _mimeTypes;

public ResourceHttpContentFactory(ResourceFactory factory, MimeTypes mimeTypes)
public ResourceHttpContentFactory(Resource baseResource, MimeTypes mimeTypes)
{
Objects.requireNonNull(mimeTypes, "MimeTypes cannot be null");
_factory = factory;
_baseResource = Objects.requireNonNullElse(baseResource, ResourceFactory.root().newResource("."));
_mimeTypes = mimeTypes;
}

Expand All @@ -44,8 +44,7 @@ public HttpContent getContent(String pathInContext) throws IOException
{
try
{
// try loading the content from our factory.
Resource resource = this._factory.newResource(pathInContext);
Resource resource = resolve(pathInContext);
if (Resources.missing(resource))
return null;
return load(pathInContext, resource);
Expand All @@ -65,6 +64,11 @@ public HttpContent getContent(String pathInContext) throws IOException
}
}

protected Resource resolve(String pathInContext)
{
return _baseResource.resolve(pathInContext);
}

private HttpContent load(String pathInContext, Resource resource)
{
if (resource == null || !resource.exists())
Expand All @@ -75,6 +79,6 @@ private HttpContent load(String pathInContext, Resource resource)
@Override
public String toString()
{
return "ResourceContentFactory[" + _factory + "]@" + hashCode();
return "ResourceContentFactory[" + _baseResource + "]@" + hashCode();
}
}
28 changes: 26 additions & 2 deletions jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/IOTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.nio.channels.ServerSocketChannel;
import java.nio.channels.SocketChannel;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileSystemException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
Expand All @@ -56,6 +57,7 @@
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assumptions.assumeTrue;

@ExtendWith(WorkDirExtension.class)
public class IOTest
Expand Down Expand Up @@ -580,7 +582,18 @@ public void testSymbolicLink(WorkDir workDir) throws Exception
FS.touch(realPath);

Path linkPath = dir.resolve("link");
Files.createSymbolicLink(linkPath, realPath);
boolean symlinkSupported;
try
{
Files.createSymbolicLink(linkPath, realPath);
symlinkSupported = true;
}
catch (UnsupportedOperationException | FileSystemException e)
{
symlinkSupported = false;
}

assumeTrue(symlinkSupported, "Symlink not supported");
Path targPath = linkPath.toRealPath();

System.err.printf("realPath = %s%n", realPath);
Expand All @@ -604,7 +617,18 @@ public void testSymbolicLinkDir(WorkDir workDir) throws Exception
Files.createDirectories(realDirPath);

Path linkDirPath = dir.resolve("link");
Files.createSymbolicLink(linkDirPath, realDirPath);
boolean symlinkSupported;
try
{
Files.createSymbolicLink(linkDirPath, realDirPath);
symlinkSupported = true;
}
catch (UnsupportedOperationException | FileSystemException e)
{
symlinkSupported = false;
}

assumeTrue(symlinkSupported, "Symlink not supported");

Path realPath = realDirPath.resolve("file");
FS.touch(realPath);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@

import org.eclipse.jetty.osgi.OSGiServerConstants;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.URIUtil;
import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
import org.osgi.framework.Filter;
Expand Down Expand Up @@ -61,22 +60,24 @@ public static URI resolvePathAsLocalizedURI(String path, Bundle bundle, Path jet
if (StringUtil.isBlank(path))
return null;

if (path.startsWith("/") || path.startsWith("file:/")) //absolute location
return URIUtil.toURI(path);
else
if (path.startsWith("file:/"))
return new URI(path);

if (path.startsWith("/") && File.separatorChar != '/')
return new URI("file:" + path);

try
{
try
{
Path p = FileSystems.getDefault().getPath(path);
if (p.isAbsolute())
return p.toUri();
}
catch (InvalidPathException x)
{
//ignore and try via the jetty bundle instead
LOG.trace("IGNORED", x);
}
Path p = FileSystems.getDefault().getPath(path);
if (p.isAbsolute())
return p.toUri();
}
catch (InvalidPathException x)
{
//ignore and try via the jetty bundle instead
LOG.trace("IGNORED", x);
}


//relative location
//try inside the bundle first
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,22 +42,17 @@
import org.slf4j.LoggerFactory;

/**
* Resource Handler.
*
* This handle will serve static content and handle If-Modified-Since headers. No caching is done. Requests for resources that do not exist are let pass (Eg no
* 404's).
* TODO there is a lot of URI manipulation, this should be factored out in a utility class.
*
* TODO GW: Work out how this logic can be reused by the DefaultServlet... potentially for wrapped output streams
*
* Missing:
* - current context' mime types
* - Default stylesheet (needs Resource impl for classpath resources)
* - request ranges
* - a way to configure caching or not
* Resource Handler will serve static content and handle If-Modified-Since headers. No caching is done.
* Requests for resources that do not exist are let pass (Eg no 404's).
*/
public class ResourceHandler extends Handler.Wrapper
{
// TODO there is a lot of URI manipulation, this should be factored out in a utility class.
// TODO Missing:
// - current context' mime types
// - Default stylesheet (needs Resource impl for classpath resources)
// - request ranges
// - a way to configure caching or not
private static final Logger LOG = LoggerFactory.getLogger(ResourceHandler.class);

private final ResourceService _resourceService = newResourceService();
Expand Down Expand Up @@ -131,7 +126,7 @@ public HttpContent.Factory getHttpContentFactory()

protected HttpContent.Factory newHttpContentFactory()
{
HttpContent.Factory contentFactory = new ResourceHttpContentFactory(ResourceFactory.of(getBaseResource()), getMimeTypes());
HttpContent.Factory contentFactory = new ResourceHttpContentFactory(getBaseResource(), getMimeTypes());
if (isUseFileMapping())
contentFactory = new FileMappingHttpContentFactory(contentFactory);
contentFactory = new VirtualHttpContentFactory(contentFactory, getStyleSheet(), "text/css");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
import org.eclipse.jetty.util.resource.ResourceFactory;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -664,7 +665,7 @@ public void before() throws Exception
@Override
protected HttpContent.Factory newHttpContentFactory()
{
HttpContent.Factory contentFactory = new ResourceHttpContentFactory(ResourceFactory.of(getBaseResource()), getMimeTypes());
HttpContent.Factory contentFactory = new ResourceHttpContentFactory(getBaseResource(), getMimeTypes());
contentFactory = new FileMappingHttpContentFactory(contentFactory);
contentFactory = new VirtualHttpContentFactory(contentFactory, getStyleSheet(), "text/css");
contentFactory = new PreCompressedHttpContentFactory(contentFactory, getPrecompressedFormats());
Expand Down Expand Up @@ -3934,10 +3935,20 @@ private void setupBigFiles(Path base) throws Exception

private void setupQuestionMarkDir(Path base) throws IOException
{
Path dirQ = base.resolve("dir?");
Files.createDirectories(dirQ);
Path welcome = dirQ.resolve("welcome.txt");
Files.writeString(welcome, "Hello");
boolean filesystemSupportsQuestionMarkDir = false;
try
{
Path dirQ = base.resolve("dir?");
Files.createDirectories(dirQ);
Path welcome = dirQ.resolve("welcome.txt");
Files.writeString(welcome, "Hello");
filesystemSupportsQuestionMarkDir = true;
}
catch (InvalidPathException e)
{
filesystemSupportsQuestionMarkDir = false;
}
Assumptions.assumeTrue(filesystemSupportsQuestionMarkDir);
}

private void setupSimpleText(Path base) throws IOException
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.component.LifeCycle;
import org.eclipse.jetty.util.resource.ResourceFactory;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -104,7 +103,7 @@ public void testTryPaths(WorkDir workDir) throws Exception
protected HttpContent.Factory newHttpContentFactory()
{
// We don't want to cache not found entries for this test.
return new ResourceHttpContentFactory(ResourceFactory.of(getBaseResource()), getMimeTypes());
return new ResourceHttpContentFactory(getBaseResource(), getMimeTypes());
}
};

Expand Down Expand Up @@ -175,7 +174,7 @@ public void testTryPathsWithPathMappings(WorkDir workDir) throws Exception
protected HttpContent.Factory newHttpContentFactory()
{
// We don't want to cache not found entries for this test.
return new ResourceHttpContentFactory(ResourceFactory.of(getBaseResource()), getMimeTypes());
return new ResourceHttpContentFactory(getBaseResource(), getMimeTypes());
}
};
resourceHandler.setDirAllowed(false);
Expand Down Expand Up @@ -303,7 +302,7 @@ public void testTryPathsHandlerAttributes(WorkDir workDir) throws Exception
protected HttpContent.Factory newHttpContentFactory()
{
// We don't want to cache not found entries for this test.
return new ResourceHttpContentFactory(ResourceFactory.of(getBaseResource()), getMimeTypes());
return new ResourceHttpContentFactory(getBaseResource(), getMimeTypes());
}
};
resourceHandler.setDirAllowed(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,23 @@ public static boolean isPathValid(String path)
return true;
}

/**
* Test if a string is a relative path or a URI
* @param uriOrPath A string that is either a path, a URI path segment or an absolute URI
* @return True if the string does not start with any absolute URI or file characters sequences.
*/
public static boolean isRelative(String uriOrPath)
{
if (uriOrPath.isEmpty())
return true;

char c = uriOrPath.charAt(0);
if (c == '/' || (File.separatorChar != '/' && c == File.separatorChar))
return false;

return !URIUtil.hasScheme(uriOrPath);
}

/**
* Test if codepoint is safe and unambiguous to pass as input to {@link URI}
*
Expand Down Expand Up @@ -1767,7 +1784,9 @@ public static URI correctURI(URI uri)
*
* @param str the input string of references
* @see #toJarFileUri(URI)
* @deprecated use {@link ResourceFactory#split(String)}
*/
@Deprecated(since = "12.0.8", forRemoval = true)
public static List<URI> split(String str)
{
List<URI> uris = new ArrayList<>();
Expand Down Expand Up @@ -1870,7 +1889,10 @@ else if (scheme.equalsIgnoreCase("file"))
* @param resource If the string starts with one of the ALLOWED_SCHEMES, then it is assumed to be a
* representation of a {@link URI}, otherwise it is treated as a {@link Path}.
* @return The {@link URI} form of the resource.
* @deprecated This method is currently resolving relative paths against the current directory, which is a mechanism
* that should be implemented by a {@link ResourceFactory}. All calls to this method need to be reviewed.
*/
@Deprecated(since = "12.0.8")
public static URI toURI(String resource)
{
Objects.requireNonNull(resource);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,9 @@ private URI toJarURIRoot(URI uri)
{
String rawURI = uri.toASCIIString();
int idx = rawURI.indexOf("!/");
return URI.create(rawURI.substring(0, idx + 2));
if (idx > 0)
return URI.create(rawURI.substring(0, idx + 2));
return uri;
}

private void unmount(URI fsUri)
Expand Down
Loading

0 comments on commit 9b6944e

Please sign in to comment.