This is an automated email from the ASF dual-hosted git repository.
wenjin272 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/flink-agents.git
The following commit(s) were added to refs/heads/main by this push:
new ae5e4399 [runtime] Resolve relative file: URLs when loading classpath
skills (#966) (#1033)
ae5e4399 is described below
commit ae5e4399d3152aca84c7d662199aee73d63317e9
Author: Femi <[email protected]>
AuthorDate: Wed Aug 26 08:00:41 2026 +0100
[runtime] Resolve relative file: URLs when loading classpath skills (#966)
(#1033)
---
.../skill/repository/ClasspathSkillRepository.java | 11 ++-
.../agents/runtime/skill/repository/LocalUrls.java | 74 +++++++++++++++++++
.../skill/repository/SkillMaterializer.java | 11 ++-
.../skill/ClasspathSkillRepositoryTest.java | 82 ++++++++++++++++++++++
.../runtime/skill/repository/LocalUrlsTest.java | 81 +++++++++++++++++++++
5 files changed, 247 insertions(+), 12 deletions(-)
diff --git
a/runtime/src/main/java/org/apache/flink/agents/runtime/skill/repository/ClasspathSkillRepository.java
b/runtime/src/main/java/org/apache/flink/agents/runtime/skill/repository/ClasspathSkillRepository.java
index 6dcdd863..4a5dcda7 100644
---
a/runtime/src/main/java/org/apache/flink/agents/runtime/skill/repository/ClasspathSkillRepository.java
+++
b/runtime/src/main/java/org/apache/flink/agents/runtime/skill/repository/ClasspathSkillRepository.java
@@ -23,12 +23,10 @@ import org.slf4j.LoggerFactory;
import java.io.File;
import java.io.IOException;
-import java.net.URISyntaxException;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.file.Files;
import java.nio.file.Path;
-import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.LinkedHashSet;
@@ -136,8 +134,8 @@ public final class ClasspathSkillRepository extends
AbstractMaterializedSkillRep
throws IOException {
Path p;
try {
- p = Paths.get(url.toURI());
- } catch (URISyntaxException e) {
+ p = LocalUrls.toLocalFile(url).toPath();
+ } catch (IOException e) {
throw new IOException("Bad classpath URL: " + url, e);
}
if (Files.isDirectory(p)) {
@@ -171,8 +169,9 @@ public final class ClasspathSkillRepository extends
AbstractMaterializedSkillRep
}
File jarFileObj;
try {
- jarFileObj = new File(u.toURI());
- } catch (URISyntaxException e) {
+ jarFileObj = LocalUrls.toLocalFile(u);
+ } catch (IOException e) {
+ // Skip URLs we can't resolve to a local file (non-file
protocols, malformed).
continue;
}
if (!jarFileObj.isFile()) {
diff --git
a/runtime/src/main/java/org/apache/flink/agents/runtime/skill/repository/LocalUrls.java
b/runtime/src/main/java/org/apache/flink/agents/runtime/skill/repository/LocalUrls.java
new file mode 100644
index 00000000..8b98addb
--- /dev/null
+++
b/runtime/src/main/java/org/apache/flink/agents/runtime/skill/repository/LocalUrls.java
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.agents.runtime.skill.repository;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.net.URL;
+
+/**
+ * Centralizes the conversion of local {@code file:} URLs to {@link File} for
the classpath skill
+ * loading path (direct resource materialization, JAR extraction, and the
{@code URLClassLoader}
+ * fallback scan). Keeping this in one place ensures the three call sites
agree on how relative
+ * {@code file:} URLs are resolved.
+ */
+final class LocalUrls {
+
+ private LocalUrls() {}
+
+ /**
+ * Resolve a local {@code file:} URL to a {@link File}.
+ *
+ * <p>A Flink deployment may add user-code JARs relative to the
TaskManager working directory,
+ * producing relative {@code file:} URLs such as {@code
file:../../flink/usrlib/job.jar}. Such a
+ * URL parses to an <em>opaque</em> URI (its scheme-specific part is not
an absolute path),
+ * which {@code new File(URI)} rejects with {@code "URI is not
hierarchical"}. This method
+ * resolves the opaque URI's decoded scheme-specific part against the
process working directory
+ * instead.
+ *
+ * <p>Absolute hierarchical {@code file:} URLs keep their existing {@code
new File(uri)}
+ * behavior. Non-{@code file} URLs are rejected explicitly.
+ *
+ * @throws IOException if the URL is not a {@code file:} URL, is a
malformed URI, or cannot be
+ * represented as a local {@link File}.
+ */
+ static File toLocalFile(URL url) throws IOException {
+ if (!"file".equals(url.getProtocol())) {
+ throw new IOException("Not a local file URL: " + url);
+ }
+ try {
+ URI uri = url.toURI();
+ if (uri.isOpaque()) {
+ // Relative file: URL (e.g. file:../../flink/usrlib/job.jar).
new File(URI)
+ // rejects opaque URIs, so resolve the decoded scheme-specific
part relative to
+ // the working directory, which is exactly how a relative File
is interpreted.
+ return new File(uri.getSchemeSpecificPart());
+ }
+ return new File(uri);
+ } catch (URISyntaxException | IllegalArgumentException e) {
+ // URISyntaxException: url.toURI() rejected the URL.
IllegalArgumentException:
+ // new File(URI) cannot represent this file: URL as a local path,
e.g. one carrying an
+ // authority component like file://host/share/job.jar. Callers
only catch IOException,
+ // so wrap both so such a URL is skipped rather than escaping
unchecked.
+ throw new IOException("Malformed file URL: " + url, e);
+ }
+ }
+}
diff --git
a/runtime/src/main/java/org/apache/flink/agents/runtime/skill/repository/SkillMaterializer.java
b/runtime/src/main/java/org/apache/flink/agents/runtime/skill/repository/SkillMaterializer.java
index 300f3225..c05f7919 100644
---
a/runtime/src/main/java/org/apache/flink/agents/runtime/skill/repository/SkillMaterializer.java
+++
b/runtime/src/main/java/org/apache/flink/agents/runtime/skill/repository/SkillMaterializer.java
@@ -25,7 +25,6 @@ import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.net.HttpURLConnection;
-import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -204,11 +203,11 @@ public final class SkillMaterializer {
URL innerUrl = new URL(innerSpec);
File jarFileObj;
try {
- jarFileObj = new File(innerUrl.toURI());
- } catch (URISyntaxException | IllegalArgumentException e) {
- // IllegalArgumentException is thrown by File(URI) when the URI
scheme is not "file"
- // (e.g. a JAR nested behind http://). Surface both as IOException
so callers that
- // catch IOException for graceful failure handling see them.
+ jarFileObj = LocalUrls.toLocalFile(innerUrl);
+ } catch (IOException e) {
+ // toLocalFile rejects a non-file inner URL (e.g. a JAR nested
behind http://) and
+ // malformed URLs. Re-wrap with the outer jar URL for context so
callers that catch
+ // IOException for graceful failure handling see it.
throw new IOException("Invalid JAR URL: " + jarUrl, e);
}
try (JarFile jarFile = new JarFile(jarFileObj)) {
diff --git
a/runtime/src/test/java/org/apache/flink/agents/runtime/skill/ClasspathSkillRepositoryTest.java
b/runtime/src/test/java/org/apache/flink/agents/runtime/skill/ClasspathSkillRepositoryTest.java
index 7f0e99cc..74141946 100644
---
a/runtime/src/test/java/org/apache/flink/agents/runtime/skill/ClasspathSkillRepositoryTest.java
+++
b/runtime/src/test/java/org/apache/flink/agents/runtime/skill/ClasspathSkillRepositoryTest.java
@@ -22,11 +22,14 @@ import
org.apache.flink.agents.runtime.skill.repository.ClasspathSkillRepository
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
+import java.io.File;
import java.io.IOException;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.file.Files;
import java.nio.file.Path;
+import java.util.Collections;
+import java.util.Enumeration;
import java.util.List;
import java.util.jar.JarEntry;
import java.util.jar.JarOutputStream;
@@ -89,6 +92,85 @@ class ClasspathSkillRepositoryTest {
.collect(Collectors.toList()));
}
+ @Test
+ void loadFromRelativeJarUrl(@TempDir Path tempDir) throws IOException {
+ // A Flink deployment can add user-code JARs relative to the
TaskManager working directory,
+ // so the class loader exposes the jar through a relative file: URL
such as
+ // file:../../flink/usrlib/job.jar. new File(URI) rejects that opaque
URI; the repository
+ // must still resolve and load the skills. Regression test for GH-966.
+ Path jar = tempDir.resolve("relative-skills.jar");
+ zipDirIntoJarUnderPrefix(resourcesRoot(), jar, "embedded-skills");
+
+ Path relativeJar = Path.of("").toAbsolutePath().relativize(jar);
+ URL relativeUrl =
+ new URL("file:" +
relativeJar.toString().replace(File.separatorChar, '/'));
+ URLClassLoader loader = new URLClassLoader(new URL[] {relativeUrl}, /*
parent */ null);
+
+ ClasspathSkillRepository repo = new
ClasspathSkillRepository("embedded-skills", loader);
+ assertEquals(
+ List.of("github", "nano-banana-pro"),
+ repo.getSkills().stream()
+ .map(AgentSkill::getName)
+ .sorted()
+ .collect(Collectors.toList()));
+ }
+
+ @Test
+ void loadFromDirectRelativeJarUrl(@TempDir Path tempDir) throws
IOException {
+ // Some class loaders return from getResources() the exact (relative)
jar: URL they were
+ // configured with, rather than an absolute one. This drives the jar
branch of materialize
+ // straight into SkillMaterializer.copyJarEntries with a relative
inner file: URL, without
+ // going through the URLClassLoader fallback scan. Regression test for
GH-966.
+ Path jar = tempDir.resolve("relative-skills.jar");
+ zipDirIntoJarUnderPrefix(resourcesRoot(), jar, "embedded-skills");
+
+ Path relativeJar = Path.of("").toAbsolutePath().relativize(jar);
+ String relativeFileUrl = "file:" +
relativeJar.toString().replace(File.separatorChar, '/');
+ URL directJarUrl = new URL("jar:" + relativeFileUrl +
"!/embedded-skills");
+ ClassLoader loader = resourcesReturning("embedded-skills",
directJarUrl);
+
+ ClasspathSkillRepository repo = new
ClasspathSkillRepository("embedded-skills", loader);
+ assertEquals(
+ List.of("github", "nano-banana-pro"),
+ repo.getSkills().stream()
+ .map(AgentSkill::getName)
+ .sorted()
+ .collect(Collectors.toList()));
+ }
+
+ @Test
+ void loadFromDirectRelativeDirectoryUrl() throws IOException {
+ // getResources() returns a relative file: directory URL, driving
+ // ClasspathSkillRepository.materializeFileUrl through LocalUrls.
Regression test for
+ // GH-966. src/test/resources/skills is the module-relative directory
holding the skills.
+ URL directDirUrl = new URL("file:src/test/resources/skills");
+ ClassLoader loader = resourcesReturning("skills", directDirUrl);
+
+ ClasspathSkillRepository repo = new ClasspathSkillRepository("skills",
loader);
+ assertEquals(
+ List.of("github", "nano-banana-pro"),
+ repo.getSkills().stream()
+ .map(AgentSkill::getName)
+ .sorted()
+ .collect(Collectors.toList()));
+ }
+
+ /**
+ * A plain (non-{@link URLClassLoader}) class loader whose {@link
ClassLoader#getResources}
+ * hands back exactly {@code url} for {@code expectedResource}. Being
non-{@code URLClassLoader}
+ * suppresses the fallback scan, so a test exercises only the direct
{@code getResources} path.
+ */
+ private static ClassLoader resourcesReturning(String expectedResource, URL
url) {
+ return new ClassLoader(null) {
+ @Override
+ public Enumeration<URL> getResources(String name) {
+ return expectedResource.equals(name)
+ ? Collections.enumeration(List.of(url))
+ : Collections.emptyEnumeration();
+ }
+ };
+ }
+
@Test
void loadFromMultipleJarsMergesSkills(@TempDir Path tempDir) throws
IOException {
// Two jars on the classpath, each carrying a different skill under
the same prefix.
diff --git
a/runtime/src/test/java/org/apache/flink/agents/runtime/skill/repository/LocalUrlsTest.java
b/runtime/src/test/java/org/apache/flink/agents/runtime/skill/repository/LocalUrlsTest.java
new file mode 100644
index 00000000..e18cd4b6
--- /dev/null
+++
b/runtime/src/test/java/org/apache/flink/agents/runtime/skill/repository/LocalUrlsTest.java
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.agents.runtime.skill.repository;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class LocalUrlsTest {
+
+ @Test
+ void resolvesAbsoluteFileUrl(@TempDir Path tempDir) throws IOException {
+ Path file = Files.createFile(tempDir.resolve("job.jar"));
+ File resolved = LocalUrls.toLocalFile(file.toUri().toURL());
+ assertEquals(file.toFile(), resolved);
+ }
+
+ @Test
+ void resolvesRelativeFileUrlAgainstWorkingDir(@TempDir Path tempDir)
throws IOException {
+ // file:../../.../job.jar parses to an opaque URI that new File(URI)
rejects; it must be
+ // resolved relative to the process working directory instead.
Regression test for GH-966.
+ Path file = Files.createFile(tempDir.resolve("job.jar"));
+ Path relative = Path.of("").toAbsolutePath().relativize(file);
+ URL relativeUrl = new URL("file:" +
relative.toString().replace(File.separatorChar, '/'));
+
+ File resolved = LocalUrls.toLocalFile(relativeUrl);
+
+ assertTrue(!resolved.isAbsolute(), "a relative file URL should stay a
relative File");
+ assertEquals(
+ file.toFile().getCanonicalFile(),
+ resolved.getCanonicalFile(),
+ "relative File must resolve to the same location as the
absolute path");
+ }
+
+ @Test
+ void rejectsNonFileUrl() throws IOException {
+ URL httpUrl = new URL("http://example.com/skills.jar";);
+ IOException ex = assertThrows(IOException.class, () ->
LocalUrls.toLocalFile(httpUrl));
+ assertTrue(
+ ex.getMessage().contains("Not a local file URL"),
+ "expected 'Not a local file URL' in message, got: " +
ex.getMessage());
+ }
+
+ @Test
+ void wrapsFileUrlWithAuthorityAsIoException() throws IOException {
+ // file://host/share/job.jar is hierarchical (not opaque) but new
File(URI) rejects it with
+ // "URI has an authority component". Callers only catch IOException,
so it must be wrapped
+ // rather than escaping as an unchecked IllegalArgumentException.
+ URL authorityUrl = new URL("file://host/share/job.jar");
+ IOException ex = assertThrows(IOException.class, () ->
LocalUrls.toLocalFile(authorityUrl));
+ assertTrue(
+ ex.getCause() instanceof IllegalArgumentException,
+ "expected the IllegalArgumentException from new File(URI) as
cause, got: "
+ + ex.getCause());
+ }
+}
