From 4c33f1f2d7672ed9eef08908eb86a262d9fdc35b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Diakowski?= Date: Thu, 23 Oct 2025 09:51:53 +0200 Subject: [PATCH] Restrict BytesType compatibility to scalar types only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- CHANGES.txt | 1 + .../schema/AlterTableStatement.java | 3 +- .../cassandra/db/marshal/BytesType.java | 8 +++ .../cql3/validation/operations/AlterTest.java | 13 ++-- .../cassandra/db/marshal/BytesTypeTest.java | 71 +++++++++++++++++++ 5 files changed, 89 insertions(+), 7 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 03f0da2f28..30e1b420b8 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -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) diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java index f3021cad9f..8d520b4d0f 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java @@ -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()); } diff --git a/src/java/org/apache/cassandra/db/marshal/BytesType.java b/src/java/org/apache/cassandra/db/marshal/BytesType.java index cabd007b09..ccd144123a 100644 --- a/src/java/org/apache/cassandra/db/marshal/BytesType.java +++ b/src/java/org/apache/cassandra/db/marshal/BytesType.java @@ -85,6 +85,14 @@ public class BytesType extends AbstractType 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; diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java index a0669495a1..4b5ea9b60b 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java @@ -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>", "frozen>", "frozen>"}; @@ -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]); } } diff --git a/test/unit/org/apache/cassandra/db/marshal/BytesTypeTest.java b/test/unit/org/apache/cassandra/db/marshal/BytesTypeTest.java index 789794044a..0e8a5fe924 100644 --- a/test/unit/org/apache/cassandra/db/marshal/BytesTypeTest.java +++ b/test/unit/org/apache/cassandra/db/marshal/BytesTypeTest.java @@ -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", + BytesType.instance.isSerializationCompatibleWith(frozenList)); + assertFalse("BytesType should NOT be serialization-compatible with frozen", + BytesType.instance.isSerializationCompatibleWith(frozenSet)); + assertFalse("BytesType should NOT be serialization-compatible with frozen", + 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)); + } }