Merge branch 'cassandra-5.0' into trunk

CHANGES.txt

@@ -155,6 +155,7 @@ Merged from 4.1:
  * Enforce CQL message size limit on multiframe messages (CASSANDRA-20052)
  * Fix race condition in DecayingEstimatedHistogramReservoir during rescale (CASSANDRA-19365)
 Merged from 4.0:
+ * CBUtil serialization of UTF8 does not handle all UTF8 properly (CASSANDRA-20234)
  * Make hint expiry use request start time rather than timeout time for TTL (CASSANDRA-20014)
  * Do not attach rows and partitions to QueryCancellationChecker when already attached (CASSANDRA-20135)
  * Allow hint delivery during schema mismatch (CASSANDRA-20188)

src/java/org/apache/cassandra/db/TypeSizes.java

@@ -45,6 +45,16 @@ public final class TypeSizes
         return sizeof((short) length) + length;
     }
 
+    /**
+     * Java uses a Modified UTF-8 (see {@link java.io.DataOutput#writeUTF(String)}), and this method attempts to
+     * calculate the modified utf-8 length; this method only works when the utf-8 writing logic is java's modified utf-8
+     * and will not work when normal utf-8 is written.
+     *
+     * If normal utf-8 is written, then {@link org.apache.cassandra.transport.CBUtil#encodedUTF8Length(String)} should be
+     * used instread of this one.
+     *
+     * @see <a href="https://docs.oracle.com/en/java/javase/23/docs/api/java.base/java/io/DataInput.html#modified-utf-8">Modified UTF 8</a>
+     */
     public static int encodedUTF8Length(String st)
     {
         int strlen = st.length();

src/java/org/apache/cassandra/gms/VersionedValue.java

@@ -22,6 +22,7 @@ import java.io.DataOutputStream;
 import java.io.IOException;
 import java.net.InetAddress;
 import java.util.Collection;
+import java.util.Objects;
 import java.util.Set;
 import java.util.UUID;
 import java.util.function.Supplier;
@@ -138,6 +139,21 @@ public class VersionedValue implements Comparable<VersionedValue>
         return "Value(" + value + ',' + version + ')';
     }
 
