From a94e4c5192f388873de99246f3ffdfe4b6c7790f Mon Sep 17 00:00:00 2001 From: Jeremiah Jordan Date: Tue, 23 Jun 2026 12:54:32 -0500 Subject: [PATCH] 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 86e1f626ff..52182afe31 100644 --- a/src/java/org/apache/cassandra/auth/AuthConfig.java +++ b/src/java/org/apache/cassandra/auth/AuthConfig.java @@ -63,7 +63,7 @@ public final class AuthConfig /* 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 final class AuthConfig // 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 final class AuthConfig // 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 final class AuthConfig // 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 final class AuthConfig // 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 final class AuthConfig 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 76aacc5b5a..2d897c2eee 100644 --- a/src/java/org/apache/cassandra/auth/MutualTlsAuthenticator.java +++ b/src/java/org/apache/cassandra/auth/MutualTlsAuthenticator.java @@ -101,7 +101,8 @@ public class MutualTlsAuthenticator implements IAuthenticator 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 1cac64dba0..3fc9553fd9 100644 --- a/src/java/org/apache/cassandra/auth/MutualTlsInternodeAuthenticator.java +++ b/src/java/org/apache/cassandra/auth/MutualTlsInternodeAuthenticator.java @@ -104,7 +104,8 @@ public class MutualTlsInternodeAuthenticator implements IInternodeAuthenticator } 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 c0fb8bc200..106ab02e24 100644 --- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java +++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java @@ -515,7 +515,7 @@ public class DatabaseDescriptor 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 class DatabaseDescriptor } 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 class DatabaseDescriptor { 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 class DatabaseDescriptor { 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 class DatabaseDescriptor { 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 class DatabaseDescriptor { 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 @@ public class DatabaseDescriptor { 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 d772629f19..803fcb1fac 100644 --- a/src/java/org/apache/cassandra/config/ParameterizedClass.java +++ b/src/java/org/apache/cassandra/config/ParameterizedClass.java @@ -64,7 +64,17 @@ public class ParameterizedClass 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 @@ public class ParameterizedClass if (!searchPackage.isEmpty() && !searchPackage.endsWith(".")) searchPackage = searchPackage + '.'; String name = searchPackage + parameterizedClass.class_name; - providerClass = Class.forName(name); + // Load without initialization so a wrong class name does not run its static initializer here. The + // type is verified below (once the search has resolved a class) and the class is only initialized + // later, when it is constructed. + providerClass = Class.forName(name, false, ParameterizedClass.class.getClassLoader()); } - catch (ClassNotFoundException e) + catch (ClassNotFoundException | NoClassDefFoundError e) { //no-op } @@ -91,6 +104,13 @@ public class ParameterizedClass throw new ConfigurationException(error); } + // Verify the resolved class is the expected extension type before it is initialized/instantiated. Done once, + // after the package search, so a wrong-type match under an earlier search package does not abort the search + // before a valid class under a later package is found. + if (expectedType != null && !expectedType.isAssignableFrom(providerClass)) + throw new ConfigurationException("Invalid parameterized class " + providerClass.getName() + + ": must extend or implement " + expectedType.getName()); + try { Constructor 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 f5fdec6a56..56938b441f 100644 --- a/src/java/org/apache/cassandra/db/StorageHook.java +++ b/src/java/org/apache/cassandra/db/StorageHook.java @@ -55,7 +55,7 @@ public interface StorageHook 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 interface StorageHook } }; } -} \ 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 990a07a1ce..ebae5d5394 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 c01e18f679..0a11734034 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 abstract class ValueGenerator 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 abstract class ValueGenerator 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 3aa9533ff5..8e93e129e0 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 abstract class ValueValidator 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 b2dc0202bc..8e2da14142 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 class TypeParser 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 @@ public class TypeParser 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 @@ public class TypeParser } } + 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 81820382f3..b1e8df8783 100644 --- a/src/java/org/apache/cassandra/diag/DiagnosticEventPersistence.java +++ b/src/java/org/apache/cassandra/diag/DiagnosticEventPersistence.java @@ -128,6 +128,7 @@ public final class DiagnosticEventPersistence 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 @@ public final class DiagnosticEventPersistence 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 2846a8ede5..aeb4556ff4 100644 --- a/src/java/org/apache/cassandra/index/SecondaryIndexManager.java +++ b/src/java/org/apache/cassandra/index/SecondaryIndexManager.java @@ -921,6 +921,11 @@ public class SecondaryIndexManager implements IndexRegistry, INotificationConsum 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 @@ public class SecondaryIndexManager implements IndexRegistry, INotificationConsum 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 c827b2b820..339a2cae5d 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.disk.OnDiskIndexBuilder.Mode; 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 class IndexMode 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 class IndexMode // 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 class IndexMode } 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 4dd68b3dfd..395c59f835 100644 --- a/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java +++ b/src/java/org/apache/cassandra/locator/AbstractReplicationStrategy.java @@ -340,11 +340,11 @@ public abstract class AbstractReplicationStrategy 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 dd5c0d6ed9..2c3ba9dc20 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 @@ public class AutoRepairConfig implements Serializable 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 9217a36376..1cfdd31458 100644 --- a/src/java/org/apache/cassandra/schema/CompactionParams.java +++ b/src/java/org/apache/cassandra/schema/CompactionParams.java @@ -311,15 +311,7 @@ public final class CompactionParams 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 a438e34eb1..aea2dd4dad 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.compress.ZstdDictionaryCompressor; 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 final class CompressionParams 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 15a0670e3a..b471260ccc 100644 --- a/src/java/org/apache/cassandra/schema/IndexMetadata.java +++ b/src/java/org/apache/cassandra/schema/IndexMetadata.java @@ -148,9 +148,7 @@ public final class IndexMetadata // 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 7d88f6518e..5491b105cf 100644 --- a/src/java/org/apache/cassandra/schema/MemtableParams.java +++ b/src/java/org/apache/cassandra/schema/MemtableParams.java @@ -228,19 +228,25 @@ public final class MemtableParams 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 c1e2643897..63cad56057 100644 --- a/src/java/org/apache/cassandra/schema/ReplicationParams.java +++ b/src/java/org/apache/cassandra/schema/ReplicationParams.java @@ -286,7 +286,7 @@ public final class ReplicationParams 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 final class ReplicationParams 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 8240c2880f..c755c06347 100644 --- a/src/java/org/apache/cassandra/service/CacheService.java +++ b/src/java/org/apache/cassandra/service/CacheService.java @@ -147,9 +147,11 @@ public class CacheService implements CacheServiceMBean ? 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 c119d5f4c3..a17688d116 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 036559a738..7556e368d4 100644 --- a/src/java/org/apache/cassandra/service/DiskErrorsHandlerService.java +++ b/src/java/org/apache/cassandra/service/DiskErrorsHandlerService.java @@ -78,7 +78,7 @@ public class DiskErrorsHandlerService 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 340760370c..111e207474 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 class AccordService implements IAccordService, Shutdownable { 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 84db420f04..df1e8ee76e 100644 --- a/src/java/org/apache/cassandra/streaming/StreamHook.java +++ b/src/java/org/apache/cassandra/streaming/StreamHook.java @@ -37,7 +37,7 @@ public interface StreamHook 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 976e327951..2165fceddd 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 class ExtensionKey> extends MetadataKey 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 class ExtensionKey> extends MetadataKey { 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 class ExtensionKey> extends MetadataKey } } } - diff --git a/src/java/org/apache/cassandra/tools/nodetool/Sjk.java b/src/java/org/apache/cassandra/tools/nodetool/Sjk.java index c117ed638b..f9d19eabd2 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 @@ public class Sjk extends AbstractCommand List> result = new ArrayList<>(); try { + ClassLoader cl = Thread.currentThread().getContextClassLoader(); String path = packageName.replace('.', '/'); for (String f : findFiles(path)) { @@ -400,7 +401,7 @@ public class Sjk extends AbstractCommand { 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 4ef2e500cb..8d621cb668 100644 --- a/src/java/org/apache/cassandra/tracing/Tracing.java +++ b/src/java/org/apache/cassandra/tracing/Tracing.java @@ -117,7 +117,7 @@ public abstract class Tracing extends ExecutorLocals.Impl { 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 e3ded966ad..6231979695 100644 --- a/src/java/org/apache/cassandra/triggers/TriggerExecutor.java +++ b/src/java/org/apache/cassandra/triggers/TriggerExecutor.java @@ -286,13 +286,28 @@ public class TriggerExecutor } } - 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 class TriggerExecutor // 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 1d7bd70368..145bab1120 100644 --- a/src/java/org/apache/cassandra/utils/Clock.java +++ b/src/java/org/apache/cassandra/utils/Clock.java @@ -63,7 +63,7 @@ public interface Clock 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 a0ec48390c..0e4c3281a4 100644 --- a/src/java/org/apache/cassandra/utils/FBUtilities.java +++ b/src/java/org/apache/cassandra/utils/FBUtilities.java @@ -679,7 +679,7 @@ public class FBUtilities assert comparator.isPresent() : "Expected a comparator for local partitioner"; return new LocalPartitioner(comparator.get()); } - return FBUtilities.instanceOrConstruct(partitionerClassName, "partitioner"); + return FBUtilities.instanceOrConstruct(partitionerClassName, "partitioner", IPartitioner.class); } public static IAuditLogger newAuditLogger(String className, Map parameters) throws ConfigurationException @@ -689,8 +689,9 @@ public class FBUtilities 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 class FBUtilities try { - Class 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 class FBUtilities if (!className.contains(".")) className = "org.apache.cassandra.security." + className; - Class 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 class FBUtilities } /** + * 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 class FBUtilities } } + /** + * 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 class FBUtilities 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 class FBUtilities 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 class FBUtilities }); 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 72851d74ad..85e5ec81f0 100644 --- a/src/java/org/apache/cassandra/utils/JMXServerUtils.java +++ b/src/java/org/apache/cassandra/utils/JMXServerUtils.java @@ -222,7 +222,7 @@ public class JMXServerUtils 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 d1d82ca1f6..691d5170d6 100644 --- a/src/java/org/apache/cassandra/utils/MBeanWrapper.java +++ b/src/java/org/apache/cassandra/utils/MBeanWrapper.java @@ -78,7 +78,7 @@ public interface MBeanWrapper 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 2300cf2e89..0b63835120 100644 --- a/src/java/org/apache/cassandra/utils/MonotonicClock.java +++ b/src/java/org/apache/cassandra/utils/MonotonicClock.java @@ -94,7 +94,7 @@ public interface MonotonicClock 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 @@ public interface MonotonicClock 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 ecbc72f0d5..90c10d16cf 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.junit.Test; 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.AuthTestUtils.loadCertificateChain; 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 class AuthConfigTest 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 0000000000..abfdfa278a --- /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 5796c4032f..6f80a3dc6c 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.exceptions.ConfigurationException; 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 class DatabaseDescriptorTest } } + @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 732812aefd..4794d3de80 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.junit.Test; 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 class ParameterizedClassTest 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 79f6d51be0..46708f4317 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.DatabaseDescriptor; 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 9afb60b966..0f4d298b87 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 javax.annotation.Nonnull; 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 class ValueGeneratorTest .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 7bc717e4bc..5480223996 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.junit.Test; 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 class ValueValidatorTest .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 e9b2e3c764..9588c72bd2 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.OrderPreservingPartitioner; 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 class TypeParserTest 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 0000000000..13c880f942 --- /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 0000000000..b18d7b6aac --- /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 6eaa6e7dcc..bc1604a5c6 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.ColumnFamilyStore; 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 class SecondaryIndexManagerTest extends CQLTester 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 546e4cfff9..63e0395588 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.AsciiType; 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 class IndexModeTest { 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 class IndexModeTest { 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 188e0da30f..4dbdf24efb 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.config.ParameterizedClass; 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 class AutoRepairConfigTest extends CQLTester 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 0000000000..3831c2db25 --- /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 05b98e86aa..e4a696e663 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.BeforeClass; 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 class CompressionParamsTest 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 0bcd0dfc28..ef221141d8 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 class IndexMetadataTest 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 0000000000..823706fbf2 --- /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 0000000000..6cb4989cba --- /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 75094ef5d1..d92a1b2ac6 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.InheritingClass; 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 class MemtableParamsTest 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 0000000000..119aa46975 --- /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 cb12d58558..108003a1cc 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.Before; 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 class CipherFactoryTest 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 be1c617a7b..b7430d2d06 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.exceptions.InvalidRequestException; 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 class TriggerExecutorTest .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 class TriggerExecutorTest } } + 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 0000000000..401d89eb36 --- /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 0000000000..8faa92286a --- /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 0000000000..41d76473f9 --- /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 c9451730f3..d5b491ddbf 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.junit.Test; 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.LocalPartitioner; 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 42da2d5af1..e7af625895 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.config.YamlConfigurationLoader; 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 @@ public class LoaderOptions { 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 @@ public class LoaderOptions { 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 141c645079..ff2d2ef2cc 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 java.util.Map; 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 @@ class OptionReplication extends OptionMulti { 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 89c26a874e..c59044a361 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.ProtocolOptions; 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 class SettingsMode implements Serializable { 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)) {