Ensure that WebAppClassLoader.addJars considers classpath entries in a deterministic order.

Signed-off-by: Jesse Glick <jglick@cloudbees.com>
This commit is contained in:
Jesse Glick 2017-09-15 13:34:00 -04:00
parent 3d5f06d05d
commit ac61f0e968
7 changed files with 36 additions and 5 deletions

View File

@ -447,8 +447,8 @@ public abstract class Resource implements ResourceFactory, Closeable
/* ------------------------------------------------------------ */ /* ------------------------------------------------------------ */
/** /**
* list of resource names contained in the given resource. * list of resource names contained in the given resource.
* * Ordering is unspecified, so callers may wish to sort the return value to ensure deterministic behavior.
* @return a list of resource names contained in the given resource. * @return a list of resource names contained in the given resource, or null.
* Note: The resource names are not URL encoded. * Note: The resource names are not URL encoded.
*/ */
public abstract String[] list(); public abstract String[] list();

View File

@ -438,7 +438,7 @@ public class ResourceCollection extends Resource
set.add(s); set.add(s);
} }
String[] result=set.toArray(new String[set.size()]); String[] result=set.toArray(new String[set.size()]);
Arrays.sort(result); Arrays.sort(result); // TODO still necessary?
return result; return result;
} }

View File

@ -29,6 +29,7 @@ import java.security.CodeSource;
import java.security.PermissionCollection; import java.security.PermissionCollection;
import java.security.PrivilegedExceptionAction; import java.security.PrivilegedExceptionAction;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import java.util.Enumeration; import java.util.Enumeration;
import java.util.HashSet; import java.util.HashSet;
@ -324,10 +325,15 @@ public class WebAppClassLoader extends URLClassLoader
*/ */
public void addJars(Resource lib) public void addJars(Resource lib)
{ {
if (lib.exists() && lib.isDirectory()) if (lib.exists() && lib.isDirectory()) // TODO perhaps redundant given null check from list()?
{ {
String[] files=lib.list(); String[] files=lib.list();
for (int f=0;files!=null && f<files.length;f++) if (files == null)
{
return;
}
Arrays.sort(files);
for (int f=0;f<files.length;f++)
{ {
try try
{ {

View File

@ -716,6 +716,7 @@ public class WebInfConfiguration extends AbstractConfiguration
Resource web_inf_lib = web_inf.addPath("/lib"); Resource web_inf_lib = web_inf.addPath("/lib");
if (web_inf_lib.exists() && web_inf_lib.isDirectory()) if (web_inf_lib.exists() && web_inf_lib.isDirectory())
{ {
// TODO should files be sorted? What is the interaction with Ordering?
String[] files=web_inf_lib.list(); String[] files=web_inf_lib.list();
for (int f=0;files!=null && f<files.length;f++) for (int f=0;files!=null && f<files.length;f++)
{ {

View File

@ -22,10 +22,13 @@ import static org.eclipse.jetty.toolchain.test.ExtraMatchers.ordered;
import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat; import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeThat; import static org.junit.Assume.assumeThat;
import java.io.InputStream;
import java.lang.instrument.ClassFileTransformer; import java.lang.instrument.ClassFileTransformer;
import java.lang.instrument.IllegalClassFormatException; import java.lang.instrument.IllegalClassFormatException;
import java.net.URI; import java.net.URI;
@ -34,9 +37,11 @@ import java.nio.file.Path;
import java.security.ProtectionDomain; import java.security.ProtectionDomain;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.Enumeration;
import java.util.List; import java.util.List;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.resource.PathResource; import org.eclipse.jetty.util.resource.PathResource;
import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.Resource;
import org.junit.Before; import org.junit.Before;
@ -323,4 +328,23 @@ public class WebAppClassLoaderTest
} }
@Test
public void ordering() throws Exception
{
Enumeration<URL> resources = _loader.getResources("org/acme/clashing.txt");
assertTrue(resources.hasMoreElements());
URL resource = resources.nextElement();
try (InputStream data = resource.openStream())
{
assertEquals("correct contents of " + resource, "alpha", IO.toString(data));
}
assertTrue(resources.hasMoreElements());
resource = resources.nextElement();
try (InputStream data = resource.openStream())
{
assertEquals("correct contents of " + resource, "omega", IO.toString(data));
}
assertFalse(resources.hasMoreElements());
}
} }

Binary file not shown.

Binary file not shown.