+ * A statement is considered "mis-prepared" if it contains hardcoded literal values + * instead of bind markers. This is a performance anti-pattern because it prevents + * query plan reuse and floods the server-side Prepared Statement Cache with + * unique entries, leading to heap exhaustion and high GC pressure. + *
+ * LWT and Batch Considerations: + * This check applies to both the {@code WHERE} clause and the {@code IF} conditions + * in Lightweight Transactions (LWT). For example, the following is considered + * mis-prepared because of the hardcoded literals: + *
{@code
+ * UPDATE users SET description = 'v2' WHERE id = 1 AND name = 'v1' IF description = 'v0'
+ * }
+ * To be compliant, it should use bind markers ({@code ?}):
+ * {@code
+ * UPDATE users SET description = ? WHERE id = ? AND name = ? IF description = ?
+ * }
+ * For {@code BATCH} statements, if any individual statement within the batch is
+ * identified as mis-prepared, the entire batch triggers this guardrail.
+
+ *
+ * @return true if the usage of mis-prepared statements is enabled, false otherwise. Returns true by default.
+ * {@code false} if mis-prepared statements should be strictly rejected, causing the query to fail.
+ * @see CASSANDRA-21139
+ */
+
+ boolean getMispreparedStatementsEnabled();
+
+ /**
+ * sets whether misprepared statements is enabled
+ */
+ void setMispreparedStatementsEnabled(boolean enabled);
+
/**
* Sets whether new columns can be created with vector type
*
@@ -509,6 +543,8 @@ public interface GuardrailsConfig
void setIntersectFilteringQueryEnabled(boolean value);
+
+
/**
* @return A timestamp that if a user supplied timestamp is after will trigger a warning
*/
diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java
index e2e9cd74ddb..31fb8d865de 100644
--- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java
+++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java
@@ -54,6 +54,13 @@ public interface GuardrailsMBean
*/
void setKeyspacesThreshold(int warn, int fail);
+ /**
+ * @return the use of misprepared statement is enabled
+ */
+ boolean getMispreparedStatementsEnabled();
+
+ void setMispreparedStatementsEnabled(boolean enabled);
+
/**
* @return The threshold to warn when creating more tables than threshold.
* -1 means disabled.
diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java
index dc9d9e3f680..d53836f626e 100644
--- a/src/java/org/apache/cassandra/service/StorageService.java
+++ b/src/java/org/apache/cassandra/service/StorageService.java
@@ -4836,6 +4836,19 @@ public void setColumnIndexCacheSize(int cacheSizeInKB)
logger.info("Updated column_index_cache_size to {}", cacheSizeInKB);
}
+ @Override
+ public boolean getMispreparedStatementsEnabled()
+ {
+ return DatabaseDescriptor.getMispreparedStatementsEnabled();
+ }
+
+ @Override
+ public void setMispreparedStatementsEnabled(boolean enabled)
+ {
+ DatabaseDescriptor.setMispreparedStatementsEnabled(enabled);
+ logger.info("Updated misprepared_statements_enabled to {}", enabled);
+ }
+
/*
* In CASSANDRA-17668, JMX setters that did not throw standard exceptions were deprecated in favor of ones that do.
* For consistency purposes, the respective getter "getColumnIndexCacheSize" was also deprecated and replaced by
diff --git a/src/java/org/apache/cassandra/service/StorageServiceMBean.java b/src/java/org/apache/cassandra/service/StorageServiceMBean.java
index 9c4ae481ec1..09db793ad84 100644
--- a/src/java/org/apache/cassandra/service/StorageServiceMBean.java
+++ b/src/java/org/apache/cassandra/service/StorageServiceMBean.java
@@ -1097,6 +1097,16 @@ default int upgradeSSTables(String keyspaceName, boolean excludeCurrentVersion,
@Deprecated(since = "5.0")
public void setColumnIndexCacheSize(int cacheSizeInKB);
+ /**
+ * Returns whether use of misprepared statements is enabled
+ */
+ public boolean getMispreparedStatementsEnabled();
+
+ /**
+ * Sets whether use of misprepared statements is enabled
+ */
+ public void setMispreparedStatementsEnabled(boolean enabled);
+
/** Returns the threshold for skipping the column index when caching partition info **/
public int getColumnIndexCacheSizeInKiB();
diff --git a/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java b/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java
index 678f6746f1d..88d583d57f5 100644
--- a/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java
+++ b/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java
@@ -781,6 +781,27 @@ public void testRowIndexSizeWarnEqAbort()
DatabaseDescriptor.applyReadThresholdsValidations(conf);
}
+ @Test
+ public void testMispreparedStatementsEnabled() {
+ boolean originalValue = DatabaseDescriptor.getMispreparedStatementsEnabled();
+ try
+ {
+ DatabaseDescriptor.setMispreparedStatementsEnabled(true);
+ Assert.assertTrue("Value should be true after setting to true",
+ DatabaseDescriptor.getMispreparedStatementsEnabled());
+
+ DatabaseDescriptor.setMispreparedStatementsEnabled(false);
+ Assert.assertFalse("Value should be false after setting to false",
+ DatabaseDescriptor.getMispreparedStatementsEnabled());
+
+ Assert.assertTrue("Default value of misprepared_statement_enabled must be true", originalValue);
+ }
+ finally
+ {
+ DatabaseDescriptor.setMispreparedStatementsEnabled(originalValue);
+ }
+ }
+
@Test
public void testRowIndexSizeWarnEnabledAbortDisabled()
{
diff --git a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java
new file mode 100644
index 00000000000..2cb437e5dca
--- /dev/null
+++ b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java
@@ -0,0 +1,391 @@
+/*
+ * 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.cql3;
+
+import java.net.InetSocketAddress;
+import java.util.List;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import org.apache.cassandra.auth.AuthenticatedUser;
+import org.apache.cassandra.auth.CassandraAuthorizer;
+import org.apache.cassandra.auth.PasswordAuthenticator;
+import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.db.guardrails.GuardrailViolatedException;
+import org.apache.cassandra.db.guardrails.Guardrails;
+import org.apache.cassandra.service.ClientState;
+import org.apache.cassandra.service.ClientWarn;
+
+import static org.junit.Assert.assertTrue;
+
+public class MispreparedStatementsTest extends CQLTester
+{
+ private static final ClientState state = ClientState.forExternalCalls(new InetSocketAddress("127.0.0.1", 9042));
+
+ @BeforeClass
+ public static void setupGlobalConfig()
+ {
+ DatabaseDescriptor.setAuthenticator(new PasswordAuthenticator());
+ DatabaseDescriptor.setAuthorizer(new CassandraAuthorizer());
+ DatabaseDescriptor.setMispreparedStatementsEnabled(false);
+ }
+
+ @Before
+ public void setUp()
+ {
+ AuthenticatedUser nonSuperUser = new AuthenticatedUser("regular-user")
+ {
+ @Override
+ public boolean isSuper()
+ {
+ return false;
+ }
+
+ @Override
+ public boolean isSystem()
+ {
+ return false;
+ }
+
+ @Override
+ public boolean isAnonymous()
+ {
+ return false;
+ }
+
+ @Override
+ public boolean canLogin()
+ {
+ return true;
+ }
+ };
+
+ state.login(nonSuperUser);
+ state.setKeyspace(KEYSPACE);
+ ClientWarn.instance.captureWarnings();
+ createTable("CREATE TABLE %s (id int, description text, age int, name text, PRIMARY KEY (id, name, age))");
+ }
+
+ @After
+ public void tearDown()
+ {
+ DatabaseDescriptor.setMispreparedStatementsEnabled(false);
+ }
+
+ @Test
+ public void testSelectWithPartitionKey()
+ {
+ assertGuardrailViolated(String.format("SELECT * FROM %s WHERE id = 1", currentTable()));
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testSelectWithClusteringKey()
+ {
+ assertGuardrailViolated(String.format("SELECT * FROM %s WHERE name = 'v1' ALLOW FILTERING", currentTable()));
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testSelectWithFullPrimaryKey()
+ {
+ assertGuardrailViolated(String.format("SELECT * FROM %s WHERE id = 1 AND name = 'v1'", currentTable()));
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testSelectWithWhereNonPrimaryKeyColumn()
+ {
+ assertGuardrailViolated(String.format("SELECT * FROM %s WHERE description = 'foo' ALLOW FILTERING", currentTable()));
+ assertNoWarnings();
+ }
+
+
+ @Test
+ public void testSelectWithCompositeRestriction()
+ {
+ assertGuardrailViolated(String.format("SELECT sum(id) from %s WHERE (name, age) = ('a', 1) ALLOW FILTERING", currentTable()));
+ }
+
+ @Test
+ public void testSelectWithFunction()
+ {
+ assertGuardrailViolated(String.format("SELECT sum(id) from %s where name = 'v1' ALLOW FILTERING", currentTable()));
+ }
+
+ @Test
+ public void testSelectWithRangeRestriction()
+ {
+ assertGuardrailViolated(String.format("SELECT * FROM %s WHERE id = 1 AND name > 'a'", currentTable()));
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testSelectInRestrictionOnPartitionKey()
+ {
+ assertGuardrailViolated(String.format("SELECT * FROM %s WHERE id IN (1, 2, 3)", currentTable()));
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testSelectInRestrictionOnClusteringKey()
+ {
+ assertGuardrailViolated(String.format("SELECT * FROM %s WHERE name IN ('a', 'b') ALLOW FILTERING", currentTable()));
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testSelectInRestrictionOnFullPrimaryKey()
+ {
+ assertGuardrailViolated(String.format("SELECT * FROM %s WHERE id IN (1, 2, 3) AND name in ('a', 'b')", currentTable()));
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testDDLStatementsBypass()
+ {
+ assertGuardrailPassed("CREATE TABLE IF NOT EXISTS test_table (id INT PRIMARY KEY)");
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testWhereLiteralWithLike()
+ {
+ assertGuardrailViolated(String.format("SELECT * FROM %s WHERE name LIKE 'prefix%%' ALLOW FILTERING", currentTable()));
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testInsertJsonGuardrail()
+ {
+ assertGuardrailViolated(String.format("INSERT INTO %s JSON '{\"id\": 1, \"name\": \"v1\"}'", currentTable()));
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testUpdateWithPartitionKey()
+ {
+ assertGuardrailViolated(String.format("UPDATE %s SET description = 'new' WHERE id = 1 AND name = 'name' AND age = 1", KEYSPACE + '.' + currentTable()));
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testUpdateWithIfCondition()
+ {
+ assertGuardrailViolated(String.format("UPDATE %s SET description = 'v2' WHERE id = 1 AND name = 'v1' AND age = 1 IF description = 'v0'", currentTable()));
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testDeleteWithFullPrimaryKey()
+ {
+ assertGuardrailViolated(String.format("DELETE FROM %s WHERE id = 1 AND name = 'v1'", currentTable()));
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testMultiTableBatchGuardrailAllLiteral()
+ {
+ String table1 = currentTable();
+ String table2 = createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)");
+ String query = String.format("BEGIN BATCH " +
+ "UPDATE %s.%s SET description = 'v1' WHERE id = 1 AND name = 'n1' AND age = 1; " +
+ "INSERT INTO %s.%s (id, val) VALUES (2, 'v2'); " +
+ "APPLY BATCH",
+ KEYSPACE, table1, KEYSPACE, table2);
+
+ assertGuardrailViolated(query);
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testProperlyPreparedWithBindMarkers()
+ {
+ assertGuardrailPassed(String.format("SELECT * FROM %s WHERE id = ? AND name = ?", currentTable()));
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testSelectAllPasses()
+ {
+ assertGuardrailPassed(String.format("SELECT * FROM %s", currentTable()));
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testLiteralInProjectionIsAllowed()
+ {
+ assertGuardrailPassed(String.format("SELECT id, (text)'const_val' FROM %s WHERE id = ?", currentTable()));
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testInternalBypass()
+ {
+ assertGuardrailPassed("SELECT * FROM " + KEYSPACE + '.' + currentTable() + " WHERE id = 1", ClientState.forInternalCalls());
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testSuperUserBypass()
+ {
+ AuthenticatedUser superUser = new AuthenticatedUser("super-user")
+ {
+ @Override
+ public boolean isSuper()
+ {
+ return true;
+ }
+
+ @Override
+ public boolean isSystem()
+ {
+ return false;
+ }
+
+ @Override
+ public boolean isAnonymous()
+ {
+ return false;
+ }
+
+ @Override
+ public boolean canLogin()
+ {
+ return true;
+ }
+ };
+ ClientState superUserState = ClientState.forExternalCalls(new InetSocketAddress("127.0.0.1", 9042));
+ superUserState.login(superUser);
+ assertGuardrailPassed("SELECT * FROM " + KEYSPACE + '.' + currentTable() + " WHERE id = 1", superUserState);
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testSystemKeyspaceBypassForRegularUser()
+ {
+ assertGuardrailPassed("SELECT * FROM system.local WHERE key = 'local'");
+ assertNoWarnings();
+ }
+
+ @Test
+ public void testGuardrailDisabledAllowsLiterals()
+ {
+ DatabaseDescriptor.setMispreparedStatementsEnabled(true);
+ assertGuardrailPassed(String.format("SELECT * FROM %s WHERE id = 1", currentTable()));
+ assertWarnings();
+ }
+
+ @Test
+ public void testWarningIsIssuedWhenGuardrailIsAllowed()
+ {
+ DatabaseDescriptor.setMispreparedStatementsEnabled(true);
+ ClientWarn.instance.captureWarnings();
+ assertGuardrailPassed(String.format("SELECT * FROM %s WHERE id = 1", currentTable()));
+ assertWarnings();
+
+ }
+
+ @Test
+ public void testGuardrailDisabledAllowsBatchLiterals()
+ {
+ DatabaseDescriptor.setMispreparedStatementsEnabled(true);
+ String tableName = currentTable();
+ assertGuardrailPassed(String.format("BEGIN BATCH " +
+ "INSERT INTO %s.%s (id, description, name, age) VALUES (1, 'v1', 'v1', 1); " +
+ "UPDATE %s.%s SET description = 'v2' WHERE id = 2 AND name = 'v1' AND age = 1; " +
+ "APPLY BATCH", KEYSPACE, tableName, KEYSPACE, tableName));
+ assertWarnings();
+ }
+
+ @Test
+ public void testBlockedStatementsDoNotEnterCache()
+ {
+ QueryProcessor.clearPreparedStatementsCache();
+ int initialCacheSize = QueryProcessor.preparedStatementsCount();
+ for (int i = 0; i < 10; i++)
+ {
+ String query = String.format("SELECT * FROM %s WHERE id = %d", currentTable(), i);
+ try
+ {
+ QueryProcessor.instance.prepare(query, state);
+ }
+ catch (GuardrailViolatedException e)
+ {
+ // Expected: Guardrail throws exception
+ }
+ }
+ Assert.assertEquals("Blocked misprepared statements should not be added to the cache", initialCacheSize, QueryProcessor.preparedStatementsCount());
+ }
+
+ private void assertGuardrailViolated(String query)
+ {
+ try
+ {
+ QueryProcessor.instance.prepare(query, state);
+ Assert.fail("Expected GuardrailViolatedException for query: " + query);
+ }
+ catch (GuardrailViolatedException e)
+ {
+ assertTrue(e.getMessage().contains("Mis-prepared statements is not allowed"));
+ }
+ catch (Exception e)
+ {
+ Assert.fail(e.getMessage());
+ }
+ }
+
+ private void assertNoWarnings()
+ {
+ List