Skip to content
Permalink
Browse files
Merge branch 'cassandra-4.1' into trunk
  • Loading branch information
adelapena committed May 20, 2022
2 parents 23e5da9 + c1d163a commit 56ce8d1f7f90fa9dd9d1b0e3507dab0cd2dec0cf
Showing 7 changed files with 308 additions and 16 deletions.
@@ -179,6 +179,7 @@ Merged from 3.0:
Merged from 4.0:
Merged from 3.11:
Merged from 3.0:
* Fix issue where frozen maps may not be serialized in the correct order (CASSANDRA-17623)
* Suppress CVE-2022-24823 (CASSANDRA-17633)
* fsync TOC and digest files (CASSANDRA-10709)

@@ -216,9 +216,9 @@ public String getText()

public static class Value extends Term.Terminal
{
public final Map<ByteBuffer, ByteBuffer> map;
public final SortedMap<ByteBuffer, ByteBuffer> map;

public Value(Map<ByteBuffer, ByteBuffer> map)
public Value(SortedMap<ByteBuffer, ByteBuffer> map)
{
this.map = map;
}
@@ -230,7 +230,8 @@ public static Value fromSerialized(ByteBuffer value, MapType type, ProtocolVersi
// Collections have this small hack that validate cannot be called on a serialized object,
// but compose does the validation (so we're fine).
Map<?, ?> m = type.getSerializer().deserializeForNativeProtocol(value, ByteBufferAccessor.instance, version);
Map<ByteBuffer, ByteBuffer> map = new LinkedHashMap<>(m.size());
// We depend on Maps to be properly sorted by their keys, so use a sorted map implementation here.
SortedMap<ByteBuffer, ByteBuffer> map = new TreeMap<>(type.getKeysType());
for (Map.Entry<?, ?> entry : m.entrySet())
map.put(type.getKeysType().decompose(entry.getKey()), type.getValuesType().decompose(entry.getValue()));
return new Value(map);
@@ -297,7 +298,7 @@ public void collectMarkerSpecification(VariableSpecifications boundNames)

public Terminal bind(QueryOptions options) throws InvalidRequestException
{
Map<ByteBuffer, ByteBuffer> buffers = new TreeMap<ByteBuffer, ByteBuffer>(comparator);
SortedMap<ByteBuffer, ByteBuffer> buffers = new TreeMap<>(comparator);
for (Map.Entry<Term, Term> entry : elements.entrySet())
{
// We don't support values > 64K because the serialization format encode the length as an unsigned short.
@@ -431,7 +432,7 @@ static void doPut(Term.Terminal value, ColumnMetadata column, UpdateParameters p
return;
}

Map<ByteBuffer, ByteBuffer> elements = ((Value) value).map;
SortedMap<ByteBuffer, ByteBuffer> elements = ((Value) value).map;

if (column.type.isMultiCell())
{
@@ -140,7 +140,7 @@ public Term prepare(String keyspace, ColumnSpecification receiver) throws Invali
// We've parsed empty maps as a set literal to break the ambiguity so
// handle that case now
if (receiver.type instanceof MapType && elements.isEmpty())
return new Maps.Value(Collections.<ByteBuffer, ByteBuffer>emptyMap());
return new Maps.Value(Collections.emptySortedMap());

ColumnSpecification valueSpec = Sets.valueSpecOf(receiver);
Set<Term> values = new HashSet<>(elements.size());
@@ -45,6 +45,7 @@
import com.google.common.base.Objects;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;

import org.junit.*;

@@ -285,7 +286,7 @@ public static void startJMXServer() throws Exception
jmxServer = JMXServerUtils.createJMXServer(jmxPort, true);
jmxServer.start();
}

public static void createMBeanServerConnection() throws Exception
{
assert jmxServer != null : "jmxServer not started";
@@ -842,14 +843,14 @@ protected void alterKeyspace(String query)
logger.info(fullQuery);
schemaChange(fullQuery);
}

protected void alterKeyspaceMayThrow(String query) throws Throwable
{
String fullQuery = String.format(query, currentKeyspace());
logger.info(fullQuery);
QueryProcessor.executeOnceInternal(fullQuery);
}

protected String createKeyspaceName()
{
String currentKeyspace = String.format("keyspace_%02d", seqNumber.getAndIncrement());
@@ -1174,7 +1175,7 @@ protected static void assertWarningsContain(List<String> warnings, String messag
Assert.assertNotNull(warnings);
assertTrue(warnings.stream().anyMatch(s -> s.contains(message)));
}

protected static void assertWarningsEquals(ResultSet rs, String... messages)
{
assertWarningsEquals(rs.getExecutionInfo().getWarnings(), messages);
@@ -1193,7 +1194,7 @@ protected static void assertNoWarningContains(Message.Response response, String

protected static void assertNoWarningContains(List<String> warnings, String message)
{
if (warnings != null)
if (warnings != null)
{
assertFalse(warnings.stream().anyMatch(s -> s.contains(message)));
}
@@ -1429,6 +1430,13 @@ protected void assertRowsNet(ProtocolVersion protocolVersion, ResultSet result,
rows.length>i ? "less" : "more", rows.length, i, protocolVersion), i == rows.length);
}

protected void assertRowCountNet(ResultSet r1, int expectedCount)
{
Assert.assertFalse("Received a null resultset when expected count was > 0", expectedCount > 0 && r1 == null);
int actualRowCount = Iterables.size(r1);
Assert.assertEquals(String.format("expected %d rows but received %d", expectedCount, actualRowCount), expectedCount, actualRowCount);
}

public static void assertRows(UntypedResultSet result, Object[]... rows)
{
if (result == null)
@@ -2120,13 +2128,27 @@ protected Object set(Object...values)
return ImmutableSet.copyOf(values);
}

// LinkedHashSets are iterable in insertion order, which is important for some tests
protected LinkedHashSet<Object> linkedHashSet(Object...values)
{
LinkedHashSet<Object> s = new LinkedHashSet<>(values.length);
s.addAll(Arrays.asList(values));
return s;
}

protected Object map(Object...values)
{
return linkedHashMap(values);
}

// LinkedHashMaps are iterable in insertion order, which is important for some tests
protected static LinkedHashMap<Object, Object> linkedHashMap(Object...values)
{
if (values.length % 2 != 0)
throw new IllegalArgumentException("Invalid number of arguments, got " + values.length);

int size = values.length / 2;
Map<Object, Object> m = new LinkedHashMap<>(size);
LinkedHashMap<Object, Object> m = new LinkedHashMap<>(size);
for (int i = 0; i < size; i++)
m.put(values[2 * i], values[(2 * i) + 1]);
return m;
@@ -167,7 +167,7 @@ private static boolean conditionContainsApplies(SortedSet<ByteBuffer> rowValue,
return bound.appliesTo(newRow(definition, rowValue));
}

private static boolean conditionApplies(Map<ByteBuffer, ByteBuffer> rowValue, Operator op, Map<ByteBuffer, ByteBuffer> conditionValue)
private static boolean conditionApplies(SortedMap<ByteBuffer, ByteBuffer> rowValue, Operator op, SortedMap<ByteBuffer, ByteBuffer> conditionValue)
{
ColumnMetadata definition = ColumnMetadata.regularColumn("ks", "cf", "c", MapType.getInstance(Int32Type.instance, Int32Type.instance, true));
ColumnCondition condition = ColumnCondition.condition(definition, op, Terms.of(new Maps.Value(conditionValue)));
@@ -465,9 +465,9 @@ public void testSetCollectionBoundAppliesTo() throws InvalidRequestException
}

// values should be a list of key, value, key, value, ...
private static Map<ByteBuffer, ByteBuffer> map(ByteBuffer... values)
private static SortedMap<ByteBuffer, ByteBuffer> map(ByteBuffer... values)
{
Map<ByteBuffer, ByteBuffer> map = new TreeMap<>();
SortedMap<ByteBuffer, ByteBuffer> map = new TreeMap<>();
for (int i = 0; i < values.length; i += 2)
map.put(values[i], values[i + 1]);

@@ -25,6 +25,7 @@
import com.datastax.driver.core.DataType;
import com.datastax.driver.core.ResultSet;
import com.datastax.driver.core.Session;
import com.datastax.driver.core.utils.UUIDs;
import org.apache.cassandra.cql3.CQLTester;
import org.apache.cassandra.cql3.ColumnSpecification;
import org.apache.cassandra.cql3.UntypedResultSet;
@@ -1980,4 +1981,102 @@ public void testSelectionOfEmptyCollections() throws Throwable
assertRows(execute("SELECT m['0'..'1']['3'..'5'], s[0..1][3..5] FROM %s WHERE k = 3"), row(null, null));
});
}

/*
Tests for CASSANDRA-17623
Before CASSANDRA-17623, parameterized queries with maps as values would fail because frozen maps were
required to be sorted by the sort order of their key type, but weren't always sorted correctly.
Also adding tests for Sets, which did work because they always used SortedSet, to make sure this behavior is maintained.
We use `executeNet` in these tests because `execute` passes parameters through CqlTester#transformValues(), which calls
AbstractType#decompose() on the value, which "fixes" the map order, but wouldn't happen normally.
*/

@Test
public void testInsertingMapDataWithParameterizedQueriesIsKeyOrderIndependent() throws Throwable
{
UUID uuid1 = UUIDs.timeBased();
UUID uuid2 = UUIDs.timeBased();
createTable("CREATE TABLE %s (k text, c frozen<map<timeuuid, text>>, PRIMARY KEY (k, c));");
executeNet("INSERT INTO %s (k, c) VALUES ('0', ?)", linkedHashMap(uuid1, "0", uuid2, "1"));
executeNet("INSERT INTO %s (k, c) VALUES ('0', ?)", linkedHashMap(uuid2, "3", uuid1, "4"));
beforeAndAfterFlush(() -> {
assertRowCountNet(executeNet("SELECT * FROM %s WHERE k='0' AND c={" + uuid1 + ": '0', " + uuid2 + ": '1'}"), 1);
assertRowCountNet(executeNet("SELECT * FROM %s WHERE k='0' AND c={" + uuid2 + ": '1', " + uuid1 + ": '0'}"), 1);
assertRowCountNet(executeNet("SELECT * FROM %s WHERE k='0' AND c={" + uuid1 + ": '4', " + uuid2 + ": '3'}"), 1);
assertRowCountNet(executeNet("SELECT * FROM %s WHERE k='0' AND c={" + uuid2 + ": '3', " + uuid1 + ": '4'}"), 1);
});
}

@Test
public void testInsertingMapDataWithParameterizedQueriesIsKeyOrderIndependentWithSelectSlice() throws Throwable
{
UUID uuid1 = UUIDs.timeBased();
UUID uuid2 = UUIDs.timeBased();
createTable("CREATE TABLE %s (k text, c frozen<map<timeuuid, text>>, PRIMARY KEY (k, c));");
beforeAndAfterFlush(() -> {
executeNet("INSERT INTO %s (k, c) VALUES ('0', ?)", linkedHashMap(uuid2, "2", uuid1, "1"));
// Make sure that we can slice either value out of the map
assertRowsNet(executeNet("SELECT k, c[" + uuid2 + "] from %s"), row("0", "2"));
assertRowsNet(executeNet("SELECT k, c[" + uuid1 + "] from %s"), row("0", "1"));
});
}

@Test
public void testSelectingMapDataWithParameterizedQueriesIsKeyOrderIndependent() throws Throwable
{
UUID uuid1 = UUIDs.timeBased();
UUID uuid2 = UUIDs.timeBased();
createTable("CREATE TABLE %s (k text, c frozen<map<timeuuid, text>>, PRIMARY KEY (k, c));");
executeNet("INSERT INTO %s (k, c) VALUES ('0', {" + uuid1 + ": '0', " + uuid2 + ": '1'})");
executeNet("INSERT INTO %s (k, c) VALUES ('0', {" + uuid2 + ": '3', " + uuid1 + ": '4'})");
beforeAndAfterFlush(() -> {
assertRowCountNet(executeNet("SELECT * FROM %s WHERE k=? AND c=?", "0", linkedHashMap(uuid1, "0", uuid2, "1")), 1);
assertRowCountNet(executeNet("SELECT * FROM %s WHERE k=? AND c=?", "0", linkedHashMap(uuid2, "1", uuid1, "0")), 1);
assertRowCountNet(executeNet("SELECT * FROM %s WHERE k=? AND c=?", "0", linkedHashMap(uuid1, "4", uuid2, "3")), 1);
assertRowCountNet(executeNet("SELECT * FROM %s WHERE k=? AND c=?", "0", linkedHashMap(uuid2, "3", uuid1, "4")), 1);
});
}

@Test
public void testInsertingSetDataWithParameterizedQueriesIsKeyOrderIndependent() throws Throwable
{
UUID uuid1 = UUIDs.timeBased();
UUID uuid2 = UUIDs.timeBased();
createTable("CREATE TABLE %s (k text, c frozen<set<timeuuid>>, PRIMARY KEY (k, c));");
executeNet("INSERT INTO %s (k, c) VALUES ('0', ?)", linkedHashSet(uuid1, uuid2));
executeNet("INSERT INTO %s (k, c) VALUES ('0', ?)", linkedHashSet(uuid2, uuid1));
beforeAndAfterFlush(() -> {
assertRowCountNet(executeNet("SELECT * FROM %s WHERE k='0' AND c={" + uuid1 + ", " + uuid2 + '}'), 1);
assertRowCountNet(executeNet("SELECT * FROM %s WHERE k='0' AND c={" + uuid2 + ", " + uuid1 + '}'), 1);
assertRowCountNet(executeNet("SELECT * FROM %s WHERE k='0' AND c={" + uuid1 + ", " + uuid2 + '}'), 1);
assertRowCountNet(executeNet("SELECT * FROM %s WHERE k='0' AND c={" + uuid2 + ", " + uuid1 + '}'), 1);
});
}

@Test
public void testInsertingSetDataWithParameterizedQueriesIsKeyOrderIndependentWithSelectSlice() throws Throwable
{
UUID uuid1 = UUIDs.timeBased();
UUID uuid2 = UUIDs.timeBased();
createTable("CREATE TABLE %s (k text, c frozen<set<timeuuid>>, PRIMARY KEY (k, c));");
beforeAndAfterFlush(() -> {
executeNet("INSERT INTO %s (k, c) VALUES ('0', ?)", linkedHashSet(uuid2, uuid1));
assertRowsNet(executeNet("SELECT k, c[" + uuid2 + "] from %s"), row("0", uuid2));
assertRowsNet(executeNet("SELECT k, c[" + uuid1 + "] from %s"), row("0", uuid1));
});
}

@Test
public void testSelectingSetDataWithParameterizedQueriesIsKeyOrderIndependent() throws Throwable
{
UUID uuid1 = UUIDs.timeBased();
UUID uuid2 = UUIDs.timeBased();
createTable("CREATE TABLE %s (k text, c frozen<set<timeuuid>>, PRIMARY KEY (k, c));");
executeNet("INSERT INTO %s (k, c) VALUES ('0', {" + uuid1 + ", " + uuid2 + "})");
beforeAndAfterFlush(() -> {
assertRowsNet(executeNet("SELECT k, c from %s where k='0' and c=?", linkedHashSet(uuid1, uuid2)), row("0", list(uuid1, uuid2)));
assertRowsNet(executeNet("SELECT k, c from %s where k='0' and c=?", linkedHashSet(uuid2, uuid1)), row("0", list(uuid1, uuid2)));
});
}
// End tests for CASSANDRA-17623
}

0 comments on commit 56ce8d1

Please sign in to comment.