diff --git a/CHANGES.txt b/CHANGES.txt index 313000abc9..9db65e981b 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -13,6 +13,7 @@ * Fix incremental repair not remove parent session on remote (CASSANDRA-8291) * Improve JBOD disk utilization (CASSANDRA-7386) Merged from 2.0: + * Validate size of indexed column values (CASSANDRA-8280) * Make LCS split compaction results over all data directories (CASSANDRA-8329) * Fix some failing queries that use multi-column relations on COMPACT STORAGE tables (CASSANDRA-8264) diff --git a/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java b/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java index 2c8717347c..09f26d63b9 100644 --- a/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java @@ -25,7 +25,7 @@ import org.apache.cassandra.config.CFMetaData; import org.apache.cassandra.config.ColumnDefinition; import org.apache.cassandra.db.*; import org.apache.cassandra.db.composites.Composite; -import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.index.SecondaryIndexManager; import org.apache.cassandra.exceptions.*; import org.apache.cassandra.utils.ByteBufferUtil; import org.apache.cassandra.utils.Pair; @@ -97,6 +97,21 @@ public class UpdateStatement extends ModificationStatement for (Operation update : updates) update.execute(key, cf, prefix, params); } + + SecondaryIndexManager indexManager = Keyspace.open(cfm.ksName).getColumnFamilyStore(cfm.cfId).indexManager; + if (indexManager.hasIndexes()) + { + for (Cell cell : cf) + { + // Indexed values must be validated by any applicable index. See CASSANDRA-3057/4240/8081 for more details + if (!indexManager.validate(cell)) + throw new InvalidRequestException(String.format("Can't index column value of size %d for index %s on %s.%s", + cell.value().remaining(), + cfm.getColumnDefinition(cell.name()).getIndexName(), + cfm.ksName, + cfm.cfName)); + } + } } public static class ParsedInsert extends ModificationStatement.Parsed diff --git a/src/java/org/apache/cassandra/io/sstable/SSTableWriter.java b/src/java/org/apache/cassandra/io/sstable/SSTableWriter.java index 0f95a9bc51..53176e3b52 100644 --- a/src/java/org/apache/cassandra/io/sstable/SSTableWriter.java +++ b/src/java/org/apache/cassandra/io/sstable/SSTableWriter.java @@ -64,6 +64,7 @@ import org.apache.cassandra.io.util.SegmentedFile; import org.apache.cassandra.io.util.SequentialWriter; import org.apache.cassandra.service.StorageService; import org.apache.cassandra.utils.ByteBufferUtil; +import org.apache.cassandra.utils.FBUtilities; import org.apache.cassandra.utils.FilterFactory; import org.apache.cassandra.utils.IFilter; import org.apache.cassandra.utils.Pair; @@ -212,6 +213,14 @@ public class SSTableWriter extends SSTable public void append(DecoratedKey decoratedKey, ColumnFamily cf) { + if (decoratedKey.getKey().remaining() > FBUtilities.MAX_UNSIGNED_SHORT) + { + logger.error("Key size {} exceeds maximum of {}, skipping row", + decoratedKey.getKey().remaining(), + FBUtilities.MAX_UNSIGNED_SHORT); + return; + } + long startPosition = beforeAppend(decoratedKey); try { diff --git a/test/unit/org/apache/cassandra/cql3/IndexedValuesValidationTest.java b/test/unit/org/apache/cassandra/cql3/IndexedValuesValidationTest.java new file mode 100644 index 0000000000..05acf86da9 --- /dev/null +++ b/test/unit/org/apache/cassandra/cql3/IndexedValuesValidationTest.java @@ -0,0 +1,86 @@ +/* + * + * * 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.nio.ByteBuffer; + +import org.junit.Test; + +import org.apache.cassandra.exceptions.InvalidRequestException; + +import static org.junit.Assert.fail; + +public class IndexedValuesValidationTest extends CQLTester +{ + // CASSANDRA-8280/8081 + // reject updates with indexed values where value > 64k + @Test + public void testIndexOnCompositeValueOver64k() throws Throwable + { + createTable("CREATE TABLE %s(a int, b int, c blob, PRIMARY KEY (a))"); + createIndex("CREATE INDEX ON %s(c)"); + performInsertWithIndexedValueOver64k("INSERT INTO %s (a, b, c) VALUES (0, 0, ?)"); + } + + @Test + public void testIndexOnClusteringValueOver64k() throws Throwable + { + createTable("CREATE TABLE %s(a int, b blob, c int, PRIMARY KEY (a, b))"); + createIndex("CREATE INDEX ON %s(b)"); + performInsertWithIndexedValueOver64k("INSERT INTO %s (a, b, c) VALUES (0, ?, 0)"); + } + + @Test + public void testIndexOnPartitionKeyOver64k() throws Throwable + { + createTable("CREATE TABLE %s(a blob, b int, c int, PRIMARY KEY ((a, b)))"); + createIndex("CREATE INDEX ON %s(a)"); + performInsertWithIndexedValueOver64k("INSERT INTO %s (a, b, c) VALUES (?, 0, 0)"); + } + + @Test + public void testCompactTableWithValueOver64k() throws Throwable + { + createTable("CREATE TABLE %s(a int, b blob, PRIMARY KEY (a)) WITH COMPACT STORAGE"); + createIndex("CREATE INDEX ON %s(b)"); + performInsertWithIndexedValueOver64k("INSERT INTO %s (a, b) VALUES (0, ?)"); + } + + public void performInsertWithIndexedValueOver64k(String insertCQL) throws Throwable + { + ByteBuffer buf = ByteBuffer.allocate(1024 * 65); + buf.clear(); + + //read more than 64k + for (int i=0; i<1024 + 1; i++) + buf.put((byte)0); + + try + { + execute(insertCQL, buf); + fail("Expected statement to fail validation"); + } + catch (InvalidRequestException e) + { + // as expected + } + } +}