From a94e4c5192f388873de99246f3ffdfe4b6c7790f Mon Sep 17 00:00:00 2001 From: Jeremiah Jordan Date: Tue, 23 Jun 2026 12:54:32 -0500 Subject: [PATCH 1/6] Verify extension type before initializing reflectively-loaded classes Cassandra resolves pluggable extensions by class name from configuration, schema, and tooling inputs. These names were loaded with an initializing Class.forName(name) and type-checked only afterward, so the named class ran its static initializer before its type was confirmed. After this change such classes will be loaded without initialization, verified against the expected interface or base class, and initialized only through normal use after validation. A shared FBUtilities.classForNameWithoutInitialization helper and typed instanceOrConstruct/construct overloads apply this to the configurable extension points loaded by class name: the guardrail value generator and validator, disk-error handler, Accord agent, TCM extension values, the authentication backends and mutual-TLS validators, seed provider, snitch and its location providers, abstract types, secondary and custom indexes, compaction strategy, compressor, replication strategy (including its metadata serializers), SASI analyzers, key and cache providers, query handler, storage and stream hooks, tracing, the JMX authorization proxy, MBeans, clocks, nodetool Sjk, triggers, the memtable factory contract, the guardrails config provider, the auto-repair token-range splitter, crypto provider, SSL context factory, the sstableloader and stress class options, and diagnostic event classes. ParameterizedClass.newInstance also takes an expected type and loads without initialization. Regression tests confirm that an invalid-type load is rejected without initializing the target class, and that valid implementations still resolve. Hard-coded JDK and internal class probes are left unchanged. patch by Jeremiah Jordan; reviewed by for CASSANDRA-21525 --- .../org/apache/cassandra/auth/AuthConfig.java | 19 ++-- .../auth/MutualTlsAuthenticator.java | 3 +- .../auth/MutualTlsInternodeAuthenticator.java | 3 +- .../cassandra/config/DatabaseDescriptor.java | 15 +-- .../cassandra/config/ParameterizedClass.java | 24 +++- .../org/apache/cassandra/db/StorageHook.java | 4 +- .../guardrails/GuardrailsConfigProvider.java | 2 +- .../db/guardrails/ValueGenerator.java | 7 +- .../db/guardrails/ValueValidator.java | 5 +- .../cassandra/db/marshal/TypeParser.java | 19 +++- .../diag/DiagnosticEventPersistence.java | 5 +- .../index/SecondaryIndexManager.java | 7 +- .../cassandra/index/sasi/conf/IndexMode.java | 49 ++++---- .../locator/AbstractReplicationStrategy.java | 10 +- .../repair/autorepair/AutoRepairConfig.java | 5 +- .../cassandra/schema/CompactionParams.java | 10 +- .../cassandra/schema/CompressionParams.java | 14 +-- .../cassandra/schema/IndexMetadata.java | 4 +- .../cassandra/schema/MemtableParams.java | 8 +- .../cassandra/schema/ReplicationParams.java | 4 +- .../security/AbstractCryptoProvider.java | 2 +- .../cassandra/security/CipherFactory.java | 8 +- .../cassandra/service/CacheService.java | 8 +- .../apache/cassandra/service/ClientState.java | 2 +- .../service/DiskErrorsHandlerService.java | 2 +- .../service/accord/AccordService.java | 4 +- .../cassandra/streaming/StreamHook.java | 2 +- .../tcm/extensions/ExtensionKey.java | 7 +- .../apache/cassandra/tools/nodetool/Sjk.java | 3 +- .../org/apache/cassandra/tracing/Tracing.java | 2 +- .../cassandra/triggers/TriggerExecutor.java | 21 +++- .../org/apache/cassandra/utils/Clock.java | 2 +- .../apache/cassandra/utils/FBUtilities.java | 102 +++++++++++++++-- .../cassandra/utils/JMXServerUtils.java | 2 +- .../apache/cassandra/utils/MBeanWrapper.java | 2 +- .../cassandra/utils/MonotonicClock.java | 5 +- .../apache/cassandra/auth/AuthConfigTest.java | 73 ++++++++++++ .../config/ClassLoadingSearchProbe.java | 37 ++++++ .../config/DatabaseDescriptorTest.java | 22 ++++ .../config/ParameterizedClassTest.java | 48 ++++++++ .../GuardrailsConfigProviderTest.java | 14 +++ .../db/guardrails/ValueGeneratorTest.java | 18 +++ .../db/guardrails/ValueValidatorTest.java | 18 +++ .../cassandra/db/marshal/TypeParserTest.java | 19 ++++ .../diag/ClassLoadingTestDiagnosticEvent.java | 44 +++++++ .../diag/DiagnosticEventPersistenceTest.java | 77 +++++++++++++ .../index/SecondaryIndexManagerTest.java | 20 ++++ .../index/sasi/conf/IndexModeTest.java | 63 ++++++++++- .../autorepair/AutoRepairConfigTest.java | 17 +++ .../schema/CompactionParamsTest.java | 44 +++++++ .../schema/CompressionParamsTest.java | 17 +++ .../cassandra/schema/IndexMetadataTest.java | 31 +++++ .../MemtableFactoryInvalidFieldType.java | 34 ++++++ .../MemtableFactoryInvalidReturnType.java | 39 +++++++ .../cassandra/schema/MemtableParamsTest.java | 55 +++++++++ .../schema/ReplicationParamsTest.java | 87 ++++++++++++++ .../cassandra/security/CipherFactoryTest.java | 29 +++++ .../triggers/TriggerExecutorTest.java | 26 +++++ .../utils/ClassLoadingSearchProbe.java | 33 ++++++ .../utils/ClassLoadingTestNonAssignable.java | 31 +++++ .../utils/ClassLoadingTestSupport.java | 47 ++++++++ .../cassandra/utils/FBUtilitiesTest.java | 107 ++++++++++++++++++ .../apache/cassandra/tools/LoaderOptions.java | 14 ++- .../stress/settings/OptionReplication.java | 7 +- .../stress/settings/SettingsMode.java | 8 +- 65 files changed, 1334 insertions(+), 136 deletions(-) create mode 100644 test/unit/org/apache/cassandra/config/ClassLoadingSearchProbe.java create mode 100644 test/unit/org/apache/cassandra/diag/ClassLoadingTestDiagnosticEvent.java create mode 100644 test/unit/org/apache/cassandra/diag/DiagnosticEventPersistenceTest.java create mode 100644 test/unit/org/apache/cassandra/schema/CompactionParamsTest.java create mode 100644 test/unit/org/apache/cassandra/schema/MemtableFactoryInvalidFieldType.java create mode 100644 test/unit/org/apache/cassandra/schema/MemtableFactoryInvalidReturnType.java create mode 100644 test/unit/org/apache/cassandra/schema/ReplicationParamsTest.java create mode 100644 test/unit/org/apache/cassandra/utils/ClassLoadingSearchProbe.java create mode 100644 test/unit/org/apache/cassandra/utils/ClassLoadingTestNonAssignable.java create mode 100644 test/unit/org/apache/cassandra/utils/ClassLoadingTestSupport.java diff --git a/src/java/org/apache/cassandra/auth/AuthConfig.java b/src/java/org/apache/cassandra/auth/AuthConfig.java index 86e1f626ff81..52182afe3186 100644 --- a/src/java/org/apache/cassandra/auth/AuthConfig.java +++ b/src/java/org/apache/cassandra/auth/AuthConfig.java @@ -63,7 +63,7 @@ public static void applyAuth() /* Authentication, authorization and role management backend, implementing IAuthenticator, I*Authorizer & IRoleManager */ - IAuthenticator authenticator = authInstantiate(conf.authenticator, AllowAllAuthenticator.class); + IAuthenticator authenticator = authInstantiate(conf.authenticator, IAuthenticator.class, AllowAllAuthenticator.class); // the configuration options regarding credentials caching are only guaranteed to // work with PasswordAuthenticator, so log a message if some other authenticator @@ -82,7 +82,7 @@ public static void applyAuth() // authorizer - IAuthorizer authorizer = authInstantiate(conf.authorizer, AllowAllAuthorizer.class); + IAuthorizer authorizer = authInstantiate(conf.authorizer, IAuthorizer.class, AllowAllAuthorizer.class); if (!authenticator.requireAuthentication() && authorizer.requireAuthorization()) { @@ -94,7 +94,7 @@ public static void applyAuth() // role manager - IRoleManager roleManager = authInstantiate(conf.role_manager, CassandraRoleManager.class); + IRoleManager roleManager = authInstantiate(conf.role_manager, IRoleManager.class, CassandraRoleManager.class); if (authenticator instanceof PasswordAuthenticator && !(roleManager instanceof CassandraRoleManager)) throw new ConfigurationException(authenticator.getClass().getName() + " requires " + CassandraRoleManager.class.getName(), false); @@ -104,12 +104,15 @@ public static void applyAuth() // authenticator IInternodeAuthenticator internodeAuthenticator = authInstantiate(conf.internode_authenticator, + IInternodeAuthenticator.class, AllowAllInternodeAuthenticator.class); DatabaseDescriptor.setInternodeAuthenticator(internodeAuthenticator); // network authorizer - INetworkAuthorizer networkAuthorizer = authInstantiate(conf.network_authorizer, AllowAllNetworkAuthorizer.class); + INetworkAuthorizer networkAuthorizer = authInstantiate(conf.network_authorizer, + INetworkAuthorizer.class, + AllowAllNetworkAuthorizer.class); if (networkAuthorizer.requireAuthorization() && !authenticator.requireAuthentication()) { @@ -120,7 +123,9 @@ public static void applyAuth() // cidr authorizer - ICIDRAuthorizer cidrAuthorizer = authInstantiate(conf.cidr_authorizer, AllowAllCIDRAuthorizer.class); + ICIDRAuthorizer cidrAuthorizer = authInstantiate(conf.cidr_authorizer, + ICIDRAuthorizer.class, + AllowAllCIDRAuthorizer.class); if (cidrAuthorizer.requireAuthorization() && !authenticator.requireAuthentication()) { @@ -140,11 +145,11 @@ public static void applyAuth() DatabaseDescriptor.getInternodeAuthenticator().validateConfiguration(); } - private static T authInstantiate(ParameterizedClass authCls, Class defaultCls) { + private static T authInstantiate(ParameterizedClass authCls, Class expectedType, Class defaultCls) { if (authCls != null && authCls.class_name != null) { String authPackage = AuthConfig.class.getPackage().getName(); - return ParameterizedClass.newInstance(authCls, List.of("", authPackage)); + return ParameterizedClass.newInstance(authCls, List.of("", authPackage), expectedType); } // for now, this has to stay and can not be replaced by ParameterizedClass.newInstance as above diff --git a/src/java/org/apache/cassandra/auth/MutualTlsAuthenticator.java b/src/java/org/apache/cassandra/auth/MutualTlsAuthenticator.java index 76aacc5b5a2b..2d897c2eee11 100644 --- a/src/java/org/apache/cassandra/auth/MutualTlsAuthenticator.java +++ b/src/java/org/apache/cassandra/auth/MutualTlsAuthenticator.java @@ -101,7 +101,8 @@ public MutualTlsAuthenticator(Map parameters) throw new ConfigurationException(message); } certificateValidator = ParameterizedClass.newInstance(new ParameterizedClass(certificateValidatorClassName), - Arrays.asList("", AuthConfig.class.getPackage().getName())); + Arrays.asList("", AuthConfig.class.getPackage().getName()), + MutualTlsCertificateValidator.class); Config config = DatabaseDescriptor.getRawConfig(); certificateValidityPeriodValidator = new MutualTlsCertificateValidityPeriodValidator(config.client_encryption_options.max_certificate_validity_period); diff --git a/src/java/org/apache/cassandra/auth/MutualTlsInternodeAuthenticator.java b/src/java/org/apache/cassandra/auth/MutualTlsInternodeAuthenticator.java index 1cac64dba0a4..3fc9553fd9fd 100644 --- a/src/java/org/apache/cassandra/auth/MutualTlsInternodeAuthenticator.java +++ b/src/java/org/apache/cassandra/auth/MutualTlsInternodeAuthenticator.java @@ -104,7 +104,8 @@ public MutualTlsInternodeAuthenticator(Map parameters) } certificateValidator = ParameterizedClass.newInstance(new ParameterizedClass(certificateValidatorClassName), - Arrays.asList("", AuthConfig.class.getPackage().getName())); + Arrays.asList("", AuthConfig.class.getPackage().getName()), + MutualTlsCertificateValidator.class); Config config = DatabaseDescriptor.getRawConfig(); if (parameters.containsKey(TRUSTED_PEER_IDENTITIES)) diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java index c0fb8bc200e0..106ab02e247e 100644 --- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java +++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java @@ -515,7 +515,7 @@ public static Config loadConfig() throws ConfigurationException String loaderClass = CONFIG_LOADER.getString(); ConfigurationLoader loader = loaderClass == null ? new YamlConfigurationLoader() - : FBUtilities.construct(loaderClass, "configuration loading"); + : FBUtilities.construct(loaderClass, "configuration loading", ConfigurationLoader.class); Config config = loader.loadConfig(); if (!hasLoggedConfig) @@ -1655,7 +1655,8 @@ public static void applySeedProvider() } try { - Class seedProviderClass = Class.forName(conf.seed_provider.class_name); + Class seedProviderClass = + FBUtilities.classForNameWithoutInitialization(conf.seed_provider.class_name, "seed provider", SeedProvider.class); seedProvider = (SeedProvider) seedProviderClass.getConstructor(Map.class).newInstance(conf.seed_provider.parameters); } // there are about 5 checked exceptions that could be thrown here. @@ -2032,7 +2033,7 @@ public static IEndpointSnitch createEndpointSnitch(String snitchClassName) throw { if (!snitchClassName.contains(".")) snitchClassName = "org.apache.cassandra.locator." + snitchClassName; - IEndpointSnitch snitch = FBUtilities.construct(snitchClassName, "snitch"); + IEndpointSnitch snitch = FBUtilities.construct(snitchClassName, "snitch", IEndpointSnitch.class); return snitch; } @@ -2040,7 +2041,7 @@ public static NodeProximity createProximityImpl(String className) throws Configu { if (!className.contains(".")) className = "org.apache.cassandra.locator." + className; - NodeProximity sorter = FBUtilities.construct(className, "node proximity measurement"); + NodeProximity sorter = FBUtilities.construct(className, "node proximity measurement", NodeProximity.class); return sorter; } @@ -2048,7 +2049,7 @@ public static InitialLocationProvider createInitialLocationProvider(String class { if (!className.contains(".")) className = "org.apache.cassandra.locator." + className; - InitialLocationProvider provider = FBUtilities.construct(className, "initial location provider"); + InitialLocationProvider provider = FBUtilities.construct(className, "initial location provider", InitialLocationProvider.class); return provider; } @@ -2056,7 +2057,7 @@ public static NodeAddressConfig createAddressConfig(String className) throws Con { if (!className.contains(".")) className = "org.apache.cassandra.locator." + className; - NodeAddressConfig config = FBUtilities.construct(className, "node address config"); + NodeAddressConfig config = FBUtilities.construct(className, "node address config", NodeAddressConfig.class); return config; } @@ -2064,7 +2065,7 @@ private static IFailureDetector createFailureDetector(String detectorClassName) { if (!detectorClassName.contains(".")) detectorClassName = "org.apache.cassandra.gms." + detectorClassName; - IFailureDetector detector = FBUtilities.construct(detectorClassName, "failure detector"); + IFailureDetector detector = FBUtilities.construct(detectorClassName, "failure detector", IFailureDetector.class); return detector; } diff --git a/src/java/org/apache/cassandra/config/ParameterizedClass.java b/src/java/org/apache/cassandra/config/ParameterizedClass.java index d772629f194c..803fcb1fac18 100644 --- a/src/java/org/apache/cassandra/config/ParameterizedClass.java +++ b/src/java/org/apache/cassandra/config/ParameterizedClass.java @@ -64,7 +64,17 @@ public ParameterizedClass(Map p) p.containsKey(PARAMETERS) ? (Map)((List)p.get(PARAMETERS)).get(0) : null); } + /** + * Prefer {@link #newInstance(ParameterizedClass, List, Class)}: passing an {@code expectedType} verifies the + * resolved class is the intended extension type before it is instantiated. This overload performs no type + * check (it still loads without initialization, so a wrong class name cannot run its static initializer here). + */ static public K newInstance(ParameterizedClass parameterizedClass, List searchPackages) + { + return newInstance(parameterizedClass, searchPackages, null); + } + + static public K newInstance(ParameterizedClass parameterizedClass, List searchPackages, Class expectedType) { Class providerClass = null; if (searchPackages == null || searchPackages.isEmpty()) @@ -76,9 +86,12 @@ static public K newInstance(ParameterizedClass parameterizedClass, List K newInstance(ParameterizedClass parameterizedClass, List mapConstructor = filterConstructor(providerClass, c -> c.getParameterTypes().length == 1 && c.getParameterTypes()[0].equals(Map.class)); diff --git a/src/java/org/apache/cassandra/db/StorageHook.java b/src/java/org/apache/cassandra/db/StorageHook.java index f5fdec6a563e..56938b441f3c 100644 --- a/src/java/org/apache/cassandra/db/StorageHook.java +++ b/src/java/org/apache/cassandra/db/StorageHook.java @@ -55,7 +55,7 @@ static StorageHook createHook() String className = STORAGE_HOOK.getString(); if (className != null) { - return FBUtilities.construct(className, StorageHook.class.getSimpleName()); + return FBUtilities.construct(className, StorageHook.class.getSimpleName(), StorageHook.class); } return new StorageHook() @@ -89,4 +89,4 @@ public UnfilteredRowIterator makeRowIterator(ColumnFamilyStore cfs, } }; } -} \ No newline at end of file +} diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfigProvider.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfigProvider.java index 990a07a1ceda..ebae5d53944c 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfigProvider.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfigProvider.java @@ -65,7 +65,7 @@ public interface GuardrailsConfigProvider */ static GuardrailsConfigProvider build(String customImpl) { - return FBUtilities.construct(customImpl, "custom guardrails config provider"); + return FBUtilities.construct(customImpl, "custom guardrails config provider", GuardrailsConfigProvider.class); } /** diff --git a/src/java/org/apache/cassandra/db/guardrails/ValueGenerator.java b/src/java/org/apache/cassandra/db/guardrails/ValueGenerator.java index c01e18f679c9..0a1173403471 100644 --- a/src/java/org/apache/cassandra/db/guardrails/ValueGenerator.java +++ b/src/java/org/apache/cassandra/db/guardrails/ValueGenerator.java @@ -143,8 +143,11 @@ public static ValueGenerator getGenerator(String name, @Nonnull C try { + Class rawGeneratorClass = + FBUtilities.classForNameWithoutInitialization(className, "generator", ValueGenerator.class); + @SuppressWarnings("unchecked") Class> generatorClass = - FBUtilities.classForName(className, "generator"); + (Class>) rawGeneratorClass; @SuppressWarnings("unchecked") ValueGenerator generator = generatorClass.getConstructor(CustomGuardrailConfig.class) @@ -165,4 +168,4 @@ public static ValueGenerator getGenerator(String name, @Nonnull C className, message), ex); } } -} \ No newline at end of file +} diff --git a/src/java/org/apache/cassandra/db/guardrails/ValueValidator.java b/src/java/org/apache/cassandra/db/guardrails/ValueValidator.java index 3aa9533ff550..8e93e129e0cf 100644 --- a/src/java/org/apache/cassandra/db/guardrails/ValueValidator.java +++ b/src/java/org/apache/cassandra/db/guardrails/ValueValidator.java @@ -127,8 +127,11 @@ public static ValueValidator getValidator(String name, @Nonnull C try { + Class rawValidatorClass = + FBUtilities.classForNameWithoutInitialization(className, "validator", ValueValidator.class); + @SuppressWarnings("unchecked") Class> validatorClass = - FBUtilities.classForName(className, "validator"); + (Class>) rawValidatorClass; @SuppressWarnings("unchecked") ValueValidator validator = validatorClass.getConstructor(CustomGuardrailConfig.class) diff --git a/src/java/org/apache/cassandra/db/marshal/TypeParser.java b/src/java/org/apache/cassandra/db/marshal/TypeParser.java index b2dc0202bcd5..8e2da14142ca 100644 --- a/src/java/org/apache/cassandra/db/marshal/TypeParser.java +++ b/src/java/org/apache/cassandra/db/marshal/TypeParser.java @@ -449,8 +449,7 @@ public Pair, List>> getU private static AbstractType getAbstractType(String compareWith) throws ConfigurationException { - String className = compareWith.contains(".") ? compareWith : "org.apache.cassandra.db.marshal." + compareWith; - Class> typeClass = FBUtilities.>classForName(className, "abstract-type"); + Class> typeClass = getAbstractTypeClass(compareWith); try { Field field = typeClass.getDeclaredField("instance"); @@ -465,8 +464,7 @@ private static AbstractType getAbstractType(String compareWith) throws Config private static AbstractType getAbstractType(String compareWith, TypeParser parser) throws SyntaxException, ConfigurationException { - String className = compareWith.contains(".") ? compareWith : "org.apache.cassandra.db.marshal." + compareWith; - Class> typeClass = FBUtilities.>classForName(className, "abstract-type"); + Class> typeClass = getAbstractTypeClass(compareWith); if (PseudoUtf8Type.class.isAssignableFrom(typeClass)) { if (StorageService.instance.isDaemonSetupCompleted()) @@ -491,6 +489,19 @@ private static AbstractType getAbstractType(String compareWith, TypeParser pa } } + private static Class> getAbstractTypeClass(String compareWith) throws ConfigurationException + { + String className = compareWith.contains(".") ? compareWith : "org.apache.cassandra.db.marshal." + compareWith; + // Defer class initialization until after confirming this is an AbstractType. The static instance field + // access or getInstance(TypeParser) invocation below performs the initialization for valid types. + @SuppressWarnings("unchecked") + Class> typeClass = + (Class>)(Class) FBUtilities.classForNameWithoutInitialization(className, + "abstract-type", + AbstractType.class); + return typeClass; + } + private static AbstractType getRawAbstractType(Class> typeClass) throws ConfigurationException { try diff --git a/src/java/org/apache/cassandra/diag/DiagnosticEventPersistence.java b/src/java/org/apache/cassandra/diag/DiagnosticEventPersistence.java index 81820382f327..b1e8df8783cf 100644 --- a/src/java/org/apache/cassandra/diag/DiagnosticEventPersistence.java +++ b/src/java/org/apache/cassandra/diag/DiagnosticEventPersistence.java @@ -128,6 +128,7 @@ private void onEvent(DiagnosticEvent event) LastEventIdBroadcaster.instance().setLastEventId(event.getClass().getName(), store.getLastEventId()); } + @SuppressWarnings("unchecked") private Class getEventClass(String eventClazz) throws ClassNotFoundException, InvalidClassException { // get class by eventClazz argument name @@ -135,12 +136,12 @@ private Class getEventClass(String eventClazz) throws ClassNotF if (!eventClazz.startsWith("org.apache.cassandra.")) throw new RuntimeException("Not a Cassandra event class: " + eventClazz); - Class clazz = (Class) Class.forName(eventClazz); + Class clazz = Class.forName(eventClazz, false, DiagnosticEventPersistence.class.getClassLoader()); if (!(DiagnosticEvent.class.isAssignableFrom(clazz))) throw new InvalidClassException("Event class must be of type DiagnosticEvent"); - return clazz; + return (Class) clazz.asSubclass(DiagnosticEvent.class); } private DiagnosticEventStore getStore(Class cls) diff --git a/src/java/org/apache/cassandra/index/SecondaryIndexManager.java b/src/java/org/apache/cassandra/index/SecondaryIndexManager.java index 2846a8ede505..aeb4556ff452 100644 --- a/src/java/org/apache/cassandra/index/SecondaryIndexManager.java +++ b/src/java/org/apache/cassandra/index/SecondaryIndexManager.java @@ -921,6 +921,11 @@ public Index getIndexByName(String indexName) return indexes.get(indexName); } + static Class loadIndexClass(String className) + { + return FBUtilities.classForNameWithoutInitialization(className, "Index", Index.class); + } + private Index createInstance(IndexMetadata indexDef) { Index newIndex; @@ -933,7 +938,7 @@ private Index createInstance(IndexMetadata indexDef) try { - Class indexClass = FBUtilities.classForName(className, "Index"); + Class indexClass = loadIndexClass(className); Constructor ctor = indexClass.getConstructor(ColumnFamilyStore.class, IndexMetadata.class); newIndex = ctor.newInstance(baseCfs, indexDef); } diff --git a/src/java/org/apache/cassandra/index/sasi/conf/IndexMode.java b/src/java/org/apache/cassandra/index/sasi/conf/IndexMode.java index c827b2b8203f..339a2cae5d89 100644 --- a/src/java/org/apache/cassandra/index/sasi/conf/IndexMode.java +++ b/src/java/org/apache/cassandra/index/sasi/conf/IndexMode.java @@ -37,6 +37,7 @@ import org.apache.cassandra.index.sasi.plan.Expression.Op; import org.apache.cassandra.schema.ColumnMetadata; import org.apache.cassandra.schema.IndexMetadata; +import org.apache.cassandra.utils.FBUtilities; public class IndexMode { @@ -60,10 +61,10 @@ public class IndexMode public final Mode mode; public final boolean isAnalyzed, isLiteral; - public final Class analyzerClass; + public final Class analyzerClass; public final long maxCompactionFlushMemoryInBytes; - private IndexMode(Mode mode, boolean isLiteral, boolean isAnalyzed, Class analyzerClass, long maxMemBytes) + private IndexMode(Mode mode, boolean isLiteral, boolean isAnalyzed, Class analyzerClass, long maxMemBytes) { this.mode = mode; this.isLiteral = isLiteral; @@ -81,7 +82,7 @@ public AbstractAnalyzer getAnalyzer(AbstractType validator) if (isAnalyzed) { if (analyzerClass != null) - analyzer = (AbstractAnalyzer) analyzerClass.newInstance(); + analyzer = analyzerClass.newInstance(); else if (TOKENIZABLE_TYPES.contains(validator)) analyzer = new StandardAnalyzer(); } @@ -99,21 +100,14 @@ public static void validateAnalyzer(Map indexOptions, ColumnMeta // validate that a valid analyzer class was provided if specified if (indexOptions.containsKey(INDEX_ANALYZER_CLASS_OPTION)) { - Class analyzerClass; - try - { - analyzerClass = Class.forName(indexOptions.get(INDEX_ANALYZER_CLASS_OPTION)); - } - catch (ClassNotFoundException e) - { - throw new ConfigurationException(String.format("Invalid analyzer class option specified [%s]", - indexOptions.get(INDEX_ANALYZER_CLASS_OPTION))); - } + Class analyzerClass = FBUtilities.classForNameWithoutInitialization(indexOptions.get(INDEX_ANALYZER_CLASS_OPTION), + "analyzer", + AbstractAnalyzer.class); AbstractAnalyzer analyzer; try { - analyzer = (AbstractAnalyzer) analyzerClass.newInstance(); + analyzer = analyzerClass.newInstance(); analyzer.validate(indexOptions, cd); } catch (InstantiationException | IllegalAccessException e) @@ -148,25 +142,30 @@ public static IndexMode getMode(ColumnMetadata column, Map index } boolean isAnalyzed = false; - Class analyzerClass = null; - try + Class analyzerClass = null; + if (indexOptions.get(INDEX_ANALYZER_CLASS_OPTION) != null) { - if (indexOptions.get(INDEX_ANALYZER_CLASS_OPTION) != null) + try { - analyzerClass = Class.forName(indexOptions.get(INDEX_ANALYZER_CLASS_OPTION)); + analyzerClass = FBUtilities.classForNameWithoutInitialization(indexOptions.get(INDEX_ANALYZER_CLASS_OPTION), + "analyzer", + AbstractAnalyzer.class); isAnalyzed = indexOptions.get(INDEX_ANALYZED_OPTION) == null - ? true : Boolean.parseBoolean(indexOptions.get(INDEX_ANALYZED_OPTION)); + ? true : Boolean.parseBoolean(indexOptions.get(INDEX_ANALYZED_OPTION)); } - else if (indexOptions.get(INDEX_ANALYZED_OPTION) != null) + catch (ConfigurationException e) { - isAnalyzed = Boolean.parseBoolean(indexOptions.get(INDEX_ANALYZED_OPTION)); + if (!(e.getCause() instanceof ClassNotFoundException)) + throw e; + + // Should not happen as we already validated we could instantiate an instance in validateAnalyzer(). + logger.error("Failed to find specified analyzer class [{}]. Falling back to default analyzer", + indexOptions.get(INDEX_ANALYZER_CLASS_OPTION)); } } - catch (ClassNotFoundException e) + else if (indexOptions.get(INDEX_ANALYZED_OPTION) != null) { - // should not happen as we already validated we could instantiate an instance in validateAnalyzer() - logger.error("Failed to find specified analyzer class [{}]. Falling back to default analyzer", - indexOptions.get(INDEX_ANALYZER_CLASS_OPTION)); + isAnalyzed = Boolean.parseBoolean(indexOptions.get(INDEX_ANALYZED_OPTION)); } boolean isLiteral = false; diff --git a/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java b/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java index 4dd68b3dfdc7..395c59f835f5 100644 --- a/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java +++ b/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java @@ -340,11 +340,11 @@ public static Class getClass(String cls) throws Con if ("org.apache.cassandra.locator.OldNetworkTopologyStrategy".equals(className)) // see CASSANDRA-16301 throw new ConfigurationException("The support for the OldNetworkTopologyStrategy has been removed in C* version 4.0. The keyspace strategy should be switch to NetworkTopologyStrategy"); - Class strategyClass = FBUtilities.classForName(className, "replication strategy"); - if (!AbstractReplicationStrategy.class.isAssignableFrom(strategyClass)) - { - throw new ConfigurationException(String.format("Specified replication strategy class (%s) is not derived from AbstractReplicationStrategy", className)); - } + @SuppressWarnings("unchecked") + Class strategyClass = + (Class)(Class) FBUtilities.classForNameWithoutInitialization(className, + "replication strategy", + AbstractReplicationStrategy.class); return strategyClass; } diff --git a/src/java/org/apache/cassandra/repair/autorepair/AutoRepairConfig.java b/src/java/org/apache/cassandra/repair/autorepair/AutoRepairConfig.java index dd5c0d6ed9f9..2c3ba9dc2069 100644 --- a/src/java/org/apache/cassandra/repair/autorepair/AutoRepairConfig.java +++ b/src/java/org/apache/cassandra/repair/autorepair/AutoRepairConfig.java @@ -395,7 +395,10 @@ static IAutoRepairTokenRangeSplitter newAutoRepairTokenRangeSplitter(RepairType className = parameterizedClass.class_name.contains(".") ? parameterizedClass.class_name : "org.apache.cassandra.repair.autorepair." + parameterizedClass.class_name; - tokenRangeSplitterClass = FBUtilities.classForName(className, "token_range_splitter"); + tokenRangeSplitterClass = + FBUtilities.classForNameWithoutInitialization(className, + "token_range_splitter", + IAutoRepairTokenRangeSplitter.class); } else { diff --git a/src/java/org/apache/cassandra/schema/CompactionParams.java b/src/java/org/apache/cassandra/schema/CompactionParams.java index 9217a3637626..1cfdd3145882 100644 --- a/src/java/org/apache/cassandra/schema/CompactionParams.java +++ b/src/java/org/apache/cassandra/schema/CompactionParams.java @@ -311,15 +311,7 @@ public static Class classFromName(String n String className = name.contains(".") ? name : "org.apache.cassandra.db.compaction." + name; - Class strategyClass = FBUtilities.classForName(className, "compaction strategy"); - - if (!AbstractCompactionStrategy.class.isAssignableFrom(strategyClass)) - { - throw new ConfigurationException(format("Compaction strategy class %s is not derived from AbstractReplicationStrategy", - className)); - } - - return strategyClass; + return FBUtilities.classForNameWithoutInitialization(className, "compaction strategy", AbstractCompactionStrategy.class); } /* diff --git a/src/java/org/apache/cassandra/schema/CompressionParams.java b/src/java/org/apache/cassandra/schema/CompressionParams.java index a438e34eb108..aea2dd4dadb4 100644 --- a/src/java/org/apache/cassandra/schema/CompressionParams.java +++ b/src/java/org/apache/cassandra/schema/CompressionParams.java @@ -46,6 +46,7 @@ import org.apache.cassandra.io.util.DataInputPlus; import org.apache.cassandra.io.util.DataOutputPlus; import org.apache.cassandra.net.MessagingService; +import org.apache.cassandra.utils.FBUtilities; import static java.lang.String.format; @@ -302,23 +303,16 @@ public int maxCompressedLength() return maxCompressedLength; } - private static Class parseCompressorClass(String className) throws ConfigurationException + private static Class parseCompressorClass(String className) throws ConfigurationException { if (className == null || className.isEmpty()) return null; className = className.contains(".") ? className : "org.apache.cassandra.io.compress." + className; - try - { - return Class.forName(className); - } - catch (Exception e) - { - throw new ConfigurationException("Could not create Compression for type " + className, e); - } + return FBUtilities.classForNameWithoutInitialization(className, "compression", ICompressor.class); } - private static ICompressor createCompressor(Class compressorClass, Map compressionOptions) throws ConfigurationException + private static ICompressor createCompressor(Class compressorClass, Map compressionOptions) throws ConfigurationException { if (compressorClass == null) { diff --git a/src/java/org/apache/cassandra/schema/IndexMetadata.java b/src/java/org/apache/cassandra/schema/IndexMetadata.java index 15a0670e3a2d..b471260ccc2d 100644 --- a/src/java/org/apache/cassandra/schema/IndexMetadata.java +++ b/src/java/org/apache/cassandra/schema/IndexMetadata.java @@ -148,9 +148,7 @@ public void validate(TableMetadata table) // Get the fully qualified class name: String className = getIndexClassName(); - Class indexerClass = FBUtilities.classForName(className, "custom indexer"); - if (!Index.class.isAssignableFrom(indexerClass)) - throw new ConfigurationException(String.format("Specified Indexer class (%s) does not implement the Indexer interface", className)); + Class indexerClass = FBUtilities.classForNameWithoutInitialization(className, "custom indexer", Index.class); validateCustomIndexOptions(table, indexerClass, options); } } diff --git a/src/java/org/apache/cassandra/schema/MemtableParams.java b/src/java/org/apache/cassandra/schema/MemtableParams.java index 7d88f6518ee5..5491b105cf63 100644 --- a/src/java/org/apache/cassandra/schema/MemtableParams.java +++ b/src/java/org/apache/cassandra/schema/MemtableParams.java @@ -228,19 +228,25 @@ private static Memtable.Factory getMemtableFactory(ParameterizedClass options) try { Memtable.Factory factory; - Class clazz = Class.forName(className); + Class clazz = Class.forName(className, false, MemtableParams.class.getClassLoader()); final Map parametersCopy = options.parameters != null ? new HashMap<>(options.parameters) : new HashMap<>(); try { Method factoryMethod = clazz.getDeclaredMethod("factory", Map.class); + if (!Memtable.Factory.class.isAssignableFrom(factoryMethod.getReturnType())) + throw new ConfigurationException("Memtable factory method on " + className + + " must return " + Memtable.Factory.class.getName()); factory = (Memtable.Factory) factoryMethod.invoke(null, parametersCopy); } catch (NoSuchMethodException e) { // continue with FACTORY field Field factoryField = clazz.getDeclaredField("FACTORY"); + if (!Memtable.Factory.class.isAssignableFrom(factoryField.getType())) + throw new ConfigurationException("Memtable FACTORY field on " + className + + " must be of type " + Memtable.Factory.class.getName()); factory = (Memtable.Factory) factoryField.get(null); } if (!parametersCopy.isEmpty()) diff --git a/src/java/org/apache/cassandra/schema/ReplicationParams.java b/src/java/org/apache/cassandra/schema/ReplicationParams.java index c1e2643897c5..63cad56057ca 100644 --- a/src/java/org/apache/cassandra/schema/ReplicationParams.java +++ b/src/java/org/apache/cassandra/schema/ReplicationParams.java @@ -286,7 +286,7 @@ public ReplicationParams deserialize(DataInputPlus in, Version version) throws I Map options = new HashMap<>(size); for (int i = 0; i < size; i++) options.put(in.readUTF(), in.readUTF()); - return new ReplicationParams(FBUtilities.classForName(klassName, "ReplicationStrategy"), options); + return new ReplicationParams(FBUtilities.classForNameWithoutInitialization(klassName, "ReplicationStrategy", AbstractReplicationStrategy.class), options); } public long serializedSize(ReplicationParams t, Version version) @@ -322,7 +322,7 @@ public ReplicationParams deserialize(DataInputPlus in, int version) throws IOExc Map options = new HashMap<>(size); for (int i=0; i keyProviderClass = (Class)Class.forName(options.key_provider.class_name); - Constructor ctor = keyProviderClass.getConstructor(TransparentDataEncryptionOptions.class); - keyProvider = (KeyProvider)ctor.newInstance(options); + Class keyProviderClass = + FBUtilities.classForNameWithoutInitialization(options.key_provider.class_name, "key provider", KeyProvider.class); + Constructor ctor = keyProviderClass.getConstructor(TransparentDataEncryptionOptions.class); + keyProvider = ctor.newInstance(options); } catch (Exception e) { diff --git a/src/java/org/apache/cassandra/service/CacheService.java b/src/java/org/apache/cassandra/service/CacheService.java index 8240c2880f78..c755c063475f 100644 --- a/src/java/org/apache/cassandra/service/CacheService.java +++ b/src/java/org/apache/cassandra/service/CacheService.java @@ -147,9 +147,11 @@ private AutoSavingCache initRowCache() ? DatabaseDescriptor.getRowCacheClassName() : "org.apache.cassandra.cache.NopCacheProvider"; try { - Class> cacheProviderClass = - (Class>) Class.forName(cacheProviderClassName); - cacheProvider = cacheProviderClass.newInstance(); + Class cacheProviderClass = + FBUtilities.classForNameWithoutInitialization(cacheProviderClassName, "row cache provider", CacheProvider.class); + @SuppressWarnings("unchecked") + CacheProvider typedCacheProvider = cacheProviderClass.newInstance(); + cacheProvider = typedCacheProvider; } catch (Exception e) { diff --git a/src/java/org/apache/cassandra/service/ClientState.java b/src/java/org/apache/cassandra/service/ClientState.java index c119d5f4c333..a17688d11699 100644 --- a/src/java/org/apache/cassandra/service/ClientState.java +++ b/src/java/org/apache/cassandra/service/ClientState.java @@ -124,7 +124,7 @@ public class ClientState { try { - handler = FBUtilities.construct(customHandlerClass, "QueryHandler"); + handler = FBUtilities.construct(customHandlerClass, "QueryHandler", QueryHandler.class); logger.info("Using {} as a query handler for native protocol queries (as requested by the {} system property)", customHandlerClass, CUSTOM_QUERY_HANDLER_CLASS.getKey()); } diff --git a/src/java/org/apache/cassandra/service/DiskErrorsHandlerService.java b/src/java/org/apache/cassandra/service/DiskErrorsHandlerService.java index 036559a7380b..7556e368d403 100644 --- a/src/java/org/apache/cassandra/service/DiskErrorsHandlerService.java +++ b/src/java/org/apache/cassandra/service/DiskErrorsHandlerService.java @@ -78,7 +78,7 @@ public static void configure() throws ConfigurationException String fsErrorHandlerClass = CassandraRelevantProperties.CUSTOM_DISK_ERROR_HANDLER.getString(); DiskErrorsHandler fsErrorHandler = fsErrorHandlerClass == null ? new DefaultDiskErrorsHandler() - : FBUtilities.construct(fsErrorHandlerClass, "disk error handler"); + : FBUtilities.construct(fsErrorHandlerClass, "disk error handler", DiskErrorsHandler.class); DiskErrorsHandlerService.set(fsErrorHandler); } } diff --git a/src/java/org/apache/cassandra/service/accord/AccordService.java b/src/java/org/apache/cassandra/service/accord/AccordService.java index 340760370c1d..111e2074745c 100644 --- a/src/java/org/apache/cassandra/service/accord/AccordService.java +++ b/src/java/org/apache/cassandra/service/accord/AccordService.java @@ -458,7 +458,9 @@ public AccordService(Id localId) { Invariants.require(localId != null, "static localId must be set before instantiating AccordService"); logger.info("Starting accord with nodeId {}", localId); - AccordAgent agent = FBUtilities.construct(CassandraRelevantProperties.ACCORD_AGENT_CLASS.getString(AccordAgent.class.getName()), "AccordAgent"); + AccordAgent agent = FBUtilities.construct(CassandraRelevantProperties.ACCORD_AGENT_CLASS.getString(AccordAgent.class.getName()), + "AccordAgent", + AccordAgent.class); agent.setup(localId); AccordTimeService time = new AccordTimeService(); this.scheduler = new AccordScheduler(); diff --git a/src/java/org/apache/cassandra/streaming/StreamHook.java b/src/java/org/apache/cassandra/streaming/StreamHook.java index 84db420f04aa..df1e8ee76ef0 100644 --- a/src/java/org/apache/cassandra/streaming/StreamHook.java +++ b/src/java/org/apache/cassandra/streaming/StreamHook.java @@ -37,7 +37,7 @@ static StreamHook createHook() String className = STREAM_HOOK.getString(); if (className != null) { - return FBUtilities.construct(className, StreamHook.class.getSimpleName()); + return FBUtilities.construct(className, StreamHook.class.getSimpleName(), StreamHook.class); } else { diff --git a/src/java/org/apache/cassandra/tcm/extensions/ExtensionKey.java b/src/java/org/apache/cassandra/tcm/extensions/ExtensionKey.java index 976e3279511f..2165fceddde3 100644 --- a/src/java/org/apache/cassandra/tcm/extensions/ExtensionKey.java +++ b/src/java/org/apache/cassandra/tcm/extensions/ExtensionKey.java @@ -41,7 +41,7 @@ public ExtensionKey(String id, Class valueType) public K newValue() { - return valueType.cast(FBUtilities.construct(valueType.getName(), "extension value")); + return FBUtilities.construct(valueType.getName(), "extension value", valueType); } public static final class Serializer implements MetadataSerializer> @@ -58,7 +58,9 @@ public void serialize(ExtensionKey t, DataOutputPlus out, Version version) { String id = in.readUTF(); String valType = in.readUTF(); - return new ExtensionKey(id, FBUtilities.classForName(valType, "value type")); + Class valueType = + FBUtilities.classForNameWithoutInitialization(valType, "value type", ExtensionValue.class); + return new ExtensionKey(id, valueType); } @Override @@ -68,4 +70,3 @@ public long serializedSize(ExtensionKey t, Version version) } } } - diff --git a/src/java/org/apache/cassandra/tools/nodetool/Sjk.java b/src/java/org/apache/cassandra/tools/nodetool/Sjk.java index c117ed638ba6..f9d19eabd2e4 100644 --- a/src/java/org/apache/cassandra/tools/nodetool/Sjk.java +++ b/src/java/org/apache/cassandra/tools/nodetool/Sjk.java @@ -393,6 +393,7 @@ private static List> findClasses(String packageName) List> result = new ArrayList<>(); try { + ClassLoader cl = Thread.currentThread().getContextClassLoader(); String path = packageName.replace('.', '/'); for (String f : findFiles(path)) { @@ -400,7 +401,7 @@ private static List> findClasses(String packageName) { f = f.substring(0, f.length() - ".class".length()); f = f.replace('/', '.'); - result.add(Class.forName(f)); + result.add(Class.forName(f, false, cl)); } } return result; diff --git a/src/java/org/apache/cassandra/tracing/Tracing.java b/src/java/org/apache/cassandra/tracing/Tracing.java index 4ef2e500cb4d..8d621cb66842 100644 --- a/src/java/org/apache/cassandra/tracing/Tracing.java +++ b/src/java/org/apache/cassandra/tracing/Tracing.java @@ -117,7 +117,7 @@ public int getTTL() { try { - tracing = FBUtilities.construct(customTracingClass, "Tracing"); + tracing = FBUtilities.construct(customTracingClass, "Tracing", Tracing.class); logger.info("Using the {} class to trace queries (as requested by the {} system property)", customTracingClass, CUSTOM_TRACING_CLASS.getKey()); } diff --git a/src/java/org/apache/cassandra/triggers/TriggerExecutor.java b/src/java/org/apache/cassandra/triggers/TriggerExecutor.java index e3ded966adab..623197969576 100644 --- a/src/java/org/apache/cassandra/triggers/TriggerExecutor.java +++ b/src/java/org/apache/cassandra/triggers/TriggerExecutor.java @@ -286,13 +286,28 @@ private List executeInternal(PartitionUpdate update) } } - public synchronized void loadTriggerClass(String triggerClass) throws Exception + public synchronized Class loadTriggerClass(String triggerClass) throws Exception { // Allow loading the class regardless of Config, since this could happen as part of TCM replay via // CreateTriggerStatement#apply. // Check that triggerClass is available on the classpath, but do not initialize the class since that would // execute static blocks. - customClassLoader.loadClass(triggerClass).getConstructor(); + Class trigger; + try + { + trigger = FBUtilities.classForNameWithoutInitialization(triggerClass, + "trigger", + ITrigger.class, + customClassLoader); + } + catch (ConfigurationException e) + { + if (e.getCause() instanceof ClassNotFoundException) + throw (ClassNotFoundException) e.getCause(); + throw e; + } + trigger.getConstructor(); + return trigger; } public synchronized ITrigger loadTriggerInstance(String triggerClass) throws Exception @@ -306,6 +321,6 @@ public synchronized ITrigger loadTriggerInstance(String triggerClass) throws Exc // double check. if (cachedTriggers.get(triggerClass) != null) return cachedTriggers.get(triggerClass); - return (ITrigger) customClassLoader.loadClass(triggerClass).getConstructor().newInstance(); + return loadTriggerClass(triggerClass).getConstructor().newInstance(); } } diff --git a/src/java/org/apache/cassandra/utils/Clock.java b/src/java/org/apache/cassandra/utils/Clock.java index 1d7bd7036831..145bab1120e6 100644 --- a/src/java/org/apache/cassandra/utils/Clock.java +++ b/src/java/org/apache/cassandra/utils/Clock.java @@ -63,7 +63,7 @@ public static class Global try { outcome = "Using custom clock implementation: " + classname; - clock = (Clock) Class.forName(classname).newInstance(); + clock = FBUtilities.construct(classname, "clock", Clock.class); } catch (Throwable t) { diff --git a/src/java/org/apache/cassandra/utils/FBUtilities.java b/src/java/org/apache/cassandra/utils/FBUtilities.java index a0ec48390cd0..0e4c3281a4d7 100644 --- a/src/java/org/apache/cassandra/utils/FBUtilities.java +++ b/src/java/org/apache/cassandra/utils/FBUtilities.java @@ -679,7 +679,7 @@ static IPartitioner newPartitioner(String partitionerClassName, Optional parameters) throws ConfigurationException @@ -689,8 +689,9 @@ public static IAuditLogger newAuditLogger(String className, Map try { - Class auditLoggerClass = FBUtilities.classForName(className, "Audit logger"); - return (IAuditLogger) auditLoggerClass.getConstructor(Map.class).newInstance(parameters); + Class auditLoggerClass = + FBUtilities.classForNameWithoutInitialization(className, "Audit logger", IAuditLogger.class); + return auditLoggerClass.getConstructor(Map.class).newInstance(parameters); } catch (Exception ex) { @@ -705,8 +706,9 @@ public static ISslContextFactory newSslContextFactory(String className, Map sslContextFactoryClass = Class.forName(className); - return (ISslContextFactory) sslContextFactoryClass.getConstructor(Map.class).newInstance(parameters); + Class sslContextFactoryClass = + FBUtilities.classForNameWithoutInitialization(className, "ISslContextFactory", ISslContextFactory.class); + return sslContextFactoryClass.getConstructor(Map.class).newInstance(parameters); } catch (Exception ex) { @@ -721,8 +723,9 @@ public static AbstractCryptoProvider newCryptoProvider(String className, Map cryptoProviderClass = FBUtilities.classForName(className, "crypto provider class"); - return (AbstractCryptoProvider) cryptoProviderClass.getConstructor(Map.class).newInstance(Collections.unmodifiableMap(parameters)); + Class cryptoProviderClass = + FBUtilities.classForNameWithoutInitialization(className, "crypto provider class", AbstractCryptoProvider.class); + return cryptoProviderClass.getConstructor(Map.class).newInstance(Collections.unmodifiableMap(parameters)); } catch (Exception e) { @@ -755,6 +758,8 @@ public static AbstractCompressionProvider newCompressionProvider(String classNam } /** + * Loads and initializes a class. + * * @return The Class for the given name. * @param classname Fully qualified classname. * @param readable Descriptive noun for the role the class plays. @@ -772,6 +777,53 @@ public static Class classForName(String classname, String readable) throw } } + /** + * Loads a class without initializing it, then verifies it extends or implements the expected base type. + * + * @return The Class for the given name. + * @param classname Fully qualified classname. + * @param readable Descriptive noun for the role the class plays. + * @param expectedType Required superclass or interface. + * @throws ConfigurationException If the class cannot be found or is not assignable to {@code expectedType}. + */ + public static Class classForNameWithoutInitialization(String classname, + String readable, + Class expectedType) throws ConfigurationException + { + return classForNameWithoutInitialization(classname, readable, expectedType, FBUtilities.class.getClassLoader()); + } + + /** + * Loads a class without initializing it, then verifies it extends or implements the expected base type. + * + * @return The Class for the given name. + * @param classname Fully qualified classname. + * @param readable Descriptive noun for the role the class plays. + * @param expectedType Required superclass or interface. + * @param classLoader ClassLoader to use. + * @throws ConfigurationException If the class cannot be found or is not assignable to {@code expectedType}. + */ + public static Class classForNameWithoutInitialization(String classname, + String readable, + Class expectedType, + ClassLoader classLoader) throws ConfigurationException + { + try + { + Class klass = Class.forName(classname, false, classLoader); + if (!expectedType.isAssignableFrom(klass)) + throw new ConfigurationException(String.format("Invalid %s class '%s': must extend or implement %s", + readable, + classname, + expectedType.getName())); + return klass.asSubclass(expectedType); + } + catch (ClassNotFoundException | NoClassDefFoundError e) + { + throw new ConfigurationException(String.format("Unable to find %s class '%s'", readable, classname), e); + } + } + /** * Constructs an instance of the given class, which must have a no-arg or default constructor. * @param classname Fully qualified classname. @@ -781,6 +833,25 @@ public static Class classForName(String classname, String readable) throw public static T instanceOrConstruct(String classname, String readable) throws ConfigurationException { Class cls = FBUtilities.classForName(classname, readable); + return instanceOrConstruct(cls, classname, readable); + } + + /** + * Constructs an instance of the given class, or gets its static {@code instance} field, after verifying the + * class without initializing it. + * @param classname Fully qualified classname. + * @param readable Descriptive noun for the role the class plays. + * @param expectedType Required superclass or interface. + * @throws ConfigurationException If the class cannot be found or is not assignable to {@code expectedType}. + */ + public static T instanceOrConstruct(String classname, String readable, Class expectedType) throws ConfigurationException + { + Class cls = FBUtilities.classForNameWithoutInitialization(classname, readable, expectedType); + return instanceOrConstruct(cls, classname, readable); + } + + private static T instanceOrConstruct(Class cls, String classname, String readable) throws ConfigurationException + { try { Field instance = cls.getField("instance"); @@ -805,7 +876,20 @@ public static T construct(String classname, String readable) throws Configur return construct(cls, classname, readable); } - private static T construct(Class cls, String classname, String readable) throws ConfigurationException + /** + * Constructs an instance of the given class after verifying it without initializing it. + * @param classname Fully qualified classname. + * @param readable Descriptive noun for the role the class plays. + * @param expectedType Required superclass or interface. + * @throws ConfigurationException If the class cannot be found or is not assignable to {@code expectedType}. + */ + public static T construct(String classname, String readable, Class expectedType) throws ConfigurationException + { + Class cls = FBUtilities.classForNameWithoutInitialization(classname, readable, expectedType); + return construct(cls, classname, readable); + } + + private static T construct(Class cls, String classname, String readable) throws ConfigurationException { try { @@ -1496,4 +1580,4 @@ public static AsyncResult futureToAsyncResult(org.apache.cassandra.utils. }); return adapter; } -} \ No newline at end of file +} diff --git a/src/java/org/apache/cassandra/utils/JMXServerUtils.java b/src/java/org/apache/cassandra/utils/JMXServerUtils.java index 72851d74ad2d..85e5ec81f023 100644 --- a/src/java/org/apache/cassandra/utils/JMXServerUtils.java +++ b/src/java/org/apache/cassandra/utils/JMXServerUtils.java @@ -222,7 +222,7 @@ private static MBeanServerForwarder configureJmxAuthorization(JMXServerOptions o String authzProxyClass = options.authorizer; if (authzProxyClass != null) { - final InvocationHandler handler = FBUtilities.construct(authzProxyClass, "JMX authz proxy"); + final InvocationHandler handler = FBUtilities.construct(authzProxyClass, "JMX authz proxy", InvocationHandler.class); final Class[] interfaces = { MBeanServerForwarder.class }; Object proxy = Proxy.newProxyInstance(MBeanServerForwarder.class.getClassLoader(), interfaces, handler); diff --git a/src/java/org/apache/cassandra/utils/MBeanWrapper.java b/src/java/org/apache/cassandra/utils/MBeanWrapper.java index d1d82ca1f658..691d5170d683 100644 --- a/src/java/org/apache/cassandra/utils/MBeanWrapper.java +++ b/src/java/org/apache/cassandra/utils/MBeanWrapper.java @@ -78,7 +78,7 @@ static MBeanWrapper getMBeanWrapper() return new PlatformMBeanWrapper(); } } - return FBUtilities.construct(klass, "mbean"); + return FBUtilities.construct(klass, "mbean", MBeanWrapper.class); } // Passing true for graceful will log exceptions instead of rethrowing them diff --git a/src/java/org/apache/cassandra/utils/MonotonicClock.java b/src/java/org/apache/cassandra/utils/MonotonicClock.java index 2300cf2e89eb..0b638351205f 100644 --- a/src/java/org/apache/cassandra/utils/MonotonicClock.java +++ b/src/java/org/apache/cassandra/utils/MonotonicClock.java @@ -94,7 +94,7 @@ private static MonotonicClock precise() try { logger.debug("Using custom clock implementation: {}", sclock); - return (MonotonicClock) Class.forName(sclock).newInstance(); + return FBUtilities.construct(sclock, "monotonic clock", MonotonicClock.class); } catch (Exception e) { @@ -113,7 +113,8 @@ private static MonotonicClock approx(MonotonicClock precise) try { logger.debug("Using custom clock implementation: {}", sclock); - Class clazz = (Class) Class.forName(sclock); + Class clazz = + FBUtilities.classForNameWithoutInitialization(sclock, "monotonic clock", MonotonicClock.class); if (SystemClock.class.equals(clazz) && SystemClock.class.equals(precise.getClass())) return precise; diff --git a/test/unit/org/apache/cassandra/auth/AuthConfigTest.java b/test/unit/org/apache/cassandra/auth/AuthConfigTest.java index ecbc72f0d517..90c10d16cf5b 100644 --- a/test/unit/org/apache/cassandra/auth/AuthConfigTest.java +++ b/test/unit/org/apache/cassandra/auth/AuthConfigTest.java @@ -31,7 +31,10 @@ import org.apache.cassandra.config.Config; import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.config.ParameterizedClass; +import org.apache.cassandra.exceptions.ConfigurationException; import org.apache.cassandra.locator.InetAddressAndPort; +import org.apache.cassandra.utils.ClassLoadingTestNonAssignable; +import org.apache.cassandra.utils.ClassLoadingTestSupport; import org.apache.cassandra.utils.MBeanWrapper; import static org.apache.cassandra.auth.AuthCache.MBEAN_NAME_BASE; @@ -39,6 +42,7 @@ import static org.apache.cassandra.auth.IInternodeAuthenticator.InternodeConnectionDirection.INBOUND; import static org.apache.cassandra.config.YamlConfigurationLoaderTest.load; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -160,6 +164,75 @@ public void testNewInstanceForMutualTlsAuthenticator() assertTrue(DatabaseDescriptor.getRoleManager().alterableOptions().containsAll(authenticator.getAlterableRoleOptions())); } + private static final String PROBE = ClassLoadingTestNonAssignable.class.getName(); + + private static Config baseConfig() + { + // Use the default (AllowAll) auth config so the non-probe auth components do not register JMX caches that + // would conflict across the per-field probe tests below. + Config config = load("cassandra.yaml"); + DatabaseDescriptor.unsafeDaemonInitialization(() -> config); + return config; + } + + private static void assertApplyAuthRejectsProbe() + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + AuthConfig.reset(); + assertThatThrownBy(AuthConfig::applyAuth) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must extend or implement"); + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + + @Test + public void testAuthenticatorWrongTypeRejectedWithoutInitializing() + { + Config config = baseConfig(); + config.authenticator = new ParameterizedClass(PROBE, Collections.emptyMap()); + assertApplyAuthRejectsProbe(); + } + + @Test + public void testAuthorizerWrongTypeRejectedWithoutInitializing() + { + Config config = baseConfig(); + config.authorizer = new ParameterizedClass(PROBE, Collections.emptyMap()); + assertApplyAuthRejectsProbe(); + } + + @Test + public void testRoleManagerWrongTypeRejectedWithoutInitializing() + { + Config config = baseConfig(); + config.role_manager = new ParameterizedClass(PROBE, Collections.emptyMap()); + assertApplyAuthRejectsProbe(); + } + + @Test + public void testInternodeAuthenticatorWrongTypeRejectedWithoutInitializing() + { + Config config = baseConfig(); + config.internode_authenticator = new ParameterizedClass(PROBE, Collections.emptyMap()); + assertApplyAuthRejectsProbe(); + } + + @Test + public void testNetworkAuthorizerWrongTypeRejectedWithoutInitializing() + { + Config config = baseConfig(); + config.network_authorizer = new ParameterizedClass(PROBE, Collections.emptyMap()); + assertApplyAuthRejectsProbe(); + } + + @Test + public void testCidrAuthorizerWrongTypeRejectedWithoutInitializing() + { + Config config = baseConfig(); + config.cidr_authorizer = new ParameterizedClass(PROBE, Collections.emptyMap()); + assertApplyAuthRejectsProbe(); + } + private void unregisterCaches() { safeUnregisterMbean(IDENTITIES_CACHE_MBEAN); diff --git a/test/unit/org/apache/cassandra/config/ClassLoadingSearchProbe.java b/test/unit/org/apache/cassandra/config/ClassLoadingSearchProbe.java new file mode 100644 index 000000000000..abfdfa278a8c --- /dev/null +++ b/test/unit/org/apache/cassandra/config/ClassLoadingSearchProbe.java @@ -0,0 +1,37 @@ +/* + * 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.cassandra.config; + +/** + * Search-package fall-through probe. Shares its simple name ({@code ClassLoadingSearchProbe}) with + * {@link org.apache.cassandra.utils.ClassLoadingSearchProbe}, but unlike that one it IS a {@link Runnable}, so it + * represents the "correct type under a later search package" case. A wrong-type match under an earlier package must + * not abort the search before this class is found. + */ +public final class ClassLoadingSearchProbe implements Runnable +{ + public ClassLoadingSearchProbe() + { + } + + @Override + public void run() + { + } +} diff --git a/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java b/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java index 5796c4032f4a..6f80a3dc6c3a 100644 --- a/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java +++ b/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java @@ -47,8 +47,11 @@ import org.apache.cassandra.io.util.File; import org.apache.cassandra.io.util.FileUtils; import org.apache.cassandra.io.util.PathUtils; +import org.apache.cassandra.locator.IEndpointSnitch; import org.apache.cassandra.security.EncryptionContext; import org.apache.cassandra.security.EncryptionContextGenerator; +import org.apache.cassandra.utils.ClassLoadingTestNonAssignable; +import org.apache.cassandra.utils.ClassLoadingTestSupport; import static org.apache.cassandra.config.CassandraRelevantProperties.ALLOW_UNLIMITED_CONCURRENT_VALIDATIONS; import static org.apache.cassandra.config.CassandraRelevantProperties.CONFIG_LOADER; @@ -133,6 +136,25 @@ public static void selectSuitableInterface() throws Exception { } } + @Test + public void testCreateEndpointSnitchWrongTypeRejectedWithoutInitializing() + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + + assertThatThrownBy(() -> DatabaseDescriptor.createEndpointSnitch(ClassLoadingTestNonAssignable.class.getName())) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must extend or implement " + IEndpointSnitch.class.getName()); + + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + + @Test + public void testCreateEndpointSnitchValidClassResolves() + { + IEndpointSnitch snitch = DatabaseDescriptor.createEndpointSnitch("SimpleSnitch"); + assertThat(snitch).isNotNull(); + } + @Test public void testRpcInterface() { diff --git a/test/unit/org/apache/cassandra/config/ParameterizedClassTest.java b/test/unit/org/apache/cassandra/config/ParameterizedClassTest.java index 732812aefd15..4794d3de8097 100644 --- a/test/unit/org/apache/cassandra/config/ParameterizedClassTest.java +++ b/test/unit/org/apache/cassandra/config/ParameterizedClassTest.java @@ -26,7 +26,10 @@ import org.apache.cassandra.auth.AllowAllAuthorizer; import org.apache.cassandra.auth.IAuthorizer; import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.utils.ClassLoadingTestNonAssignable; +import org.apache.cassandra.utils.ClassLoadingTestSupport; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -71,6 +74,51 @@ public void testNewInstanceWithSingleEmptyConstructorUsesEmptyConstructor() assertNotNull(instance); } + @Test + public void testTypedNewInstanceRejectsWithoutInitializing() + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + ParameterizedClass parameterizedClass = new ParameterizedClass(ClassLoadingTestNonAssignable.class.getName()); + + assertThatThrownBy(() -> ParameterizedClass.newInstance(parameterizedClass, null, Runnable.class)) + .hasMessageContaining("must extend or implement " + Runnable.class.getName()) + .isInstanceOf(ConfigurationException.class); + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + + @Test + public void testTwoArgNewInstanceDoesNotInitializeAtLoadTime() + { + // The 2-arg overload performs no type check, but it must still load without running . + // ClassLoadingTestNonAssignable has no usable constructor, so instantiation fails -- but only after the + // class has been loaded; loading must not have run its static initializer. + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + ParameterizedClass parameterizedClass = new ParameterizedClass(ClassLoadingTestNonAssignable.class.getName()); + + assertThatThrownBy(() -> ParameterizedClass.newInstance(parameterizedClass, null)) + .isInstanceOf(ConfigurationException.class); + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + + @Test + public void testNewInstanceSearchPackageFallThroughDoesNotAbortOnWrongType() + { + // org.apache.cassandra.utils.ClassLoadingSearchProbe (wrong type, not a Runnable) resolves under the earlier + // search package, while org.apache.cassandra.config.ClassLoadingSearchProbe (correct type, a Runnable) + // resolves under the later one. A wrong-type match under the earlier package must not abort the search. + ClassLoadingTestSupport.assertNotInitialized(org.apache.cassandra.utils.ClassLoadingSearchProbe.class); + ParameterizedClass parameterizedClass = new ParameterizedClass("ClassLoadingSearchProbe"); + + Runnable instance = ParameterizedClass.newInstance(parameterizedClass, + List.of("org.apache.cassandra.utils", + "org.apache.cassandra.config"), + Runnable.class); + assertNotNull(instance); + assertThat(instance).isInstanceOf(org.apache.cassandra.config.ClassLoadingSearchProbe.class); + // The wrong-type class that was probed under the earlier package must not have been initialized. + assertThat(ClassLoadingTestSupport.wasInitialized(org.apache.cassandra.utils.ClassLoadingSearchProbe.class)).isFalse(); + } + @Test public void testNewInstanceWithValidConstructorsFavorsMapConstructor() { diff --git a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java index 79f6d51be065..46708f4317ba 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/GuardrailsConfigProviderTest.java @@ -26,11 +26,25 @@ import org.apache.cassandra.config.GuardrailsOptions; import org.apache.cassandra.exceptions.ConfigurationException; import org.apache.cassandra.service.ClientState; +import org.apache.cassandra.utils.ClassLoadingTestNonAssignable; +import org.apache.cassandra.utils.ClassLoadingTestSupport; import static java.lang.String.format; public class GuardrailsConfigProviderTest extends GuardrailTester { + @Test + public void testBuildWrongTypeRejectedWithoutInitializing() + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + + Assertions.assertThatThrownBy(() -> GuardrailsConfigProvider.build(ClassLoadingTestNonAssignable.class.getName())) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must extend or implement " + GuardrailsConfigProvider.class.getName()); + + Assertions.assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + @Test public void testBuildCustom() throws Throwable { diff --git a/test/unit/org/apache/cassandra/db/guardrails/ValueGeneratorTest.java b/test/unit/org/apache/cassandra/db/guardrails/ValueGeneratorTest.java index 9afb60b9663c..0f4d298b87a1 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/ValueGeneratorTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/ValueGeneratorTest.java @@ -26,9 +26,12 @@ import org.junit.Test; import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.utils.ClassLoadingTestNonAssignable; +import org.apache.cassandra.utils.ClassLoadingTestSupport; import static java.lang.String.format; import static org.apache.cassandra.db.guardrails.ValueGenerator.GENERATOR_CLASS_NAME_KEY; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -68,6 +71,21 @@ public void testValidatorCreationWithInvalidConfiguration() .message().isEqualTo(format("Unable to create instance of generator of class %s: does not contain property 'expecting_true'", BooleanGenerator.class.getName())); } + @Test + public void testGeneratorWrongTypeRejectedWithoutInitializing() + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + + CustomGuardrailConfig config = new CustomGuardrailConfig(); + config.put(GENERATOR_CLASS_NAME_KEY, ClassLoadingTestNonAssignable.class.getName()); + + assertThatThrownBy(() -> ValueGenerator.getGenerator("probe generator", config)) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must extend or implement " + ValueGenerator.class.getName()); + + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + public static class BooleanGenerator extends ValueGenerator { private final boolean expectingTrue; diff --git a/test/unit/org/apache/cassandra/db/guardrails/ValueValidatorTest.java b/test/unit/org/apache/cassandra/db/guardrails/ValueValidatorTest.java index 7bc717e4bcfa..548022399634 100644 --- a/test/unit/org/apache/cassandra/db/guardrails/ValueValidatorTest.java +++ b/test/unit/org/apache/cassandra/db/guardrails/ValueValidatorTest.java @@ -26,10 +26,13 @@ import org.apache.cassandra.db.guardrails.ValueValidator.ValidationViolation; import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.utils.ClassLoadingTestNonAssignable; +import org.apache.cassandra.utils.ClassLoadingTestSupport; import static java.lang.Boolean.FALSE; import static java.lang.String.format; import static org.apache.cassandra.db.guardrails.ValueValidator.VALIDATOR_CLASS_NAME_KEY; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -83,6 +86,21 @@ public void testValidatorCreationWithInvalidConfiguration() .message().isEqualTo(format("Unable to create instance of validator of class %s: does not contain property 'expecting_true'", BooleanValidator.class.getName())); } + @Test + public void testValidatorWrongTypeRejectedWithoutInitializing() + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + + CustomGuardrailConfig config = new CustomGuardrailConfig(); + config.put(VALIDATOR_CLASS_NAME_KEY, ClassLoadingTestNonAssignable.class.getName()); + + assertThatThrownBy(() -> ValueValidator.getValidator("probe validator", config)) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must extend or implement " + ValueValidator.class.getName()); + + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + public static class BooleanValidator extends ValueValidator { private final boolean expectingTrue; diff --git a/test/unit/org/apache/cassandra/db/marshal/TypeParserTest.java b/test/unit/org/apache/cassandra/db/marshal/TypeParserTest.java index e9b2e3c764b2..9588c72bd21d 100644 --- a/test/unit/org/apache/cassandra/db/marshal/TypeParserTest.java +++ b/test/unit/org/apache/cassandra/db/marshal/TypeParserTest.java @@ -33,6 +33,8 @@ import org.apache.cassandra.dht.RandomPartitioner; import org.apache.cassandra.exceptions.ConfigurationException; import org.apache.cassandra.exceptions.SyntaxException; +import org.apache.cassandra.utils.ClassLoadingTestNonAssignable; +import org.apache.cassandra.utils.ClassLoadingTestSupport; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -102,6 +104,23 @@ public void testParseError() catch (SyntaxException e) {} } + @Test + public void testRejectsNonAbstractTypeWithoutInitializing() throws SyntaxException + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + try + { + TypeParser.parse(ClassLoadingTestNonAssignable.class.getName()); + fail("Should not pass"); + } + catch (ConfigurationException e) + { + assertTrue(e.getMessage().contains("must extend or implement " + AbstractType.class.getName())); + } + + assertFalse(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)); + } + @Test public void testParsePartitionerOrder() throws ConfigurationException, SyntaxException { diff --git a/test/unit/org/apache/cassandra/diag/ClassLoadingTestDiagnosticEvent.java b/test/unit/org/apache/cassandra/diag/ClassLoadingTestDiagnosticEvent.java new file mode 100644 index 000000000000..13c880f942c2 --- /dev/null +++ b/test/unit/org/apache/cassandra/diag/ClassLoadingTestDiagnosticEvent.java @@ -0,0 +1,44 @@ +/* + * 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.cassandra.diag; + +import java.io.Serializable; +import java.util.Map; + +/** + * A minimal, valid {@link DiagnosticEvent} subclass used by {@code DiagnosticEventPersistenceTest} to confirm that + * the type-checked load path in {@link DiagnosticEventPersistence} resolves a correct subtype. Lives in the + * {@code org.apache.cassandra} namespace so it passes the persistence package-prefix guard. + */ +public final class ClassLoadingTestDiagnosticEvent extends DiagnosticEvent +{ + public enum TestType { TEST } + + @Override + public Enum getType() + { + return TestType.TEST; + } + + @Override + public Map toMap() + { + return Map.of(); + } +} diff --git a/test/unit/org/apache/cassandra/diag/DiagnosticEventPersistenceTest.java b/test/unit/org/apache/cassandra/diag/DiagnosticEventPersistenceTest.java new file mode 100644 index 000000000000..b18d7b6aac11 --- /dev/null +++ b/test/unit/org/apache/cassandra/diag/DiagnosticEventPersistenceTest.java @@ -0,0 +1,77 @@ +/* + * 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.cassandra.diag; + +import java.io.InvalidClassException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; + +import org.junit.Test; + +import org.apache.cassandra.utils.ClassLoadingTestNonAssignable; +import org.apache.cassandra.utils.ClassLoadingTestSupport; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.fail; + +public class DiagnosticEventPersistenceTest +{ + /** + * Reflectively drives the (private) bespoke type-checked load path + * {@link DiagnosticEventPersistence#getEventClass(String)}. + */ + private static Class getEventClass(String eventClazz) throws Throwable + { + Method method = DiagnosticEventPersistence.class.getDeclaredMethod("getEventClass", String.class); + method.setAccessible(true); + try + { + return (Class) method.invoke(DiagnosticEventPersistence.instance(), eventClazz); + } + catch (InvocationTargetException e) + { + throw e.getCause(); + } + } + + @Test + public void testNonDiagnosticEventRejectedWithoutInitializing() throws Throwable + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + + try + { + getEventClass(ClassLoadingTestNonAssignable.class.getName()); + fail("Expected InvalidClassException for a non-DiagnosticEvent class"); + } + catch (InvalidClassException e) + { + assertThat(e.getMessage()).contains("must be of type DiagnosticEvent"); + } + + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + + @Test + public void testValidDiagnosticEventSubclassResolves() throws Throwable + { + Class resolved = getEventClass(ClassLoadingTestDiagnosticEvent.class.getName()); + assertThat(resolved).isEqualTo(ClassLoadingTestDiagnosticEvent.class); + } +} diff --git a/test/unit/org/apache/cassandra/index/SecondaryIndexManagerTest.java b/test/unit/org/apache/cassandra/index/SecondaryIndexManagerTest.java index 6eaa6e7dcc17..bc1604a5c6b2 100644 --- a/test/unit/org/apache/cassandra/index/SecondaryIndexManagerTest.java +++ b/test/unit/org/apache/cassandra/index/SecondaryIndexManagerTest.java @@ -38,9 +38,12 @@ import org.apache.cassandra.db.SystemKeyspace; import org.apache.cassandra.db.compaction.CompactionInfo; import org.apache.cassandra.db.lifecycle.SSTableSet; +import org.apache.cassandra.exceptions.ConfigurationException; import org.apache.cassandra.io.sstable.format.SSTableReader; import org.apache.cassandra.notifications.SSTableAddedNotification; import org.apache.cassandra.schema.IndexMetadata; +import org.apache.cassandra.utils.ClassLoadingTestNonAssignable; +import org.apache.cassandra.utils.ClassLoadingTestSupport; import org.apache.cassandra.utils.JVMStabilityInspector; import org.apache.cassandra.utils.KillerForTests; import org.apache.cassandra.utils.concurrent.Refs; @@ -58,6 +61,23 @@ public void after() TestingIndex.clear(); } + @Test + public void rejectsNonIndexClassWithoutInitializing() + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + try + { + SecondaryIndexManager.loadIndexClass(ClassLoadingTestNonAssignable.class.getName()); + fail("Should not pass"); + } + catch (ConfigurationException e) + { + assertTrue(e.getMessage().contains("must extend or implement " + Index.class.getName())); + } + + assertFalse(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)); + } + @Test public void createSasiAfterSai() { diff --git a/test/unit/org/apache/cassandra/index/sasi/conf/IndexModeTest.java b/test/unit/org/apache/cassandra/index/sasi/conf/IndexModeTest.java index 546e4cfff905..63e039558825 100644 --- a/test/unit/org/apache/cassandra/index/sasi/conf/IndexModeTest.java +++ b/test/unit/org/apache/cassandra/index/sasi/conf/IndexModeTest.java @@ -31,10 +31,14 @@ import org.apache.cassandra.db.marshal.BytesType; import org.apache.cassandra.db.marshal.UTF8Type; import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.index.sasi.analyzer.AbstractAnalyzer; +import org.apache.cassandra.index.sasi.analyzer.NonTokenizingAnalyzer; import org.apache.cassandra.index.sasi.disk.OnDiskIndexBuilder.Mode; import org.apache.cassandra.schema.ColumnMetadata; import org.apache.cassandra.schema.TableMetadata; import org.apache.cassandra.utils.ByteBufferUtil; +import org.apache.cassandra.utils.ClassLoadingTestNonAssignable; +import org.apache.cassandra.utils.ClassLoadingTestSupport; public class IndexModeTest @@ -185,8 +189,8 @@ public void test_bytesType_analyzer() { ColumnMetadata cd = ColumnMetadata.regularColumn(cfm, ByteBufferUtil.bytes("TestColumnMetadata"), BytesType.instance, ColumnMetadata.NO_UNIQUE_ID); - IndexMode result = IndexMode.getMode(cd, Collections.singletonMap("analyzer_class", "java.lang.Object")); - Assert.assertEquals(Object.class, result.analyzerClass); + IndexMode result = IndexMode.getMode(cd, Collections.singletonMap("analyzer_class", NonTokenizingAnalyzer.class.getName())); + Assert.assertEquals(NonTokenizingAnalyzer.class, result.analyzerClass); Assert.assertTrue(result.isAnalyzed); Assert.assertFalse(result.isLiteral); Assert.assertEquals((long)(1073741824 * 0.15), result.maxCompactionFlushMemoryInBytes); @@ -198,16 +202,67 @@ public void test_bytesType_analyzer_unanalyzed() { ColumnMetadata cd = ColumnMetadata.regularColumn(cfm, ByteBufferUtil.bytes("TestColumnMetadata"), BytesType.instance, ColumnMetadata.NO_UNIQUE_ID); - IndexMode result = IndexMode.getMode(cd, ImmutableMap.of("analyzer_class", "java.lang.Object", + IndexMode result = IndexMode.getMode(cd, ImmutableMap.of("analyzer_class", NonTokenizingAnalyzer.class.getName(), "analyzed", "false")); - Assert.assertEquals(Object.class, result.analyzerClass); + Assert.assertEquals(NonTokenizingAnalyzer.class, result.analyzerClass); Assert.assertFalse(result.isAnalyzed); Assert.assertFalse(result.isLiteral); Assert.assertEquals((long)(1073741824 * 0.15), result.maxCompactionFlushMemoryInBytes); Assert.assertEquals(Mode.PREFIX, result.mode); } + @Test + public void test_bytesType_rejectsNonAnalyzerWithoutInitializing() + { + ColumnMetadata cd = ColumnMetadata.regularColumn(cfm, ByteBufferUtil.bytes("TestColumnMetadata"), BytesType.instance, ColumnMetadata.NO_UNIQUE_ID); + + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + try + { + IndexMode.getMode(cd, Collections.singletonMap("analyzer_class", ClassLoadingTestNonAssignable.class.getName())); + Assert.fail("Should not pass"); + } + catch (ConfigurationException e) + { + Assert.assertTrue(e.getMessage().contains("must extend or implement " + AbstractAnalyzer.class.getName())); + } + + Assert.assertFalse(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)); + } + + @Test + public void test_bytesType_missingAnalyzerFallsBack() + { + ColumnMetadata cd = ColumnMetadata.regularColumn(cfm, ByteBufferUtil.bytes("TestColumnMetadata"), BytesType.instance, ColumnMetadata.NO_UNIQUE_ID); + + IndexMode result = IndexMode.getMode(cd, Collections.singletonMap("analyzer_class", "does.not.ExistAnalyzer")); + Assert.assertNull(result.analyzerClass); + Assert.assertFalse(result.isAnalyzed); + Assert.assertFalse(result.isLiteral); + Assert.assertEquals((long)(1073741824 * 0.15), result.maxCompactionFlushMemoryInBytes); + Assert.assertEquals(Mode.PREFIX, result.mode); + } + + @Test + public void test_validateAnalyzer_rejectsNonAnalyzerWithoutInitializing() + { + ColumnMetadata cd = ColumnMetadata.regularColumn(cfm, ByteBufferUtil.bytes("TestColumnMetadata"), UTF8Type.instance, ColumnMetadata.NO_UNIQUE_ID); + + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + try + { + IndexMode.validateAnalyzer(Collections.singletonMap("analyzer_class", ClassLoadingTestNonAssignable.class.getName()), cd); + Assert.fail("Should not pass"); + } + catch (ConfigurationException e) + { + Assert.assertTrue(e.getMessage().contains("must extend or implement " + AbstractAnalyzer.class.getName())); + } + + Assert.assertFalse(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)); + } + @Test public void test_bytesType_maxCompactionFlushMemoryInBytes() { diff --git a/test/unit/org/apache/cassandra/repair/autorepair/AutoRepairConfigTest.java b/test/unit/org/apache/cassandra/repair/autorepair/AutoRepairConfigTest.java index 188e0da30fa7..4dbdf24efb63 100644 --- a/test/unit/org/apache/cassandra/repair/autorepair/AutoRepairConfigTest.java +++ b/test/unit/org/apache/cassandra/repair/autorepair/AutoRepairConfigTest.java @@ -36,7 +36,11 @@ import org.apache.cassandra.cql3.CQLTester; import org.apache.cassandra.exceptions.ConfigurationException; import org.apache.cassandra.repair.autorepair.AutoRepairConfig.Options; +import org.apache.cassandra.utils.ClassLoadingTestNonAssignable; +import org.apache.cassandra.utils.ClassLoadingTestSupport; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -68,6 +72,19 @@ public void setUp() AutoRepair.SLEEP_IF_REPAIR_FINISHES_QUICKLY = new DurationSpec.IntSecondsBound("0s"); } + @Test + public void testTokenRangeSplitterWrongTypeRejectedWithoutInitializing() + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + + ParameterizedClass pc = new ParameterizedClass(ClassLoadingTestNonAssignable.class.getName(), Collections.emptyMap()); + assertThatThrownBy(() -> AutoRepairConfig.newAutoRepairTokenRangeSplitter(repairType, pc)) + .isInstanceOf(ConfigurationException.class) + .hasStackTraceContaining("must extend or implement " + IAutoRepairTokenRangeSplitter.class.getName()); + + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + @Test public void autoRepairConfigDefaultsAreNotNull() { diff --git a/test/unit/org/apache/cassandra/schema/CompactionParamsTest.java b/test/unit/org/apache/cassandra/schema/CompactionParamsTest.java new file mode 100644 index 000000000000..3831c2db254c --- /dev/null +++ b/test/unit/org/apache/cassandra/schema/CompactionParamsTest.java @@ -0,0 +1,44 @@ +/* + * 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.cassandra.schema; + +import org.junit.Test; + +import org.apache.cassandra.db.compaction.AbstractCompactionStrategy; +import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.utils.ClassLoadingTestNonAssignable; +import org.apache.cassandra.utils.ClassLoadingTestSupport; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +public class CompactionParamsTest +{ + @Test + public void testRejectsNonCompactionStrategyWithoutInitializing() + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + + assertThatThrownBy(() -> CompactionParams.classFromName(ClassLoadingTestNonAssignable.class.getName())) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must extend or implement " + AbstractCompactionStrategy.class.getName()); + + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } +} diff --git a/test/unit/org/apache/cassandra/schema/CompressionParamsTest.java b/test/unit/org/apache/cassandra/schema/CompressionParamsTest.java index 05b98e86aafb..e4a696e663ab 100644 --- a/test/unit/org/apache/cassandra/schema/CompressionParamsTest.java +++ b/test/unit/org/apache/cassandra/schema/CompressionParamsTest.java @@ -30,10 +30,14 @@ import org.junit.Test; import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.exceptions.ConfigurationException; import org.apache.cassandra.io.compress.BufferType; import org.apache.cassandra.io.compress.ICompressor; +import org.apache.cassandra.utils.ClassLoadingTestNonAssignable; +import org.apache.cassandra.utils.ClassLoadingTestSupport; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; public class CompressionParamsTest { @@ -97,6 +101,19 @@ public void testNonBuiltInCompressorClassResolvesViaDefaultProvider() assertThat(params.klass()).isEqualTo(CustomTestCompressor.class); } + @Test + public void testRejectsNonCompressorWithoutInitializing() + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + + assertThatThrownBy(() -> CompressionParams.fromMap(Collections.singletonMap(CompressionParams.CLASS, + ClassLoadingTestNonAssignable.class.getName()))) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must extend or implement " + ICompressor.class.getName()); + + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + public static class CustomTestCompressor implements ICompressor { public static CustomTestCompressor create(Map options) diff --git a/test/unit/org/apache/cassandra/schema/IndexMetadataTest.java b/test/unit/org/apache/cassandra/schema/IndexMetadataTest.java index 0bcd0dfc2899..ef221141d880 100644 --- a/test/unit/org/apache/cassandra/schema/IndexMetadataTest.java +++ b/test/unit/org/apache/cassandra/schema/IndexMetadataTest.java @@ -20,10 +20,21 @@ */ package org.apache.cassandra.schema; +import java.util.Collections; + import org.junit.Assert; import org.junit.Test; import org.apache.cassandra.cql3.ColumnIdentifier; +import org.apache.cassandra.cql3.statements.schema.IndexTarget; +import org.apache.cassandra.db.marshal.UTF8Type; +import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.index.Index; +import org.apache.cassandra.utils.ClassLoadingTestNonAssignable; +import org.apache.cassandra.utils.ClassLoadingTestSupport; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; public class IndexMetadataTest { @@ -33,4 +44,24 @@ public void testGetDefaultIndexName() Assert.assertEquals("aB4__idx", IndexMetadata.generateDefaultIndexName("a B-4@!_+")); Assert.assertEquals("34_Ddd_F6_idx", IndexMetadata.generateDefaultIndexName("34_()Ddd", new ColumnIdentifier("#F%6*", true))); } + + @Test + public void testRejectsNonCustomIndexWithoutInitializing() + { + TableMetadata table = TableMetadata.builder("ks", "tbl") + .addPartitionKeyColumn("pk", UTF8Type.instance) + .build(); + IndexMetadata index = IndexMetadata.fromSchemaMetadata("idx", + IndexMetadata.Kind.CUSTOM, + Collections.singletonMap(IndexTarget.CUSTOM_INDEX_OPTION_NAME, + ClassLoadingTestNonAssignable.class.getName())); + + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + + assertThatThrownBy(() -> index.validate(table)) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must extend or implement " + Index.class.getName()); + + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } } diff --git a/test/unit/org/apache/cassandra/schema/MemtableFactoryInvalidFieldType.java b/test/unit/org/apache/cassandra/schema/MemtableFactoryInvalidFieldType.java new file mode 100644 index 000000000000..823706fbf262 --- /dev/null +++ b/test/unit/org/apache/cassandra/schema/MemtableFactoryInvalidFieldType.java @@ -0,0 +1,34 @@ +/* + * 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.cassandra.schema; + +import org.apache.cassandra.utils.ClassLoadingTestSupport; + +public final class MemtableFactoryInvalidFieldType +{ + public static final Object FACTORY = new Object(); + + static + { + ClassLoadingTestSupport.markInitialized(MemtableFactoryInvalidFieldType.class); + } + + private MemtableFactoryInvalidFieldType() + { + } +} diff --git a/test/unit/org/apache/cassandra/schema/MemtableFactoryInvalidReturnType.java b/test/unit/org/apache/cassandra/schema/MemtableFactoryInvalidReturnType.java new file mode 100644 index 000000000000..6cb4989cbaf1 --- /dev/null +++ b/test/unit/org/apache/cassandra/schema/MemtableFactoryInvalidReturnType.java @@ -0,0 +1,39 @@ +/* + * 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.cassandra.schema; + +import java.util.Map; + +import org.apache.cassandra.utils.ClassLoadingTestSupport; + +public final class MemtableFactoryInvalidReturnType +{ + static + { + ClassLoadingTestSupport.markInitialized(MemtableFactoryInvalidReturnType.class); + } + + private MemtableFactoryInvalidReturnType() + { + } + + public static Object factory(Map parameters) + { + return new Object(); + } +} diff --git a/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java b/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java index 75094ef5d185..d92a1b2ac685 100644 --- a/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java +++ b/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java @@ -18,6 +18,8 @@ package org.apache.cassandra.schema; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.util.LinkedHashMap; import java.util.Map; @@ -32,6 +34,7 @@ import org.apache.cassandra.config.ParameterizedClass; import org.apache.cassandra.db.memtable.SkipListMemtableFactory; import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.utils.ClassLoadingTestSupport; import org.apache.cassandra.utils.ConfigGenBuilder; import static accord.utils.Property.qt; @@ -52,6 +55,58 @@ public void testDefault() assertEquals(ImmutableMap.of("default", DEFAULT), map); } + @Test + public void testInvalidFactoryMethodDoesNotInitializeClass() throws Exception + { + ClassLoadingTestSupport.assertNotInitialized(MemtableFactoryInvalidReturnType.class); + + try + { + getMemtableFactory(MemtableFactoryInvalidReturnType.class); + fail("Expected exception."); + } + catch (ConfigurationException e) + { + assertThat(e.getMessage()).contains("must return"); + } + + assertThat(ClassLoadingTestSupport.wasInitialized(MemtableFactoryInvalidReturnType.class)).isFalse(); + } + + @Test + public void testInvalidFactoryFieldDoesNotInitializeClass() throws Exception + { + ClassLoadingTestSupport.assertNotInitialized(MemtableFactoryInvalidFieldType.class); + + try + { + getMemtableFactory(MemtableFactoryInvalidFieldType.class); + fail("Expected exception."); + } + catch (ConfigurationException e) + { + assertThat(e.getMessage()).contains("must be of type"); + } + + assertThat(ClassLoadingTestSupport.wasInitialized(MemtableFactoryInvalidFieldType.class)).isFalse(); + } + + private static void getMemtableFactory(Class memtableClass) throws Exception + { + Method method = MemtableParams.class.getDeclaredMethod("getMemtableFactory", ParameterizedClass.class); + method.setAccessible(true); + try + { + method.invoke(null, new ParameterizedClass(memtableClass.getName())); + } + catch (InvocationTargetException e) + { + if (e.getCause() instanceof ConfigurationException) + throw (ConfigurationException) e.getCause(); + throw e; + } + } + @Test public void testDefaultRemapped() { diff --git a/test/unit/org/apache/cassandra/schema/ReplicationParamsTest.java b/test/unit/org/apache/cassandra/schema/ReplicationParamsTest.java new file mode 100644 index 000000000000..119aa4697537 --- /dev/null +++ b/test/unit/org/apache/cassandra/schema/ReplicationParamsTest.java @@ -0,0 +1,87 @@ +/* + * 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.cassandra.schema; + +import java.io.IOException; +import java.util.Collections; + +import org.junit.Test; + +import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.io.util.DataInputBuffer; +import org.apache.cassandra.io.util.DataOutputBuffer; +import org.apache.cassandra.locator.AbstractReplicationStrategy; +import org.apache.cassandra.net.MessagingService; +import org.apache.cassandra.tcm.serialization.Version; +import org.apache.cassandra.utils.ClassLoadingTestNonAssignable; +import org.apache.cassandra.utils.ClassLoadingTestSupport; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +public class ReplicationParamsTest +{ + @Test + public void testRejectsNonReplicationStrategyWithoutInitializing() + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + + assertThatThrownBy(() -> ReplicationParams.fromMap(Collections.singletonMap(ReplicationParams.CLASS, + ClassLoadingTestNonAssignable.class.getName()))) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must extend or implement " + AbstractReplicationStrategy.class.getName()); + + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + + @Test + public void testSerializerRejectsNonReplicationStrategyWithoutInitializing() throws IOException + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + + assertThatThrownBy(() -> ReplicationParams.serializer.deserialize(replicationParamsInput(), Version.V8)) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must extend or implement " + AbstractReplicationStrategy.class.getName()); + + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + + @Test + public void testMessageSerializerRejectsNonReplicationStrategyWithoutInitializing() throws IOException + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + + assertThatThrownBy(() -> ReplicationParams.messageSerializer.deserialize(replicationParamsInput(), + MessagingService.current_version)) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must extend or implement " + AbstractReplicationStrategy.class.getName()); + + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + + private static DataInputBuffer replicationParamsInput() throws IOException + { + try (DataOutputBuffer out = new DataOutputBuffer()) + { + out.writeUTF(ClassLoadingTestNonAssignable.class.getName()); + out.writeUnsignedVInt32(0); + return new DataInputBuffer(out.toByteArray()); + } + } +} diff --git a/test/unit/org/apache/cassandra/security/CipherFactoryTest.java b/test/unit/org/apache/cassandra/security/CipherFactoryTest.java index cb12d585583e..108003a1cc02 100644 --- a/test/unit/org/apache/cassandra/security/CipherFactoryTest.java +++ b/test/unit/org/apache/cassandra/security/CipherFactoryTest.java @@ -35,7 +35,11 @@ import org.junit.Test; import org.apache.cassandra.config.TransparentDataEncryptionOptions; +import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.utils.ClassLoadingTestNonAssignable; +import org.apache.cassandra.utils.ClassLoadingTestSupport; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; @@ -121,6 +125,31 @@ public void buildCipher_DifferentAliases() throws Exception Assert.assertFalse(c1 == c2); } + @Test + public void keyProviderWrongTypeRejectedWithoutInitializing() + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + + TransparentDataEncryptionOptions options = EncryptionContextGenerator.createEncryptionOptions(); + options.key_provider.class_name = ClassLoadingTestNonAssignable.class.getName(); + + try + { + new CipherFactory(options); + fail("Expected CipherFactory construction to be rejected for a non-KeyProvider class"); + } + catch (RuntimeException e) + { + // CipherFactory wraps the load failure; the cause is the type-check ConfigurationException + Throwable cause = e.getCause(); + assertNotNull(cause); + assertThat(cause).isInstanceOf(ConfigurationException.class); + assertThat(cause.getMessage()).contains("must extend or implement " + KeyProvider.class.getName()); + } + + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + @Test(expected = AssertionError.class) public void getDecryptor_NullIv() throws IOException { diff --git a/test/unit/org/apache/cassandra/triggers/TriggerExecutorTest.java b/test/unit/org/apache/cassandra/triggers/TriggerExecutorTest.java index be1c617a7b38..b7430d2d06e3 100644 --- a/test/unit/org/apache/cassandra/triggers/TriggerExecutorTest.java +++ b/test/unit/org/apache/cassandra/triggers/TriggerExecutorTest.java @@ -47,9 +47,11 @@ import org.apache.cassandra.schema.TableMetadata; import org.apache.cassandra.schema.TriggerMetadata; import org.apache.cassandra.schema.Triggers; +import org.apache.cassandra.utils.ClassLoadingTestSupport; import org.apache.cassandra.utils.FBUtilities; import static org.apache.cassandra.utils.ByteBufferUtil.bytes; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; @@ -114,6 +116,18 @@ public void triggerClassNotFound() .withMessageContaining("Trigger class NotExistedTriggerClass couldn't be found."); } + @Test + public void nonTriggerClassRejectedWithoutInitializing() + { + ClassLoadingTestSupport.assertNotInitialized(NonTrigger.class); + + assertThatExceptionOfType(ConfigurationException.class) + .isThrownBy(() -> TriggerExecutor.instance.loadTriggerClass(NonTrigger.class.getName())) + .withMessageContaining("must extend or implement " + ITrigger.class.getName()); + + assertThat(ClassLoadingTestSupport.wasInitialized(NonTrigger.class)).isFalse(); + } + @Test public void noTriggerMutations() throws ConfigurationException, InvalidRequestException { @@ -326,6 +340,18 @@ public Collection augment(Partition partition) } } + public static class NonTrigger + { + static + { + ClassLoadingTestSupport.markInitialized(NonTrigger.class); + } + + public NonTrigger() + { + } + } + public static class SameKeySameCfTrigger implements ITrigger { public Collection augment(Partition partition) diff --git a/test/unit/org/apache/cassandra/utils/ClassLoadingSearchProbe.java b/test/unit/org/apache/cassandra/utils/ClassLoadingSearchProbe.java new file mode 100644 index 000000000000..401d89eb3663 --- /dev/null +++ b/test/unit/org/apache/cassandra/utils/ClassLoadingSearchProbe.java @@ -0,0 +1,33 @@ +/* + * 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.cassandra.utils; + +/** + * Search-package fall-through probe. Shares its simple name ({@code ClassLoadingSearchProbe}) with + * {@link org.apache.cassandra.config.ClassLoadingSearchProbe}, but is NOT a {@link Runnable}, so it represents the + * "wrong type under an earlier search package" case. Its static initializer records initialization so tests can + * confirm the search resolves without initializing this class. + */ +public final class ClassLoadingSearchProbe +{ + static + { + ClassLoadingTestSupport.markInitialized(ClassLoadingSearchProbe.class); + } +} diff --git a/test/unit/org/apache/cassandra/utils/ClassLoadingTestNonAssignable.java b/test/unit/org/apache/cassandra/utils/ClassLoadingTestNonAssignable.java new file mode 100644 index 000000000000..8faa92286adb --- /dev/null +++ b/test/unit/org/apache/cassandra/utils/ClassLoadingTestNonAssignable.java @@ -0,0 +1,31 @@ +/* + * 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.cassandra.utils; + +public final class ClassLoadingTestNonAssignable +{ + static + { + ClassLoadingTestSupport.markInitialized(ClassLoadingTestNonAssignable.class); + } + + private ClassLoadingTestNonAssignable() + { + } +} diff --git a/test/unit/org/apache/cassandra/utils/ClassLoadingTestSupport.java b/test/unit/org/apache/cassandra/utils/ClassLoadingTestSupport.java new file mode 100644 index 000000000000..41d76473f9f4 --- /dev/null +++ b/test/unit/org/apache/cassandra/utils/ClassLoadingTestSupport.java @@ -0,0 +1,47 @@ +/* + * 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.cassandra.utils; + +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +public final class ClassLoadingTestSupport +{ + private static final Set initializedClasses = ConcurrentHashMap.newKeySet(); + + private ClassLoadingTestSupport() + { + } + + public static void markInitialized(Class initializedClass) + { + initializedClasses.add(initializedClass.getName()); + } + + public static boolean wasInitialized(Class initializedClass) + { + return initializedClasses.contains(initializedClass.getName()); + } + + public static void assertNotInitialized(Class initializedClass) + { + if (wasInitialized(initializedClass)) + throw new AssertionError(initializedClass.getName() + " has already been initialized"); + } +} diff --git a/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java b/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java index c9451730f312..d5b491ddbf17 100644 --- a/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java +++ b/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java @@ -46,6 +46,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.cassandra.audit.IAuditLogger; import org.apache.cassandra.config.Config; import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.db.marshal.AbstractType; @@ -59,6 +60,9 @@ import org.apache.cassandra.dht.Murmur3Partitioner; import org.apache.cassandra.dht.OrderPreservingPartitioner; import org.apache.cassandra.dht.RandomPartitioner; +import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.security.AbstractCryptoProvider; +import org.apache.cassandra.security.ISslContextFactory; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; @@ -69,6 +73,109 @@ public class FBUtilitiesTest public static final Logger LOGGER = LoggerFactory.getLogger(FBUtilitiesTest.class); + @Test + public void testTypedClassForNameRejectsWithoutInitializing() + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + try + { + FBUtilities.classForNameWithoutInitialization(ClassLoadingTestNonAssignable.class.getName(), "test class", Runnable.class); + fail("Should not pass"); + } + catch (ConfigurationException e) + { + assertThat(e.getMessage()).contains("must extend or implement " + Runnable.class.getName()); + } + + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + + @Test + public void testTypedConstructRejectsWithoutInitializing() + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + try + { + FBUtilities.construct(ClassLoadingTestNonAssignable.class.getName(), "test class", Runnable.class); + fail("Should not pass"); + } + catch (ConfigurationException e) + { + assertThat(e.getMessage()).contains("must extend or implement " + Runnable.class.getName()); + } + + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + + @Test + public void testTypedInstanceOrConstructRejectsWithoutInitializing() + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + try + { + FBUtilities.instanceOrConstruct(ClassLoadingTestNonAssignable.class.getName(), "test class", Runnable.class); + fail("Should not pass"); + } + catch (ConfigurationException e) + { + assertThat(e.getMessage()).contains("must extend or implement " + Runnable.class.getName()); + } + + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + + @Test + public void testNewAuditLoggerRejectsWrongTypeWithoutInitializing() + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + try + { + FBUtilities.newAuditLogger(ClassLoadingTestNonAssignable.class.getName(), Map.of()); + fail("Should not pass"); + } + catch (ConfigurationException e) + { + assertThat(e).hasRootCauseInstanceOf(ConfigurationException.class); + assertThat(e).hasStackTraceContaining("must extend or implement " + IAuditLogger.class.getName()); + } + + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + + @Test + public void testNewSslContextFactoryRejectsWrongTypeWithoutInitializing() + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + try + { + FBUtilities.newSslContextFactory(ClassLoadingTestNonAssignable.class.getName(), Map.of()); + fail("Should not pass"); + } + catch (ConfigurationException e) + { + assertThat(e).hasStackTraceContaining("must extend or implement " + ISslContextFactory.class.getName()); + } + + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + + @Test + public void testNewCryptoProviderRejectsWrongTypeWithoutInitializing() + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + try + { + FBUtilities.newCryptoProvider(ClassLoadingTestNonAssignable.class.getName(), Map.of()); + fail("Should not pass"); + } + catch (ConfigurationException e) + { + assertThat(e.getMessage()).contains("must extend or implement " + AbstractCryptoProvider.class.getName()); + } + + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + @Test public void testCompareByteSubArrays() { diff --git a/tools/sstableloader/src/org/apache/cassandra/tools/LoaderOptions.java b/tools/sstableloader/src/org/apache/cassandra/tools/LoaderOptions.java index 42da2d5af168..e7af62589516 100644 --- a/tools/sstableloader/src/org/apache/cassandra/tools/LoaderOptions.java +++ b/tools/sstableloader/src/org/apache/cassandra/tools/LoaderOptions.java @@ -50,6 +50,7 @@ import org.apache.cassandra.exceptions.ConfigurationException; import org.apache.cassandra.io.util.File; import org.apache.cassandra.locator.InetAddressAndPort; +import org.apache.cassandra.utils.FBUtilities; import static org.apache.cassandra.config.DataRateSpec.DataRateUnit.MEBIBYTES_PER_SECOND; import static org.apache.cassandra.config.EncryptionOptions.ClientEncryptionOptions.ClientAuth.REQUIRED; @@ -715,11 +716,12 @@ private void constructAuthProvider() { try { - Class authProviderClass = Class.forName(authProviderName); - Constructor constructor = authProviderClass.getConstructor(String.class, String.class); - authProvider = (AuthProvider)constructor.newInstance(user, passwd); + Class authProviderClass = + FBUtilities.classForNameWithoutInitialization(authProviderName, "auth provider", AuthProvider.class); + Constructor constructor = authProviderClass.getConstructor(String.class, String.class); + authProvider = constructor.newInstance(user, passwd); } - catch (ClassNotFoundException e) + catch (ConfigurationException e) { errorMsg("Unknown auth provider: " + e.getMessage(), getCmdLineOptions()); } @@ -746,9 +748,9 @@ else if (authProviderName != null) { try { - authProvider = (AuthProvider)Class.forName(authProviderName).newInstance(); + authProvider = FBUtilities.construct(authProviderName, "auth provider", AuthProvider.class); } - catch (ClassNotFoundException | InstantiationException | IllegalAccessException e) + catch (ConfigurationException e) { errorMsg("Unknown auth provider: " + e.getMessage(), getCmdLineOptions()); } diff --git a/tools/stress/src/org/apache/cassandra/stress/settings/OptionReplication.java b/tools/stress/src/org/apache/cassandra/stress/settings/OptionReplication.java index 141c64507964..ff2d2ef2cc36 100644 --- a/tools/stress/src/org/apache/cassandra/stress/settings/OptionReplication.java +++ b/tools/stress/src/org/apache/cassandra/stress/settings/OptionReplication.java @@ -28,6 +28,7 @@ import com.google.common.base.Function; import org.apache.cassandra.locator.AbstractReplicationStrategy; +import org.apache.cassandra.utils.FBUtilities; /** * For specifying replication options @@ -76,9 +77,9 @@ public String apply(String name) { try { - Class clazz = Class.forName(fullname); - if (!AbstractReplicationStrategy.class.isAssignableFrom(clazz)) - throw new IllegalArgumentException(clazz + " is not a replication strategy"); + FBUtilities.classForNameWithoutInitialization(fullname, + "replication strategy", + AbstractReplicationStrategy.class); strategy = fullname; break; } catch (Exception ignore) diff --git a/tools/stress/src/org/apache/cassandra/stress/settings/SettingsMode.java b/tools/stress/src/org/apache/cassandra/stress/settings/SettingsMode.java index 89c26a874e48..c59044a36119 100644 --- a/tools/stress/src/org/apache/cassandra/stress/settings/SettingsMode.java +++ b/tools/stress/src/org/apache/cassandra/stress/settings/SettingsMode.java @@ -33,6 +33,7 @@ import com.datastax.driver.core.ProtocolVersion; import org.apache.cassandra.stress.util.ResultLogger; +import org.apache.cassandra.utils.FBUtilities; import static java.lang.String.format; import static org.apache.cassandra.stress.settings.SettingsCredentials.CQL_PASSWORD_PROPERTY_KEY; @@ -91,9 +92,10 @@ public SettingsMode(GroupedOptions options, SettingsCredentials credentials) { try { - Class clazz = Class.forName(authProviderClassname); - if (!AuthProvider.class.isAssignableFrom(clazz)) - throw new IllegalArgumentException(clazz + " is not a valid auth provider"); + Class clazz = + FBUtilities.classForNameWithoutInitialization(authProviderClassname, + "auth provider", + AuthProvider.class); // check we can instantiate it if (PlainTextAuthProvider.class.equals(clazz)) { From 69c103e399274995d72ff639855f5dd446b1c036 Mon Sep 17 00:00:00 2001 From: Jeremiah Jordan Date: Thu, 16 Jul 2026 07:23:32 -0500 Subject: [PATCH 2/6] Resolve the trigger constructor once when loading trigger classes --- .../cassandra/triggers/TriggerExecutor.java | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/java/org/apache/cassandra/triggers/TriggerExecutor.java b/src/java/org/apache/cassandra/triggers/TriggerExecutor.java index 623197969576..f4e97982147b 100644 --- a/src/java/org/apache/cassandra/triggers/TriggerExecutor.java +++ b/src/java/org/apache/cassandra/triggers/TriggerExecutor.java @@ -292,13 +292,20 @@ public synchronized Class loadTriggerClass(String triggerCla // CreateTriggerStatement#apply. // Check that triggerClass is available on the classpath, but do not initialize the class since that would // execute static blocks. - Class trigger; + Class trigger = loadTriggerClassWithoutInitialization(triggerClass); + // Validate that the class exposes a public no-argument constructor before it is accepted. + trigger.getConstructor(); + return trigger; + } + + private Class loadTriggerClassWithoutInitialization(String triggerClass) throws Exception + { try { - trigger = FBUtilities.classForNameWithoutInitialization(triggerClass, - "trigger", - ITrigger.class, - customClassLoader); + return FBUtilities.classForNameWithoutInitialization(triggerClass, + "trigger", + ITrigger.class, + customClassLoader); } catch (ConfigurationException e) { @@ -306,8 +313,6 @@ public synchronized Class loadTriggerClass(String triggerCla throw (ClassNotFoundException) e.getCause(); throw e; } - trigger.getConstructor(); - return trigger; } public synchronized ITrigger loadTriggerInstance(String triggerClass) throws Exception @@ -321,6 +326,6 @@ public synchronized ITrigger loadTriggerInstance(String triggerClass) throws Exc // double check. if (cachedTriggers.get(triggerClass) != null) return cachedTriggers.get(triggerClass); - return loadTriggerClass(triggerClass).getConstructor().newInstance(); + return loadTriggerClassWithoutInitialization(triggerClass).getConstructor().newInstance(); } } From 0140d9a5fa848b8a732d48f6a3399367c7efe610 Mon Sep 17 00:00:00 2001 From: Jeremiah Jordan Date: Thu, 16 Jul 2026 07:23:32 -0500 Subject: [PATCH 3/6] Remove redundant intermediate casts in reflective type resolution --- src/java/org/apache/cassandra/db/marshal/TypeParser.java | 6 +++--- .../cassandra/locator/AbstractReplicationStrategy.java | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/java/org/apache/cassandra/db/marshal/TypeParser.java b/src/java/org/apache/cassandra/db/marshal/TypeParser.java index 8e2da14142ca..d3a20fc7ca7f 100644 --- a/src/java/org/apache/cassandra/db/marshal/TypeParser.java +++ b/src/java/org/apache/cassandra/db/marshal/TypeParser.java @@ -496,9 +496,9 @@ private static Class> getAbstractTypeClass(String comp // access or getInstance(TypeParser) invocation below performs the initialization for valid types. @SuppressWarnings("unchecked") Class> typeClass = - (Class>)(Class) FBUtilities.classForNameWithoutInitialization(className, - "abstract-type", - AbstractType.class); + (Class>) FBUtilities.classForNameWithoutInitialization(className, + "abstract-type", + AbstractType.class); return typeClass; } diff --git a/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java b/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java index 395c59f835f5..4be3f0e60579 100644 --- a/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java +++ b/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java @@ -342,9 +342,9 @@ public static Class getClass(String cls) throws Con @SuppressWarnings("unchecked") Class strategyClass = - (Class)(Class) FBUtilities.classForNameWithoutInitialization(className, - "replication strategy", - AbstractReplicationStrategy.class); + (Class) FBUtilities.classForNameWithoutInitialization(className, + "replication strategy", + AbstractReplicationStrategy.class); return strategyClass; } From 13c3d8dee8d9f358f0def23224c58620e0980c47 Mon Sep 17 00:00:00 2001 From: Jeremiah Jordan Date: Thu, 16 Jul 2026 08:30:19 -0500 Subject: [PATCH 4/6] Use assertThatThrownBy in reflective class-loading tests --- .../cassandra/db/marshal/TypeParserTest.java | 13 +--- .../diag/DiagnosticEventPersistenceTest.java | 14 +--- .../index/SecondaryIndexManagerTest.java | 13 +--- .../index/sasi/conf/IndexModeTest.java | 26 ++----- .../cassandra/schema/MemtableParamsTest.java | 25 ++----- .../cassandra/security/CipherFactoryTest.java | 19 ++--- .../cassandra/utils/FBUtilitiesTest.java | 75 +++++-------------- 7 files changed, 53 insertions(+), 132 deletions(-) diff --git a/test/unit/org/apache/cassandra/db/marshal/TypeParserTest.java b/test/unit/org/apache/cassandra/db/marshal/TypeParserTest.java index 9588c72bd21d..fa50c1d02a83 100644 --- a/test/unit/org/apache/cassandra/db/marshal/TypeParserTest.java +++ b/test/unit/org/apache/cassandra/db/marshal/TypeParserTest.java @@ -36,6 +36,7 @@ import org.apache.cassandra.utils.ClassLoadingTestNonAssignable; import org.apache.cassandra.utils.ClassLoadingTestSupport; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -108,15 +109,9 @@ public void testParseError() public void testRejectsNonAbstractTypeWithoutInitializing() throws SyntaxException { ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); - try - { - TypeParser.parse(ClassLoadingTestNonAssignable.class.getName()); - fail("Should not pass"); - } - catch (ConfigurationException e) - { - assertTrue(e.getMessage().contains("must extend or implement " + AbstractType.class.getName())); - } + assertThatThrownBy(() -> TypeParser.parse(ClassLoadingTestNonAssignable.class.getName())) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must extend or implement " + AbstractType.class.getName()); assertFalse(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)); } diff --git a/test/unit/org/apache/cassandra/diag/DiagnosticEventPersistenceTest.java b/test/unit/org/apache/cassandra/diag/DiagnosticEventPersistenceTest.java index b18d7b6aac11..9315054d617e 100644 --- a/test/unit/org/apache/cassandra/diag/DiagnosticEventPersistenceTest.java +++ b/test/unit/org/apache/cassandra/diag/DiagnosticEventPersistenceTest.java @@ -28,7 +28,7 @@ import org.apache.cassandra.utils.ClassLoadingTestSupport; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.Assert.fail; +import static org.assertj.core.api.Assertions.assertThatThrownBy; public class DiagnosticEventPersistenceTest { @@ -55,15 +55,9 @@ public void testNonDiagnosticEventRejectedWithoutInitializing() throws Throwable { ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); - try - { - getEventClass(ClassLoadingTestNonAssignable.class.getName()); - fail("Expected InvalidClassException for a non-DiagnosticEvent class"); - } - catch (InvalidClassException e) - { - assertThat(e.getMessage()).contains("must be of type DiagnosticEvent"); - } + assertThatThrownBy(() -> getEventClass(ClassLoadingTestNonAssignable.class.getName())) + .isInstanceOf(InvalidClassException.class) + .hasMessageContaining("must be of type DiagnosticEvent"); assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); } diff --git a/test/unit/org/apache/cassandra/index/SecondaryIndexManagerTest.java b/test/unit/org/apache/cassandra/index/SecondaryIndexManagerTest.java index bc1604a5c6b2..5bda8313e779 100644 --- a/test/unit/org/apache/cassandra/index/SecondaryIndexManagerTest.java +++ b/test/unit/org/apache/cassandra/index/SecondaryIndexManagerTest.java @@ -48,6 +48,7 @@ import org.apache.cassandra.utils.KillerForTests; import org.apache.cassandra.utils.concurrent.Refs; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -65,15 +66,9 @@ public void after() public void rejectsNonIndexClassWithoutInitializing() { ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); - try - { - SecondaryIndexManager.loadIndexClass(ClassLoadingTestNonAssignable.class.getName()); - fail("Should not pass"); - } - catch (ConfigurationException e) - { - assertTrue(e.getMessage().contains("must extend or implement " + Index.class.getName())); - } + assertThatThrownBy(() -> SecondaryIndexManager.loadIndexClass(ClassLoadingTestNonAssignable.class.getName())) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must extend or implement " + Index.class.getName()); assertFalse(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)); } diff --git a/test/unit/org/apache/cassandra/index/sasi/conf/IndexModeTest.java b/test/unit/org/apache/cassandra/index/sasi/conf/IndexModeTest.java index 63e039558825..b4a02c92baa6 100644 --- a/test/unit/org/apache/cassandra/index/sasi/conf/IndexModeTest.java +++ b/test/unit/org/apache/cassandra/index/sasi/conf/IndexModeTest.java @@ -40,6 +40,8 @@ import org.apache.cassandra.utils.ClassLoadingTestNonAssignable; import org.apache.cassandra.utils.ClassLoadingTestSupport; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + public class IndexModeTest { @@ -218,15 +220,9 @@ public void test_bytesType_rejectsNonAnalyzerWithoutInitializing() ColumnMetadata cd = ColumnMetadata.regularColumn(cfm, ByteBufferUtil.bytes("TestColumnMetadata"), BytesType.instance, ColumnMetadata.NO_UNIQUE_ID); ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); - try - { - IndexMode.getMode(cd, Collections.singletonMap("analyzer_class", ClassLoadingTestNonAssignable.class.getName())); - Assert.fail("Should not pass"); - } - catch (ConfigurationException e) - { - Assert.assertTrue(e.getMessage().contains("must extend or implement " + AbstractAnalyzer.class.getName())); - } + assertThatThrownBy(() -> IndexMode.getMode(cd, Collections.singletonMap("analyzer_class", ClassLoadingTestNonAssignable.class.getName()))) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must extend or implement " + AbstractAnalyzer.class.getName()); Assert.assertFalse(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)); } @@ -250,15 +246,9 @@ public void test_validateAnalyzer_rejectsNonAnalyzerWithoutInitializing() ColumnMetadata cd = ColumnMetadata.regularColumn(cfm, ByteBufferUtil.bytes("TestColumnMetadata"), UTF8Type.instance, ColumnMetadata.NO_UNIQUE_ID); ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); - try - { - IndexMode.validateAnalyzer(Collections.singletonMap("analyzer_class", ClassLoadingTestNonAssignable.class.getName()), cd); - Assert.fail("Should not pass"); - } - catch (ConfigurationException e) - { - Assert.assertTrue(e.getMessage().contains("must extend or implement " + AbstractAnalyzer.class.getName())); - } + assertThatThrownBy(() -> IndexMode.validateAnalyzer(Collections.singletonMap("analyzer_class", ClassLoadingTestNonAssignable.class.getName()), cd)) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must extend or implement " + AbstractAnalyzer.class.getName()); Assert.assertFalse(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)); } diff --git a/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java b/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java index d92a1b2ac685..d87077b067d2 100644 --- a/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java +++ b/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java @@ -40,6 +40,7 @@ import static accord.utils.Property.qt; import static org.apache.cassandra.config.YamlConfigurationLoader.fromMap; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -60,15 +61,9 @@ public void testInvalidFactoryMethodDoesNotInitializeClass() throws Exception { ClassLoadingTestSupport.assertNotInitialized(MemtableFactoryInvalidReturnType.class); - try - { - getMemtableFactory(MemtableFactoryInvalidReturnType.class); - fail("Expected exception."); - } - catch (ConfigurationException e) - { - assertThat(e.getMessage()).contains("must return"); - } + assertThatThrownBy(() -> getMemtableFactory(MemtableFactoryInvalidReturnType.class)) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must return"); assertThat(ClassLoadingTestSupport.wasInitialized(MemtableFactoryInvalidReturnType.class)).isFalse(); } @@ -78,15 +73,9 @@ public void testInvalidFactoryFieldDoesNotInitializeClass() throws Exception { ClassLoadingTestSupport.assertNotInitialized(MemtableFactoryInvalidFieldType.class); - try - { - getMemtableFactory(MemtableFactoryInvalidFieldType.class); - fail("Expected exception."); - } - catch (ConfigurationException e) - { - assertThat(e.getMessage()).contains("must be of type"); - } + assertThatThrownBy(() -> getMemtableFactory(MemtableFactoryInvalidFieldType.class)) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must be of type"); assertThat(ClassLoadingTestSupport.wasInitialized(MemtableFactoryInvalidFieldType.class)).isFalse(); } diff --git a/test/unit/org/apache/cassandra/security/CipherFactoryTest.java b/test/unit/org/apache/cassandra/security/CipherFactoryTest.java index 108003a1cc02..4115af05751e 100644 --- a/test/unit/org/apache/cassandra/security/CipherFactoryTest.java +++ b/test/unit/org/apache/cassandra/security/CipherFactoryTest.java @@ -40,6 +40,7 @@ import org.apache.cassandra.utils.ClassLoadingTestSupport; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; @@ -133,19 +134,11 @@ public void keyProviderWrongTypeRejectedWithoutInitializing() TransparentDataEncryptionOptions options = EncryptionContextGenerator.createEncryptionOptions(); options.key_provider.class_name = ClassLoadingTestNonAssignable.class.getName(); - try - { - new CipherFactory(options); - fail("Expected CipherFactory construction to be rejected for a non-KeyProvider class"); - } - catch (RuntimeException e) - { - // CipherFactory wraps the load failure; the cause is the type-check ConfigurationException - Throwable cause = e.getCause(); - assertNotNull(cause); - assertThat(cause).isInstanceOf(ConfigurationException.class); - assertThat(cause.getMessage()).contains("must extend or implement " + KeyProvider.class.getName()); - } + // CipherFactory wraps the load failure; the cause is the type-check ConfigurationException + assertThatThrownBy(() -> new CipherFactory(options)) + .isInstanceOf(RuntimeException.class) + .hasCauseInstanceOf(ConfigurationException.class) + .hasStackTraceContaining("must extend or implement " + KeyProvider.class.getName()); assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); } diff --git a/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java b/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java index d5b491ddbf17..171fb729eb3b 100644 --- a/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java +++ b/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java @@ -65,6 +65,7 @@ import org.apache.cassandra.security.ISslContextFactory; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; @@ -77,15 +78,9 @@ public class FBUtilitiesTest public void testTypedClassForNameRejectsWithoutInitializing() { ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); - try - { - FBUtilities.classForNameWithoutInitialization(ClassLoadingTestNonAssignable.class.getName(), "test class", Runnable.class); - fail("Should not pass"); - } - catch (ConfigurationException e) - { - assertThat(e.getMessage()).contains("must extend or implement " + Runnable.class.getName()); - } + assertThatThrownBy(() -> FBUtilities.classForNameWithoutInitialization(ClassLoadingTestNonAssignable.class.getName(), "test class", Runnable.class)) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must extend or implement " + Runnable.class.getName()); assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); } @@ -94,15 +89,9 @@ public void testTypedClassForNameRejectsWithoutInitializing() public void testTypedConstructRejectsWithoutInitializing() { ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); - try - { - FBUtilities.construct(ClassLoadingTestNonAssignable.class.getName(), "test class", Runnable.class); - fail("Should not pass"); - } - catch (ConfigurationException e) - { - assertThat(e.getMessage()).contains("must extend or implement " + Runnable.class.getName()); - } + assertThatThrownBy(() -> FBUtilities.construct(ClassLoadingTestNonAssignable.class.getName(), "test class", Runnable.class)) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must extend or implement " + Runnable.class.getName()); assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); } @@ -111,15 +100,9 @@ public void testTypedConstructRejectsWithoutInitializing() public void testTypedInstanceOrConstructRejectsWithoutInitializing() { ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); - try - { - FBUtilities.instanceOrConstruct(ClassLoadingTestNonAssignable.class.getName(), "test class", Runnable.class); - fail("Should not pass"); - } - catch (ConfigurationException e) - { - assertThat(e.getMessage()).contains("must extend or implement " + Runnable.class.getName()); - } + assertThatThrownBy(() -> FBUtilities.instanceOrConstruct(ClassLoadingTestNonAssignable.class.getName(), "test class", Runnable.class)) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must extend or implement " + Runnable.class.getName()); assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); } @@ -128,16 +111,10 @@ public void testTypedInstanceOrConstructRejectsWithoutInitializing() public void testNewAuditLoggerRejectsWrongTypeWithoutInitializing() { ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); - try - { - FBUtilities.newAuditLogger(ClassLoadingTestNonAssignable.class.getName(), Map.of()); - fail("Should not pass"); - } - catch (ConfigurationException e) - { - assertThat(e).hasRootCauseInstanceOf(ConfigurationException.class); - assertThat(e).hasStackTraceContaining("must extend or implement " + IAuditLogger.class.getName()); - } + assertThatThrownBy(() -> FBUtilities.newAuditLogger(ClassLoadingTestNonAssignable.class.getName(), Map.of())) + .isInstanceOf(ConfigurationException.class) + .hasRootCauseInstanceOf(ConfigurationException.class) + .hasStackTraceContaining("must extend or implement " + IAuditLogger.class.getName()); assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); } @@ -146,15 +123,9 @@ public void testNewAuditLoggerRejectsWrongTypeWithoutInitializing() public void testNewSslContextFactoryRejectsWrongTypeWithoutInitializing() { ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); - try - { - FBUtilities.newSslContextFactory(ClassLoadingTestNonAssignable.class.getName(), Map.of()); - fail("Should not pass"); - } - catch (ConfigurationException e) - { - assertThat(e).hasStackTraceContaining("must extend or implement " + ISslContextFactory.class.getName()); - } + assertThatThrownBy(() -> FBUtilities.newSslContextFactory(ClassLoadingTestNonAssignable.class.getName(), Map.of())) + .isInstanceOf(ConfigurationException.class) + .hasStackTraceContaining("must extend or implement " + ISslContextFactory.class.getName()); assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); } @@ -163,15 +134,9 @@ public void testNewSslContextFactoryRejectsWrongTypeWithoutInitializing() public void testNewCryptoProviderRejectsWrongTypeWithoutInitializing() { ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); - try - { - FBUtilities.newCryptoProvider(ClassLoadingTestNonAssignable.class.getName(), Map.of()); - fail("Should not pass"); - } - catch (ConfigurationException e) - { - assertThat(e.getMessage()).contains("must extend or implement " + AbstractCryptoProvider.class.getName()); - } + assertThatThrownBy(() -> FBUtilities.newCryptoProvider(ClassLoadingTestNonAssignable.class.getName(), Map.of())) + .isInstanceOf(ConfigurationException.class) + .hasMessageContaining("must extend or implement " + AbstractCryptoProvider.class.getName()); assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); } From e94bc8890a2acb0caf89ff891dedd08c09b81765 Mon Sep 17 00:00:00 2001 From: Jeremiah Jordan Date: Thu, 16 Jul 2026 08:30:31 -0500 Subject: [PATCH 5/6] Verify compression provider type before initialization --- src/java/org/apache/cassandra/utils/FBUtilities.java | 5 +++-- .../org/apache/cassandra/utils/FBUtilitiesTest.java | 12 ++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/java/org/apache/cassandra/utils/FBUtilities.java b/src/java/org/apache/cassandra/utils/FBUtilities.java index 0e4c3281a4d7..8ef98363d204 100644 --- a/src/java/org/apache/cassandra/utils/FBUtilities.java +++ b/src/java/org/apache/cassandra/utils/FBUtilities.java @@ -744,8 +744,9 @@ public static AbstractCompressionProvider newCompressionProvider(String classNam if (!className.contains(".")) className = "org.apache.cassandra.io.compress." + className; - Class compressionProviderClass = FBUtilities.classForName(className, "compression service provider"); - return (AbstractCompressionProvider) compressionProviderClass.getConstructor().newInstance(); + Class compressionProviderClass = + FBUtilities.classForNameWithoutInitialization(className, "compression service provider", AbstractCompressionProvider.class); + return compressionProviderClass.getConstructor().newInstance(); } catch (ConfigurationException e) { diff --git a/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java b/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java index 171fb729eb3b..00e94230a02d 100644 --- a/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java +++ b/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java @@ -61,6 +61,7 @@ import org.apache.cassandra.dht.OrderPreservingPartitioner; import org.apache.cassandra.dht.RandomPartitioner; import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.io.compress.AbstractCompressionProvider; import org.apache.cassandra.security.AbstractCryptoProvider; import org.apache.cassandra.security.ISslContextFactory; @@ -141,6 +142,17 @@ public void testNewCryptoProviderRejectsWrongTypeWithoutInitializing() assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); } + @Test + public void testNewCompressionProviderRejectsWrongTypeWithoutInitializing() + { + ClassLoadingTestSupport.assertNotInitialized(ClassLoadingTestNonAssignable.class); + assertThatThrownBy(() -> FBUtilities.newCompressionProvider(ClassLoadingTestNonAssignable.class.getName())) + .isInstanceOf(ConfigurationException.class) + .hasStackTraceContaining("must extend or implement " + AbstractCompressionProvider.class.getName()); + + assertThat(ClassLoadingTestSupport.wasInitialized(ClassLoadingTestNonAssignable.class)).isFalse(); + } + @Test public void testCompareByteSubArrays() { From 963e652032fff520f137a0ab3af20f9b105983d4 Mon Sep 17 00:00:00 2001 From: Jeremiah Jordan Date: Fri, 17 Jul 2026 08:14:06 -0500 Subject: [PATCH 6/6] Preserve existing error messages and exception causes when loading classes without initialization Keep the reflectively-loaded classes' failure messages and exception cause chains identical to the previous behavior so existing tests continue to pass: route the memtable factory type mismatch through the existing catch that reports "Could not create memtable factory for class", restore the original "Could not create Compression for type" message when a compressor class is missing, and surface the underlying ClassNotFoundException as the direct cause of an unresolvable ISslContextFactory. --- .../apache/cassandra/schema/CompressionParams.java | 11 ++++++++++- .../org/apache/cassandra/schema/MemtableParams.java | 8 ++++---- src/java/org/apache/cassandra/utils/FBUtilities.java | 5 ++++- .../apache/cassandra/schema/MemtableParamsTest.java | 4 ++-- .../apache/cassandra/security/CryptoProviderTest.java | 2 +- 5 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/java/org/apache/cassandra/schema/CompressionParams.java b/src/java/org/apache/cassandra/schema/CompressionParams.java index aea2dd4dadb4..078f58d8f757 100644 --- a/src/java/org/apache/cassandra/schema/CompressionParams.java +++ b/src/java/org/apache/cassandra/schema/CompressionParams.java @@ -309,7 +309,16 @@ private static Class parseCompressorClass(String classNam return null; className = className.contains(".") ? className : "org.apache.cassandra.io.compress." + className; - return FBUtilities.classForNameWithoutInitialization(className, "compression", ICompressor.class); + try + { + return FBUtilities.classForNameWithoutInitialization(className, "compression", ICompressor.class); + } + catch (ConfigurationException e) + { + if (e.getCause() instanceof ClassNotFoundException || e.getCause() instanceof NoClassDefFoundError) + throw new ConfigurationException("Could not create Compression for type " + className, e); + throw e; + } } private static ICompressor createCompressor(Class compressorClass, Map compressionOptions) throws ConfigurationException diff --git a/src/java/org/apache/cassandra/schema/MemtableParams.java b/src/java/org/apache/cassandra/schema/MemtableParams.java index 5491b105cf63..c2111b9b0b50 100644 --- a/src/java/org/apache/cassandra/schema/MemtableParams.java +++ b/src/java/org/apache/cassandra/schema/MemtableParams.java @@ -236,8 +236,8 @@ private static Memtable.Factory getMemtableFactory(ParameterizedClass options) { Method factoryMethod = clazz.getDeclaredMethod("factory", Map.class); if (!Memtable.Factory.class.isAssignableFrom(factoryMethod.getReturnType())) - throw new ConfigurationException("Memtable factory method on " + className + - " must return " + Memtable.Factory.class.getName()); + throw new ClassCastException("Memtable factory method on " + className + + " must return " + Memtable.Factory.class.getName()); factory = (Memtable.Factory) factoryMethod.invoke(null, parametersCopy); } catch (NoSuchMethodException e) @@ -245,8 +245,8 @@ private static Memtable.Factory getMemtableFactory(ParameterizedClass options) // continue with FACTORY field Field factoryField = clazz.getDeclaredField("FACTORY"); if (!Memtable.Factory.class.isAssignableFrom(factoryField.getType())) - throw new ConfigurationException("Memtable FACTORY field on " + className + - " must be of type " + Memtable.Factory.class.getName()); + throw new ClassCastException("Memtable FACTORY field on " + className + + " must be of type " + Memtable.Factory.class.getName()); factory = (Memtable.Factory) factoryField.get(null); } if (!parametersCopy.isEmpty()) diff --git a/src/java/org/apache/cassandra/utils/FBUtilities.java b/src/java/org/apache/cassandra/utils/FBUtilities.java index 8ef98363d204..8bf13f9ea51d 100644 --- a/src/java/org/apache/cassandra/utils/FBUtilities.java +++ b/src/java/org/apache/cassandra/utils/FBUtilities.java @@ -712,7 +712,10 @@ public static ISslContextFactory newSslContextFactory(String className, Map getMemtableFactory(MemtableFactoryInvalidReturnType.class)) .isInstanceOf(ConfigurationException.class) - .hasMessageContaining("must return"); + .hasStackTraceContaining("must return"); assertThat(ClassLoadingTestSupport.wasInitialized(MemtableFactoryInvalidReturnType.class)).isFalse(); } @@ -75,7 +75,7 @@ public void testInvalidFactoryFieldDoesNotInitializeClass() throws Exception assertThatThrownBy(() -> getMemtableFactory(MemtableFactoryInvalidFieldType.class)) .isInstanceOf(ConfigurationException.class) - .hasMessageContaining("must be of type"); + .hasStackTraceContaining("must be of type"); assertThat(ClassLoadingTestSupport.wasInitialized(MemtableFactoryInvalidFieldType.class)).isFalse(); } diff --git a/test/unit/org/apache/cassandra/security/CryptoProviderTest.java b/test/unit/org/apache/cassandra/security/CryptoProviderTest.java index 2d9d8ae0035e..edf90acca1a7 100644 --- a/test/unit/org/apache/cassandra/security/CryptoProviderTest.java +++ b/test/unit/org/apache/cassandra/security/CryptoProviderTest.java @@ -167,7 +167,7 @@ public void testUnableToCreateDefaultCryptoProvider() DatabaseDescriptor.getRawConfig().crypto_provider = new ParameterizedClass(DefaultCryptoProvider.class.getName(), of("k1", "v1", "k2", "v2")); - fbUtilitiesMock.when(() -> FBUtilities.classForName(DefaultCryptoProvider.class.getName(), "crypto provider class")) + fbUtilitiesMock.when(() -> FBUtilities.classForNameWithoutInitialization(DefaultCryptoProvider.class.getName(), "crypto provider class", AbstractCryptoProvider.class)) .thenThrow(new RuntimeException("exception from test")); fbUtilitiesMock.when(() -> FBUtilities.newCryptoProvider(anyString(), anyMap())).thenCallRealMethod();