Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Issue #9973 - use URIUtil.addPaths() instead of URI.resolve() (#10058) 

* use URIUtil.addPaths() instead of URI.resolve()
* Better Connection / InputStream locks
* Removing URLResource.close()
* Adding URLResourceFactory.setReadTimeout()
* restore existence check in isDirectory
* Simplify URLResource.resolve

---------

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Co-authored-by: Ludovic Orban <lorban@bitronix.be>
This commit is contained in:
Joakim Erdfelt 2023-07-03 15:51:38 -05:00 committed by GitHub
parent 643c11c7a9
commit dbe9a25508
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 225 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

170
jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResourceFactory.java
View File

@ -15,6 +15,9 @@ package org.eclipse.jetty.util.resource;
import java.io.IOException;
import java.io.InputStream;
import java.lang.ref.Cleaner;
import java.lang.ref.Reference;
import java.lang.ref.SoftReference;
import java.net.MalformedURLException;
import java.net.URI;
import java.net.URL;
@ -23,22 +26,35 @@ import java.nio.channels.Channels;
import java.nio.channels.ReadableByteChannel;
import java.nio.file.Path;
import java.time.Instant;
import java.util.function.Consumer;
import org.eclipse.jetty.util.FileID;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.URIUtil;
import org.eclipse.jetty.util.thread.AutoLock;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* {@link ResourceFactory} for {@link java.net.URL} based resources.
*/
public class URLResourceFactory implements ResourceFactory
{
private int connectTimeout = 1000;
static Consumer<Reference<InputStream>> ON_SWEEP_LISTENER;
private int connectTimeout = 0;
private int readTimeout = 0;
private boolean useCaches = true;
public URLResourceFactory()
{
}
/**
* URL Connection Connect Timeout
*
* @return the connect timeout
* @see URLConnection#getConnectTimeout()
*/
public int getConnectTimeout()
{
return connectTimeout;
@ -49,6 +65,22 @@ public class URLResourceFactory implements ResourceFactory
this.connectTimeout = connectTimeout;
}
/**
* URL Connection Read Timeout.
*
* @return the read timeout
* @see URLConnection#getReadTimeout()
*/
public int getReadTimeout()
{
return readTimeout;
}
public void setReadTimeout(int readTimeout)
{
this.readTimeout = readTimeout;
}
public boolean isUseCaches()
{
return useCaches;
@ -64,7 +96,7 @@ public class URLResourceFactory implements ResourceFactory
{
try
{
return new URLResource(uri, this.connectTimeout, this.useCaches);
return new URLResource(uri, this.connectTimeout, this.readTimeout, this.useCaches);
}
catch (MalformedURLException e)
{
@ -74,25 +106,47 @@ public class URLResourceFactory implements ResourceFactory
private static class URLResource extends Resource
{
private static final Logger LOG = LoggerFactory.getLogger(URLResource.class);
private static final Cleaner CLEANER = Cleaner.create();
private final AutoLock lock = new AutoLock();
private final URI uri;
private final URL url;
private final int connectTimeout;
private final int readTimeout;
private final boolean useCaches;
private URLConnection connection;
private InputStream in = null;
public URLResource(URI uri, int connectTimeout, boolean useCaches) throws MalformedURLException
public URLResource(URI uri, int connectTimeout, int readTimeout, boolean useCaches) throws MalformedURLException
{
this.uri = uri;
this.url = uri.toURL();
this.connectTimeout = connectTimeout;
this.readTimeout = readTimeout;
this.useCaches = useCaches;
}
private URLConnection newConnection() throws IOException
protected boolean checkConnection() throws IOException
{
URLConnection urlConnection = url.openConnection();
urlConnection.setUseCaches(this.useCaches);
urlConnection.setConnectTimeout(this.connectTimeout);
return urlConnection;
try (AutoLock l = lock.lock())
{
if (connection == null)
{
try
{
connection = url.openConnection();
connection.setConnectTimeout(connectTimeout);
connection.setReadTimeout(readTimeout);
connection.setUseCaches(useCaches);
}
catch (IOException e)
{
LOG.trace("IGNORED", e);
}
}
return connection != null;
}
}
@Override
@ -111,7 +165,7 @@ public class URLResourceFactory implements ResourceFactory
@Override
public boolean isDirectory()
{
return uri.getSchemeSpecificPart().endsWith("/");
return exists() && uri.getSchemeSpecificPart().endsWith("/");
}
@Override
@ -141,10 +195,16 @@ public class URLResourceFactory implements ResourceFactory
@Override
public Resource resolve(String subUriPath)
{
URI newURI = resolve(uri, subUriPath);
if (URIUtil.isNotNormalWithinSelf(subUriPath))
throw new IllegalArgumentException(subUriPath);
if ("/".equals(subUriPath))
return this;
URI newURI = URIUtil.addPath(uri, subUriPath);
try
{
return new URLResource(newURI, this.connectTimeout, this.useCaches);
return new URLResource(newURI, this.connectTimeout, this.readTimeout, this.useCaches);
}
catch (MalformedURLException e)
{
@ -152,53 +212,71 @@ public class URLResourceFactory implements ResourceFactory
}
}
// This could probably live in URIUtil, but it's awefully specific to URLResourceFactory.
private static URI resolve(URI parent, String path)
{
if (parent.isOpaque() && parent.getPath() == null)
{
URI resolved = resolve(URI.create(parent.getRawSchemeSpecificPart()), path);
return URI.create(parent.getScheme() + ":" + resolved.toASCIIString());
}
else if (parent.getPath() != null)
{
return parent.resolve(path);
}
else
{
// Not possible to use URLs that without a path in Jetty.
throw new RuntimeException("URL without path not supported by Jetty: " + parent);
}
}
@Override
public boolean exists()
{
try
boolean ret = false;
try (AutoLock l = lock.lock())
{
newConnection();
return true;
if (checkConnection())
{
if (in == null)
{
in = connection.getInputStream();
Reference<InputStream> ref = new SoftReference<>(in);
CLEANER.register(this, () ->
{
IO.close(ref.get());
if (ON_SWEEP_LISTENER != null)
ON_SWEEP_LISTENER.accept(ref);
});
}
ret = in != null;
}
}
catch (IOException e)
{
return false;
LOG.trace("IGNORED", e);
}
return ret;
}
@Override
public InputStream newInputStream() throws IOException
{
URLConnection urlConnection = newConnection();
return urlConnection.getInputStream();
try (AutoLock l = lock.lock())
{
if (!checkConnection())
throw new IOException("Invalid resource");
try
{
if (in != null)
{
InputStream stream = in;
in = null;
return stream;
}
return connection.getInputStream();
}
finally
{
connection = null;
if (LOG.isDebugEnabled())
LOG.debug("Connection nulled");
}
}
}
@Override
public Instant lastModified()
{
try
try (AutoLock l = lock.lock())
{
URLConnection urlConnection = newConnection();
return Instant.ofEpochMilli(urlConnection.getLastModified());
if (!checkConnection())
throw new IOException("Invalid resource");
return Instant.ofEpochMilli(connection.getLastModified());
}
catch (IOException e)
{
@ -209,10 +287,12 @@ public class URLResourceFactory implements ResourceFactory
@Override
public long length()
{
try
try (AutoLock l = lock.lock())
{
URLConnection urlConnection = newConnection();
return urlConnection.getContentLengthLong();
if (!checkConnection())
throw new IOException("Invalid resource");
return connection.getContentLengthLong();
}
catch (IOException e)
{
@ -226,12 +306,6 @@ public class URLResourceFactory implements ResourceFactory
return Channels.newChannel(newInputStream());
}
@Override
public boolean isAlias()
{
return false;
}
@Override
public URI getRealURI()
{

6
jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/PathResourceTest.java
View File

@ -250,6 +250,12 @@ public class PathResourceTest
assertTrue(dirResource.exists());
assertFalse(dirResource.isAlias());
// Resolve content in dir to name, with slash
Resource textResource = dirResource.resolve("/test.txt");
assertTrue(textResource.exists());
assertFalse(textResource.isAlias());
assertTrue(Resources.isReadableFile(textResource));
// Resolve directory to name, with slash
dirResource = archiveResource.resolve("/dir/");
assertTrue(dirResource.exists());

104
jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/UrlResourceFactoryTest.java
View File

@ -24,6 +24,8 @@ import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.time.Instant;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.util.IO;
@ -31,12 +33,14 @@ import org.eclipse.jetty.util.URIUtil;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import static org.awaitility.Awaitility.await;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class UrlResourceFactoryTest
@ -45,7 +49,10 @@ public class UrlResourceFactoryTest
@Tag("external")
public void testHttps() throws IOException
{
ResourceFactory.registerResourceFactory("https", new URLResourceFactory());
URLResourceFactory urlResourceFactory = new URLResourceFactory();
urlResourceFactory.setConnectTimeout(1000);
ResourceFactory.registerResourceFactory("https", urlResourceFactory);
try
{
Resource resource = ResourceFactory.root().newResource(URI.create("https://webtide.com/"));
@ -63,7 +70,7 @@ public class UrlResourceFactoryTest
assertTrue(resource.isDirectory());
assertThat(resource.getFileName(), is(""));
Resource blogs = resource.resolve("blogs/");
Resource blogs = resource.resolve("blog/");
assertThat(blogs, notNullValue());
assertTrue(blogs.exists());
assertThat(blogs.lastModified().toEpochMilli(), not(Instant.EPOCH));
@ -85,6 +92,33 @@ public class UrlResourceFactoryTest
}
}
@Test
public void testInputStreamCleanedUp() throws Exception
{
Path path = MavenTestingUtils.getTestResourcePath("example.jar");
URI jarFileUri = URI.create("jar:" + path.toUri().toASCIIString() + "!/WEB-INF/");
AtomicInteger cleanedRefCount = new AtomicInteger();
URLResourceFactory urlResourceFactory = new URLResourceFactory();
URLResourceFactory.ON_SWEEP_LISTENER = ref ->
{
if (ref != null && ref.get() != null)
cleanedRefCount.incrementAndGet();
};
Resource resource = urlResourceFactory.newResource(jarFileUri.toURL());
Resource webResource = resource.resolve("/web.xml");
assertTrue(webResource.exists());
webResource = null;
await().atMost(5, TimeUnit.SECONDS).until(() ->
{
System.gc();
return cleanedRefCount.get() > 0;
});
}
@Test
public void testFileUrl() throws Exception
{
@ -104,6 +138,14 @@ public class UrlResourceFactoryTest
}
}
@Test
public void testIsDirectory()
{
URLResourceFactory urlResourceFactory = new URLResourceFactory();
Resource resource = urlResourceFactory.newResource("file:/does/not/exist/ends/with/a/slash/");
assertThat(resource.isDirectory(), is(false));
}
@Test
public void testJarFileUrl() throws Exception
{
@ -132,25 +174,73 @@ public class UrlResourceFactoryTest
URLResourceFactory urlResourceFactory = new URLResourceFactory();
Resource resource = urlResourceFactory.newResource(jarFileUri.toURL());
Resource webResource = resource.resolve("web.xml");
assertThat(webResource.isDirectory(), is(false));
Resource webResource = resource.resolve("/web.xml");
assertTrue(Resources.isReadableFile(webResource));
URI expectedURI = URI.create(jarFileUri.toASCIIString() + "web.xml");
assertThat(webResource.getURI(), is(expectedURI));
}
/**
* Test resolve where the input path is for a parent directory location.
* (An attempt to break out of the base resource)
*/
@Test
public void testResolveUriNoPath() throws MalformedURLException
public void testResolveUriParent() throws MalformedURLException
{
Path path = MavenTestingUtils.getTestResourcePath("example.jar");
URI jarFileUri = URI.create("jar:" + path.toUri().toASCIIString() + "!/WEB-INF/");
URLResourceFactory urlResourceFactory = new URLResourceFactory();
Resource baseResource = urlResourceFactory.newResource(jarFileUri.toURL());
assertThrows(IllegalArgumentException.class, () ->
{
baseResource.resolve("../META-INF/MANIFEST.MF");
});
}
/**
* Test resolve where the base URI has no path.
*/
@Test
public void testResolveNestedUriNoPath() throws MalformedURLException
{
Path path = MavenTestingUtils.getTestResourcePath("example.jar");
URI jarFileUri = URI.create("file:" + path.toUri().toASCIIString());
// We now have `file:file:/path/to/example.jar` URI (with two nested "file" schemes)
// The first `file` will be opaque and contain no path.
URLResourceFactory urlResourceFactory = new URLResourceFactory();
Resource resource = urlResourceFactory.newResource(jarFileUri.toURL());
assertThat("file:file:/path/to/example.jar cannot exist", resource.exists(), is(false));
assertThat(resource.isDirectory(), is(false));
Resource webResource = resource.resolve("web.xml");
Resource webResource = resource.resolve("/WEB-INF/web.xml");
assertThat("resource /path/to/example.jar/WEB-INF/web.xml doesn't exist", webResource.exists(), is(false));
assertThat(webResource.isDirectory(), is(false));
URI expectedURI = URIUtil.correctFileURI(URI.create("file:" + path.toUri().resolve("web.xml").toASCIIString()));
URI expectedURI = URIUtil.correctFileURI(URI.create("file:" + path.toUri().toASCIIString() + "/WEB-INF/web.xml"));
assertThat(webResource.getURI(), is(expectedURI));
}
/**
* Test resolve where the base URI has no path.
*/
@Test
public void testResolveFromFile() throws MalformedURLException
{
Path path = MavenTestingUtils.getTestResourcePath("example.jar");
URI jarFileUri = path.toUri();
URLResourceFactory urlResourceFactory = new URLResourceFactory();
Resource baseResource = urlResourceFactory.newResource(jarFileUri.toURL());
assertThat(baseResource.exists(), is(true));
assertThat(baseResource.isDirectory(), is(false));
Resource webResource = baseResource.resolve("/WEB-INF/web.xml");
assertThat("resource /path/to/example.jar/WEB-INF/web.xml doesn't exist", webResource.exists(), is(false));
assertThat(webResource.isDirectory(), is(false));
URI expectedURI = URIUtil.correctFileURI(URI.create(path.toUri().toASCIIString() + "/WEB-INF/web.xml"));
assertThat(webResource.getURI(), is(expectedURI));
}