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

Updates from review

This commit is contained in:
Joakim Erdfelt 2022-08-01 07:39:40 -05:00
parent 98fa52ef30
commit e6be9fb14c
No known key found for this signature in database
GPG Key ID: 2D0E1FB8FE4B68B4
9 changed files with 165 additions and 87 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
jetty-core/jetty-deploy/src/test/java/org/eclipse/jetty/deploy/MockAppProvider.java
View File

@ -13,7 +13,6 @@
package org.eclipse.jetty.deploy; package org.eclipse.jetty.deploy;
import java.io.File;
import java.nio.file.Path; import java.nio.file.Path;
import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.handler.ContextHandler;
@ -26,7 +25,7 @@ import org.eclipse.jetty.util.component.Environment;
public class MockAppProvider extends AbstractLifeCycle implements AppProvider public class MockAppProvider extends AbstractLifeCycle implements AppProvider
{ {
private DeploymentManager deployMan; private DeploymentManager deployMan;
private File webappsDir; private Path webappsDir;
@Override @Override
public String getEnvironmentName() public String getEnvironmentName()
@ -43,7 +42,7 @@ public class MockAppProvider extends AbstractLifeCycle implements AppProvider
@Override @Override
public void doStart() public void doStart()
{ {
this.webappsDir = MavenTestingUtils.getTestResourceDir("webapps"); this.webappsDir = MavenTestingUtils.getTestResourcePathDir("webapps");
} }
public App createWebapp(String name) public App createWebapp(String name)
@ -54,13 +53,13 @@ public class MockAppProvider extends AbstractLifeCycle implements AppProvider
} }
@Override @Override
public ContextHandler createContextHandler(App app) throws Exception public ContextHandler createContextHandler(App app)
{ {
ContextHandler contextHandler = new ContextHandler(); ContextHandler contextHandler = new ContextHandler();
String name = app.getPath().toString(); String name = app.getPath().toString();
name = name.substring(name.lastIndexOf("-") + 1); name = name.substring(name.lastIndexOf("-") + 1);
Path war = new File(webappsDir, name).toPath(); Path war = webappsDir.resolve(name);
String path = war.toString(); String path = war.toString();

9
jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/FileID.java
View File

@ -57,7 +57,10 @@ public class FileID
*/ */
public static String getBasename(Path path) public static String getBasename(Path path)
{ {
String basename = path.getFileName().toString(); Path filename = path.getFileName();
if (filename == null)
return "";
String basename = filename.toString();
int dot = basename.lastIndexOf('.'); int dot = basename.lastIndexOf('.');
if (dot >= 0) if (dot >= 0)
basename = basename.substring(0, dot); basename = basename.substring(0, dot);
@ -354,7 +357,7 @@ public class FileID
* @param path the path to test * @param path the path to test
* @return true if not in {@code META-INF/versions/*} tree * @return true if not in {@code META-INF/versions/*} tree
*/ */
public static boolean skipMetaInfVersions(Path path) public static boolean isNotMetaInfVersions(Path path)
{ {
return !isMetaInfVersions(path); return !isMetaInfVersions(path);
} }
@ -369,7 +372,7 @@ public class FileID
* @param path the path to test * @param path the path to test
* @return true if not a {@code module-info.class} file * @return true if not a {@code module-info.class} file
*/ */
public static boolean skipModuleInfoClass(Path path) public static boolean isNotModuleInfoClass(Path path)
{ {
Path filenameSegment = path.getFileName(); Path filenameSegment = path.getFileName();
if (filenameSegment == null) if (filenameSegment == null)

2
jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/MultiReleaseJarFile.java
View File

@ -72,7 +72,7 @@ public class MultiReleaseJarFile implements Closeable
return Files.walk(rootPath) return Files.walk(rootPath)
// skip the entire META-INF/versions tree // skip the entire META-INF/versions tree
.filter(FileID::skipMetaInfVersions); .filter(FileID::isNotMetaInfVersions);
} }
@Override @Override

28
jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java
View File

@ -13,6 +13,7 @@
package org.eclipse.jetty.util; package org.eclipse.jetty.util;
import java.io.File;
import java.io.IOException; import java.io.IOException;
import java.net.URI; import java.net.URI;
import java.net.URL; import java.net.URL;
@ -1655,7 +1656,28 @@ public final class URIUtil
return query1 + '&' + query2; return query1 + '&' + query2;
} }
public static URI correctBadFileURI(URI uri) /**
* <p>
* Corrects any bad {@code file} based URIs (even within a {@code jar:file:} based URIs) from the bad out-of-spec
* format that various older Java APIs creates (most notably: {@link java.io.File} creates with it's {@link File#toURL()}
* and {@link File#toURI()}, along with the side effects of using {@link URL#toURI()})
* </p>
*
* <p>
* This correction is limited to only the {@code file:/} substring in the URI.
* If there is a {@code file:/<not-a-slash>} detected, that substring is corrected to
* {@code file:///<not-a-slash>}, all other uses of {@code file:}, and URIs without a {@core file:}
* substring are left alone.
* </p>
*
* <p>
* Note that Windows UNC based URIs are left alone, along with non-absolute URIs.
* </p>
*
* @param uri the URI to (possibly) correct
* @return the new URI with the {@code file:/} substring corrected, or the original URI.
*/
public static URI correctFileURI(URI uri)
{ {
if ((uri == null) || (uri.getScheme() == null)) if ((uri == null) || (uri.getScheme() == null))
return uri; return uri;
@ -1869,6 +1891,6 @@ public final class URIUtil
.map(URL::toString) .map(URL::toString)
.map(URI::create) .map(URI::create)
.map(URIUtil::unwrapContainer) .map(URIUtil::unwrapContainer)
.map(URIUtil::correctBadFileURI); .map(URIUtil::correctFileURI);
} }
} }

17
jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/Resource.java
View File

@ -707,31 +707,18 @@ public abstract class Resource implements ResourceFactory
if (!subUri.getPath().endsWith(URIUtil.SLASH)) if (!subUri.getPath().endsWith(URIUtil.SLASH))
subUri = URI.create(subUri + URIUtil.SLASH); subUri = URI.create(subUri + URIUtil.SLASH);
URI subUriResolved = uriResolve(subUri, subUriPath); URI subUriResolved = subUri.resolve(subUriPath);
resolvedUri = URI.create(scheme + ":" + subUriResolved); resolvedUri = URI.create(scheme + ":" + subUriResolved);
} }
else else
{ {
if (!uri.getPath().endsWith(URIUtil.SLASH)) if (!uri.getPath().endsWith(URIUtil.SLASH))
uri = URI.create(uri + URIUtil.SLASH); uri = URI.create(uri + URIUtil.SLASH);
resolvedUri = uriResolve(uri, subUriPath); resolvedUri = uri.resolve(subUriPath);
} }
return newResource(resolvedUri); return newResource(resolvedUri);
} }
// TODO: move to URIUtil
private static URI uriResolve(URI uri, String subUriPath) throws IOException
{
try
{
return uri.resolve(subUriPath);
}
catch (IllegalArgumentException iae)
{
throw new IOException(iae);
}
}
/** /**
* @return true if this Resource is an alias to another real Resource * @return true if this Resource is an alias to another real Resource
*/ */

