From 369daf56bca1cf56ceddfc5808b8e91a67d4a7ef Mon Sep 17 00:00:00 2001 From: Marcus Eriksson Date: Mon, 3 Mar 2025 08:41:34 +0100 Subject: [PATCH] Fix PartitionUpdate.isEmpty deserialization issue to avoid potential EOFException Patch by marcuse; reviewed by Sam Tunnicliffe for CASSANDRA-20345 --- CHANGES.txt | 1 + .../db/partitions/PartitionUpdate.java | 2 +- .../distributed/test/PaxosRepairTest.java | 33 +++++++++++++++++++ .../paxos/uncommitted/PaxosRowsTest.java | 5 +-- 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 76e6fc4872..f6cbace4f0 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 5.1 + * Fix PartitionUpdate.isEmpty deserialization issue to avoid potential EOFException (CASSANDRA-20345) * Avoid adding LEFT nodes to tokenMap on upgrade from gossip (CASSANDRA-20344) * Allow empty placements when deserializing cluster metadata (CASSANDRA-20343) * Reduce heap pressure when initializing CMS (CASSANDRA-20267) diff --git a/src/java/org/apache/cassandra/db/partitions/PartitionUpdate.java b/src/java/org/apache/cassandra/db/partitions/PartitionUpdate.java index c20a8490a4..ff5d0f9035 100644 --- a/src/java/org/apache/cassandra/db/partitions/PartitionUpdate.java +++ b/src/java/org/apache/cassandra/db/partitions/PartitionUpdate.java @@ -800,7 +800,7 @@ public class PartitionUpdate extends AbstractBTreePartition if (version >= MessagingService.VERSION_51) { long epoch = VIntCoding.getUnsignedVInt(in, position); - position += VIntCoding.computeVIntSize(epoch); + position += VIntCoding.computeUnsignedVIntSize(epoch); } // DecoratedKey key = metadata.decorateKey(ByteBufferUtil.readWithVIntLength(in)); diff --git a/test/distributed/org/apache/cassandra/distributed/test/PaxosRepairTest.java b/test/distributed/org/apache/cassandra/distributed/test/PaxosRepairTest.java index e22a6be3ea..0197106bc6 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/PaxosRepairTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/PaxosRepairTest.java @@ -74,6 +74,7 @@ import org.apache.cassandra.tcm.ClusterMetadata; import org.apache.cassandra.tcm.ClusterMetadataService; import org.apache.cassandra.tcm.membership.Directory; import org.apache.cassandra.tcm.membership.NodeVersion; +import org.apache.cassandra.tcm.transformations.CustomTransformation; import org.apache.cassandra.tcm.transformations.ForceSnapshot; import org.apache.cassandra.utils.*; @@ -271,6 +272,38 @@ public class PaxosRepairTest extends TestBaseImpl } } + @Test + public void epochBadDeserializationTest() throws Throwable + { + try (Cluster cluster = init(Cluster.build(3).withConfig(WITH_NETWORK).withoutVNodes().start())) + { + cluster.schemaChange("CREATE TABLE " + KEYSPACE + '.' + TABLE + " (pk text primary key, v int)"); + cluster.get(1).runOnInstance(() -> { + // just execute transformations to get epoch bumped enough for the bug to occur + for (int i = 0; i < 75; i++) + ClusterMetadataService.instance().commit(CustomTransformation.make("x"+i)); + }); + // and bump the epoch in the tablemetadata: + cluster.schemaChange("ALTER TABLE " + KEYSPACE + '.' + TABLE + " WITH comment='abc'"); + + cluster.verbs(PAXOS_COMMIT_REQ).drop(); + try + { + cluster.coordinator(1).execute("INSERT INTO " + KEYSPACE + '.' + TABLE + " (pk, v) VALUES ('xyzxyzxyzxyzxyzxyzxyzxyz', 1) IF NOT EXISTS", ConsistencyLevel.QUORUM); + Assert.fail("expected write timeout"); + } + catch (RuntimeException e) + { + // exception expected + } + + cluster.filters().reset(); + cluster.get(1).shutdown().get(); + cluster.get(1).startup(); + } + } + + @Ignore @Test public void topologyChangePaxosTest() throws Throwable diff --git a/test/unit/org/apache/cassandra/service/paxos/uncommitted/PaxosRowsTest.java b/test/unit/org/apache/cassandra/service/paxos/uncommitted/PaxosRowsTest.java index 911ac72d82..529a2cf0a8 100644 --- a/test/unit/org/apache/cassandra/service/paxos/uncommitted/PaxosRowsTest.java +++ b/test/unit/org/apache/cassandra/service/paxos/uncommitted/PaxosRowsTest.java @@ -43,6 +43,7 @@ import org.apache.cassandra.schema.TableId; import org.apache.cassandra.schema.TableMetadata; import org.apache.cassandra.service.paxos.Ballot; import org.apache.cassandra.service.paxos.Commit; +import org.apache.cassandra.tcm.Epoch; import org.apache.cassandra.utils.ByteBufferUtil; import org.apache.cassandra.utils.CloseableIterator; import org.apache.cassandra.utils.FBUtilities; @@ -96,7 +97,7 @@ public class PaxosRowsTest SchemaLoader.prepareServer(); ks = "coordinatorsessiontest"; - metadata = CreateTableStatement.parse("CREATE TABLE tbl (k INT PRIMARY KEY, v INT)", ks).build(); + metadata = CreateTableStatement.parse("CREATE TABLE tbl (k INT PRIMARY KEY, v INT)", ks).epoch(Epoch.create(100)).build(); tableId = metadata.id; } @@ -109,7 +110,7 @@ public class PaxosRowsTest @Test public void testRowInterpretation() { - DecoratedKey key = dk(5); + DecoratedKey key = dk(Integer.MAX_VALUE); Ballot[] ballots = createBallots(3); SystemKeyspace.savePaxosWritePromise(key, metadata, ballots[0]);