Restrict BytesType compatibility to scalar types only

Converting collections or UDTs to raw bytes is nonsensical - it
allows reading raw serialized bytes which have no meaningful
interpretation.

patch by Mikołaj Diakowski; reviewed by Stefan Miklosovic, Brandon Williams for CASSANDRA-20982
This commit is contained in:
Mikołaj Diakowski 2025-10-23 09:51:53 +02:00 committed by Stefan Miklosovic
parent 34c3736c9e
commit 4c33f1f2d7
No known key found for this signature in database
GPG Key ID: 32F35CB2F546D93E
5 changed files with 89 additions and 7 deletions

View File

@ -1,4 +1,5 @@
4.0.20
* Restrict BytesType compatibility to scalar types only (CASSANDRA-20982)
* Backport fix to nodetool gcstats output for direct memory (CASSANDRA-21037)
* ArrayIndexOutOfBoundsException with repaired data tracking and counters (CASSANDRA-20871)
* Fix cleanup of old incremental repair sessions in case of owned token range changes or a table deleting (CASSANDRA-20877)

View File

@ -219,9 +219,10 @@ public abstract class AlterTableStatement extends AlterSchemaStatement
// columns is pushed deeper down the line. The latter would still be problematic in cases of schema races.
if (!type.isSerializationCompatibleWith(droppedColumn.type))
{
throw ire("Cannot re-add previously dropped column '%s' of type %s, incompatible with previous type %s",
throw ire("Cannot add a column '%s' of type %s, incompatible with previously dropped column '%s' of type %s",
name,
type.asCQL3Type(),
name,
droppedColumn.type.asCQL3Type());
}

View File

@ -85,6 +85,14 @@ public class BytesType extends AbstractType<ByteBuffer>
return true;
}
@Override
public boolean isSerializationCompatibleWith(AbstractType<?> previous)
{
// BytesType should only be compatible with simple scalar types, not with collections or UDTs
// because converting a collection or UDT to raw bytes is nonsensical
return !previous.isCollection() && !previous.isUDT() && super.isSerializationCompatibleWith(previous);
}
public CQL3Type asCQL3Type()
{
return CQL3Type.Native.BLOB;

View File

@ -52,7 +52,7 @@ public class AlterTest extends CQLTester
{
createTable("CREATE TABLE %s (a int, b " + type + ", PRIMARY KEY (a));");
alterTable("ALTER TABLE %s DROP b;");
assertInvalidMessage("Cannot re-add previously dropped column 'b' of type blob, incompatible with previous type " + type,
assertInvalidMessage("Cannot add a column 'b' of type blob, incompatible with previously dropped column 'b' of type " + type,
"ALTER TABLE %s ADD b blob;");
}
@ -60,13 +60,13 @@ public class AlterTest extends CQLTester
{
createTable("CREATE TABLE %s (a int, b blob, PRIMARY KEY (a));");
alterTable("ALTER TABLE %s DROP b;");
assertInvalidMessage("Cannot re-add previously dropped column 'b' of type " + type + ", incompatible with previous type blob",
assertInvalidMessage("Cannot add a column 'b' of type " + type + ", incompatible with previously dropped column 'b' of type blob",
"ALTER TABLE %s ADD b " + type + ';');
}
}
@Test
public void testFrozenCollectionsAreCompatibleWithBlob()
public void testFrozenCollectionsAreNotCompatibleWithBlob() throws Throwable
{
String[] collectionTypes = new String[] {"frozen<map<int, int>>", "frozen<set<int>>", "frozen<list<int>>"};
@ -74,7 +74,8 @@ public class AlterTest extends CQLTester
{
createTable("CREATE TABLE %s (a int, b " + type + ", PRIMARY KEY (a));");
alterTable("ALTER TABLE %s DROP b;");
alterTable("ALTER TABLE %s ADD b blob;");
assertInvalidMessage("Cannot add a column 'b' of type blob, incompatible with previously dropped column 'b' of type " + type,
"ALTER TABLE %s ADD b blob;");
}
}
@ -482,8 +483,8 @@ public class AlterTest extends CQLTester
{
createTable("create table %s (k int, c int, v " + typePair[0] + ", PRIMARY KEY (k, c))");
execute("alter table %s drop v");
assertInvalidMessage("Cannot re-add previously dropped column 'v' of type "
+ typePair[1] + ", incompatible with previous type " + typePair[0],
assertInvalidMessage("Cannot add a column 'v' of type "
+ typePair[1] + ", incompatible with previously dropped column 'v' of type " + typePair[0],
"alter table %s add v " + typePair[1]);
}
}

View File

@ -19,9 +19,16 @@
*/
package org.apache.cassandra.db.marshal;
import java.util.Arrays;
import org.apache.cassandra.cql3.FieldIdentifier;
import org.apache.cassandra.serializers.MarshalException;
import org.apache.cassandra.utils.ByteBufferUtil;
import org.junit.Test;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
public class BytesTypeTest
{
private static final String INVALID_HEX = "33AG45F"; // Invalid (has a G)
@ -38,4 +45,68 @@ public class BytesTypeTest
{
BytesType.instance.fromString(VALID_HEX);
}
@Test
public void testValueCompatibilityWithScalarTypes()
{
// BytesType should be value-compatible with simple scalar types
assertTrue("BytesType should be value-compatible with AsciiType",
BytesType.instance.isValueCompatibleWith(AsciiType.instance));
assertTrue("BytesType should be value-compatible with UTF8Type",
BytesType.instance.isValueCompatibleWith(UTF8Type.instance));
assertTrue("BytesType should be value-compatible with Int32Type",
BytesType.instance.isValueCompatibleWith(Int32Type.instance));
assertTrue("BytesType should be value-compatible with LongType",
BytesType.instance.isValueCompatibleWith(LongType.instance));
assertTrue("BytesType should be value-compatible with UUIDType",
BytesType.instance.isValueCompatibleWith(UUIDType.instance));
assertTrue("BytesType should be value-compatible with BooleanType",
BytesType.instance.isValueCompatibleWith(BooleanType.instance));
assertTrue("BytesType should be value-compatible with itself",
BytesType.instance.isValueCompatibleWith(BytesType.instance));
}
@Test
public void testSerializationIncompatibilityWithCollections()
{
// BytesType should NOT be serialization-compatible with collections (even frozen ones)
// because converting a collection to raw bytes for schema changes is nonsensical
ListType<?> frozenList = ListType.getInstance(Int32Type.instance, false);
SetType<?> frozenSet = SetType.getInstance(Int32Type.instance, false);
MapType<?, ?> frozenMap = MapType.getInstance(Int32Type.instance, UTF8Type.instance, false);
assertFalse("BytesType should NOT be serialization-compatible with frozen<list>",
BytesType.instance.isSerializationCompatibleWith(frozenList));
assertFalse("BytesType should NOT be serialization-compatible with frozen<set>",
BytesType.instance.isSerializationCompatibleWith(frozenSet));
assertFalse("BytesType should NOT be serialization-compatible with frozen<map>",
BytesType.instance.isSerializationCompatibleWith(frozenMap));
// Also test with multi-cell collections
ListType<?> multiCellList = ListType.getInstance(Int32Type.instance, true);
SetType<?> multiCellSet = SetType.getInstance(Int32Type.instance, true);
MapType<?, ?> multiCellMap = MapType.getInstance(Int32Type.instance, UTF8Type.instance, true);
assertFalse("BytesType should NOT be serialization-compatible with list",
BytesType.instance.isSerializationCompatibleWith(multiCellList));
assertFalse("BytesType should NOT be serialization-compatible with set",
BytesType.instance.isSerializationCompatibleWith(multiCellSet));
assertFalse("BytesType should NOT be serialization-compatible with map",
BytesType.instance.isSerializationCompatibleWith(multiCellMap));
}
@Test
public void testSerializationIncompatibilityWithUDT()
{
// BytesType should NOT be serialization-compatible with User Defined Types
// because converting a UDT to raw bytes for schema changes is nonsensical
UserType udt = new UserType("ks",
ByteBufferUtil.bytes("myType"),
Arrays.asList(FieldIdentifier.forQuoted("field1"), FieldIdentifier.forQuoted("field2")),
Arrays.asList(Int32Type.instance, UTF8Type.instance),
true);
assertFalse("BytesType should NOT be serialization-compatible with UDT",
BytesType.instance.isSerializationCompatibleWith(udt));
}
}