From d3b253071a7e3796f608ef6899b1cb7478637b2f Mon Sep 17 00:00:00 2001 From: Keith Turner Date: Fri, 16 Jan 2026 20:31:07 +0000 Subject: [PATCH 1/3] use URI instead of URL --- ...LocalCachingContextClassLoaderFactory.java | 12 +- .../lcc/definition/ContextDefinition.java | 20 ++- .../classloader/lcc/definition/Resource.java | 8 +- .../lcc/resolvers/FileResolver.java | 33 ++--- .../lcc/resolvers/HdfsFileResolver.java | 19 +-- .../lcc/resolvers/HttpFileResolver.java | 10 +- .../lcc/resolvers/LocalFileResolver.java | 23 ++-- .../classloader/lcc/util/LocalStore.java | 15 ++- ...lCachingContextClassLoaderFactoryTest.java | 127 +++++++++--------- ...AccumuloClusterClassLoaderFactoryTest.java | 20 +-- .../accumulo/classloader/lcc/TestUtils.java | 11 +- .../lcc/resolvers/FileResolversTest.java | 29 ++-- .../classloader/lcc/util/LocalStoreTest.java | 24 ++-- 13 files changed, 168 insertions(+), 183 deletions(-) diff --git a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/LocalCachingContextClassLoaderFactory.java b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/LocalCachingContextClassLoaderFactory.java index 94fed63..38948de 100644 --- a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/LocalCachingContextClassLoaderFactory.java +++ b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/LocalCachingContextClassLoaderFactory.java @@ -23,6 +23,7 @@ import java.io.IOException; import java.io.UncheckedIOException; import java.lang.management.ManagementFactory; +import java.net.URI; import java.net.URISyntaxException; import java.net.URL; import java.net.URLClassLoader; @@ -200,6 +201,8 @@ private ContextDefinition computeDefinitionAndClassLoader( monitorContext(contextLocation, computedDefinition.getMonitorIntervalSeconds()); } catch (IOException e) { throw new UncheckedIOException(e); + } catch (URISyntaxException e) { + throw new IllegalArgumentException(e); } } else { computedDefinition = previousDefinition; @@ -216,10 +219,11 @@ private ContextDefinition computeDefinitionAndClassLoader( return computedDefinition; } - private static ContextDefinition getDefinition(String contextLocation) throws IOException { + private static ContextDefinition getDefinition(String contextLocation) + throws IOException, URISyntaxException { LOG.trace("Retrieving context definition file from {}", contextLocation); - URL url = new URL(contextLocation); - return ContextDefinition.fromRemoteURL(url); + URI uri = new URI(contextLocation); + return ContextDefinition.fromRemoteURL(uri); } private static String newCacheKey(String contextLocation, String contextChecksum) { @@ -268,7 +272,7 @@ private void checkMonitoredLocation(String contextLocation, long interval) { } else { LOG.trace("Context definition for {} has not changed", contextLocation); } - } catch (IOException | RuntimeException e) { + } catch (IOException | RuntimeException | URISyntaxException e) { LOG.error("Error parsing updated context definition at {}. Classloader NOT updated!", contextLocation, e); final Stopwatch failureTimer = classloaderFailures.get(contextLocation); diff --git a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/definition/ContextDefinition.java b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/definition/ContextDefinition.java index 8c13fee..3ba76d0 100644 --- a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/definition/ContextDefinition.java +++ b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/definition/ContextDefinition.java @@ -27,7 +27,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; -import java.net.URL; +import java.net.URI; import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashSet; @@ -39,8 +39,6 @@ import org.apache.accumulo.classloader.lcc.resolvers.FileResolver; import org.apache.accumulo.core.cli.Help; import org.apache.accumulo.start.spi.KeywordExecutable; -import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.fs.FsUrlStreamHandlerFactory; import com.beust.jcommander.Parameter; import com.google.auto.service.AutoService; @@ -66,10 +64,10 @@ static class Opts extends Help { private static final Gson GSON = new GsonBuilder().disableJdkUnsafe().setPrettyPrinting().create(); - public static ContextDefinition create(int monitorIntervalSecs, URL... sources) + public static ContextDefinition create(int monitorIntervalSecs, URI... sources) throws IOException { LinkedHashSet resources = new LinkedHashSet<>(); - for (URL u : sources) { + for (URI u : sources) { FileResolver resolver = FileResolver.resolve(u); try (InputStream is = resolver.getInputStream()) { String checksum = DIGESTER.digestAsHex(is); @@ -79,12 +77,12 @@ public static ContextDefinition create(int monitorIntervalSecs, URL... sources) return new ContextDefinition(monitorIntervalSecs, resources); } - public static ContextDefinition fromRemoteURL(final URL url) throws IOException { - final FileResolver resolver = FileResolver.resolve(url); + public static ContextDefinition fromRemoteURL(final URI uri) throws IOException { + final FileResolver resolver = FileResolver.resolve(uri); try (InputStream is = resolver.getInputStream()) { var def = GSON.fromJson(new InputStreamReader(is, UTF_8), ContextDefinition.class); if (def == null) { - throw new EOFException("InputStream does not contain a valid ContextDefinition at " + url); + throw new EOFException("InputStream does not contain a valid ContextDefinition at " + uri); } return def; } @@ -160,14 +158,12 @@ public String description() { @Override public void execute(String[] args) throws Exception { - URL.setURLStreamHandlerFactory(new FsUrlStreamHandlerFactory(new Configuration())); - Opts opts = new Opts(); opts.parseArgs(ContextDefinition.class.getName(), args); - URL[] urls = new URL[opts.files.size()]; + URI[] urls = new URI[opts.files.size()]; int count = 0; for (String f : opts.files) { - urls[count++] = new URL(f); + urls[count++] = new URI(f); } ContextDefinition def = create(opts.monitorInterval, urls); System.out.print(def.toJson()); diff --git a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/definition/Resource.java b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/definition/Resource.java index d31694b..4a770fe 100644 --- a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/definition/Resource.java +++ b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/definition/Resource.java @@ -18,22 +18,22 @@ */ package org.apache.accumulo.classloader.lcc.definition; -import java.net.URL; +import java.net.URI; import java.util.Objects; public class Resource { - private URL location; + private URI location; private String checksum; public Resource() {} - public Resource(URL location, String checksum) { + public Resource(URI location, String checksum) { this.location = location; this.checksum = checksum; } - public URL getLocation() { + public URI getLocation() { return location; } diff --git a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/FileResolver.java b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/FileResolver.java index e32db52..0f2e7c7 100644 --- a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/FileResolver.java +++ b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/FileResolver.java @@ -23,34 +23,37 @@ import java.io.IOException; import java.io.InputStream; -import java.net.URL; +import java.net.URI; import java.util.Objects; +import com.google.common.base.Preconditions; + public abstract class FileResolver { - public static FileResolver resolve(URL url) throws IOException { - requireNonNull(url, "URL must be supplied"); - switch (url.getProtocol()) { + public static FileResolver resolve(URI uri) throws IOException { + requireNonNull(uri, "URI must be supplied"); + Preconditions.checkArgument(uri.getScheme() != null, "URI : %s has no scheme", uri); + switch (uri.getScheme()) { case "http": case "https": - return new HttpFileResolver(url); + return new HttpFileResolver(uri); case "file": - return new LocalFileResolver(url); + return new LocalFileResolver(uri); case "hdfs": - return new HdfsFileResolver(url); + return new HdfsFileResolver(uri); default: - throw new IOException("Unhandled protocol: " + url.getProtocol()); + throw new IOException("Unhandled protocol: " + uri.getScheme()); } } - private final URL url; + private final URI uri; - protected FileResolver(URL url) { - this.url = url; + protected FileResolver(URI uri) { + this.uri = uri; } - protected URL getURL() { - return this.url; + protected URI getURI() { + return this.uri; } public abstract String getFileName(); @@ -59,7 +62,7 @@ protected URL getURL() { @Override public int hashCode() { - return hash(url); + return hash(uri); } @Override @@ -74,7 +77,7 @@ public boolean equals(Object obj) { return false; } FileResolver other = (FileResolver) obj; - return Objects.equals(url, other.url); + return Objects.equals(uri, other.uri); } } diff --git a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/HdfsFileResolver.java b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/HdfsFileResolver.java index 6eda92d..27f3f4e 100644 --- a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/HdfsFileResolver.java +++ b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/HdfsFileResolver.java @@ -21,8 +21,6 @@ import java.io.IOException; import java.io.InputStream; import java.net.URI; -import java.net.URISyntaxException; -import java.net.URL; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; @@ -34,17 +32,12 @@ public final class HdfsFileResolver extends FileResolver { private final FileSystem fs; private final Path path; - protected HdfsFileResolver(URL url) throws IOException { - super(url); - try { - final URI uri = url.toURI(); - this.fs = FileSystem.get(uri, hadoopConf); - this.path = fs.makeQualified(new Path(uri)); - if (!fs.exists(this.path)) { - throw new IOException("File: " + url + " does not exist."); - } - } catch (URISyntaxException e) { - throw new IOException("Error creating URI from url: " + url, e); + protected HdfsFileResolver(URI uri) throws IOException { + super(uri); + this.fs = FileSystem.get(uri, hadoopConf); + this.path = fs.makeQualified(new Path(uri)); + if (!fs.exists(this.path)) { + throw new IOException("File: " + uri + " does not exist."); } } diff --git a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/HttpFileResolver.java b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/HttpFileResolver.java index 68da39c..0130993 100644 --- a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/HttpFileResolver.java +++ b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/HttpFileResolver.java @@ -20,19 +20,19 @@ import java.io.IOException; import java.io.InputStream; -import java.net.URL; +import java.net.URI; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; public final class HttpFileResolver extends FileResolver { - protected HttpFileResolver(URL url) throws IOException { - super(url); + protected HttpFileResolver(URI uri) throws IOException { + super(uri); } @Override public String getFileName() { - String path = getURL().getPath(); + String path = getURI().getPath(); return path.substring(path.lastIndexOf("/") + 1); } @@ -40,6 +40,6 @@ public String getFileName() { @SuppressFBWarnings(value = "URLCONNECTION_SSRF_FD", justification = "user-supplied URL is the intended functionality") public InputStream getInputStream() throws IOException { - return getURL().openStream(); + return getURI().toURL().openStream(); } } diff --git a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/LocalFileResolver.java b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/LocalFileResolver.java index ece5914..ed35959 100644 --- a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/LocalFileResolver.java +++ b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/LocalFileResolver.java @@ -23,8 +23,6 @@ import java.io.IOException; import java.io.InputStream; import java.net.URI; -import java.net.URISyntaxException; -import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; @@ -32,22 +30,17 @@ public final class LocalFileResolver extends FileResolver { private final File file; - public LocalFileResolver(URL url) throws IOException { - super(url); - if (url.getHost() != null && !url.getHost().isBlank()) { + public LocalFileResolver(URI uri) throws IOException { + super(uri); + if (uri.getHost() != null && !uri.getHost().isBlank()) { throw new IOException( - "Unsupported file url, only local files are supported. host = " + url.getHost()); + "Unsupported file uri, only local files are supported. host = " + uri.getHost()); } - try { - final URI uri = url.toURI(); - final Path path = Path.of(uri); - if (Files.notExists(Path.of(uri))) { - throw new IOException("File: " + url + " does not exist."); - } - file = path.toFile(); - } catch (URISyntaxException e) { - throw new IOException("Error creating URI from url: " + url, e); + final Path path = Path.of(uri); + if (Files.notExists(path)) { + throw new IOException("File: " + uri + " does not exist."); } + file = path.toFile(); } @Override diff --git a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/util/LocalStore.java b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/util/LocalStore.java index 96e1ed5..dc796b6 100644 --- a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/util/LocalStore.java +++ b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/util/LocalStore.java @@ -33,6 +33,7 @@ import java.io.IOException; import java.io.UncheckedIOException; import java.net.MalformedURLException; +import java.net.URI; import java.net.URL; import java.net.URLClassLoader; import java.nio.file.FileAlreadyExistsException; @@ -194,15 +195,15 @@ private void storeContextDefinition(final ContextDefinition contextDefinition, * Failures to download are not re-attempted, but will propagate up to the caller. */ private Path storeResource(final Resource resource) throws IOException { - final URL url = resource.getLocation(); - final FileResolver source = FileResolver.resolve(url); + final URI uri = resource.getLocation(); + final FileResolver source = FileResolver.resolve(uri); final String baseName = localResourceName(source.getFileName(), resource.getChecksum()); final Path destinationPath = resourcesDir.resolve(baseName); final Path tempPath = resourcesDir.resolve(tempName(baseName)); final Path downloadingProgressPath = resourcesDir.resolve("." + baseName + ".downloading"); if (Files.exists(destinationPath)) { - LOG.trace("Resource {} is already cached at {}", url, destinationPath); + LOG.trace("Resource {} is already cached at {}", uri, destinationPath); verifyDownload(resource, destinationPath, null); try { // clean up any in progress files that may have been left behind by previous failed attempts @@ -235,9 +236,9 @@ private Path storeResource(final Resource resource) throws IOException { var task = new FutureTask(() -> downloadFile(source, tempPath, resource), null); var t = new Thread(task); t.setDaemon(true); - t.setName("downloading " + url + " to " + tempPath); + t.setName("downloading " + uri + " to " + tempPath); - LOG.trace("Storing remote resource {} locally at {} via temp file {}", url, destinationPath, + LOG.trace("Storing remote resource {} locally at {} via temp file {}", uri, destinationPath, tempPath); t.start(); try { @@ -258,11 +259,11 @@ private Path storeResource(final Resource resource) throws IOException { task.cancel(true); Thread.currentThread().interrupt(); throw new IllegalStateException( - "Thread was interrupted while waiting on resource to copy from " + url + " to " + "Thread was interrupted while waiting on resource to copy from " + uri + " to " + tempPath, e); } catch (ExecutionException e) { - throw new IllegalStateException("Error copying resource from " + url + " to " + tempPath, + throw new IllegalStateException("Error copying resource from " + uri + " to " + tempPath, e); } } diff --git a/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/LocalCachingContextClassLoaderFactoryTest.java b/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/LocalCachingContextClassLoaderFactoryTest.java index 47bd6d7..0fe1a18 100644 --- a/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/LocalCachingContextClassLoaderFactoryTest.java +++ b/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/LocalCachingContextClassLoaderFactoryTest.java @@ -35,8 +35,7 @@ import java.io.File; import java.io.IOException; import java.io.UncheckedIOException; -import java.net.MalformedURLException; -import java.net.URL; +import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; @@ -58,7 +57,6 @@ import org.apache.accumulo.core.spi.common.ContextClassLoaderFactory.ContextClassLoaderException; import org.apache.accumulo.core.util.ConfigurationImpl; import org.apache.hadoop.fs.FileSystem; -import org.apache.hadoop.fs.FsUrlStreamHandlerFactory; import org.apache.hadoop.hdfs.MiniDFSCluster; import org.eclipse.jetty.server.Server; import org.junit.jupiter.api.AfterAll; @@ -75,14 +73,14 @@ public class LocalCachingContextClassLoaderFactoryTest { private static MiniDFSCluster hdfs; private static FileSystem fs; private static Server jetty; - private static URL jarAOrigLocation; - private static URL jarBOrigLocation; - private static URL jarCOrigLocation; - private static URL jarDOrigLocation; - private static URL jarEOrigLocation; - private static URL localAllContext; - private static URL hdfsAllContext; - private static URL jettyAllContext; + private static URI jarAOrigLocation; + private static URI jarBOrigLocation; + private static URI jarCOrigLocation; + private static URI jarDOrigLocation; + private static URI jarEOrigLocation; + private static URI localAllContext; + private static URI hdfsAllContext; + private static URI jettyAllContext; private static TestClassInfo classA; private static TestClassInfo classB; private static TestClassInfo classC; @@ -97,38 +95,37 @@ public class LocalCachingContextClassLoaderFactoryTest { @BeforeAll public static void beforeAll() throws Exception { // Find the Test jar files - jarAOrigLocation = - LocalCachingContextClassLoaderFactoryTest.class.getResource("/ClassLoaderTestA/TestA.jar"); + jarAOrigLocation = LocalCachingContextClassLoaderFactoryTest.class + .getResource("/ClassLoaderTestA/TestA.jar").toURI(); assertNotNull(jarAOrigLocation); - jarBOrigLocation = - LocalCachingContextClassLoaderFactoryTest.class.getResource("/ClassLoaderTestB/TestB.jar"); + jarBOrigLocation = LocalCachingContextClassLoaderFactoryTest.class + .getResource("/ClassLoaderTestB/TestB.jar").toURI(); assertNotNull(jarBOrigLocation); - jarCOrigLocation = - LocalCachingContextClassLoaderFactoryTest.class.getResource("/ClassLoaderTestC/TestC.jar"); + jarCOrigLocation = LocalCachingContextClassLoaderFactoryTest.class + .getResource("/ClassLoaderTestC/TestC.jar").toURI(); assertNotNull(jarCOrigLocation); - jarDOrigLocation = - LocalCachingContextClassLoaderFactoryTest.class.getResource("/ClassLoaderTestD/TestD.jar"); + jarDOrigLocation = LocalCachingContextClassLoaderFactoryTest.class + .getResource("/ClassLoaderTestD/TestD.jar").toURI(); assertNotNull(jarDOrigLocation); - jarEOrigLocation = - LocalCachingContextClassLoaderFactoryTest.class.getResource("/ClassLoaderTestE/TestE.jar"); + jarEOrigLocation = LocalCachingContextClassLoaderFactoryTest.class + .getResource("/ClassLoaderTestE/TestE.jar").toURI(); assertNotNull(jarEOrigLocation); // Put B into HDFS hdfs = TestUtils.getMiniCluster(); - URL.setURLStreamHandlerFactory(new FsUrlStreamHandlerFactory(hdfs.getConfiguration(0))); fs = hdfs.getFileSystem(); assertTrue(fs.mkdirs(new org.apache.hadoop.fs.Path("/contextB"))); final var dst = new org.apache.hadoop.fs.Path("/contextB/TestB.jar"); - fs.copyFromLocalFile(new org.apache.hadoop.fs.Path(jarBOrigLocation.toURI()), dst); + fs.copyFromLocalFile(new org.apache.hadoop.fs.Path(jarBOrigLocation), dst); assertTrue(fs.exists(dst)); - final URL jarBHdfsLocation = new URL(fs.getUri().toString() + dst.toUri().toString()); + final URI jarBHdfsLocation = new URI(fs.getUri().toString() + dst.toUri().toString()); // Have Jetty serve up files from Jar C directory - var jarCParentDirectory = Path.of(jarCOrigLocation.toURI()).getParent(); + var jarCParentDirectory = Path.of(jarCOrigLocation).getParent(); assertNotNull(jarCParentDirectory); jetty = TestUtils.getJetty(jarCParentDirectory); - final URL jarCJettyLocation = jetty.getURI().resolve("TestC.jar").toURL(); + final URI jarCJettyLocation = jetty.getURI().resolve("TestC.jar"); // ContextDefinition with all jars var allJarsDef = ContextDefinition.create(MONITOR_INTERVAL_SECS, jarAOrigLocation, @@ -144,9 +141,9 @@ public static void beforeAll() throws Exception { fs.copyFromLocalFile(new org.apache.hadoop.fs.Path(localDefFile.toURI()), hdfsDefFile); assertTrue(fs.exists(hdfsDefFile)); - localAllContext = localDefFile.toURI().toURL(); - hdfsAllContext = new URL(fs.getUri().toString() + hdfsDefFile.toUri().toString()); - jettyAllContext = jetty.getURI().resolve("allContextDefinition.json").toURL(); + localAllContext = localDefFile.toURI(); + hdfsAllContext = new URI(fs.getUri().toString() + hdfsDefFile.toUri().toString()); + jettyAllContext = jetty.getURI().resolve("allContextDefinition.json"); classA = new TestClassInfo("test.TestObjectA", "Hello from A"); classB = new TestClassInfo("test.TestObjectB", "Hello from B"); @@ -169,7 +166,7 @@ public static void afterAll() throws Exception { public void beforeEach() throws Exception { baseCacheDir = tempDir.resolve("base"); ConfigurationCopy acuConf = new ConfigurationCopy( - Map.of(CACHE_DIR_PROPERTY, baseCacheDir.toAbsolutePath().toUri().toURL().toExternalForm())); + Map.of(CACHE_DIR_PROPERTY, baseCacheDir.toAbsolutePath().toUri().toString())); FACTORY = new LocalCachingContextClassLoaderFactory(); FACTORY.init(() -> new ConfigurationImpl(acuConf)); } @@ -205,16 +202,16 @@ public void testCreateFromHttp() throws Exception { public void testInvalidContextDefinitionURL() { var ex = assertThrows(ContextClassLoaderException.class, () -> FACTORY.getClassLoader("/not/a/URL")); - assertTrue(ex.getCause() instanceof UncheckedIOException); - assertTrue(ex.getCause().getCause() instanceof MalformedURLException); - assertEquals("no protocol: /not/a/URL", ex.getCause().getCause().getMessage()); + ex.printStackTrace(); + assertTrue(ex.getCause() instanceof IllegalArgumentException, ex::toString); + assertEquals("URI : /not/a/URL has no scheme", ex.getCause().getMessage()); } @Test public void testInitialContextDefinitionEmpty() throws Exception { // Create a new context definition file in HDFS, but with no content final var def = createContextDefinitionFile(fs, "EmptyContextDefinitionFile.json", null); - final URL emptyDefUrl = new URL(fs.getUri().toString() + def.toUri().toString()); + final URI emptyDefUrl = new URI(fs.getUri().toString() + def.toUri().toString()); var ex = assertThrows(ContextClassLoaderException.class, () -> FACTORY.getClassLoader(emptyDefUrl.toString())); @@ -232,7 +229,7 @@ public void testInitialInvalidJson() throws Exception { // write out invalid json final var invalid = createContextDefinitionFile(fs, "InvalidContextDefinitionFile.json", def.toJson().substring(0, 4)); - final URL invalidDefUrl = new URL(fs.getUri().toString() + invalid.toUri().toString()); + final URI invalidDefUrl = new URI(fs.getUri().toString() + invalid.toUri().toString()); var ex = assertThrows(ContextClassLoaderException.class, () -> FACTORY.getClassLoader(invalidDefUrl.toString())); @@ -245,7 +242,7 @@ public void testInitial() throws Exception { var def = ContextDefinition.create(MONITOR_INTERVAL_SECS, jarAOrigLocation); final var initial = createContextDefinitionFile(fs, "InitialContextDefinitionFile.json", def.toJson()); - final URL initialDefUrl = new URL(fs.getUri().toString() + initial.toUri().toString()); + final URI initialDefUrl = new URI(fs.getUri().toString() + initial.toUri().toString()); ClassLoader cl = FACTORY.getClassLoader(initialDefUrl.toString()); @@ -258,7 +255,7 @@ public void testInitial() throws Exception { @Test public void testInitialNonExistentResource() throws Exception { // copy jarA to some other name - var jarAPath = Path.of(jarAOrigLocation.toURI()); + var jarAPath = Path.of(jarAOrigLocation); var jarAPathParent = jarAPath.getParent(); assertNotNull(jarAPathParent); var jarACopy = jarAPathParent.resolve("jarACopy.jar"); @@ -266,14 +263,14 @@ public void testInitialNonExistentResource() throws Exception { Files.copy(jarAPath, jarACopy, StandardCopyOption.REPLACE_EXISTING); assertTrue(Files.exists(jarACopy)); - var def = ContextDefinition.create(MONITOR_INTERVAL_SECS, jarACopy.toUri().toURL()); + var def = ContextDefinition.create(MONITOR_INTERVAL_SECS, jarACopy.toUri()); Files.delete(jarACopy); assertTrue(!Files.exists(jarACopy)); final var initial = createContextDefinitionFile(fs, "InitialContextDefinitionFileMissingResource.json", def.toJson()); - final URL initialDefUrl = new URL(fs.getUri().toString() + initial.toUri().toString()); + final URI initialDefUrl = new URI(fs.getUri().toString() + initial.toUri().toString()); var ex = assertThrows(ContextClassLoaderException.class, () -> FACTORY.getClassLoader(initialDefUrl.toString())); @@ -293,16 +290,14 @@ public void testInitialBadResourceURL() throws Exception { final var initial = createContextDefinitionFile(fs, "InitialContextDefinitionBadResourceURL.json", badJson); - final URL initialDefUrl = new URL(fs.getUri().toString() + initial.toUri().toString()); + final URI initialDefUrl = new URI(fs.getUri().toString() + initial.toUri().toString()); var ex = assertThrows(ContextClassLoaderException.class, () -> FACTORY.getClassLoader(initialDefUrl.toString())); assertTrue(ex.getMessage().startsWith("Error getting classloader for context:"), ex::getMessage); - assertTrue(ex.getCause() instanceof JsonSyntaxException); - assertTrue(ex.getCause().getCause() instanceof MalformedURLException); - assertTrue(ex.getCause().getCause().getMessage().startsWith("no protocol"), - ex.getCause().getCause()::getMessage); + assertTrue(ex.getCause() instanceof IllegalArgumentException); + assertTrue(ex.getCause().getMessage().contains("no scheme"), ex.getCause()::getMessage); } @Test @@ -315,7 +310,7 @@ public void testInitialBadResourceChecksum() throws Exception { final var initial = createContextDefinitionFile(fs, "InitialContextDefinitionBadResourceChecksum.json", def.toJson()); - final URL initialDefUrl = new URL(fs.getUri().toString() + initial.toUri().toString()); + final URI initialDefUrl = new URI(fs.getUri().toString() + initial.toUri().toString()); var ex = assertThrows(ContextClassLoaderException.class, () -> FACTORY.getClassLoader(initialDefUrl.toString())); @@ -337,7 +332,7 @@ public void testUpdate() throws Exception { final var def = ContextDefinition.create(MONITOR_INTERVAL_SECS, jarAOrigLocation); final var defFilePath = createContextDefinitionFile(fs, "UpdateContextDefinitionFile.json", def.toJson()); - final URL updateDefUrl = new URL(fs.getUri().toString() + defFilePath.toUri().toString()); + final URI updateDefUrl = new URI(fs.getUri().toString() + defFilePath.toUri().toString()); final ClassLoader cl = FACTORY.getClassLoader(updateDefUrl.toString()); @@ -368,7 +363,7 @@ public void testUpdateSameClassNameDifferentContent() throws Exception { final var def = ContextDefinition.create(MONITOR_INTERVAL_SECS, jarAOrigLocation); final var defFilePath = createContextDefinitionFile(fs, "UpdateContextDefinitionFile.json", def.toJson()); - final URL updateDefUrl = new URL(fs.getUri().toString() + defFilePath.toUri().toString()); + final URI updateDefUrl = new URI(fs.getUri().toString() + defFilePath.toUri().toString()); final ClassLoader cl = FACTORY.getClassLoader(updateDefUrl.toString()); @@ -401,7 +396,7 @@ public void testUpdateContextDefinitionEmpty() throws Exception { final var def = ContextDefinition.create(MONITOR_INTERVAL_SECS, jarAOrigLocation); final var defFilePath = createContextDefinitionFile(fs, "UpdateEmptyContextDefinitionFile.json", def.toJson()); - final URL updateDefUrl = new URL(fs.getUri().toString() + defFilePath.toUri().toString()); + final URI updateDefUrl = new URI(fs.getUri().toString() + defFilePath.toUri().toString()); final ClassLoader cl = FACTORY.getClassLoader(updateDefUrl.toString()); @@ -432,7 +427,7 @@ public void testUpdateNonExistentResource() throws Exception { final var def = ContextDefinition.create(MONITOR_INTERVAL_SECS, jarAOrigLocation); final var defFilePath = createContextDefinitionFile(fs, "UpdateNonExistentResource.json", def.toJson()); - final URL updateDefUrl = new URL(fs.getUri().toString() + defFilePath.toUri().toString()); + final URI updateDefUrl = new URI(fs.getUri().toString() + defFilePath.toUri().toString()); final ClassLoader cl = FACTORY.getClassLoader(updateDefUrl.toString()); @@ -444,14 +439,14 @@ public void testUpdateNonExistentResource() throws Exception { // copy jarA to jarACopy // create a ContextDefinition that references it // delete jarACopy - var jarAPath = Path.of(jarAOrigLocation.toURI()); + var jarAPath = Path.of(jarAOrigLocation); var jarAPathParent = jarAPath.getParent(); assertNotNull(jarAPathParent); var jarACopy = jarAPathParent.resolve("jarACopy.jar"); assertTrue(!Files.exists(jarACopy)); Files.copy(jarAPath, jarACopy, StandardCopyOption.REPLACE_EXISTING); assertTrue(Files.exists(jarACopy)); - var def2 = ContextDefinition.create(MONITOR_INTERVAL_SECS, jarACopy.toUri().toURL()); + var def2 = ContextDefinition.create(MONITOR_INTERVAL_SECS, jarACopy.toUri()); Files.delete(jarACopy); assertTrue(!Files.exists(jarACopy)); @@ -475,7 +470,7 @@ public void testUpdateBadResourceChecksum() throws Exception { final var def = ContextDefinition.create(MONITOR_INTERVAL_SECS, jarAOrigLocation); final var defFilePath = createContextDefinitionFile(fs, "UpdateBadResourceChecksum.json", def.toJson()); - final URL updateDefUrl = new URL(fs.getUri().toString() + defFilePath.toUri().toString()); + final URI updateDefUrl = new URI(fs.getUri().toString() + defFilePath.toUri().toString()); final ClassLoader cl = FACTORY.getClassLoader(updateDefUrl.toString()); @@ -510,7 +505,7 @@ public void testUpdateBadResourceURL() throws Exception { final var def = ContextDefinition.create(MONITOR_INTERVAL_SECS, jarAOrigLocation); final var defFilePath = createContextDefinitionFile(fs, "UpdateBadResourceChecksum.json", def.toJson()); - final URL updateDefUrl = new URL(fs.getUri().toString() + defFilePath.toUri().toString()); + final URI updateDefUrl = new URI(fs.getUri().toString() + defFilePath.toUri().toString()); final ClassLoader cl = FACTORY.getClassLoader(updateDefUrl.toString()); @@ -547,7 +542,7 @@ public void testUpdateInvalidJson() throws Exception { final var def = ContextDefinition.create(MONITOR_INTERVAL_SECS, jarAOrigLocation); final var defFilePath = createContextDefinitionFile(fs, "UpdateInvalidContextDefinitionFile.json", def.toJson()); - final URL updateDefUrl = new URL(fs.getUri().toString() + defFilePath.toUri().toString()); + final URI updateDefUrl = new URI(fs.getUri().toString() + defFilePath.toUri().toString()); final ClassLoader cl = FACTORY.getClassLoader(updateDefUrl.toString()); @@ -593,7 +588,7 @@ public void testChangingContext() throws Exception { jarCOrigLocation, jarDOrigLocation); final var update = createContextDefinitionFile(fs, "UpdateChangingContextDefinition.json", def.toJson()); - final URL updatedDefUrl = new URL(fs.getUri().toString() + update.toUri().toString()); + final URI updatedDefUrl = new URI(fs.getUri().toString() + update.toUri().toString()); final ClassLoader cl = FACTORY.getClassLoader(updatedDefUrl.toString()); testClassLoads(cl, classA); @@ -601,23 +596,23 @@ public void testChangingContext() throws Exception { testClassLoads(cl, classC); testClassLoads(cl, classD); - final List masterList = new ArrayList<>(); + final List masterList = new ArrayList<>(); masterList.add(jarAOrigLocation); masterList.add(jarBOrigLocation); masterList.add(jarCOrigLocation); masterList.add(jarDOrigLocation); - List priorList = masterList; + List priorList = masterList; ClassLoader priorCL = cl; for (int i = 0; i < 20; i++) { - final List updatedList = new ArrayList<>(masterList); + final List updatedList = new ArrayList<>(masterList); Collections.shuffle(updatedList); - final URL removed = updatedList.remove(0); + final URI removed = updatedList.remove(0); // Update the contents of the context definition json file var updateDef = - ContextDefinition.create(MONITOR_INTERVAL_SECS, updatedList.toArray(new URL[0])); + ContextDefinition.create(MONITOR_INTERVAL_SECS, updatedList.toArray(new URI[0])); updateContextDefinitionFile(fs, update, updateDef.toJson()); // wait 2x the monitor interval @@ -629,7 +624,7 @@ public void testChangingContext() throws Exception { assertEquals(priorCL, updatedClassLoader); } else { assertNotEquals(cl, updatedClassLoader); - for (URL u : updatedList) { + for (URI u : updatedList) { if (u.toString().equals(jarAOrigLocation.toString())) { testClassLoads(updatedClassLoader, classA); } else if (u.toString().equals(jarBOrigLocation.toString())) { @@ -672,7 +667,7 @@ public void testGracePeriod() throws Exception { final var def = ContextDefinition.create(MONITOR_INTERVAL_SECS, jarAOrigLocation); final var defFilePath = createContextDefinitionFile(fs, "UpdateNonExistentResource.json", def.toJson()); - final URL updateDefUrl = new URL(fs.getUri().toString() + defFilePath.toUri().toString()); + final URI updateDefUrl = new URI(fs.getUri().toString() + defFilePath.toUri().toString()); final ClassLoader cl = localFactory.getClassLoader(updateDefUrl.toString()); @@ -684,14 +679,14 @@ public void testGracePeriod() throws Exception { // copy jarA to jarACopy // create a ContextDefinition that references it // delete jarACopy - var jarAPath = Path.of(jarAOrigLocation.toURI()); + var jarAPath = Path.of(jarAOrigLocation); var jarAPathParent = jarAPath.getParent(); assertNotNull(jarAPathParent); var jarACopy = jarAPathParent.resolve("jarACopy.jar"); assertTrue(!Files.exists(jarACopy)); Files.copy(jarAPath, jarACopy, StandardCopyOption.REPLACE_EXISTING); assertTrue(Files.exists(jarACopy)); - var def2 = ContextDefinition.create(MONITOR_INTERVAL_SECS, jarACopy.toUri().toURL()); + var def2 = ContextDefinition.create(MONITOR_INTERVAL_SECS, jarACopy.toUri()); Files.delete(jarACopy); assertTrue(!Files.exists(jarACopy)); @@ -724,7 +719,7 @@ public void testExternalFileModification() throws Exception { jarCOrigLocation, jarDOrigLocation); final var update = createContextDefinitionFile(fs, "UpdateChangingContextDefinition.json", def.toJson()); - final URL updatedDefUrl = new URL(fs.getUri().toString() + update.toUri().toString()); + final URI updatedDefUrl = new URI(fs.getUri().toString() + update.toUri().toString()); final ClassLoader cl = FACTORY.getClassLoader(updatedDefUrl.toString()); testClassLoads(cl, classA); @@ -750,7 +745,7 @@ public void testExternalFileModification() throws Exception { final var update2 = createContextDefinitionFile(fs, "UpdateChangingContextDefinition2.json", def.toJson()); - final URL updatedDefUrl2 = new URL(fs.getUri().toString() + update2.toUri().toString()); + final URI updatedDefUrl2 = new URI(fs.getUri().toString() + update2.toUri().toString()); // The classloader should fail to create because one of the files in the local filesystem cache // has a checksum mismatch diff --git a/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/MiniAccumuloClusterClassLoaderFactoryTest.java b/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/MiniAccumuloClusterClassLoaderFactoryTest.java index 07909eb..55e794c 100644 --- a/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/MiniAccumuloClusterClassLoaderFactoryTest.java +++ b/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/MiniAccumuloClusterClassLoaderFactoryTest.java @@ -32,7 +32,7 @@ import java.io.File; import java.io.IOException; -import java.net.URL; +import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; @@ -129,18 +129,18 @@ public void configureMiniCluster(MiniAccumuloConfigImpl cfg, private static final int MONITOR_INTERVAL_SECS = LocalCachingContextClassLoaderFactoryTest.MONITOR_INTERVAL_SECS; - private static URL jarAOrigLocation; - private static URL jarBOrigLocation; + private static URI jarAOrigLocation; + private static URI jarBOrigLocation; @BeforeAll public static void beforeAll() throws Exception { // Find the Test jar files jarAOrigLocation = MiniAccumuloClusterClassLoaderFactoryTest.class - .getResource("/ExampleIteratorsA/example-iterators-a.jar"); + .getResource("/ExampleIteratorsA/example-iterators-a.jar").toURI(); assertNotNull(jarAOrigLocation); jarBOrigLocation = MiniAccumuloClusterClassLoaderFactoryTest.class - .getResource("/ExampleIteratorsB/example-iterators-b.jar"); + .getResource("/ExampleIteratorsB/example-iterators-b.jar").toURI(); assertNotNull(jarBOrigLocation); startMiniClusterWithConfig(new TestMACConfiguration()); @@ -216,10 +216,10 @@ public void testClassLoader() throws Exception { vp.cols = params.cols; VerifyIngest.verifyIngest(client, vp); - // Set the table classloader context. The context is the URL to the context definition file - final String contextURL = testContextDefFile.toURI().toURL().toString(); + // Set the table classloader context. The context is the URI to the context definition file + final String contextURI = testContextDefFile.toURI().toString(); client.tableOperations().setProperty(tableName, Property.TABLE_CLASSLOADER_CONTEXT.getKey(), - contextURL); + contextURI); // check that the table is returning unique values // before applying the iterator @@ -276,7 +276,7 @@ public void testClassLoader() throws Exception { // Copy jar A, create a context definition using the copy, then // remove the copy so that it's not found when the context classloader // updates. - var jarAPath = Path.of(jarAOrigLocation.toURI()); + var jarAPath = Path.of(jarAOrigLocation); var jarAPathParent = jarAPath.getParent(); assertNotNull(jarAPathParent); var jarACopy = jarAPathParent.resolve("jarACopy.jar"); @@ -285,7 +285,7 @@ public void testClassLoader() throws Exception { assertTrue(Files.exists(jarACopy)); final ContextDefinition testContextDefUpdate2 = - ContextDefinition.create(MONITOR_INTERVAL_SECS, jarACopy.toUri().toURL()); + ContextDefinition.create(MONITOR_INTERVAL_SECS, jarACopy.toUri()); Files.delete(jarACopy); assertTrue(!Files.exists(jarACopy)); diff --git a/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/TestUtils.java b/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/TestUtils.java index ca0043f..413a81c 100644 --- a/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/TestUtils.java +++ b/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/TestUtils.java @@ -29,9 +29,10 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; -import java.net.URL; +import java.net.URI; import java.nio.file.Path; +import org.apache.accumulo.classloader.lcc.resolvers.FileResolver; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.FileSystem; @@ -167,14 +168,14 @@ public static Server getJetty(Path resourceDirectory) throws Exception { @SuppressFBWarnings(value = "URLCONNECTION_SSRF_FD", justification = "user-supplied URL is the intended functionality") - public static String computeResourceChecksum(URL resourceLocation) throws IOException { - try (InputStream is = resourceLocation.openStream()) { + public static String computeResourceChecksum(URI resourceLocation) throws IOException { + try (InputStream is = FileResolver.resolve(resourceLocation).getInputStream()) { return DIGESTER.digestAsHex(is); } } - public static String getFileName(URL url) { - String path = url.getPath(); + public static String getFileName(URI uri) { + String path = uri.getPath(); return path.substring(path.lastIndexOf("/") + 1); } diff --git a/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/resolvers/FileResolversTest.java b/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/resolvers/FileResolversTest.java index 8be9907..283c315 100644 --- a/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/resolvers/FileResolversTest.java +++ b/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/resolvers/FileResolversTest.java @@ -24,7 +24,7 @@ import java.io.IOException; import java.io.InputStream; -import java.net.URL; +import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; @@ -32,7 +32,6 @@ import org.apache.accumulo.classloader.lcc.TestUtils; import org.apache.commons.io.IOUtils; import org.apache.hadoop.fs.FileSystem; -import org.apache.hadoop.fs.FsUrlStreamHandlerFactory; import org.apache.hadoop.hdfs.MiniDFSCluster; import org.eclipse.jetty.server.Server; import org.junit.jupiter.api.Test; @@ -57,13 +56,13 @@ private long getFileSize(FileResolver resolver) throws IOException { @Test public void testLocalFile() throws Exception { - URL jarPath = FileResolversTest.class.getResource("/HelloWorld.jar"); + URI jarPath = FileResolversTest.class.getResource("/HelloWorld.jar").toURI(); assertNotNull(jarPath); - var p = Path.of(jarPath.toURI()); + var p = Path.of(jarPath); final long origFileSize = getFileSize(p); FileResolver resolver = FileResolver.resolve(jarPath); assertTrue(resolver instanceof LocalFileResolver); - assertEquals(jarPath, resolver.getURL()); + assertEquals(jarPath, resolver.getURI()); assertEquals("HelloWorld.jar", resolver.getFileName()); assertEquals(origFileSize, getFileSize(resolver)); } @@ -71,17 +70,17 @@ public void testLocalFile() throws Exception { @Test public void testHttpFile() throws Exception { - URL jarPath = FileResolversTest.class.getResource("/HelloWorld.jar"); + URI jarPath = FileResolversTest.class.getResource("/HelloWorld.jar").toURI(); assertNotNull(jarPath); - var p = Path.of(jarPath.toURI()); + var p = Path.of(jarPath); final long origFileSize = getFileSize(p); Server jetty = TestUtils.getJetty(p.getParent()); LOG.debug("Jetty listening at: {}", jetty.getURI()); - URL httpPath = jetty.getURI().resolve("HelloWorld.jar").toURL(); + URI httpPath = jetty.getURI().resolve("HelloWorld.jar"); FileResolver resolver = FileResolver.resolve(httpPath); assertTrue(resolver instanceof HttpFileResolver); - assertEquals(httpPath, resolver.getURL()); + assertEquals(httpPath, resolver.getURI()); assertEquals("HelloWorld.jar", resolver.getFileName()); assertEquals(origFileSize, getFileSize(resolver)); @@ -92,9 +91,9 @@ public void testHttpFile() throws Exception { @Test public void testHdfsFile() throws Exception { - URL jarPath = FileResolversTest.class.getResource("/HelloWorld.jar"); + URI jarPath = FileResolversTest.class.getResource("/HelloWorld.jar").toURI(); assertNotNull(jarPath); - var p = Path.of(jarPath.toURI()); + var p = Path.of(jarPath); final long origFileSize = getFileSize(p); MiniDFSCluster cluster = TestUtils.getMiniCluster(); @@ -102,17 +101,15 @@ public void testHdfsFile() throws Exception { FileSystem fs = cluster.getFileSystem(); assertTrue(fs.mkdirs(new org.apache.hadoop.fs.Path("/context1"))); var dst = new org.apache.hadoop.fs.Path("/context1/HelloWorld.jar"); - fs.copyFromLocalFile(new org.apache.hadoop.fs.Path(jarPath.toURI()), dst); + fs.copyFromLocalFile(new org.apache.hadoop.fs.Path(jarPath), dst); assertTrue(fs.exists(dst)); - URL.setURLStreamHandlerFactory(new FsUrlStreamHandlerFactory(cluster.getConfiguration(0))); - - URL fullPath = new URL(fs.getUri().toString() + dst.toUri().toString()); + URI fullPath = new URI(fs.getUri().toString() + dst.toUri().toString()); LOG.info("Path to hdfs file: {}", fullPath); FileResolver resolver = FileResolver.resolve(fullPath); assertTrue(resolver instanceof HdfsFileResolver); - assertEquals(fullPath, resolver.getURL()); + assertEquals(fullPath, resolver.getURI()); assertEquals("HelloWorld.jar", resolver.getFileName()); assertEquals(origFileSize, getFileSize(resolver)); diff --git a/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/util/LocalStoreTest.java b/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/util/LocalStoreTest.java index 995ff77..014dd44 100644 --- a/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/util/LocalStoreTest.java +++ b/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/util/LocalStoreTest.java @@ -28,7 +28,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.File; -import java.net.URL; +import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; import java.util.Comparator; @@ -40,7 +40,6 @@ import org.apache.accumulo.classloader.lcc.definition.ContextDefinition; import org.apache.accumulo.classloader.lcc.definition.Resource; import org.apache.hadoop.fs.FileSystem; -import org.apache.hadoop.fs.FsUrlStreamHandlerFactory; import org.apache.hadoop.hdfs.MiniDFSCluster; import org.eclipse.jetty.server.Server; import org.junit.jupiter.api.AfterAll; @@ -69,11 +68,14 @@ public static void beforeAll() throws Exception { baseCacheDir = tempDir.resolve("base"); // Find the Test jar files - final URL jarAOrigLocation = LocalStoreTest.class.getResource("/ClassLoaderTestA/TestA.jar"); + final URI jarAOrigLocation = + LocalStoreTest.class.getResource("/ClassLoaderTestA/TestA.jar").toURI(); assertNotNull(jarAOrigLocation); - final URL jarBOrigLocation = LocalStoreTest.class.getResource("/ClassLoaderTestB/TestB.jar"); + final URI jarBOrigLocation = + LocalStoreTest.class.getResource("/ClassLoaderTestB/TestB.jar").toURI(); assertNotNull(jarBOrigLocation); - final URL jarCOrigLocation = LocalStoreTest.class.getResource("/ClassLoaderTestC/TestC.jar"); + final URI jarCOrigLocation = + LocalStoreTest.class.getResource("/ClassLoaderTestC/TestC.jar").toURI(); assertNotNull(jarCOrigLocation); // Put B into HDFS @@ -81,15 +83,14 @@ public static void beforeAll() throws Exception { final FileSystem fs = hdfs.getFileSystem(); assertTrue(fs.mkdirs(new org.apache.hadoop.fs.Path("/contextB"))); final var dst = new org.apache.hadoop.fs.Path("/contextB/TestB.jar"); - fs.copyFromLocalFile(new org.apache.hadoop.fs.Path(jarBOrigLocation.toURI()), dst); + fs.copyFromLocalFile(new org.apache.hadoop.fs.Path(jarBOrigLocation), dst); assertTrue(fs.exists(dst)); - URL.setURLStreamHandlerFactory(new FsUrlStreamHandlerFactory(hdfs.getConfiguration(0))); - final URL jarBNewLocation = new URL(fs.getUri().toString() + dst.toUri().toString()); + final URI jarBNewLocation = new URI(fs.getUri().toString() + dst.toUri().toString()); // Put C into Jetty - var jarCParentDirectory = Path.of(jarCOrigLocation.toURI()).getParent(); + var jarCParentDirectory = Path.of(jarCOrigLocation).getParent(); jetty = TestUtils.getJetty(jarCParentDirectory); - final URL jarCNewLocation = jetty.getURI().resolve("TestC.jar").toURL(); + final URI jarCNewLocation = jetty.getURI().resolve("TestC.jar"); // Create ContextDefinition with all three resources final LinkedHashSet resources = new LinkedHashSet<>(); @@ -225,7 +226,8 @@ public void testClassLoaderUpdate() throws Exception { var removedResource = def.getResources().stream().reduce((a, b) -> b).orElseThrow(); // Add D - final URL jarDOrigLocation = LocalStoreTest.class.getResource("/ClassLoaderTestD/TestD.jar"); + final URI jarDOrigLocation = + LocalStoreTest.class.getResource("/ClassLoaderTestD/TestD.jar").toURI(); assertNotNull(jarDOrigLocation); updatedResources .add(new Resource(jarDOrigLocation, TestUtils.computeResourceChecksum(jarDOrigLocation))); From c902660a33b023281fd59fef077bcdbfe1aea94b Mon Sep 17 00:00:00 2001 From: Keith Turner Date: Fri, 16 Jan 2026 21:04:04 +0000 Subject: [PATCH 2/3] remove unneeded supression --- .../java/org/apache/accumulo/classloader/lcc/TestUtils.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/TestUtils.java b/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/TestUtils.java index 413a81c..c3037c8 100644 --- a/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/TestUtils.java +++ b/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/TestUtils.java @@ -42,8 +42,6 @@ import org.eclipse.jetty.server.handler.ResourceHandler; import org.eclipse.jetty.util.resource.PathResource; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; - public class TestUtils { public static class TestClassInfo { @@ -166,8 +164,6 @@ public static Server getJetty(Path resourceDirectory) throws Exception { return jetty; } - @SuppressFBWarnings(value = "URLCONNECTION_SSRF_FD", - justification = "user-supplied URL is the intended functionality") public static String computeResourceChecksum(URI resourceLocation) throws IOException { try (InputStream is = FileResolver.resolve(resourceLocation).getInputStream()) { return DIGESTER.digestAsHex(is); From c1f94388e8230e6381530a2b1018d67831b3296f Mon Sep 17 00:00:00 2001 From: Keith Turner Date: Fri, 16 Jan 2026 23:18:00 +0000 Subject: [PATCH 3/3] code review updates --- .../accumulo/classloader/lcc/resolvers/FileResolver.java | 2 +- .../classloader/lcc/resolvers/HdfsFileResolver.java | 5 +++++ .../classloader/lcc/resolvers/HttpFileResolver.java | 7 +++++++ .../classloader/lcc/resolvers/LocalFileResolver.java | 8 ++++++++ .../lcc/LocalCachingContextClassLoaderFactoryTest.java | 5 +++-- 5 files changed, 24 insertions(+), 3 deletions(-) diff --git a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/FileResolver.java b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/FileResolver.java index 0f2e7c7..5ed8274 100644 --- a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/FileResolver.java +++ b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/FileResolver.java @@ -32,7 +32,7 @@ public abstract class FileResolver { public static FileResolver resolve(URI uri) throws IOException { requireNonNull(uri, "URI must be supplied"); - Preconditions.checkArgument(uri.getScheme() != null, "URI : %s has no scheme", uri); + Preconditions.checkArgument(uri.isAbsolute(), "URI is not absolute : %s", uri); switch (uri.getScheme()) { case "http": case "https": diff --git a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/HdfsFileResolver.java b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/HdfsFileResolver.java index 27f3f4e..e7890f1 100644 --- a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/HdfsFileResolver.java +++ b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/HdfsFileResolver.java @@ -26,6 +26,8 @@ import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; +import com.google.common.base.Preconditions; + public final class HdfsFileResolver extends FileResolver { private final Configuration hadoopConf = new Configuration(); @@ -34,6 +36,9 @@ public final class HdfsFileResolver extends FileResolver { protected HdfsFileResolver(URI uri) throws IOException { super(uri); + + Preconditions.checkArgument(uri.getScheme().equals("hdfs"), "Not HDFS URI : " + uri); + this.fs = FileSystem.get(uri, hadoopConf); this.path = fs.makeQualified(new Path(uri)); if (!fs.exists(this.path)) { diff --git a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/HttpFileResolver.java b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/HttpFileResolver.java index 0130993..88aed18 100644 --- a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/HttpFileResolver.java +++ b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/HttpFileResolver.java @@ -22,12 +22,19 @@ import java.io.InputStream; import java.net.URI; +import com.google.common.base.Preconditions; + import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; public final class HttpFileResolver extends FileResolver { protected HttpFileResolver(URI uri) throws IOException { super(uri); + // Converting to a URL will perform more strict checks, also ensure the protocol is correct. + var url = uri.toURL(); + Preconditions.checkArgument( + url.getProtocol().equals("http") || url.getProtocol().equals("https"), + "Not an HTTP url " + url); } @Override diff --git a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/LocalFileResolver.java b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/LocalFileResolver.java index ed35959..ce20390 100644 --- a/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/LocalFileResolver.java +++ b/modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/resolvers/LocalFileResolver.java @@ -26,16 +26,24 @@ import java.nio.file.Files; import java.nio.file.Path; +import com.google.common.base.Preconditions; + public final class LocalFileResolver extends FileResolver { private final File file; public LocalFileResolver(URI uri) throws IOException { super(uri); + + // Converting to a URL will perform more strict checks, also ensure the protocol is correct. + var url = uri.toURL(); + Preconditions.checkArgument(url.getProtocol().equals("file"), "Not a file url : " + url); + if (uri.getHost() != null && !uri.getHost().isBlank()) { throw new IOException( "Unsupported file uri, only local files are supported. host = " + uri.getHost()); } + final Path path = Path.of(uri); if (Files.notExists(path)) { throw new IOException("File: " + uri + " does not exist."); diff --git a/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/LocalCachingContextClassLoaderFactoryTest.java b/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/LocalCachingContextClassLoaderFactoryTest.java index 0fe1a18..7531ee8 100644 --- a/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/LocalCachingContextClassLoaderFactoryTest.java +++ b/modules/local-caching-classloader/src/test/java/org/apache/accumulo/classloader/lcc/LocalCachingContextClassLoaderFactoryTest.java @@ -204,7 +204,7 @@ public void testInvalidContextDefinitionURL() { assertThrows(ContextClassLoaderException.class, () -> FACTORY.getClassLoader("/not/a/URL")); ex.printStackTrace(); assertTrue(ex.getCause() instanceof IllegalArgumentException, ex::toString); - assertEquals("URI : /not/a/URL has no scheme", ex.getCause().getMessage()); + assertEquals("URI is not absolute : /not/a/URL", ex.getCause().getMessage()); } @Test @@ -297,7 +297,8 @@ public void testInitialBadResourceURL() throws Exception { assertTrue(ex.getMessage().startsWith("Error getting classloader for context:"), ex::getMessage); assertTrue(ex.getCause() instanceof IllegalArgumentException); - assertTrue(ex.getCause().getMessage().contains("no scheme"), ex.getCause()::getMessage); + assertTrue(ex.getCause().getMessage().contains("URI is not absolute"), + ex.getCause()::getMessage); } @Test