171
jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/FileIDTest.java
View File

@ -23,10 +23,8 @@ import java.util.Map;
import java.util.stream.Stream; import java.util.stream.Stream;
import org.eclipse.jetty.toolchain.test.FS; import org.eclipse.jetty.toolchain.test.FS;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.condition.EnabledOnOs; import org.junit.jupiter.api.condition.EnabledOnOs;
import org.junit.jupiter.api.condition.OS; import org.junit.jupiter.api.condition.OS;
import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.ExtendWith;
@ -43,15 +41,8 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
@ExtendWith(WorkDirExtension.class) @ExtendWith(WorkDirExtension.class)
public class FileIDTest public class FileIDTest
{ {
private static FileSystem metaInfVersionTestFileSystem;
public WorkDir workDir; public WorkDir workDir;
@AfterAll
public static void closeFileSystems()
{
IO.close(metaInfVersionTestFileSystem);
}
private Path touchTestPath(String input) throws IOException private Path touchTestPath(String input) throws IOException
{ {
return touchTestPath(workDir.getPath(), input); return touchTestPath(workDir.getPath(), input);
@ -62,7 +53,7 @@ public class FileIDTest
Path path = base.resolve(input); Path path = base.resolve(input);
if (input.endsWith("/")) if (input.endsWith("/"))
{ {
FS.ensureEmpty(path); FS.ensureDirExists(path);
} }
else else
{ {
@ -72,45 +63,37 @@ public class FileIDTest
return path; return path;
} }
/**
* Create an empty ZipFs FileSystem useful for testing skipMetaInfVersions and isMetaInfVersions rules
*/
private static FileSystem getMetaInfVersionTestJar() throws IOException
{
if (metaInfVersionTestFileSystem != null)
return metaInfVersionTestFileSystem;
Path outputJar = MavenTestingUtils.getTargetTestingPath("getMetaInfVersionTestJar").resolve("metainf-versions.jar");
FS.ensureEmpty(outputJar.getParent());
Map<String, String> env = new HashMap<>();
env.put("create", "true");
URI uri = URI.create("jar:" + outputJar.toUri().toASCIIString());
metaInfVersionTestFileSystem = FileSystems.newFileSystem(uri, env);
// this is an empty FileSystem, I don't need to create the files for the skipMetaInfVersions() tests
return metaInfVersionTestFileSystem;
}
public static Stream<Arguments> basenameCases() public static Stream<Arguments> basenameCases()
{ {
return Stream.of( return Stream.of(
Arguments.of("foo.xml", "foo"), Arguments.of("/foo.xml", "foo"),
Arguments.of("dir/foo.xml", "foo"), Arguments.of("/dir/foo.xml", "foo"),
Arguments.of("dir/foo", "foo"), Arguments.of("/dir/foo", "foo"),
Arguments.of("foo", "foo") Arguments.of("/dir/foo/", "foo"),
Arguments.of("/foo", "foo"),
Arguments.of("/foo/", "foo"),
Arguments.of("/", "")
); );
} }
@ParameterizedTest @ParameterizedTest
@MethodSource("basenameCases") @MethodSource("basenameCases")
public void testGetBasename(String input, String expected) public void testGetBasename(String input, String expected) throws IOException
{ {
Path path = workDir.getEmptyPathDir().resolve(input);
String actual = FileID.getBasename(path); Path outputJar = workDir.getEmptyPathDir().resolve("test.jar");
assertThat(actual, is(expected)); Map<String, String> env = new HashMap<>();
env.put("create", "true");
URI uri = URI.create("jar:" + outputJar.toUri().toASCIIString());
// Use ZipFS so that we can create paths that are just "/"
try (FileSystem zipfs = FileSystems.newFileSystem(uri, env))
{
Path root = zipfs.getPath("/");
Path path = touchTestPath(root, input);
String actual = FileID.getBasename(path);
assertThat("getBasename(%s, \"%s\")".formatted(path, expected), actual, is(expected));
}
} }
public static Stream<Arguments> containsDirectoryTrueCases() public static Stream<Arguments> containsDirectoryTrueCases()
@ -124,6 +107,9 @@ public class FileIDTest
); );
} }
/**
* Tests of {@link FileID#containsDirectory(Path, String)} on a real file system
*/
@ParameterizedTest @ParameterizedTest
@MethodSource("containsDirectoryTrueCases") @MethodSource("containsDirectoryTrueCases")
public void testContainsDirectoryTrue(String input, String dirname) throws IOException public void testContainsDirectoryTrue(String input, String dirname) throws IOException
@ -142,6 +128,9 @@ public class FileIDTest
); );
} }
/**
* Tests of {@link FileID#containsDirectory(Path, String)} on a real file system
*/
@ParameterizedTest @ParameterizedTest
@MethodSource("containsDirectoryFalseCases") @MethodSource("containsDirectoryFalseCases")
public void testContainsDirectoryFalse(String input, String dirname) throws IOException public void testContainsDirectoryFalse(String input, String dirname) throws IOException
@ -150,6 +139,66 @@ public class FileIDTest
assertFalse(FileID.containsDirectory(path, dirname), "containsDirectory(%s, \"%s\")".formatted(path, dirname)); assertFalse(FileID.containsDirectory(path, dirname), "containsDirectory(%s, \"%s\")".formatted(path, dirname));
} }
public static Stream<Arguments> containsDirectoryTrueZipFsCases()
{
return Stream.of(
Arguments.of("/META-INF/services/org.eclipse.jetty.FooService", "META-INF"),
Arguments.of("/WEB-INF/lib/lib-1.jar", "WEB-INF"),
Arguments.of("/WEB-INF/dir/foo.tld", "WEB-INF")
);
}
/**
* Tests of {@link FileID#containsDirectory(Path, String)} on a ZipFS based file system
*/
@ParameterizedTest
@MethodSource("containsDirectoryTrueZipFsCases")
public void containsDirectoryTrueZipFs(String input, String dirname) throws IOException
{
Path outputJar = workDir.getEmptyPathDir().resolve("test.jar");
Map<String, String> env = new HashMap<>();
env.put("create", "true");
URI uri = URI.create("jar:" + outputJar.toUri().toASCIIString());
try (FileSystem zipfs = FileSystems.newFileSystem(uri, env))
{
Path root = zipfs.getPath("/");
Path path = touchTestPath(root, input);
assertTrue(FileID.containsDirectory(path, dirname), "containsDirectory(%s, \"%s\")".formatted(path, dirname));
}
}
public static Stream<Arguments> containsDirectoryFalseZipFsCases()
{
return Stream.of(
Arguments.of("/css/main.css", "WEB-INF"),
Arguments.of("/META-INF/classes/module-info.class", "WEB-INF"),
Arguments.of("/", "util"),
Arguments.of("/index.html", "target") // shouldn't detect that the zipfs archive is in the target directory
);
}
/**
* Tests of {@link FileID#containsDirectory(Path, String)} on a ZipFS based file system
*/
@ParameterizedTest
@MethodSource("containsDirectoryFalseZipFsCases")
public void containsDirectoryFalseZipFs(String input, String dirname) throws IOException
{
Path outputJar = workDir.getEmptyPathDir().resolve("test.jar");
Map<String, String> env = new HashMap<>();
env.put("create", "true");
URI uri = URI.create("jar:" + outputJar.toUri().toASCIIString());
// Use ZipFS to be able to test Path objects from root references (like "/")
try (FileSystem zipfs = FileSystems.newFileSystem(uri, env))
{
Path root = zipfs.getPath("/");
Path path = touchTestPath(root, input);
assertFalse(FileID.containsDirectory(path, dirname), "containsDirectory(%s, \"%s\")".formatted(path, dirname));
}
}
public static Stream<Arguments> extensionCases() public static Stream<Arguments> extensionCases()
{ {
return Stream.of( return Stream.of(
@ -270,10 +319,19 @@ public class FileIDTest
}) })
public void testIsMetaInfVersions(String input) throws IOException public void testIsMetaInfVersions(String input) throws IOException
{ {
FileSystem zipfs = getMetaInfVersionTestJar(); Path outputJar = workDir.getEmptyPathDir().resolve("test.jar");
Path testPath = zipfs.getPath(input); Map<String, String> env = new HashMap<>();
assertTrue(FileID.isMetaInfVersions(testPath), "isMetaInfVersions(" + testPath + ")"); env.put("create", "true");
assertFalse(FileID.skipMetaInfVersions(testPath), "skipMetaInfVersions(" + testPath + ")");
URI uri = URI.create("jar:" + outputJar.toUri().toASCIIString());
// Use ZipFS to be able to test Path objects from root references (like "/")
try (FileSystem zipfs = FileSystems.newFileSystem(uri, env))
{
Path root = zipfs.getPath("/");
Path testPath = touchTestPath(root, input);
assertTrue(FileID.isMetaInfVersions(testPath), "isMetaInfVersions(" + testPath + ")");
assertFalse(FileID.isNotMetaInfVersions(testPath), "isNotMetaInfVersions(" + testPath + ")");
}
} }
@ParameterizedTest @ParameterizedTest
@ -353,10 +411,19 @@ public class FileIDTest
}) })
public void testNotMetaInfVersions(String input) throws IOException public void testNotMetaInfVersions(String input) throws IOException
{ {
FileSystem zipfs = getMetaInfVersionTestJar(); Path outputJar = workDir.getEmptyPathDir().resolve("test.jar");
Path testPath = zipfs.getPath(input); Map<String, String> env = new HashMap<>();
assertTrue(FileID.skipMetaInfVersions(testPath), "skipMetaInfVersions(" + testPath + ")"); env.put("create", "true");
assertFalse(FileID.isMetaInfVersions(testPath), "isMetaInfVersions(" + testPath + ")");
URI uri = URI.create("jar:" + outputJar.toUri().toASCIIString());
// Use ZipFS to be able to test Path objects from root references (like "/")
try (FileSystem zipfs = FileSystems.newFileSystem(uri, env))
{
Path root = zipfs.getPath("/");
Path testPath = touchTestPath(root, input);
assertTrue(FileID.isNotMetaInfVersions(testPath), "isNotMetaInfVersions(" + testPath + ")");
assertFalse(FileID.isMetaInfVersions(testPath), "isMetaInfVersions(" + testPath + ")");
}
} }
@ParameterizedTest @ParameterizedTest
@ -366,10 +433,10 @@ public class FileIDTest
"Module-Info.Class", // case differences "Module-Info.Class", // case differences
"META-INF/versions/17/module-info.class" "META-INF/versions/17/module-info.class"
}) })
public void testSkipModuleInfoClassFalse(String input) throws IOException public void testIsNotModuleInfoClassFalse(String input) throws IOException
{ {
Path path = touchTestPath(input); Path path = touchTestPath(input);
assertFalse(FileID.skipModuleInfoClass(path), "skipModuleInfoClass(" + path + ")"); assertFalse(FileID.isNotModuleInfoClass(path), "isNotModuleInfoClass(" + path + ")");
} }
@ParameterizedTest @ParameterizedTest
@ -380,9 +447,9 @@ public class FileIDTest
"META-INF/versions/9/foo/module-info.class/Zed.class", // as path segment "META-INF/versions/9/foo/module-info.class/Zed.class", // as path segment
"", // no segment "", // no segment
}) })
public void testSkipModuleInfoClassTrue(String input) throws IOException public void testIsNotModuleInfoClassTrue(String input) throws IOException
{ {
Path path = touchTestPath(input); Path path = touchTestPath(input);
assertTrue(FileID.skipModuleInfoClass(path), "skipModuleInfoClass(" + path + ")"); assertTrue(FileID.isNotModuleInfoClass(path), "isNotModuleInfoClass(" + path + ")");
} }
} }

