From e773bbd9c6c52a2fedc127cb7ab77a1fdbeb63d2 Mon Sep 17 00:00:00 2001 From: cclive1601 Date: Fri, 21 Jan 2022 11:07:37 -0800 Subject: [PATCH] Reject snapshot names with special character Patch by Maxwell Guo, Saranya Krishnakumar and Francisco Guerrero; review by Benjamin Lerer, Chris Lohfink, Stefan Miklosovic for CASSANDRA-15297 This commit also adds unit tests for the snapshot nodetool command. Co-authored-by: Saranya Krishnakumar Co-authored-by: Francisco Guerrero --- CHANGES.txt | 1 + .../cassandra/tools/nodetool/Snapshot.java | 5 + .../tools/nodetool/SnapshotTest.java | 152 ++++++++++++++++++ 3 files changed, 158 insertions(+) create mode 100644 test/unit/org/apache/cassandra/tools/nodetool/SnapshotTest.java diff --git a/CHANGES.txt b/CHANGES.txt index 86eae2b536..5c0477ba25 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.0.4 + * Reject snapshot names with special character (CASSANDRA-15297) * Fix ObjectSizes implementation and usages (CASSANDRA-17402) * Fix race condition bug during local session repair (CASSANDRA-17335) * Fix ignored streaming encryption settings in sstableloader (CASSANDRA-17367) diff --git a/src/java/org/apache/cassandra/tools/nodetool/Snapshot.java b/src/java/org/apache/cassandra/tools/nodetool/Snapshot.java index 6619c531bf..ac485ea89d 100644 --- a/src/java/org/apache/cassandra/tools/nodetool/Snapshot.java +++ b/src/java/org/apache/cassandra/tools/nodetool/Snapshot.java @@ -23,6 +23,7 @@ import io.airlift.airline.Arguments; import io.airlift.airline.Command; import io.airlift.airline.Option; +import java.io.File; import java.io.IOException; import java.io.PrintStream; import java.util.ArrayList; @@ -64,6 +65,10 @@ public class Snapshot extends NodeToolCmd Map options = new HashMap(); options.put("skipFlush", Boolean.toString(skipFlush)); + if (!snapshotName.isEmpty() && snapshotName.contains(File.pathSeparator)) + { + throw new IOException("Snapshot name cannot contain " + File.pathSeparatorChar); + } // Create a separate path for kclist to avoid breaking of already existing scripts if (null != ktList && !ktList.isEmpty()) { diff --git a/test/unit/org/apache/cassandra/tools/nodetool/SnapshotTest.java b/test/unit/org/apache/cassandra/tools/nodetool/SnapshotTest.java new file mode 100644 index 0000000000..428438fcd5 --- /dev/null +++ b/test/unit/org/apache/cassandra/tools/nodetool/SnapshotTest.java @@ -0,0 +1,152 @@ +/* + * 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.tools.nodetool; + +import java.io.File; + +import org.junit.BeforeClass; +import org.junit.Test; + +import org.apache.cassandra.cql3.CQLTester; +import org.apache.cassandra.tools.ToolRunner; + +import static org.apache.cassandra.tools.ToolRunner.invokeNodetool; +import static org.assertj.core.api.Assertions.assertThat; + +/**o + * Tests for the {@code nodetool snapshot} command + */ +public class SnapshotTest extends CQLTester +{ + @BeforeClass + public static void setup() throws Exception + { + startJMXServer(); + } + + @Test + public void testSnapshotAllKeyspaces() + { + ToolRunner.ToolResult tool = invokeNodetool("snapshot"); + tool.assertOnCleanExit(); + + assertThat(tool.getExitCode()).isEqualTo(0); + assertThat(tool.getStdout()).contains("Requested creating snapshot(s) for [all keyspaces]"); + } + + @Test + public void testSnapshotSpecifySingleKeyspace() + { + ToolRunner.ToolResult tool = invokeNodetool("snapshot", "system_schema"); + tool.assertOnCleanExit(); + + assertThat(tool.getExitCode()).isEqualTo(0); + assertThat(tool.getStdout()).contains("Requested creating snapshot(s) for [system_schema]"); + } + + @Test + public void testSnapshotSpecifyMultipleKeyspaces() + { + ToolRunner.ToolResult tool = invokeNodetool("snapshot", "system", "system_schema"); + tool.assertOnCleanExit(); + + assertThat(tool.getExitCode()).isEqualTo(0); + assertThat(tool.getStdout()).contains("Requested creating snapshot(s) for [system, system_schema]"); + } + + @Test + public void testSnapshotWithName() + { + ToolRunner.ToolResult tool = invokeNodetool("snapshot", "-t", "custom_snapshot_name"); + tool.assertOnCleanExit(); + + assertThat(tool.getExitCode()).isEqualTo(0); + assertThat(tool.getStdout()).contains("Requested creating snapshot(s) for [all keyspaces] with snapshot name [custom_snapshot_name] and options {skipFlush=false}"); + } + + @Test + public void testInvalidSnapshotName() + { + ToolRunner.ToolResult tool = invokeNodetool("snapshot", "-t", "invalid" + File.pathSeparatorChar + "name"); + assertThat(tool.getExitCode()).isEqualTo(2); + assertThat(tool.getStderr()).contains("Snapshot name cannot contain " + File.pathSeparatorChar); + } + + @Test + public void testSkipFlushOption() + { + ToolRunner.ToolResult tool = invokeNodetool("snapshot", "-t", "skip_flush", "-sf"); + tool.assertOnCleanExit(); + + assertThat(tool.getExitCode()).isEqualTo(0); + assertThat(tool.getStdout()).contains("Requested creating snapshot(s) for [all keyspaces] with snapshot name [skip_flush] and options {skipFlush=true}"); + } + + @Test + public void testTableOption() + { + ToolRunner.ToolResult tool = invokeNodetool("snapshot", "-t", "table", "--table", "keyspaces", "system_schema"); + tool.assertOnCleanExit(); + + assertThat(tool.getExitCode()).isEqualTo(0); + assertThat(tool.getStdout()).contains("Requested creating snapshot(s) for [system_schema] with snapshot name [table] and options {skipFlush=false}"); + } + + @Test + public void testInvalidTableWithMultipleKeyspacesOption() + { + ToolRunner.ToolResult tool = invokeNodetool("snapshot", "-t", "table", "--table", "keyspaces", "system", "system_schema"); + assertThat(tool.getExitCode()).isEqualTo(2); + assertThat(tool.getStderr()).contains("When specifying the table for a snapshot, you must specify one and only one keyspace"); + } + + @Test + public void testInvalidTableWithKeyspaceTableListOption() + { + ToolRunner.ToolResult tool = invokeNodetool("snapshot", "-t", "table", "--table", "keyspaces", "-kt", "ks.table"); + assertThat(tool.getExitCode()).isEqualTo(2); + assertThat(tool.getStderr()).contains("When specifying the Keyspace table list (using -kt,--kt-list,-kc,--kc.list), you must not also specify keyspaces to snapshot"); + } + + @Test + public void testInvalidTableWithoutKeyspaceOption() + { + ToolRunner.ToolResult tool = invokeNodetool("snapshot", "-t", "table", "--table", "keyspaces"); + assertThat(tool.getExitCode()).isEqualTo(2); + assertThat(tool.getStderr()).contains("When specifying the table for a snapshot, you must specify one and only one keyspace"); + } + + @Test + public void testKeyspaceTableListOption() + { + ToolRunner.ToolResult tool = invokeNodetool("snapshot", "-t", "kt_option", "-kt", "system_schema.keyspaces,system_schema.aggregates"); + tool.assertOnCleanExit(); + + assertThat(tool.getExitCode()).isEqualTo(0); + assertThat(tool.getStdout()).contains("Requested creating snapshot(s) for [system_schema.keyspaces,system_schema.aggregates] with snapshot name [kt_option] and options {skipFlush=false}"); + } + + @Test + public void testInvalidKeyspacesWithKeyspaceTableListOptions() + { + ToolRunner.ToolResult tool = invokeNodetool("snapshot", "-kt", "ks.table", "ks"); + assertThat(tool.getExitCode()).isEqualTo(2); + assertThat(tool.getStderr()).contains("When specifying the Keyspace table list (using -kt,--kt-list,-kc,--kc.list), you must not also specify keyspaces to snapshot"); + } +}