mirror of https://github.com/apache/cassandra
Add a guardrail for misprepared statements
patch by Rishabh Saraswat; reviewed by Stefan Miklosovic, Brad Schoening for CASSANDRA-21139
This commit is contained in:
parent
cd75b9c6a1
commit
eabd2a27f5
|
|
@ -1,4 +1,5 @@
|
||||||
7.0
|
7.0
|
||||||
|
* Add a guardrail for misprepared statements (CASSANDRA-21139)
|
||||||
Merged from 6.0:
|
Merged from 6.0:
|
||||||
* Ensure schema created before 2.1 without tableId in folder name can be loaded in SnapshotLoader (CASSANDRA-21246)
|
* Ensure schema created before 2.1 without tableId in folder name can be loaded in SnapshotLoader (CASSANDRA-21246)
|
||||||
* Differentiate between legitimate cases where the first entry is the same as the last entry and empty bounds in SSTableCursorWriter#addIndexBlock() (CASSANDRA-21255)
|
* Differentiate between legitimate cases where the first entry is the same as the last entry and empty bounds in SSTableCursorWriter#addIndexBlock() (CASSANDRA-21255)
|
||||||
|
|
|
||||||
|
|
@ -2565,6 +2565,13 @@ drop_compact_storage_enabled: false
|
||||||
# This would also apply to system keyspaces.
|
# This would also apply to system keyspaces.
|
||||||
# maximum_replication_factor_warn_threshold: -1
|
# maximum_replication_factor_warn_threshold: -1
|
||||||
# maximum_replication_factor_fail_threshold: -1
|
# maximum_replication_factor_fail_threshold: -1
|
||||||
|
#
|
||||||
|
# Guardrail to warn or fail when a statement is prepared without bind markers (parameters).
|
||||||
|
# This prevents the Prepared Statement Cache from being flooded with unique entries caused
|
||||||
|
# by hardcoded literals. When warned is true, logs a warning on the usage of misprepared statements.
|
||||||
|
# When enabled is true, rejects misprepared statements with an error.
|
||||||
|
# prepared_statements_require_parameters_warned: true
|
||||||
|
# prepared_statements_require_parameters_enabled: false
|
||||||
|
|
||||||
#role_name_policy:
|
#role_name_policy:
|
||||||
# # Implementation class of a validator. When not in form of FQCN, the
|
# # Implementation class of a validator. When not in form of FQCN, the
|
||||||
|
|
|
||||||
|
|
@ -2343,6 +2343,13 @@ drop_compact_storage_enabled: false
|
||||||
# This would also apply to system keyspaces.
|
# This would also apply to system keyspaces.
|
||||||
# maximum_replication_factor_warn_threshold: -1
|
# maximum_replication_factor_warn_threshold: -1
|
||||||
# maximum_replication_factor_fail_threshold: -1
|
# maximum_replication_factor_fail_threshold: -1
|
||||||
|
#
|
||||||
|
# Guardrail to warn or fail when a statement is prepared without bind markers (parameters).
|
||||||
|
# This prevents the Prepared Statement Cache from being flooded with unique entries caused
|
||||||
|
# by hardcoded literals. When warned is true, logs a warning on the usage of misprepared statements.
|
||||||
|
# When enabled is true, rejects misprepared statements with an error.
|
||||||
|
# prepared_statements_require_parameters_warned: true
|
||||||
|
# prepared_statements_require_parameters_enabled: false
|
||||||
|
|
||||||
#role_name_policy:
|
#role_name_policy:
|
||||||
# # Implementation class of a validator. When not in form of FQCN, the
|
# # Implementation class of a validator. When not in form of FQCN, the
|
||||||
|
|
|
||||||
|
|
@ -938,6 +938,8 @@ public class Config
|
||||||
public volatile boolean user_timestamps_enabled = true;
|
public volatile boolean user_timestamps_enabled = true;
|
||||||
public volatile boolean alter_table_enabled = true;
|
public volatile boolean alter_table_enabled = true;
|
||||||
public volatile boolean group_by_enabled = true;
|
public volatile boolean group_by_enabled = true;
|
||||||
|
public volatile boolean prepared_statements_require_parameters_warned = true;
|
||||||
|
public volatile boolean prepared_statements_require_parameters_enabled = false;
|
||||||
public volatile boolean bulk_load_enabled = true;
|
public volatile boolean bulk_load_enabled = true;
|
||||||
public volatile boolean drop_truncate_table_enabled = true;
|
public volatile boolean drop_truncate_table_enabled = true;
|
||||||
public volatile boolean drop_keyspace_enabled = true;
|
public volatile boolean drop_keyspace_enabled = true;
|
||||||
|
|
|
||||||
|
|
@ -1403,6 +1403,36 @@ public class GuardrailsOptions implements GuardrailsConfig
|
||||||
x -> config.minimum_client_driver_versions_disallowed = x);
|
x -> config.minimum_client_driver_versions_disallowed = x);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public boolean getPreparedStatementsRequireParametersWarned()
|
||||||
|
{
|
||||||
|
return config.prepared_statements_require_parameters_warned;
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public boolean getPreparedStatementsRequireParametersEnabled()
|
||||||
|
{
|
||||||
|
return config.prepared_statements_require_parameters_enabled;
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public void setPreparedStatementsRequireParametersWarned(boolean warned)
|
||||||
|
{
|
||||||
|
updatePropertyWithLogging("prepared_statements_require_parameters_warned",
|
||||||
|
warned,
|
||||||
|
() -> config.prepared_statements_require_parameters_warned,
|
||||||
|
x -> config.prepared_statements_require_parameters_warned = x);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public void setPreparedStatementsRequireParametersEnabled(boolean enabled)
|
||||||
|
{
|
||||||
|
updatePropertyWithLogging("prepared_statements_require_parameters_enabled",
|
||||||
|
enabled,
|
||||||
|
() -> config.prepared_statements_require_parameters_enabled,
|
||||||
|
x -> config.prepared_statements_require_parameters_enabled = x);
|
||||||
|
}
|
||||||
|
|
||||||
private static <T> void updatePropertyWithLogging(String propertyName, T newValue, Supplier<T> getter, Consumer<T> setter)
|
private static <T> void updatePropertyWithLogging(String propertyName, T newValue, Supplier<T> getter, Consumer<T> setter)
|
||||||
{
|
{
|
||||||
T oldValue = getter.get();
|
T oldValue = getter.get();
|
||||||
|
|
|
||||||
|
|
@ -73,6 +73,18 @@ public interface CQLStatement
|
||||||
*/
|
*/
|
||||||
void validate(ClientState state);
|
void validate(ClientState state);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Performs validation specific to the preparation phase of a CQL statement.
|
||||||
|
* <p>
|
||||||
|
* Unlike {@link #validate(ClientState)}, which is invoked during normal execution,
|
||||||
|
* this method is explicitly triggered only when a statement is being prepared by a client.
|
||||||
|
* By default, this is a no-op. Subclasses can override this to enforce preparation-specific
|
||||||
|
* rules or guardrails (e.g., ensuring prepared statements contain bind markers).
|
||||||
|
*
|
||||||
|
* @param state the current client state
|
||||||
|
*/
|
||||||
|
default void validatePrepare(ClientState state) {}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Execute the statement and return the resulting result or null if there is no result.
|
* Execute the statement and return the resulting result or null if there is no result.
|
||||||
*
|
*
|
||||||
|
|
|
||||||
|
|
@ -494,6 +494,7 @@ public class QueryProcessor implements QueryHandler
|
||||||
// Note: if 2 threads prepare the same query, we'll live so don't bother synchronizing
|
// Note: if 2 threads prepare the same query, we'll live so don't bother synchronizing
|
||||||
CQLStatement statement = raw.prepare(clientState);
|
CQLStatement statement = raw.prepare(clientState);
|
||||||
statement.validate(clientState);
|
statement.validate(clientState);
|
||||||
|
statement.validatePrepare(clientState);
|
||||||
|
|
||||||
// Set CQL string for AlterSchemaStatement as this is used to serialize the transformation
|
// Set CQL string for AlterSchemaStatement as this is used to serialize the transformation
|
||||||
// in the cluster metadata log
|
// in the cluster metadata log
|
||||||
|
|
|
||||||
|
|
@ -338,6 +338,15 @@ public class BatchStatement implements CQLStatement.CompositeCQLStatement
|
||||||
statement.validate(state);
|
statement.validate(state);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public void validatePrepare(ClientState state)
|
||||||
|
{
|
||||||
|
for (ModificationStatement statement : statements)
|
||||||
|
{
|
||||||
|
statement.validatePrepare(state);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public List<ModificationStatement> getStatements()
|
public List<ModificationStatement> getStatements()
|
||||||
{
|
{
|
||||||
|
|
|
||||||
|
|
@ -414,6 +414,12 @@ public abstract class ModificationStatement implements CQLStatement.SingleKeyspa
|
||||||
Guardrails.userTimestampsEnabled.ensureEnabled(state);
|
Guardrails.userTimestampsEnabled.ensureEnabled(state);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public void validatePrepare(ClientState state)
|
||||||
|
{
|
||||||
|
Guardrails.preparedStatementsRequireParameters.guard(this, restrictions, state, metadata.keyspace, metadata.getTableName());
|
||||||
|
}
|
||||||
|
|
||||||
public void validateDiskUsage(QueryOptions options, ClientState state)
|
public void validateDiskUsage(QueryOptions options, ClientState state)
|
||||||
{
|
{
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -344,6 +344,12 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement,
|
||||||
Guardrails.allowFilteringEnabled.ensureEnabled(state);
|
Guardrails.allowFilteringEnabled.ensureEnabled(state);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public void validatePrepare(ClientState state)
|
||||||
|
{
|
||||||
|
Guardrails.preparedStatementsRequireParameters.guard(this, restrictions, state, table.keyspace, table.getTableName());
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public ResultMessage.Rows execute(QueryState state, QueryOptions options, Dispatcher.RequestTime requestTime)
|
public ResultMessage.Rows execute(QueryState state, QueryOptions options, Dispatcher.RequestTime requestTime)
|
||||||
{
|
{
|
||||||
|
|
|
||||||
|
|
@ -709,6 +709,9 @@ public final class Guardrails implements GuardrailsMBean
|
||||||
state -> CONFIG_PROVIDER.getOrCreate(state).getUnsetTrainingMinFrequencyEnabled(),
|
state -> CONFIG_PROVIDER.getOrCreate(state).getUnsetTrainingMinFrequencyEnabled(),
|
||||||
"unset minimum frequency of training for dictionary compressor");
|
"unset minimum frequency of training for dictionary compressor");
|
||||||
|
|
||||||
|
public static final PreparedStatementParameterRequirementGuardrail preparedStatementsRequireParameters =
|
||||||
|
new PreparedStatementParameterRequirementGuardrail();
|
||||||
|
|
||||||
private Guardrails()
|
private Guardrails()
|
||||||
{
|
{
|
||||||
MBeanWrapper.instance.registerMBean(this, MBEAN_NAME);
|
MBeanWrapper.instance.registerMBean(this, MBEAN_NAME);
|
||||||
|
|
@ -1921,6 +1924,30 @@ public final class Guardrails implements GuardrailsMBean
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public boolean getPreparedStatementsRequireParametersWarned()
|
||||||
|
{
|
||||||
|
return DEFAULT_CONFIG.getPreparedStatementsRequireParametersWarned();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public boolean getPreparedStatementsRequireParametersEnabled()
|
||||||
|
{
|
||||||
|
return DEFAULT_CONFIG.getPreparedStatementsRequireParametersEnabled();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public void setPreparedStatementsRequireParametersWarned(boolean warned)
|
||||||
|
{
|
||||||
|
DEFAULT_CONFIG.setPreparedStatementsRequireParametersWarned(warned);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public void setPreparedStatementsRequireParametersEnabled(boolean enabled)
|
||||||
|
{
|
||||||
|
DEFAULT_CONFIG.setPreparedStatementsRequireParametersEnabled(enabled);
|
||||||
|
}
|
||||||
|
|
||||||
private static String toCSV(Set<String> values)
|
private static String toCSV(Set<String> values)
|
||||||
{
|
{
|
||||||
return values == null || values.isEmpty() ? "" : String.join(",", values);
|
return values == null || values.isEmpty() ? "" : String.join(",", values);
|
||||||
|
|
|
||||||
|
|
@ -695,4 +695,30 @@ public interface GuardrailsConfig
|
||||||
* driver version is below the specified minimum.
|
* driver version is below the specified minimum.
|
||||||
*/
|
*/
|
||||||
Map<String, String> getMinimumClientDriverVersionsDisallowed();
|
Map<String, String> getMinimumClientDriverVersionsDisallowed();
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @return {@code true} if a warning is logged when a prepared statement is created without parameters,
|
||||||
|
* {@code false} otherwise.
|
||||||
|
*/
|
||||||
|
boolean getPreparedStatementsRequireParametersWarned();
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @return {@code true} if creating a prepared statement without parameters is rejected,
|
||||||
|
* {@code false} otherwise.
|
||||||
|
*/
|
||||||
|
boolean getPreparedStatementsRequireParametersEnabled();
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Sets whether to log a warning when a prepared statement is created without parameters.
|
||||||
|
*
|
||||||
|
* @param warned {@code true} to enable the warning, {@code false} to disable it.
|
||||||
|
*/
|
||||||
|
void setPreparedStatementsRequireParametersWarned(boolean warned);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Sets whether to reject creating a prepared statement without parameters.
|
||||||
|
*
|
||||||
|
* @param enabled {@code true} to reject, {@code false} to allow.
|
||||||
|
*/
|
||||||
|
void setPreparedStatementsRequireParametersEnabled(boolean enabled);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1214,4 +1214,30 @@ public interface GuardrailsMBean
|
||||||
* @param value JSON representation of a map of driver name to minimum version string.
|
* @param value JSON representation of a map of driver name to minimum version string.
|
||||||
*/
|
*/
|
||||||
void setMinimumClientDriverVersionsDisallowed(String value);
|
void setMinimumClientDriverVersionsDisallowed(String value);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @return {@code true} if a warning is logged when a prepared statement is created without parameters,
|
||||||
|
* {@code false} otherwise.
|
||||||
|
*/
|
||||||
|
boolean getPreparedStatementsRequireParametersWarned();
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @return {@code true} if creating a prepared statement without parameters is rejected,
|
||||||
|
* {@code false} otherwise.
|
||||||
|
*/
|
||||||
|
boolean getPreparedStatementsRequireParametersEnabled();
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Sets whether to log a warning when a prepared statement is created without parameters.
|
||||||
|
*
|
||||||
|
* @param warned {@code true} to enable the warning, {@code false} to disable it.
|
||||||
|
*/
|
||||||
|
void setPreparedStatementsRequireParametersWarned(boolean warned);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Sets whether to reject creating a prepared statement without parameters.
|
||||||
|
*
|
||||||
|
* @param enabled {@code true} to reject, {@code false} to allow.
|
||||||
|
*/
|
||||||
|
void setPreparedStatementsRequireParametersEnabled(boolean enabled);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,66 @@
|
||||||
|
/*
|
||||||
|
* 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.db.guardrails;
|
||||||
|
|
||||||
|
import javax.annotation.Nullable;
|
||||||
|
|
||||||
|
import com.google.common.annotations.VisibleForTesting;
|
||||||
|
|
||||||
|
import org.apache.cassandra.cql3.CQLStatement;
|
||||||
|
import org.apache.cassandra.cql3.restrictions.StatementRestrictions;
|
||||||
|
import org.apache.cassandra.service.ClientState;
|
||||||
|
|
||||||
|
public class PreparedStatementParameterRequirementGuardrail extends Guardrail
|
||||||
|
{
|
||||||
|
@VisibleForTesting
|
||||||
|
public static final String MISPREPARED_STATEMENT_MESSAGE = "The query contains only literal values and no bind markers. Using one or more '?' placeholder values (bind markers) allows a prepared statement to be reused.";
|
||||||
|
|
||||||
|
PreparedStatementParameterRequirementGuardrail()
|
||||||
|
{
|
||||||
|
super("prepared_statements_require_parameters", null);
|
||||||
|
}
|
||||||
|
|
||||||
|
public void guard(CQLStatement statement, StatementRestrictions restrictions, @Nullable ClientState state, String keyspace, String table)
|
||||||
|
{
|
||||||
|
if (restrictions == null
|
||||||
|
|| !statement.eligibleAsPreparedStatement()
|
||||||
|
|| !statement.getBindVariables().isEmpty()
|
||||||
|
|| (!restrictions.hasPartitionKeyRestrictions()
|
||||||
|
&& !restrictions.hasClusteringColumnsRestrictions()
|
||||||
|
&& !restrictions.hasNonPrimaryKeyRestrictions()))
|
||||||
|
return;
|
||||||
|
|
||||||
|
if (!enabled(state))
|
||||||
|
return;
|
||||||
|
|
||||||
|
final GuardrailsConfig config = Guardrails.CONFIG_PROVIDER.getOrCreate(state);
|
||||||
|
|
||||||
|
boolean failOn = config.getPreparedStatementsRequireParametersEnabled();
|
||||||
|
if (!failOn && !config.getPreparedStatementsRequireParametersWarned())
|
||||||
|
return;
|
||||||
|
|
||||||
|
final String message = MISPREPARED_STATEMENT_MESSAGE + " Query executed on keyspace '" + keyspace + "', table '" + table + "'.";
|
||||||
|
|
||||||
|
if (failOn)
|
||||||
|
fail(message, state);
|
||||||
|
else
|
||||||
|
warn(message);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
|
@ -373,6 +373,7 @@ public abstract class GuardrailsConfigCommand extends AbstractCommand
|
||||||
* Set of guardrails which are flags, even though their suffix would suggest they are part of "values" which have warned, ignored, and disallowed sub-categories
|
* Set of guardrails which are flags, even though their suffix would suggest they are part of "values" which have warned, ignored, and disallowed sub-categories
|
||||||
*/
|
*/
|
||||||
private static final Set<String> specialFlags = Set.of("intersect_filtering_query_warned",
|
private static final Set<String> specialFlags = Set.of("intersect_filtering_query_warned",
|
||||||
|
"prepared_statements_require_parameters_warned",
|
||||||
"unset_training_min_frequency_warned",
|
"unset_training_min_frequency_warned",
|
||||||
"zero_ttl_on_twcs_warned");
|
"zero_ttl_on_twcs_warned");
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,126 @@
|
||||||
|
/*
|
||||||
|
* 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.distributed.test.guardrails;
|
||||||
|
|
||||||
|
import java.io.IOException;
|
||||||
|
|
||||||
|
import com.datastax.driver.core.Session;
|
||||||
|
import com.datastax.driver.core.SimpleStatement;
|
||||||
|
import com.datastax.driver.core.exceptions.InvalidQueryException;
|
||||||
|
|
||||||
|
import org.assertj.core.api.ThrowableAssert;
|
||||||
|
import org.junit.AfterClass;
|
||||||
|
import org.junit.BeforeClass;
|
||||||
|
import org.junit.Test;
|
||||||
|
|
||||||
|
import org.apache.cassandra.db.guardrails.Guardrails;
|
||||||
|
import org.apache.cassandra.db.guardrails.PreparedStatementParameterRequirementGuardrail;
|
||||||
|
import org.apache.cassandra.distributed.Cluster;
|
||||||
|
import org.apache.cassandra.distributed.api.Feature;
|
||||||
|
|
||||||
|
import static org.assertj.core.api.Assertions.assertThatCode;
|
||||||
|
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||||
|
|
||||||
|
public class MispreparedStatementsTest extends GuardrailTester
|
||||||
|
{
|
||||||
|
private static Cluster cluster;
|
||||||
|
private static com.datastax.driver.core.Cluster driverCluster;
|
||||||
|
private static Session driverSession;
|
||||||
|
|
||||||
|
@BeforeClass
|
||||||
|
public static void setupCluster() throws IOException
|
||||||
|
{
|
||||||
|
cluster = init(Cluster.build(1)
|
||||||
|
.withConfig(c -> c.with(Feature.GOSSIP, Feature.NATIVE_PROTOCOL)
|
||||||
|
.set("authenticator", "PasswordAuthenticator")
|
||||||
|
.set("prepared_statements_require_parameters_warned", true)
|
||||||
|
.set("prepared_statements_require_parameters_enabled", false))
|
||||||
|
.start());
|
||||||
|
driverCluster = buildDriverCluster(cluster);
|
||||||
|
driverSession = driverCluster.connect();
|
||||||
|
}
|
||||||
|
|
||||||
|
@AfterClass
|
||||||
|
public static void teardownCluster()
|
||||||
|
{
|
||||||
|
if (driverSession != null)
|
||||||
|
driverSession.close();
|
||||||
|
|
||||||
|
if (driverCluster != null)
|
||||||
|
driverCluster.close();
|
||||||
|
|
||||||
|
if (cluster != null)
|
||||||
|
cluster.close();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
protected Cluster getCluster()
|
||||||
|
{
|
||||||
|
return cluster;
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testInvalidConstantSelectStatements()
|
||||||
|
{
|
||||||
|
schemaChange("create table %s (pk int, ck int, data text, primary key((pk), ck))");
|
||||||
|
|
||||||
|
cluster.get(1).runOnInstance(() -> {
|
||||||
|
Guardrails.instance.setPreparedStatementsRequireParametersEnabled(true);
|
||||||
|
});
|
||||||
|
assertGuardrailViolated(() -> prepare("select * from %s where pk = 1"));
|
||||||
|
assertGuardrailViolated(() -> prepare("select * from %s where ck = 1 allow filtering"));
|
||||||
|
assertGuardrailViolated(() -> prepare("select * from %s where data = 'a' allow filtering"));
|
||||||
|
assertGuardrailViolated(() -> prepare("insert into %s(pk, ck, data) values (1,1,'a')"));
|
||||||
|
assertGuardrailViolated(() -> prepare("update %s set data = 'a' where pk = 1 and ck = 1"));
|
||||||
|
|
||||||
|
// non-prepare queries remain unaffected
|
||||||
|
assertThatCode(() -> execute("select * from %s where pk = 1")).doesNotThrowAnyException();
|
||||||
|
// at-least 1 bind marker
|
||||||
|
assertThatCode(() -> prepare("select * from %s where pk = 1 AND data = ? allow filtering")).doesNotThrowAnyException();
|
||||||
|
// statement having bind marker but no where clause
|
||||||
|
assertThatCode(() -> prepare("insert into %s(pk, ck, data) values (1,1,?)")).doesNotThrowAnyException();
|
||||||
|
}
|
||||||
|
|
||||||
|
private void assertGuardrailViolated(ThrowableAssert.ThrowingCallable throwable)
|
||||||
|
{
|
||||||
|
assertThatThrownBy(throwable)
|
||||||
|
.isInstanceOf(InvalidQueryException.class)
|
||||||
|
.hasMessageContaining(PreparedStatementParameterRequirementGuardrail.MISPREPARED_STATEMENT_MESSAGE);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
protected Session getSession()
|
||||||
|
{
|
||||||
|
return driverSession;
|
||||||
|
}
|
||||||
|
|
||||||
|
private void execute(String query)
|
||||||
|
{
|
||||||
|
SimpleStatement stmt = new SimpleStatement(format(query));
|
||||||
|
stmt.setConsistencyLevel(com.datastax.driver.core.ConsistencyLevel.ALL);
|
||||||
|
driverSession.execute(stmt);
|
||||||
|
}
|
||||||
|
|
||||||
|
private void prepare(String query)
|
||||||
|
{
|
||||||
|
SimpleStatement stmt = new SimpleStatement(format(query));
|
||||||
|
stmt.setConsistencyLevel(com.datastax.driver.core.ConsistencyLevel.ALL);
|
||||||
|
driverSession.prepare(stmt);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,134 @@
|
||||||
|
/*
|
||||||
|
* 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.db.guardrails;
|
||||||
|
|
||||||
|
import java.util.List;
|
||||||
|
|
||||||
|
import org.junit.After;
|
||||||
|
import org.junit.Before;
|
||||||
|
import org.junit.Test;
|
||||||
|
|
||||||
|
import org.apache.cassandra.cql3.QueryProcessor;
|
||||||
|
|
||||||
|
import static org.apache.cassandra.db.guardrails.PreparedStatementParameterRequirementGuardrail.MISPREPARED_STATEMENT_MESSAGE;
|
||||||
|
|
||||||
|
public class MispreparedStatementsTest extends GuardrailTester
|
||||||
|
{
|
||||||
|
private boolean preparedStatementsRequireParametersWarned;
|
||||||
|
private boolean preparedStatementsRequireParametersEnabled;
|
||||||
|
private String[] mispreparedQueries;
|
||||||
|
private String[] preparableQueries;
|
||||||
|
|
||||||
|
@Before
|
||||||
|
public void setup()
|
||||||
|
{
|
||||||
|
preparedStatementsRequireParametersWarned = guardrails().getPreparedStatementsRequireParametersWarned();
|
||||||
|
preparedStatementsRequireParametersEnabled = guardrails().getPreparedStatementsRequireParametersEnabled();
|
||||||
|
createTable("create table %s (pk int, ck int, v text, PRIMARY KEY (pk, ck))");
|
||||||
|
mispreparedQueries = new String[]{
|
||||||
|
String.format("SELECT * FROM %s WHERE pk = 1", currentTable()),
|
||||||
|
String.format("SELECT * FROM %s WHERE pk = 1 AND ck = 1", currentTable()),
|
||||||
|
String.format("SELECT * FROM %s WHERE ck = 1 ALLOW FILTERING", currentTable()),
|
||||||
|
String.format("SELECT * FROM %s WHERE v = 'a' ALLOW FILTERING", currentTable()),
|
||||||
|
String.format("INSERT INTO %s (pk, ck, v) VALUES (1, 1, 'a')", currentTable()),
|
||||||
|
String.format("UPDATE %s SET v = 'b' WHERE pk = 1 AND ck = 1", currentTable()),
|
||||||
|
String.format("DELETE FROM %s WHERE pk = 1 AND ck = 1", currentTable()),
|
||||||
|
String.format("BEGIN BATCH " +
|
||||||
|
"INSERT INTO %s (pk, ck, v) VALUES (1, 1, 'a');" +
|
||||||
|
"UPDATE %s SET v = 'b' WHERE pk = 2 AND ck = 2;" +
|
||||||
|
"APPLY BATCH;", currentTable(), currentTable()) };
|
||||||
|
preparableQueries = new String[]{
|
||||||
|
String.format("SELECT * FROM %s WHERE pk = ?", currentTable()),
|
||||||
|
String.format("INSERT INTO %s (pk, ck, v) VALUES (?, 1, 'a')", currentTable()),
|
||||||
|
String.format("UPDATE %s SET v = ? WHERE pk = 1 AND ck = 1", currentTable()),
|
||||||
|
String.format("SELECT * FROM %s WHERE pk = 1 AND ck = ?", currentTable()),
|
||||||
|
String.format("TRUNCATE %s", currentTable())
|
||||||
|
};
|
||||||
|
userClientState.setKeyspace(KEYSPACE);
|
||||||
|
}
|
||||||
|
|
||||||
|
@After
|
||||||
|
public void tear()
|
||||||
|
{
|
||||||
|
guardrails().setPreparedStatementsRequireParametersEnabled(preparedStatementsRequireParametersEnabled);
|
||||||
|
guardrails().setPreparedStatementsRequireParametersWarned(preparedStatementsRequireParametersWarned);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testPreparedStatementsRequireParametersEnabledGuardrailEnabled() throws Throwable
|
||||||
|
{
|
||||||
|
guardrails().setPreparedStatementsRequireParametersEnabled(true);
|
||||||
|
|
||||||
|
for (String query : mispreparedQueries)
|
||||||
|
assertGuardrailViolated(query);
|
||||||
|
|
||||||
|
for (String query : preparableQueries)
|
||||||
|
assertGuardrailAllowed(query);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testPreparedStatementsRequireParametersWarnEnabled() throws Throwable
|
||||||
|
{
|
||||||
|
guardrails().setPreparedStatementsRequireParametersEnabled(false);
|
||||||
|
for (String query : mispreparedQueries)
|
||||||
|
{
|
||||||
|
// Skip the batch query in this loop because it generates multiple warnings
|
||||||
|
if (query.contains("BEGIN BATCH")) continue;
|
||||||
|
|
||||||
|
assertWarns(() -> QueryProcessor.getStatement(query, userClientState).validatePrepare(userClientState),
|
||||||
|
MISPREPARED_STATEMENT_MESSAGE + " Query executed on keyspace '" + KEYSPACE + "', table '" + currentTable() + "'.");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testDoesNotWarnTwiceForSameQuery() throws Throwable
|
||||||
|
{
|
||||||
|
guardrails().setPreparedStatementsRequireParametersEnabled(false);
|
||||||
|
// We use a fully qualified name (KEYSPACE.table) here and ensure ClientState keyspace is null
|
||||||
|
// to suppress the "USE <keyspace> anti-pattern" warning, which otherwise conflicts with
|
||||||
|
// assertWarns by adding an unexpected second warning to the result.
|
||||||
|
String query = String.format("SELECT * FROM %s.%s WHERE pk = 1", KEYSPACE, currentTable());
|
||||||
|
assertWarns(() -> QueryProcessor.instance.prepare(query, userClientState),
|
||||||
|
MISPREPARED_STATEMENT_MESSAGE + " Query executed on keyspace '" + KEYSPACE + "', table '" + currentTable() + "'.");
|
||||||
|
assertValid(() -> QueryProcessor.instance.prepare(query, userClientState));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testBatchPreparedStatementsWarnEnabled() throws Throwable
|
||||||
|
{
|
||||||
|
guardrails().setPreparedStatementsRequireParametersEnabled(false);
|
||||||
|
String batchQuery = mispreparedQueries[7];
|
||||||
|
|
||||||
|
String message = MISPREPARED_STATEMENT_MESSAGE + " Query executed on keyspace '" + KEYSPACE + "', table '" + currentTable() + "'.";
|
||||||
|
|
||||||
|
assertWarns(() -> QueryProcessor.getStatement(batchQuery, userClientState).validatePrepare(userClientState),
|
||||||
|
List.of(message, message));
|
||||||
|
}
|
||||||
|
|
||||||
|
private void assertGuardrailViolated(String query) throws Throwable
|
||||||
|
{
|
||||||
|
assertFails(() -> QueryProcessor.getStatement(query, userClientState).validatePrepare(userClientState),
|
||||||
|
MISPREPARED_STATEMENT_MESSAGE + " Query executed on keyspace '" + KEYSPACE + "', table '" + currentTable() + "'.");
|
||||||
|
}
|
||||||
|
|
||||||
|
private void assertGuardrailAllowed(String query) throws Throwable
|
||||||
|
{
|
||||||
|
assertValid(() -> QueryProcessor.getStatement(query, userClientState).validatePrepare(userClientState));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -209,6 +209,8 @@ public class GuardrailsConfigCommandsTest extends CQLTester
|
||||||
"intersect_filtering_query_enabled true\n" +
|
"intersect_filtering_query_enabled true\n" +
|
||||||
"intersect_filtering_query_warned true\n" +
|
"intersect_filtering_query_warned true\n" +
|
||||||
"non_partition_restricted_index_query_enabled true\n" +
|
"non_partition_restricted_index_query_enabled true\n" +
|
||||||
|
"prepared_statements_require_parameters_enabled false\n" +
|
||||||
|
"prepared_statements_require_parameters_warned true\n" +
|
||||||
"read_before_write_list_operations_enabled true\n" +
|
"read_before_write_list_operations_enabled true\n" +
|
||||||
"secondary_indexes_enabled true\n" +
|
"secondary_indexes_enabled true\n" +
|
||||||
"simplestrategy_enabled true\n" +
|
"simplestrategy_enabled true\n" +
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue