natas_hw
/
cassandra
mirror of https://github.com/apache/cassandra
0
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'cassandra-4.1' into cassandra-5.0

This commit is contained in:
Stefan Miklosovic 2026-06-04 10:23:10 +02:00
parent 1656a9cb01 eacc60fa37
commit 2a219ed873
No known key found for this signature in database
GPG Key ID: 32F35CB2F546D93E
4 changed files with 159 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGES.txt
View File

@ -5,6 +5,7 @@
Merged from 4.1:
* Add Paxos v2 option and informatin in cassandra.yaml (CASSANDRA-21316)
Merged from 4.0:
* Validate snapshot names (CASSANDRA-21389)
* BTree.FastBuilder.reset() fails to clear savedBuffer and savedNextKey, causing ClassCastException and SSTable header corruption during schema disagreement (CASSANDRA-21216, CASSANDRA-21260)
* Backport CASSANDRA-17810 fix and improve RTBoundValidator error messages (CASSANDRA-18282)

4
src/java/org/apache/cassandra/config/CassandraRelevantProperties.java
View File

@ -511,6 +511,10 @@ public enum CassandraRelevantProperties
SNAPSHOT_CLEANUP_PERIOD_SECONDS("cassandra.snapshot.ttl_cleanup_period_seconds", "60"),
/** minimum allowed TTL for snapshots */
SNAPSHOT_MIN_ALLOWED_TTL_SECONDS("cassandra.snapshot.min_allowed_ttl_seconds", "60"),
/**
* Validates names of to-be-taken snapshots, defaults to false.
*/
SNAPSHOT_NAME_VALIDATION("cassandra.snapshot.validation", "false"),
SSL_ENABLE("ssl.enable"),
SSL_STORAGE_PORT("cassandra.ssl_storage_port"),
START_GOSSIP("cassandra.start_gossip", "true"),

43
src/java/org/apache/cassandra/db/ColumnFamilyStore.java
View File

@ -78,6 +78,7 @@ import org.apache.cassandra.cache.RowCacheKey;
import org.apache.cassandra.cache.RowCacheSentinel;
import org.apache.cassandra.concurrent.ExecutorPlus;
import org.apache.cassandra.concurrent.FutureTask;
import org.apache.cassandra.config.CassandraRelevantProperties;
import org.apache.cassandra.config.DatabaseDescriptor;
import org.apache.cassandra.config.DurationSpec;
import org.apache.cassandra.db.commitlog.CommitLog;
@ -178,9 +179,11 @@ import org.apache.cassandra.utils.concurrent.OpOrder;
import org.apache.cassandra.utils.concurrent.Refs;
import org.apache.cassandra.utils.concurrent.UncheckedInterruptedException;
import static java.lang.String.format;
import static org.apache.cassandra.concurrent.ExecutorFactory.Global.executorFactory;
import static org.apache.cassandra.config.DatabaseDescriptor.getFlushWriters;
import static org.apache.cassandra.db.commitlog.CommitLogPosition.NONE;
import static org.apache.cassandra.schema.SchemaConstants.FILENAME_LENGTH;
import static org.apache.cassandra.utils.Clock.Global.currentTimeMillis;
import static org.apache.cassandra.utils.Clock.Global.nanoTime;
import static org.apache.cassandra.utils.FBUtilities.now;
@ -2138,6 +2141,8 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean, Memtable.Owner
throw new IllegalStateException(String.format("can not take ephemeral snapshot (%s) while ttl is specified too", snapshotName));
}
validateSnapshotName(snapshotName);
if (rateLimiter == null)
rateLimiter = DatabaseDescriptor.getSnapshotRateLimiter();
@ -2160,6 +2165,44 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean, Memtable.Owner
return createSnapshot(snapshotName, ephemeral, ttl, snapshottedSSTables, creationTime);
}
private void validateSnapshotName(String snapshotName)
{
if (snapshotName.contains(File.pathSeparator()))
{
throw new IllegalArgumentException("Snapshot name cannot contain " + File.pathSeparator());
}
if (snapshotName.equals(".") || snapshotName.equals(".."))
{
throw new IllegalArgumentException("Snapshot name '" + snapshotName + "' is reserved");
}
if (!CassandraRelevantProperties.SNAPSHOT_NAME_VALIDATION.getBoolean())
return;
// the length of valid snapshot name has to be less than or equal to FILENAME_LEGTH - that is 255 -
// we are following the max length as it is in SchemaConstants for table name.
if (snapshotName.length() > SchemaConstants.FILENAME_LENGTH)
{
throw new IllegalArgumentException(format("Snapshot name must not be more than %d characters long for " +
"resolved snapshot name (got %d characters for \"%s\")",
FILENAME_LENGTH, snapshotName.length(), snapshotName));
}
// Allowed characters are a conservative subset of the AWS S3 "Safe characters" set
// (https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html#object-key-guidelines):
// 0-9 a-z A-Z - _ .
// plus '+', which is not an S3 "Safe character" but can legitimately appear in system
// snapshot names via version build metadata (e.g. an upgrade snapshot "<millis>-upgrade-5.0.4+build-...").
// The remaining S3-safe characters (! * ' ( )) are intentionally excluded as they are
// shell-significant and error-prone in paths, and the path separator '/' is excluded too,
// which is what blocks traversal attempts such as "../../mysnapshot"
if (!Pattern.compile("[a-zA-Z0-9_.+-]+").matcher(snapshotName).matches())
{
throw new IllegalArgumentException("Snapshot name contains illegal characters: " + snapshotName);
}
}
protected TableSnapshot createSnapshot(String tag, boolean ephemeral, DurationSpec.IntSecondsBound ttl, Set<SSTableReader> sstables, Instant creationTime) {
Set<File> snapshotDirs = sstables.stream()
.map(s -> Directories.getSnapshotDirectory(s.descriptor, tag).toAbsolute())

113
test/unit/org/apache/cassandra/db/SnapshotTest.java
View File

@ -21,20 +21,32 @@ package org.apache.cassandra.db;
import java.nio.file.Files;
import java.nio.file.StandardOpenOption;
import org.junit.Before;
import org.junit.Test;
import org.apache.cassandra.config.CassandraRelevantProperties;
import org.apache.cassandra.cql3.CQLTester;
import org.apache.cassandra.distributed.shared.WithProperties;
import org.apache.cassandra.io.sstable.format.SSTableFormat.Components;
import org.apache.cassandra.io.sstable.format.SSTableReader;
import org.apache.cassandra.io.util.File;
import org.apache.cassandra.schema.SchemaConstants;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
public class SnapshotTest extends CQLTester
{
@Test
public void testEmptyTOC() throws Throwable
@Before
public void setUpTable() throws Throwable
{
createTable("create table %s (id int primary key, k int)");
execute("insert into %s (id, k) values (1,1)");
}
@Test
public void testEmptyTOC() throws Throwable
{
getCurrentColumnFamilyStore().forceBlockingFlush(ColumnFamilyStore.FlushReason.UNIT_TESTS);
for (SSTableReader sstable : getCurrentColumnFamilyStore().getLiveSSTables())
{
@ -43,4 +55,101 @@ public class SnapshotTest extends CQLTester
}
getCurrentColumnFamilyStore().snapshot("hello");
}
@Test
public void testSnapshotNameValidation()
{
ColumnFamilyStore cfs = getCurrentColumnFamilyStore();
String sep = File.pathSeparator();
try (WithProperties p = new WithProperties().set(CassandraRelevantProperties.SNAPSHOT_NAME_VALIDATION, true))
{
// Previously-allowed alphanumerics, '-' and '_' must still be accepted.
assertThatCode(() -> cfs.snapshot("atag")).doesNotThrowAnyException();
assertThatCode(() -> cfs.snapshot("a-tag")).doesNotThrowAnyException();
assertThatCode(() -> cfs.snapshot("a_tag")).doesNotThrowAnyException();
assertThatCode(() -> cfs.snapshot("a_tag_1and_something2-more")).doesNotThrowAnyException();
assertThatCode(() -> cfs.snapshot(repeat('a', SchemaConstants.FILENAME_LENGTH))).doesNotThrowAnyException();
// Only alphanumerics, '-', '_' and '.' are accepted.
assertThatCode(() -> cfs.snapshot("snap.2026-05-20")).doesNotThrowAnyException();
// Dots embedded in a name are not traversal: with '/' excluded, "a..tag" is just a literal directory.
assertThatCode(() -> cfs.snapshot("a..tag")).doesNotThrowAnyException();
// "+" is part of the allowed set because it can appear in a Cassandra version
// (build metadata, e.g. "7.0.0+abc123"), which ends up in system snapshot names.
assertThatCode(() -> cfs.snapshot("this_is_snapshot-7.0.0+abc123")).doesNotThrowAnyException();
String tooLong = repeat('a', SchemaConstants.FILENAME_LENGTH + 1);
assertThatThrownBy(() -> cfs.snapshot(tooLong))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Snapshot name must not be more than 255 characters long for " +
"resolved snapshot name (got 256 characters for \"" + tooLong + "\")");
// '/' is not in the allowed set; this is what kills traversal attempts like "../../mysnapshot".
assertThatThrownBy(() -> cfs.snapshot("a" + sep + "tag"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Snapshot name cannot contain " + sep);
// The shell-significant S3 "safe" characters (! * ' ( )) are deliberately NOT allowed.
assertThatThrownBy(() -> cfs.snapshot("important!"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Snapshot name contains illegal characters: important!");
assertThatThrownBy(() -> cfs.snapshot("backup*"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Snapshot name contains illegal characters: backup*");
assertThatThrownBy(() -> cfs.snapshot("o'snap"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Snapshot name contains illegal characters: o'snap");
assertThatThrownBy(() -> cfs.snapshot("snap(1)"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Snapshot name contains illegal characters: snap(1)");
// Other characters outside the allowed set must still be rejected.
assertThatThrownBy(() -> cfs.snapshot("a tag"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Snapshot name contains illegal characters: a tag");
assertThatThrownBy(() -> cfs.snapshot("a:tag"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Snapshot name contains illegal characters: a:tag");
// "." and ".." pass the charset check but resolve to the snapshots/ dir itself
// and its parent (the live table dir) respectively, so they must be rejected as reserved.
assertThatThrownBy(() -> cfs.snapshot("."))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Snapshot name '.' is reserved");
assertThatThrownBy(() -> cfs.snapshot(".."))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Snapshot name '..' is reserved");
}
try (WithProperties p = new WithProperties().set(CassandraRelevantProperties.SNAPSHOT_NAME_VALIDATION, false))
{
// The character check is bypassed entirely: space, ':', and the now-disallowed
// shell-significant characters (! * ' ( )) are all accepted.
assertThatCode(() -> cfs.snapshot("a tag")).doesNotThrowAnyException();
assertThatCode(() -> cfs.snapshot("a:tag")).doesNotThrowAnyException();
assertThatCode(() -> cfs.snapshot("important!")).doesNotThrowAnyException();
assertThatCode(() -> cfs.snapshot("snap(1)")).doesNotThrowAnyException();
// Path separator and "." / ".." rejections are unconditional — they guard against
// traversal regardless of the toggle.
assertThatThrownBy(() -> cfs.snapshot("a" + sep + "tag"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Snapshot name cannot contain " + sep);
assertThatThrownBy(() -> cfs.snapshot("."))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Snapshot name '.' is reserved");
assertThatThrownBy(() -> cfs.snapshot(".."))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Snapshot name '..' is reserved");
}
}
private static String repeat(char c, int times)
{
char[] chars = new char[times];
java.util.Arrays.fill(chars, c);
return new String(chars);
}
}