8
jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilTest.java
View File

@ -552,10 +552,10 @@ public class URIUtilTest
@ParameterizedTest @ParameterizedTest
@MethodSource("correctBadFileURICases") @MethodSource("correctBadFileURICases")
public void testCorrectBadFileURI(String input, String expected) public void testCorrectFileURI(String input, String expected)
{ {
URI inputUri = URI.create(input); URI inputUri = URI.create(input);
URI actualUri = URIUtil.correctBadFileURI(inputUri); URI actualUri = URIUtil.correctFileURI(inputUri);
URI expectedUri = URI.create(expected); URI expectedUri = URI.create(expected);
assertThat(actualUri.toASCIIString(), is(expectedUri.toASCIIString())); assertThat(actualUri.toASCIIString(), is(expectedUri.toASCIIString()));
} }
@ -577,8 +577,8 @@ public class URIUtilTest
assertThat(fileUri.toASCIIString(), not(containsString("://"))); assertThat(fileUri.toASCIIString(), not(containsString("://")));
assertThat(fileUrlUri.toASCIIString(), not(containsString("://"))); assertThat(fileUrlUri.toASCIIString(), not(containsString("://")));
assertThat(URIUtil.correctBadFileURI(fileUri).toASCIIString(), is(expectedUri.toASCIIString())); assertThat(URIUtil.correctFileURI(fileUri).toASCIIString(), is(expectedUri.toASCIIString()));
assertThat(URIUtil.correctBadFileURI(fileUrlUri).toASCIIString(), is(expectedUri.toASCIIString())); assertThat(URIUtil.correctFileURI(fileUrlUri).toASCIIString(), is(expectedUri.toASCIIString()));
} }
public static Stream<Arguments> encodeSpacesSource() public static Stream<Arguments> encodeSpacesSource()

