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

Validate snapshot names 

The validation of snapshot names is turned off by default.
It is controlled by system property cassandra.snapshot.validation which is by default
set to false except 7.0 branch.

When the validation is turned on, the allowed characters are AWS safe characters together
with "+" character. A user can not specify any path separators in snapshot names even if
the validation as such is turned off.

patch by Stefan Miklosovic; reviewed by Francisco Guerrero, Matt Byrd for CASSANDRA-21389
This commit is contained in:
Stefan Miklosovic 2026-05-27 13:51:58 +02:00
parent 863cb651e7
commit 1362ac6f69
No known key found for this signature in database
GPG Key ID: 32F35CB2F546D93E
4 changed files with 165 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGES.txt
View File

@ -1,4 +1,5 @@
4.0.21
* 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)

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

@ -210,6 +210,11 @@ public enum CassandraRelevantProperties
/** This property indicates whether disable_mbean_registration is true */
IS_DISABLED_MBEAN_REGISTRATION("org.apache.cassandra.disable_mbean_registration"),
/**
* Validates names of to-be-taken snapshots, defaults to false.
*/
SNAPSHOT_NAME_VALIDATION("cassandra.snapshot.validation", "false"),
/** what class to use for mbean registeration */
MBEAN_REGISTRATION_CLASS("org.apache.cassandra.mbean_registration_class"),

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

@ -93,6 +93,8 @@ import org.json.simple.JSONObject;
import static java.util.concurrent.TimeUnit.NANOSECONDS;
import static java.lang.String.format;
import static org.apache.cassandra.schema.SchemaConstants.FILENAME_LENGTH;
import static org.apache.cassandra.utils.Throwables.maybeFail;
import static org.apache.cassandra.utils.Throwables.merge;
@ -1837,6 +1839,8 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean
*/
public Set<SSTableReader> snapshotWithoutFlush(String snapshotName, Predicate<SSTableReader> predicate, boolean ephemeral, RateLimiter rateLimiter)
{
validateSnapshotName(snapshotName);
if (rateLimiter == null)
rateLimiter = DatabaseDescriptor.getSnapshotRateLimiter();
@ -1868,6 +1872,46 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean
return snapshottedSSTables;
}
private void validateSnapshotName(String snapshotName)
{
if (snapshotName.contains(File.separator))
{
throw new IllegalArgumentException("Snapshot name cannot contain " + File.separator);
}
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_LENGTH - 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 + ". " +
"Allowed characters must match the pattern: [a-zA-Z0-9_.+-]+" +
" with a maximum of length of " + FILENAME_LENGTH + " characters.");
}
}
private void writeSnapshotManifest(final JSONArray filesJSONArr, final String snapshotName)
{
final File manifestFile = getDirectories().getSnapshotManifestFile(snapshotName);

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

@ -22,19 +22,31 @@ import java.io.File;
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.Component;
import org.apache.cassandra.io.sstable.format.SSTableReader;
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();
for (SSTableReader sstable : getCurrentColumnFamilyStore().getLiveSSTables())
{
@ -43,4 +55,105 @@ public class SnapshotTest extends CQLTester
}
getCurrentColumnFamilyStore().snapshot("hello");
}
@Test
public void testSnapshotNameValidation()
{
ColumnFamilyStore cfs = getCurrentColumnFamilyStore();
String sep = File.separator;
try (WithProperties p = new WithProperties())
{
p.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)
.hasMessageContaining("Snapshot name contains illegal characters: important!");
assertThatThrownBy(() -> cfs.snapshot("backup*"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Snapshot name contains illegal characters: backup*");
assertThatThrownBy(() -> cfs.snapshot("o'snap"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Snapshot name contains illegal characters: o'snap");
assertThatThrownBy(() -> cfs.snapshot("snap(1)"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("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)
.hasMessageContaining("Snapshot name contains illegal characters: a tag");
assertThatThrownBy(() -> cfs.snapshot("a:tag"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("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())
{
p.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);
}
}