+    @Override
+    public boolean equals(Object o)
+    {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        VersionedValue that = (VersionedValue) o;
+        return version == that.version && value.equals(that.value);
+    }
+
+    @Override
+    public int hashCode()
+    {
+        return Objects.hash(version, value);
+    }
+
     public byte[] toBytes()
     {
         return value.getBytes(ISO_8859_1);

src/java/org/apache/cassandra/transport/CBUtil.java

@@ -32,6 +32,9 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
+
+import com.google.common.base.Preconditions;
+
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.ByteBufAllocator;
 import io.netty.buffer.ByteBufUtil;
@@ -39,8 +42,8 @@ import io.netty.buffer.PooledByteBufAllocator;
 import io.netty.buffer.UnpooledByteBufAllocator;
 import io.netty.util.concurrent.FastThreadLocal;
 import org.apache.cassandra.db.ConsistencyLevel;
-import org.apache.cassandra.db.TypeSizes;
 import org.apache.cassandra.utils.ByteBufferUtil;
+import org.apache.cassandra.utils.LazyToString;
 import org.apache.cassandra.utils.Pair;
 import org.apache.cassandra.utils.TimeUUID;
 import org.apache.cassandra.utils.UUIDGen;
@@ -161,14 +164,30 @@ public abstract class CBUtil
 
     public static void writeString(String str, ByteBuf cb)
     {
-        int length = TypeSizes.encodedUTF8Length(str);
+        int length = encodedUTF8Length(str);
+        Preconditions.checkArgument(length <= Short.MAX_VALUE,
+                                    LazyToString.lazy(() -> String.format("String too large; expected %d <= %d", length, Short.MAX_VALUE)));
         cb.writeShort(length);
         ByteBufUtil.reserveAndWriteUtf8(cb, str, length);
     }
 
     public static int sizeOfString(String str)
     {
-        return 2 + TypeSizes.encodedUTF8Length(str);
+        return 2 + encodedUTF8Length(str);
+    }
+
+    /**
+     * Java uses a Modified UTF-8, whereas Netty uses UTF-8 proper... this means that the encoded UTF8 lengths
+     * do not match, and you must be careful to use the correct length method...
+     *
+     * When using {@link ByteBufUtil#reserveAndWriteUtf8(ByteBuf, CharSequence, int)} or similiar logic, you must use
+     * this method.  When using {@link java.io.DataOutput#writeUTF(String)} you must use {@link org.apache.cassandra.db.TypeSizes#encodedUTF8Length(String)}.
+     *
+     * @see <a href="https://docs.oracle.com/en/java/javase/23/docs/api/java.base/java/io/DataInput.html#modified-utf-8">Modified UTF 8</a>
+     */
+    public static int encodedUTF8Length(String str)
+    {
+        return ByteBufUtil.utf8Bytes(str);
     }
 
     /**
@@ -196,14 +215,14 @@ public abstract class CBUtil
 
     public static void writeLongString(String str, ByteBuf cb)
     {
-        int length = TypeSizes.encodedUTF8Length(str);
+        int length = encodedUTF8Length(str);
         cb.writeInt(length);
         ByteBufUtil.reserveAndWriteUtf8(cb, str, length);
     }
 
     public static int sizeOfLongString(String str)
     {
-        return 4 + TypeSizes.encodedUTF8Length(str);
+        return 4 + encodedUTF8Length(str);
     }
 
     public static byte[] readBytes(ByteBuf cb)

test/unit/org/apache/cassandra/gms/VersionedValueTest.java

@@ -0,0 +1,53 @@
+/*
+ * 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.gms;
+
+import org.junit.Test;
+
+import accord.utils.Gen;
+import org.apache.cassandra.db.TypeSizes;
+import org.apache.cassandra.io.IVersionedSerializers;
+import org.apache.cassandra.io.util.DataOutputBuffer;
+import org.apache.cassandra.net.MessagingService;
+import org.apache.cassandra.utils.CassandraGenerators;
+import org.apache.cassandra.utils.Generators;
+
+import static accord.utils.Property.qt;
+
+public class VersionedValueTest
+{
+    @Test
+    public void serde()
+    {
+        DataOutputBuffer buffer = new DataOutputBuffer();
+        qt().forAll(values()).check(state -> {
+            for (MessagingService.Version version : MessagingService.Version.supportedVersions())
+                IVersionedSerializers.testSerde(buffer, VersionedValue.serializer, state, version.value);
+        });
+    }
+
+    private static Gen<VersionedValue> values()
+    {
+        return Generators.toGen(CassandraGenerators.partitioners().flatMap(CassandraGenerators::gossipStatusValue))
+                         // when status == "" this returns null... skip
+                         .filter(vv -> vv != null)
+                         // sometimes the text is too big, must not be larger than Short.MAX_VALUE
+                         .filter(vv -> TypeSizes.encodedUTF8Length(vv.value) <= Short.MAX_VALUE);
+    }
+}

test/unit/org/apache/cassandra/io/IVersionedSerializers.java

@@ -0,0 +1,39 @@
+/*
+ * 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.io;
+
+import java.io.IOException;
+
+import org.apache.cassandra.io.util.DataInputBuffer;
+import org.apache.cassandra.io.util.DataOutputBuffer;
+import org.assertj.core.api.Assertions;
+
+public class IVersionedSerializers
+{
+    public static <T> void testSerde(DataOutputBuffer output, IVersionedSerializer<T> serializer, T input, int version) throws IOException
+    {
+        output.clear();
+        long expectedSize = serializer.serializedSize(input, version);
+        serializer.serialize(input, output, version);
+        Assertions.assertThat(output.getLength()).describedAs("The serialized size and bytes written do not match").isEqualTo(expectedSize);
+        DataInputBuffer in = new DataInputBuffer(output.unsafeGetBufferAndFlip(), false);
+        T read = serializer.deserialize(in, version);
+        Assertions.assertThat(read).describedAs("The deserialized output does not match the serialized input").isEqualTo(input);
+    }
+}

test/unit/org/apache/cassandra/transport/CBUtilTest.java

@@ -22,9 +22,13 @@ import org.junit.After;
 import org.junit.Assert;
 import org.junit.Test;
 
+import accord.utils.Gens;
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.ByteBufAllocator;
 import io.netty.buffer.PooledByteBufAllocator;
+import org.assertj.core.api.Assertions;
+
+import static accord.utils.Property.qt;
 
 public class CBUtilTest
 {
@@ -38,6 +42,21 @@ public class CBUtilTest
             buf.release(buf.refCnt());
     }
 
+    @Test
+    public void stringList()
+    {
+        qt().forAll(Gens.lists(Gens.strings().all().ofLengthBetween(0, 20)).ofSizeBetween(0, 10)).check(list -> {
+            int expectedSize = CBUtil.sizeOfStringList(list);
+            var body = CBUtil.allocator.buffer(expectedSize * 2);
+            CBUtil.writeStringList(list, body);
+            Assertions.assertThat(body.readableBytes()).isEqualTo(expectedSize);
+            // In CASSANDRA-20234 the sizeOf method now reflects the write method, but the work to do this in read
+            // was not done; read took a stance that it wants to limit things in CASSANDRA-8101 but that never made
+            // it to the other methods... write and read are not compatable with the full domain of UTF-8, and fixing
+            // that was higher bar than CASSANDRA-20234 wanted to tackle out of fear of unknown regressions it would cause.
+        });
+    }
+
     @Test
     public void writeAndReadString()
     {

test/unit/org/apache/cassandra/utils/CassandraGenerators.java

@@ -721,9 +721,13 @@ public final class CassandraGenerators
                         );
     }
 
-    private static Gen<VersionedValue> gossipStatusValue()
+    public static Gen<VersionedValue> gossipStatusValue()
+    {
+        return gossipStatusValue(DatabaseDescriptor.getPartitioner());
+    }
+
+    public static Gen<VersionedValue> gossipStatusValue(IPartitioner partitioner)
     {
-        IPartitioner partitioner = DatabaseDescriptor.getPartitioner();
         Gen<String> statusGen = gossipStatus();
         Gen<Token> tokenGen = token(partitioner);
         Gen<? extends Collection<Token>> tokensGen = tokens(partitioner);