4
jetty-ee10/jetty-ee10-annotations/src/main/java/org/eclipse/jetty/ee10/annotations/AnnotationParser.java
View File

@ -826,8 +826,8 @@ public class AnnotationParser
try (MultiReleaseJarFile jarFile = new MultiReleaseJarFile(jarResource.getPath()); try (MultiReleaseJarFile jarFile = new MultiReleaseJarFile(jarResource.getPath());
Stream<Path> jarEntryStream = jarFile.stream() Stream<Path> jarEntryStream = jarFile.stream()
.filter(Files::isRegularFile) .filter(Files::isRegularFile)
.filter(FileID::skipModuleInfoClass) .filter(FileID::isNotModuleInfoClass)
.filter(FileID::skipMetaInfVersions) .filter(FileID::isNotMetaInfVersions)
.filter(FileID::isClassFile) .filter(FileID::isClassFile)
) )
{ {

4
jetty-ee9/jetty-ee9-annotations/src/main/java/org/eclipse/jetty/ee9/annotations/AnnotationParser.java
View File

@ -827,8 +827,8 @@ public class AnnotationParser
try (MultiReleaseJarFile jarFile = new MultiReleaseJarFile(jarResource.getPath()); try (MultiReleaseJarFile jarFile = new MultiReleaseJarFile(jarResource.getPath());
Stream<Path> jarEntryStream = jarFile.stream() Stream<Path> jarEntryStream = jarFile.stream()
.filter(Files::isRegularFile) .filter(Files::isRegularFile)
.filter(FileID::skipModuleInfoClass) .filter(FileID::isNotModuleInfoClass)
.filter(FileID::skipMetaInfVersions) .filter(FileID::isNotMetaInfVersions)
.filter(FileID::isClassFile)) .filter(FileID::isClassFile))
{ {
jarEntryStream.forEach(e -> jarEntryStream.forEach(e ->