From fb66800a00aeaa8046cb3e6b1401fdc4f81848d5 Mon Sep 17 00:00:00 2001 From: Ekaterina Dimitrova Date: Fri, 8 Apr 2022 15:22:59 -0400 Subject: [PATCH] Fix data corruption in AbstractCompositeType due to static boolean byte buffers patch by Stefania Alborghetti and Marcus Eriksson; reviewed by Marcus Eriksson, Benjamin Lerer and Ekaterina Dimitrova for CASSANDRA-14752 Co-authored-by: Stefania Alborghetti Co-authored-by: Marcuse Eriksson --- CHANGES.txt | 1 + .../db/marshal/AbstractCompositeType.java | 6 +-- .../distributed/test/BooleanTest.java | 45 +++++++++++++++++++ .../db/ColumnFamilyStoreCQLHelperTest.java | 18 ++++++++ 4 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 test/distributed/org/apache/cassandra/distributed/test/BooleanTest.java diff --git a/CHANGES.txt b/CHANGES.txt index fdf64110fa..c481d73fd2 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.0.27 + * Fix data corruption in AbstractCompositeType due to static boolean byte buffers (CASSANDRA-14752) * Add procps dependency to RPM/Debian packages (CASSANDRA-17516) * Suppress CVE-2021-44521 (CASSANDRA-17492) * ConnectionLimitHandler may leaks connection count if remote connection drops (CASSANDRA-17252) diff --git a/src/java/org/apache/cassandra/db/marshal/AbstractCompositeType.java b/src/java/org/apache/cassandra/db/marshal/AbstractCompositeType.java index b0d6a5da15..3d74c992e8 100644 --- a/src/java/org/apache/cassandra/db/marshal/AbstractCompositeType.java +++ b/src/java/org/apache/cassandra/db/marshal/AbstractCompositeType.java @@ -190,8 +190,8 @@ public abstract class AbstractCompositeType extends AbstractType public ByteBuffer fromString(String source) { List parts = split(source); - List components = new ArrayList(parts.size()); - List comparators = new ArrayList(parts.size()); + List components = new ArrayList<>(parts.size()); + List comparators = new ArrayList<>(parts.size()); int totalLength = 0, i = 0; boolean lastByteIsOne = false; boolean lastByteIsMinusOne = false; @@ -226,7 +226,7 @@ public abstract class AbstractCompositeType extends AbstractType { comparators.get(i).serializeComparator(bb); ByteBufferUtil.writeShortLength(bb, component.remaining()); - bb.put(component); // it's ok to consume component as we won't use it anymore + bb.put(component.duplicate()); // it's not ok to consume component as we did not create it (CASSANDRA-14752) bb.put((byte)0); ++i; } diff --git a/test/distributed/org/apache/cassandra/distributed/test/BooleanTest.java b/test/distributed/org/apache/cassandra/distributed/test/BooleanTest.java new file mode 100644 index 0000000000..fa6722d9a9 --- /dev/null +++ b/test/distributed/org/apache/cassandra/distributed/test/BooleanTest.java @@ -0,0 +1,45 @@ +/* + * 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; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.cassandra.distributed.Cluster; +import org.apache.cassandra.distributed.api.ConsistencyLevel; +import org.apache.cassandra.serializers.BooleanSerializer; + +public class BooleanTest extends TestBaseImpl +{ + @Test + public void booleanTest() throws Throwable + { + try(Cluster cluster = init(Cluster.build(1).start())) + { + cluster.schemaChange("create table " + KEYSPACE + ".tbl (id int, ck boolean, t int, primary key ((id, ck)))"); + for (int i = 0; i < 10; i++) + cluster.coordinator(1).execute(withKeyspace("insert into %s.tbl (id, ck, t) values (?, true, ?)"), ConsistencyLevel.ALL, i, i); + cluster.get(1).nodetoolResult("getsstables", KEYSPACE, "tbl", "1:true"); + cluster.forEach(i -> i.runOnInstance(() -> { + Assert.assertEquals(0, BooleanSerializer.instance.serialize(true).position()); + Assert.assertEquals(0, BooleanSerializer.instance.serialize(false).position()); + })); + } + } +} diff --git a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreCQLHelperTest.java b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreCQLHelperTest.java index 58d7e25794..9dac79bb14 100644 --- a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreCQLHelperTest.java +++ b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreCQLHelperTest.java @@ -674,4 +674,22 @@ public class ColumnFamilyStoreCQLHelperTest extends CQLTester "\tWITH ID = " + cfs.metadata.cfId + "\n" + "\tAND COMPACT STORAGE")); } + + @Test + public void testBooleanCompositeKey() throws Throwable + { + createTable("CREATE TABLE %s (t_id boolean, id boolean, ck boolean, nk boolean, PRIMARY KEY ((t_id, id), ck))"); + + execute("insert into %s (t_id, id, ck, nk) VALUES (true, false, false, true)"); + assertRows(execute("select * from %s"), row(true, false, false, true)); + + // CASSANDRA-14752 - + // a problem with composite boolean types meant that calling this would + // prevent any boolean values to be inserted afterwards + ColumnFamilyStore cfs = getCurrentColumnFamilyStore(); + cfs.getSSTablesForKey("false:true"); + + execute("insert into %s (t_id, id, ck, nk) VALUES (true, true, false, true)"); + assertRows(execute("select t_id, id, ck, nk from %s"), row(true, true, false, true), row(true, false, false, true)); + } }