From fb61aeeb63feb10eaeb08ce1a42a70fc4ec76bda Mon Sep 17 00:00:00 2001 From: Calvin Kirs Date: Fri, 30 Oct 2026 22:19:47 +0800 Subject: [PATCH] [fix](plugin) Allow legacy unversioned authentication plugins (#67286) ### What problem does this PR solve? #66211 Problem Summary: The authentication plugin API-version gate rejects every plugin jar that does not contain `Doris-Authentication-Plugin-Api-Version`. Authentication plugins were distributed before this manifest contract existed, so upgrading FE breaks those legacy artifacts even when they implement the current v1 API. This change adds an opt-in legacy mapping to the generic API gate and configures only the authentication family to treat a genuinely absent declaration as API 1.0: - unversioned legacy authentication plugins load with an operator warning; - empty, malformed, and explicitly incompatible declarations remain rejected; - connector, filesystem, lineage, and other strict gates keep their existing fail-closed behavior; - when the FE authentication API moves to another major, legacy v1 plugins are rejected automatically. --- .../handler/AuthenticationPluginManager.java | 8 +- .../AuthenticationPluginManagerTest.java | 24 ++-- .../extension/loader/ApiVersionGate.java | 94 ++++++++++++-- .../loader/DirectoryPluginRuntimeManager.java | 49 +++++--- .../extension/loader/ManifestVersions.java | 8 +- .../extension/loader/ApiVersionGateTest.java | 37 +++++- ...oryPluginRuntimeManagerApiVersionTest.java | 119 ++++++++++++++++++ 7 files changed, 292 insertions(+), 47 deletions(-) diff --git a/fe/fe-authentication/fe-authentication-handler/src/main/java/org/apache/doris/authentication/handler/AuthenticationPluginManager.java b/fe/fe-authentication/fe-authentication-handler/src/main/java/org/apache/doris/authentication/handler/AuthenticationPluginManager.java index 3565013dca8..202bef69339 100644 --- a/fe/fe-authentication/fe-authentication-handler/src/main/java/org/apache/doris/authentication/handler/AuthenticationPluginManager.java +++ b/fe/fe-authentication/fe-authentication-handler/src/main/java/org/apache/doris/authentication/handler/AuthenticationPluginManager.java @@ -66,15 +66,19 @@ public class AuthenticationPluginManager { /** Family label in the process-wide {@link PluginRegistry}. */ private static final String PLUGIN_FAMILY = "AUTHENTICATION"; + private static final String LEGACY_UNVERSIONED_API_VERSION = "1.0"; /** * The authentication plugin API contract this FE serves. Built from the version filtered into * fe-authentication-spi at build time, anchored on {@link AuthenticationPluginFactory} so that it is read * from the very artifact carrying the SPI. A missing or malformed resource is a build defect and fails - * class initialization loudly rather than degrading into a check that admits everything. + * class initialization loudly rather than degrading into a check that admits everything. Authentication + * plugins shipped before the manifest contract are treated as API 1.0 for upgrade compatibility; explicit + * malformed or incompatible declarations remain rejected. */ private static final ApiVersionGate API_VERSION_GATE = - ApiVersionGate.forFamily("authentication", AuthenticationPluginFactory.class); + ApiVersionGate.forFamilyAllowingUnversioned( + "authentication", AuthenticationPluginFactory.class, LEGACY_UNVERSIONED_API_VERSION); /** Factories by plugin name (e.g., "ldap", "oidc", "password") */ private final Map factories = new ConcurrentHashMap<>(); diff --git a/fe/fe-authentication/fe-authentication-handler/src/test/java/org/apache/doris/authentication/handler/AuthenticationPluginManagerTest.java b/fe/fe-authentication/fe-authentication-handler/src/test/java/org/apache/doris/authentication/handler/AuthenticationPluginManagerTest.java index 10f929712ee..0c38463d220 100644 --- a/fe/fe-authentication/fe-authentication-handler/src/test/java/org/apache/doris/authentication/handler/AuthenticationPluginManagerTest.java +++ b/fe/fe-authentication/fe-authentication-handler/src/test/java/org/apache/doris/authentication/handler/AuthenticationPluginManagerTest.java @@ -27,7 +27,6 @@ import org.apache.doris.authentication.spi.AuthenticationPluginFactory; import org.apache.doris.extension.loader.ApiVersionGate; import org.apache.doris.extension.loader.ClassLoadingPolicy; import org.apache.doris.extension.loader.DirectoryPluginRuntimeManager; -import org.apache.doris.extension.loader.LoadFailure; import org.apache.doris.extension.loader.LoadReport; import org.apache.doris.extension.loader.PluginHandle; @@ -405,27 +404,22 @@ public class AuthenticationPluginManagerTest { } @Test - @DisplayName("UT-HANDLER-PM-022: Plugin declaring no API version is refused (fail-closed)") - void testLoadAll_UndeclaredApiVersionIsRefused() throws Exception { - // Given: a jar built with no awareness of this contract at all. + @DisplayName("UT-HANDLER-PM-022: Legacy plugin declaring no API version remains loadable") + void testLoadAll_UndeclaredLegacyApiVersionIsAccepted() throws Exception { + // Given: an authentication plugin built before the manifest version contract existed. Path root = Files.createTempDirectory("plugin-root-no-api-version"); createPluginJar( Files.createDirectories(root.resolve("external-dir-test")).resolve("external-dir-test.jar"), DirectoryPluginFactory.class.getName(), null); - // When: this is the only directory, so loadAll reports total failure... - AuthenticationException ex = Assertions.assertThrows( - AuthenticationException.class, - () -> pluginManager.loadAll( - Arrays.asList(root), - Thread.currentThread().getContextClassLoader())); + // When + pluginManager.loadAll(Arrays.asList(root), Thread.currentThread().getContextClassLoader()); - // Then: ...and the thrown message already carries the version reason, so the caller that wraps it - // does not have to. - Assertions.assertFalse(pluginManager.hasFactory("external-dir-test")); - Assertions.assertTrue(ex.getMessage().contains(LoadFailure.STAGE_API_VERSION), ex.getMessage()); - Assertions.assertTrue(ex.getMessage().contains(AUTH_GATE.getManifestAttribute()), ex.getMessage()); + // Then: absence maps to the documented legacy API 1.0, while explicitly incompatible versions are + // still covered by UT-HANDLER-PM-021. + Assertions.assertTrue(pluginManager.hasFactory("external-dir-test")); + Assertions.assertEquals("", pluginManager.apiVersionRejectionHint()); } @Test diff --git a/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/ApiVersionGate.java b/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/ApiVersionGate.java index 63040c303ac..e0949edfa99 100644 --- a/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/ApiVersionGate.java +++ b/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/ApiVersionGate.java @@ -64,7 +64,9 @@ import java.util.jar.JarFile; * *

Major must match exactly; minor and patch are ignored in both directions. That is only sound because * "major" is defined as any change to the SPI surface — including additions — so a compatible minor - * can never change the set of API elements a plugin can see. + * can never change the set of API elements a plugin can see. Families that distributed plugins before this + * manifest contract existed may explicitly map an absent declaration to their documented legacy version; + * strict rejection remains the default, and explicit declarations always follow the same major rule. */ public final class ApiVersionGate { @@ -75,14 +77,19 @@ public final class ApiVersionGate { private final String manifestAttribute; private final String expectedVersion; private final int expectedMajor; + private final String legacyUnversionedVersion; + private final int legacyUnversionedMajor; private final ClassLoader kernelClassLoader; private ApiVersionGate(String familyLabel, String manifestAttribute, String expectedVersion, - int expectedMajor, ClassLoader kernelClassLoader) { + int expectedMajor, String legacyUnversionedVersion, int legacyUnversionedMajor, + ClassLoader kernelClassLoader) { this.familyLabel = familyLabel; this.manifestAttribute = manifestAttribute; this.expectedVersion = expectedVersion; this.expectedMajor = expectedMajor; + this.legacyUnversionedVersion = legacyUnversionedVersion; + this.legacyUnversionedMajor = legacyUnversionedMajor; this.kernelClassLoader = kernelClassLoader; } @@ -100,6 +107,29 @@ public final class ApiVersionGate { * degrading into a check that admits everything. */ public static ApiVersionGate forFamily(String family, Class spiAnchor) { + return forFamily(family, spiAnchor, null); + } + + /** + * Builds a gate that maps a genuinely absent manifest declaration to a documented legacy API version. + * + *

This is only for a plugin family that shipped external plugins before the manifest contract existed. + * It preserves upgrade compatibility for those artifacts without weakening validation for malformed + * declarations or explicitly incompatible majors. If the FE later moves to another major, unversioned + * plugins are rejected automatically because their legacy major no longer matches. + * + * @param family lower-case single-token family name + * @param spiAnchor a type from the family's SPI artifact + * @param legacyUnversionedVersion API version implemented by artifacts that predate the manifest contract + */ + public static ApiVersionGate forFamilyAllowingUnversioned( + String family, Class spiAnchor, String legacyUnversionedVersion) { + Objects.requireNonNull(legacyUnversionedVersion, "legacyUnversionedVersion"); + return forFamily(family, spiAnchor, legacyUnversionedVersion); + } + + private static ApiVersionGate forFamily( + String family, Class spiAnchor, String legacyUnversionedVersion) { Objects.requireNonNull(family, "family"); Objects.requireNonNull(spiAnchor, "spiAnchor"); String resource = versionResourceOf(family); @@ -110,8 +140,19 @@ public final class ApiVersionGate { + " (from " + spiAnchor.getName() + "); expected major[.minor[.patch]]. This is a build" + " defect: the maven property feeding this resource is missing or was not filtered."); } + int legacyMajor = -1; + if (legacyUnversionedVersion != null) { + OptionalInt parsedLegacyMajor = parseMajor(legacyUnversionedVersion); + if (!parsedLegacyMajor.isPresent()) { + throw new IllegalArgumentException("Malformed legacy unversioned API version='" + + legacyUnversionedVersion + "' for " + family + "; expected major[.minor[.patch]]"); + } + legacyUnversionedVersion = legacyUnversionedVersion.trim(); + legacyMajor = parsedLegacyMajor.getAsInt(); + } return new ApiVersionGate(family.toUpperCase(Locale.ROOT), manifestAttributeOf(family), - declared.trim(), major.getAsInt(), spiAnchor.getClassLoader()); + declared.trim(), major.getAsInt(), legacyUnversionedVersion, legacyMajor, + spiAnchor.getClassLoader()); } /** Kernel-side resource path for a family, by convention. */ @@ -155,18 +196,28 @@ public final class ApiVersionGate { /** * Judges what a plugin jar declared. * - *

Fail-closed: a jar that declares nothing is rejected, so that a plugin built without any awareness of - * this contract cannot slip through. A third party can still get in by declaring a version it was not - * built against, but that is an active false claim rather than an omission. + *

Strict gates reject a jar that declares nothing. A family that explicitly configures a legacy + * unversioned version may accept an absent declaration when that legacy major matches the FE. Blank, + * malformed, and explicitly incompatible declarations always remain fail-closed. * * @param declaredVersion the value of {@link #getManifestAttribute()} in the plugin jar, or null when absent * @return null when the plugin may load, otherwise a diagnostic naming the declared and expected values */ public String rejectionReason(String declaredVersion) { - if (declaredVersion == null || declaredVersion.trim().isEmpty()) { - return "no " + manifestAttribute + " in the plugin jar MANIFEST; this FE serves " + familyLabel - + " plugin API " + expectedVersion + ". Declare the attribute and rebuild the plugin" - + " against this Doris release."; + if (declaredVersion == null) { + if (legacyUnversionedVersion == null) { + return missingDeclarationReason(); + } + if (legacyUnversionedMajor != expectedMajor) { + return "unversioned legacy " + familyLabel + " plugin API " + legacyUnversionedVersion + + " is incompatible with this FE, which serves plugin API " + expectedVersion + + ". Rebuild the plugin against this Doris release."; + } + return null; + } + if (declaredVersion.trim().isEmpty()) { + return "empty " + manifestAttribute + " in the plugin jar MANIFEST; this FE serves " + familyLabel + + " plugin API " + expectedVersion + "."; } OptionalInt major = parseMajor(declaredVersion); if (!major.isPresent()) { @@ -183,21 +234,40 @@ public final class ApiVersionGate { return null; } + /** + * Returns an operator-facing warning when an accepted plugin relied on the legacy unversioned mapping. + */ + public String acceptanceWarning(String declaredVersion) { + if (declaredVersion == null && legacyUnversionedVersion != null + && legacyUnversionedMajor == expectedMajor) { + return "plugin jar has no " + manifestAttribute + "; treating it as legacy " + familyLabel + + " plugin API " + legacyUnversionedVersion + ". Rebuild the plugin with an explicit" + + " API version declaration."; + } + return null; + } + /** Applies the same manifest-major contract to a classpath-discovered provider. */ public String rejectionReasonForClass(Class pluginClass) { Path definingJar = ManifestVersions.jarOf(pluginClass); if (definingJar == null) { // Maven/IDE runs expose kernel-built providers as class directories, but an independently // loaded exploded provider has no immutable declaration and must remain fail-closed. - return pluginClass.getClassLoader() == kernelClassLoader ? null : rejectionReason((String) null); + return pluginClass.getClassLoader() == kernelClassLoader ? null : missingDeclarationReason(); } try (JarFile jar = new JarFile(definingJar.toFile())) { return rejectionReason(ManifestVersions.mainAttribute(jar, manifestAttribute)); } catch (IOException e) { - return rejectionReason((String) null); + return missingDeclarationReason(); } } + private String missingDeclarationReason() { + return "no " + manifestAttribute + " in the plugin jar MANIFEST; this FE serves " + familyLabel + + " plugin API " + expectedVersion + ". Declare the attribute and rebuild the plugin" + + " against this Doris release."; + } + /** * Major segment of {@code major[.minor[.patch]]}, or empty when the value is not that shape. * diff --git a/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/DirectoryPluginRuntimeManager.java b/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/DirectoryPluginRuntimeManager.java index 556242738c5..e9c6d0b71c5 100644 --- a/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/DirectoryPluginRuntimeManager.java +++ b/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/DirectoryPluginRuntimeManager.java @@ -19,6 +19,9 @@ package org.apache.doris.extension.loader; import org.apache.doris.extension.spi.PluginFactory; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + import java.io.Closeable; import java.io.IOException; import java.net.MalformedURLException; @@ -104,6 +107,8 @@ import java.util.stream.Stream; */ public class DirectoryPluginRuntimeManager { + private static final Logger LOG = LogManager.getLogger(DirectoryPluginRuntimeManager.class); + private final ConcurrentMap> handlesByName = new ConcurrentHashMap<>(); private final Object lifecycleLock = new Object(); @@ -278,8 +283,18 @@ public class DirectoryPluginRuntimeManager { // Version check before construction, and before asSubclass(): a plugin built against a // different SPI major is precisely the one whose types may no longer line up, so it must be // answered with a diagnosis naming both versions rather than with a cast or linkage error. - String declaredApiVersion = readManifestMainAttribute( - discoveredClass, allJars, apiVersionGate.getManifestAttribute()); + String declaredApiVersion; + try { + declaredApiVersion = readManifestMainAttribute( + discoveredClass, allJars, apiVersionGate.getManifestAttribute()); + } catch (IOException e) { + throw new PluginLoadException( + normalizedDir, + LoadFailure.STAGE_API_VERSION, + "Failed to read " + apiVersionGate.getManifestAttribute() + " for factory '" + + factoryClassName + "' in " + normalizedDir + ": " + e.getMessage(), + e); + } String rejection = apiVersionGate.rejectionReason(declaredApiVersion); if (rejection != null) { throw new PluginLoadException( @@ -288,6 +303,10 @@ public class DirectoryPluginRuntimeManager { "Rejected plugin in " + normalizedDir + ": " + rejection, null); } + String acceptanceWarning = apiVersionGate.acceptanceWarning(declaredApiVersion); + if (acceptanceWarning != null) { + LOG.warn("Loading legacy unversioned plugin from {}: {}", normalizedDir, acceptanceWarning); + } try { @SuppressWarnings("unchecked") @@ -366,17 +385,16 @@ public class DirectoryPluginRuntimeManager { * source when available (covers layouts where the service descriptor sits in a root jar but the * implementation lives in lib/), otherwise the first candidate jar containing the class entry. * - *

An unreadable jar degrades to null rather than throwing. For the display-only version that leaves - * the inventory row's version empty; for the plugin API version it reads as "declared nothing", which - * {@link ApiVersionGate} rejects — the fail-closed direction in both cases. + *

Null means a readable defining jar genuinely omitted the requested datum. An unreadable or + * unresolved defining jar throws: the API gate must not mistake failed verification for an absent + * legacy declaration. Once the code source identifies a jar, another candidate cannot replace it. */ - private String readFromDefiningJar(Class factoryClass, List candidateJars, ManifestReader reader) { + private String readFromDefiningJar(Class factoryClass, List candidateJars, ManifestReader reader) + throws IOException { Path definingJar = ManifestVersions.jarOf(factoryClass); if (definingJar != null) { try (JarFile jarFile = new JarFile(definingJar.toFile())) { return reader.read(jarFile); - } catch (IOException ignored) { - // Fall through to scanning the candidate jars. } } String classEntry = factoryClass.getName().replace('.', '/') + ".class"; @@ -386,23 +404,26 @@ public class DirectoryPluginRuntimeManager { continue; } return reader.read(jarFile); - } catch (IOException ignored) { - // Fall through to the next candidate jar. } } - return null; + throw new IOException("Cannot locate defining jar for factory '" + factoryClass.getName() + "'"); } /** Implementation-Version, honoring the class's package section. Display-only metadata. */ private String readImplementationVersion(Class factoryClass, List candidateJars) { String packagePath = ManifestVersions.packagePathOf(factoryClass); - return readFromDefiningJar(factoryClass, candidateJars, - jarFile -> ManifestVersions.fromManifest(jarFile, packagePath)); + try { + return readFromDefiningJar(factoryClass, candidateJars, + jarFile -> ManifestVersions.fromManifest(jarFile, packagePath)); + } catch (IOException ignored) { + // Display-only metadata may be unavailable without making a verified plugin unloadable. + return null; + } } /** The plugin API version a jar declares, as a MANIFEST main attribute. Null when absent. */ private String readManifestMainAttribute(Class factoryClass, List candidateJars, - String attributeName) { + String attributeName) throws IOException { return readFromDefiningJar(factoryClass, candidateJars, jarFile -> ManifestVersions.mainAttribute(jarFile, attributeName)); } diff --git a/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/ManifestVersions.java b/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/ManifestVersions.java index 82c28c3710d..18c986c8a44 100644 --- a/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/ManifestVersions.java +++ b/fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/ManifestVersions.java @@ -91,7 +91,11 @@ final class ManifestVersions { return lastDot < 0 ? null : className.substring(0, lastDot).replace('.', '/') + "/"; } - /** Jar file that defined the class per its protection domain, or null. */ + /** + * Defining jar location from the protection domain, or null for class directories or unavailable + * provenance. Keep the location even if the jar was removed after class loading, so callers report + * the read failure instead of falling back to an unrelated candidate with the same class entry. + */ static Path jarOf(Class clazz) { try { CodeSource codeSource = clazz.getProtectionDomain().getCodeSource(); @@ -99,7 +103,7 @@ final class ManifestVersions { return null; } Path path = Paths.get(codeSource.getLocation().toURI()); - return Files.isRegularFile(path) ? path : null; + return Files.isDirectory(path) ? null : path; } catch (URISyntaxException | RuntimeException e) { return null; } diff --git a/fe/fe-extension-loader/src/test/java/org/apache/doris/extension/loader/ApiVersionGateTest.java b/fe/fe-extension-loader/src/test/java/org/apache/doris/extension/loader/ApiVersionGateTest.java index fa4ccb24151..06ed183f926 100644 --- a/fe/fe-extension-loader/src/test/java/org/apache/doris/extension/loader/ApiVersionGateTest.java +++ b/fe/fe-extension-loader/src/test/java/org/apache/doris/extension/loader/ApiVersionGateTest.java @@ -21,8 +21,9 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; /** - * The decision rule itself: major must match, minor and patch never matter, and anything a plugin did not - * clearly declare is refused. + * The decision rule itself: major must match and minor and patch never matter. Strict gates refuse missing + * declarations; an explicitly configured legacy gate accepts only a genuinely absent declaration for the + * documented legacy major. * *

Each case here encodes a promise made to plugin authors, not just current behavior. The two-way minor * cases are the load-bearing ones: they are only sound because "major" is defined to cover additions @@ -37,6 +38,8 @@ class ApiVersionGateTest { private static final ApiVersionGate GATE = ApiVersionGate.forFamily("test", ApiVersionGateTest.class); + private static final ApiVersionGate LEGACY_GATE = + ApiVersionGate.forFamilyAllowingUnversioned("test", ApiVersionGateTest.class, "7.0"); @Test void testKernelVersionComesFromTheFilteredResource() { @@ -116,6 +119,36 @@ class ApiVersionGateTest { Assertions.assertNotNull(GATE.rejectionReason(" ")); } + @Test + void testDocumentedLegacyUnversionedPluginIsAcceptedWithWarning() { + Assertions.assertNull(LEGACY_GATE.rejectionReason(null)); + String warning = LEGACY_GATE.acceptanceWarning(null); + Assertions.assertNotNull(warning); + Assertions.assertTrue(warning.contains("legacy TEST plugin API 7.0"), warning); + + Assertions.assertNotNull(LEGACY_GATE.rejectionReason(""), + "an explicit empty declaration is malformed, not a legacy artifact"); + Assertions.assertNull(LEGACY_GATE.acceptanceWarning("7.0"), + "a versioned plugin must not be mislabeled as legacy"); + } + + @Test + void testLegacyUnversionedPluginIsRejectedAfterKernelMajorUpgrade() { + ApiVersionGate oldLegacy = + ApiVersionGate.forFamilyAllowingUnversioned("test", ApiVersionGateTest.class, "6.9"); + String reason = oldLegacy.rejectionReason(null); + Assertions.assertNotNull(reason); + Assertions.assertTrue(reason.contains("6.9") && reason.contains("7.2"), reason); + Assertions.assertNull(oldLegacy.acceptanceWarning(null)); + } + + @Test + void testMalformedLegacyVersionConfigurationFailsLoudly() { + Assertions.assertThrows(IllegalArgumentException.class, + () -> ApiVersionGate.forFamilyAllowingUnversioned( + "test", ApiVersionGateTest.class, "legacy-v7")); + } + @Test void testMalformedDeclarationIsRejectedRatherThanTruncated() { // "7.x" must not be read as major 7. Silently salvaging a major out of a typo would admit a plugin diff --git a/fe/fe-extension-loader/src/test/java/org/apache/doris/extension/loader/DirectoryPluginRuntimeManagerApiVersionTest.java b/fe/fe-extension-loader/src/test/java/org/apache/doris/extension/loader/DirectoryPluginRuntimeManagerApiVersionTest.java index 3ff1f1964d6..73b3fad9ac2 100644 --- a/fe/fe-extension-loader/src/test/java/org/apache/doris/extension/loader/DirectoryPluginRuntimeManagerApiVersionTest.java +++ b/fe/fe-extension-loader/src/test/java/org/apache/doris/extension/loader/DirectoryPluginRuntimeManagerApiVersionTest.java @@ -25,9 +25,17 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.nio.file.Path; +import java.security.CodeSource; +import java.security.ProtectionDomain; +import java.security.cert.Certificate; import java.util.Arrays; import java.util.Collections; +import java.util.jar.JarEntry; +import java.util.jar.JarOutputStream; /** * The gate as the directory loader applies it: what a real plugin jar declares in its MANIFEST decides @@ -45,6 +53,8 @@ import java.util.Collections; class DirectoryPluginRuntimeManagerApiVersionTest { private static final ApiVersionGate GATE = DirectoryPluginRuntimeManagerMetadataTest.TEST_GATE; + private static final ApiVersionGate LEGACY_GATE = ApiVersionGate.forFamilyAllowingUnversioned( + "test", DirectoryPluginRuntimeManagerApiVersionTest.class, "7.0"); @TempDir Path tempDir; @@ -80,6 +90,70 @@ class DirectoryPluginRuntimeManagerApiVersionTest { assertRefused(report, "seven-point-two", "7.2"); } + @Test + void testReadableUnversionedLegacyPluginLoads() throws IOException { + Path root = tempDir.resolve("legacy"); + DirectoryPluginRuntimeManagerMetadataTest.createPluginJar( + root.resolve("metadata-test").resolve("metadata-test.jar"), + MetadataTestPluginFactory.class, "3.1.4", null); + DirectoryPluginRuntimeManager manager = new DirectoryPluginRuntimeManager<>(); + LoadReport report = manager.loadAll(Collections.singletonList(root), + Thread.currentThread().getContextClassLoader(), PluginFactory.class, null, LEGACY_GATE); + + Assertions.assertEquals(1, report.getSuccesses().size(), () -> describe(report)); + Assertions.assertEquals("3.1.4", report.getSuccesses().get(0).getVersion()); + manager.discard("metadata-test"); + } + + @Test + void testLegacyGateStillRejectsExplicitInvalidDeclarations() throws IOException { + for (String version : Arrays.asList("", "seven-point-two", "6.0")) { + Path root = tempDir.resolve("invalid-legacy-" + version); + DirectoryPluginRuntimeManagerMetadataTest.createPluginJar( + root.resolve("metadata-test").resolve("metadata-test.jar"), + MetadataTestPluginFactory.class, "3.1.4", version); + LoadReport report = new DirectoryPluginRuntimeManager().loadAll( + Collections.singletonList(root), Thread.currentThread().getContextClassLoader(), + PluginFactory.class, null, LEGACY_GATE); + + assertRefused(report, "Doris-Test-Plugin-Api-Version", "7.2"); + } + } + + @Test + void testUnreadableDefiningJarIsRefusedInsteadOfUsingAnotherCandidate() throws IOException { + Path definingJar = tempDir.resolve("unreadable.jar"); + Files.write(definingJar, "not a jar".getBytes(StandardCharsets.UTF_8)); + + assertRefused(loadWithFactoryCodeSource(definingJar, true), + "Failed to read Doris-Test-Plugin-Api-Version", "metadata-test"); + } + + @Test + void testUnreadableManifestIsRefusedAsApiVersionFailure() throws IOException { + Path definingJar = tempDir.resolve("bad-manifest.jar"); + try (JarOutputStream jar = new JarOutputStream(Files.newOutputStream(definingJar))) { + jar.putNextEntry(new JarEntry("META-INF/MANIFEST.MF")); + jar.write("invalid manifest header\n".getBytes(StandardCharsets.UTF_8)); + jar.closeEntry(); + } + + assertRefused(loadWithFactoryCodeSource(definingJar, false), + "Failed to read Doris-Test-Plugin-Api-Version", "invalid header"); + } + + @Test + void testMissingDefiningJarIsRefusedInsteadOfBeingTreatedAsLegacy() throws IOException { + assertRefused(loadWithFactoryCodeSource(tempDir.resolve("missing.jar"), true), + "Failed to read Doris-Test-Plugin-Api-Version", "missing.jar"); + } + + @Test + void testUnresolvedDefiningJarIsRefusedInsteadOfBeingTreatedAsLegacy() throws IOException { + assertRefused(loadWithFactoryCodeSource(null, false), + "Failed to read Doris-Test-Plugin-Api-Version", "Cannot locate defining jar"); + } + @Test void testRefusedPluginLeavesNoTraceAndRepeatsIdentically() throws IOException { Path root = tempDir.resolve("repeat"); @@ -127,6 +201,51 @@ class DirectoryPluginRuntimeManagerApiVersionTest { GATE); } + private LoadReport loadWithFactoryCodeSource(Path definingJar, boolean includeFactory) + throws IOException { + Path root = tempDir.resolve("lookup-failure"); + Path pluginJar = root.resolve("metadata-test").resolve("metadata-test.jar"); + if (includeFactory) { + // This readable candidate must not hide a failure to read the actual defining jar. + DirectoryPluginRuntimeManagerMetadataTest.createPluginJar( + pluginJar, MetadataTestPluginFactory.class, "3.1.4", null); + } else { + Files.createDirectories(pluginJar.getParent()); + try (JarOutputStream jar = new JarOutputStream(Files.newOutputStream(pluginJar))) { + jar.putNextEntry(new JarEntry("META-INF/services/" + PluginFactory.class.getName())); + jar.write((MetadataTestPluginFactory.class.getName() + "\n").getBytes(StandardCharsets.UTF_8)); + jar.closeEntry(); + } + } + // Define the factory before the separate manifest lookup, with controlled provenance. This makes + // missing/unreadable metadata deterministic without racing filesystem mutation against loading. + ClassLoader parent = new FactoryClassLoader(definingJar); + ClassLoadingPolicy policy = new ClassLoadingPolicy( + Collections.singletonList(MetadataTestPluginFactory.class.getName())); + DirectoryPluginRuntimeManager manager = new DirectoryPluginRuntimeManager<>(); + LoadReport report = manager.loadAll(Collections.singletonList(root), + parent, PluginFactory.class, policy, LEGACY_GATE); + + Assertions.assertTrue(manager.list().isEmpty(), "failed verification must not publish a handle"); + Assertions.assertFalse(manager.get("metadata-test").isPresent()); + return report; + } + + private static class FactoryClassLoader extends ClassLoader { + FactoryClassLoader(Path definingJar) throws IOException { + super(Thread.currentThread().getContextClassLoader()); + String name = MetadataTestPluginFactory.class.getName(); + String classEntry = name.replace('.', '/') + ".class"; + try (InputStream in = MetadataTestPluginFactory.class.getResourceAsStream("/" + classEntry)) { + Assertions.assertNotNull(in); + byte[] bytes = in.readAllBytes(); + CodeSource source = definingJar == null ? null + : new CodeSource(definingJar.toUri().toURL(), (Certificate[]) null); + defineClass(name, bytes, 0, bytes.length, new ProtectionDomain(source, null)); + } + } + } + private static void assertRefused(LoadReport report, String... mustMention) { Assertions.assertTrue(report.getSuccesses().isEmpty(), "an incompatible plugin must never be published: " + describe(report));