diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/AlterSchemaStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/AlterSchemaStatement.java index f474ddacbc0b..88a327992329 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/AlterSchemaStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/AlterSchemaStatement.java @@ -28,7 +28,6 @@ import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.cql3.CQLStatement; import org.apache.cassandra.cql3.QueryOptions; -import org.apache.cassandra.cql3.statements.RawKeyspaceAwareStatement; import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.schema.KeyspaceMetadata; import org.apache.cassandra.schema.Keyspaces.KeyspacesDiff; @@ -147,7 +146,7 @@ public ResultMessage execute(QueryState state, boolean locally) private void validateKeyspaceName() { - if (!SchemaConstants.isValidName(keyspaceName)) + if (!SchemaConstants.isNameSafeForFilename(keyspaceName)) { throw ire("Keyspace name must not be empty, more than %d characters long, " + "or contain non-alphanumeric-underscore characters (got '%s')", diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/CreateIndexStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/CreateIndexStatement.java index 19030d361f82..67aef9ab1549 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/CreateIndexStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/CreateIndexStatement.java @@ -224,7 +224,7 @@ private void validateIndexTarget(TableMetadata table, IndexMetadata.Kind kind, I if (null == column) throw ire("Column '%s' doesn't exist", target.column); - if ((kind == IndexMetadata.Kind.CUSTOM) && !SchemaConstants.isValidName(target.column.toString())) + if ((kind == IndexMetadata.Kind.CUSTOM) && !SchemaConstants.isNameSafeForFilename(target.column.toString())) throw ire("Column '%s' is longer than the permissible name length of %d characters or" + " contains non-alphanumeric-underscore characters", target.column, SchemaConstants.NAME_LENGTH); diff --git a/src/java/org/apache/cassandra/db/compaction/unified/AdaptiveController.java b/src/java/org/apache/cassandra/db/compaction/unified/AdaptiveController.java index 490fbd64b89f..5d8b46872b40 100644 --- a/src/java/org/apache/cassandra/db/compaction/unified/AdaptiveController.java +++ b/src/java/org/apache/cassandra/db/compaction/unified/AdaptiveController.java @@ -210,15 +210,15 @@ static Controller fromOptions(Environment env, { logger.warn("Unable to parse saved options. Using starting value instead:", e); } - catch (FSError e) - { - logger.warn("Unable to read controller config file. Using starting value instead:", e); - } - catch (Throwable e) - { - logger.warn("Unable to read controller config file. Using starting value instead:", e); - JVMStabilityInspector.inspectThrowable(e); - } +// catch (FSError e) +// { +// logger.warn("Unable to read controller config file. Using starting value instead:", e); +// } +// catch (Throwable e) +// { +// logger.warn("Unable to read controller config file. Using starting value instead:", e); +// JVMStabilityInspector.inspectThrowable(e); +// } if (scalingParameters == null) { diff --git a/src/java/org/apache/cassandra/db/compaction/unified/Controller.java b/src/java/org/apache/cassandra/db/compaction/unified/Controller.java index b65bd250f1c2..8d9962f7cfed 100644 --- a/src/java/org/apache/cassandra/db/compaction/unified/Controller.java +++ b/src/java/org/apache/cassandra/db/compaction/unified/Controller.java @@ -425,15 +425,15 @@ public static void storeOptions(String keyspaceName, String tableName, int[] sca logger.debug(String.format("Writing current scaling parameters and flush size to file %s: %s", f.toPath().toString(), jsonObject)); } - catch (IOException | FSError e) + catch (IOException e) { logger.warn("Unable to save current scaling parameters and flush size. Current controller configuration will be lost if a node restarts: ", e); } - catch (Throwable e) - { - logger.warn("Unable to save current scaling parameters and flush size. Current controller configuration will be lost if a node restarts: ", e); - JVMStabilityInspector.inspectThrowable(e); - } +// catch (Throwable e) +// { +// logger.warn("Unable to save current scaling parameters and flush size. Current controller configuration will be lost if a node restarts: ", e); +// JVMStabilityInspector.inspectThrowable(e); +// } } public abstract void storeControllerConfig(); diff --git a/src/java/org/apache/cassandra/db/compaction/unified/StaticController.java b/src/java/org/apache/cassandra/db/compaction/unified/StaticController.java index d8636b0da455..7ce11897bb63 100644 --- a/src/java/org/apache/cassandra/db/compaction/unified/StaticController.java +++ b/src/java/org/apache/cassandra/db/compaction/unified/StaticController.java @@ -152,15 +152,15 @@ static Controller fromOptions(Environment env, { logger.warn("Unable to parse saved flush size. Using starting value instead:", e); } - catch (FSError e) - { - logger.warn("Unable to read controller config file. Using starting value instead:", e); - } - catch (Throwable e) - { - logger.warn("Unable to read controller config file. Using starting value instead:", e); - JVMStabilityInspector.inspectThrowable(e); - } +// catch (FSError e) +// { +// logger.warn("Unable to read controller config file. Using starting value instead:", e); +// } +// catch (Throwable e) +// { +// logger.warn("Unable to read controller config file. Using starting value instead:", e); +// JVMStabilityInspector.inspectThrowable(e); +// } return new StaticController(env, scalingParameters, survivalFactors, diff --git a/src/java/org/apache/cassandra/schema/IndexMetadata.java b/src/java/org/apache/cassandra/schema/IndexMetadata.java index 138d231ed85d..62c36b0af9d7 100644 --- a/src/java/org/apache/cassandra/schema/IndexMetadata.java +++ b/src/java/org/apache/cassandra/schema/IndexMetadata.java @@ -46,6 +46,8 @@ import org.apache.cassandra.utils.FBUtilities; import org.apache.cassandra.utils.UUIDSerializer; +import static org.apache.cassandra.schema.SchemaConstants.isValidCharsName; + /** * An immutable representation of secondary index metadata. */ @@ -108,11 +110,6 @@ public static IndexMetadata fromIndexTargets(List targets, return new IndexMetadata(name, newOptions, kind); } - public static boolean isNameValid(String name) - { - return name != null && !name.isEmpty() && PATTERN_WORD_CHARS.matcher(name).matches(); - } - public static String generateDefaultIndexName(String table, ColumnIdentifier column) { return PATTERN_NON_WORD_CHAR.matcher(table + "_" + column.toString() + "_idx").replaceAll(""); @@ -125,7 +122,7 @@ public static String generateDefaultIndexName(String table) public void validate(TableMetadata table) { - if (!isNameValid(name)) + if (!isValidCharsName(name)) throw new ConfigurationException("Illegal index name " + name); if (kind == null) diff --git a/src/java/org/apache/cassandra/schema/KeyspaceMetadata.java b/src/java/org/apache/cassandra/schema/KeyspaceMetadata.java index 79185d10b27d..76df3682bd0d 100644 --- a/src/java/org/apache/cassandra/schema/KeyspaceMetadata.java +++ b/src/java/org/apache/cassandra/schema/KeyspaceMetadata.java @@ -363,7 +363,7 @@ public String toCqlString(boolean withInternals, boolean ifNotExists) public void validate() { - if (!SchemaConstants.isValidName(name)) + if (!SchemaConstants.isNameSafeForFilename(name)) { throw new ConfigurationException(format("Keyspace name must not be empty, more than %s characters long, " + "or contain non-alphanumeric-underscore characters (got \"%s\")", diff --git a/src/java/org/apache/cassandra/schema/SchemaConstants.java b/src/java/org/apache/cassandra/schema/SchemaConstants.java index e2b8280946b4..ad3f06e2af6c 100644 --- a/src/java/org/apache/cassandra/schema/SchemaConstants.java +++ b/src/java/org/apache/cassandra/schema/SchemaConstants.java @@ -72,9 +72,31 @@ public final class SchemaConstants public static final List LEGACY_AUTH_TABLES = Arrays.asList("credentials", "users", "permissions"); - public static boolean isValidName(String name) + public static boolean isSafeLengthForFilename(String name) { - return name != null && !name.isEmpty() && name.length() <= NAME_LENGTH && PATTERN_WORD_CHARS.matcher(name).matches(); + return name.length() <= NAME_LENGTH; + } + + /** + * Names such as keyspace, table, index names are used in file paths and file names, + * so, they need to be safe for the use there, i.e., short enough and + * containing only alphanumeric characters and underscores. + * @param name the name to check + * @return whether the name is safe for use in file paths and file names + */ + public static boolean isNameSafeForFilename(String name) + { + return name != null && !name.isEmpty() && isSafeLengthForFilename(name) && PATTERN_WORD_CHARS.matcher(name).matches(); + } + + /** + * Checks if the name contains only alphanumeric characters and underscores and is not empty. + * @param name the name to check + * @return true if the name is valid, false otherwise + */ + public static boolean isValidCharsName(String name) + { + return name != null && !name.isEmpty() && PATTERN_WORD_CHARS.matcher(name).matches(); } static diff --git a/src/java/org/apache/cassandra/schema/TableMetadata.java b/src/java/org/apache/cassandra/schema/TableMetadata.java index ba5e1db8d84d..d6e792b2f032 100644 --- a/src/java/org/apache/cassandra/schema/TableMetadata.java +++ b/src/java/org/apache/cassandra/schema/TableMetadata.java @@ -51,7 +51,7 @@ import static java.lang.String.format; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toSet; -import static org.apache.cassandra.schema.IndexMetadata.isNameValid; +import static org.apache.cassandra.schema.SchemaConstants.isValidCharsName; @Unmetered public class TableMetadata implements SchemaElement @@ -439,11 +439,11 @@ public final void validate() public void validate(boolean durationLegacyMode) { - if (!isNameValid(keyspace)) - except("Keyspace name must not be empty, more than %s characters long, or contain non-alphanumeric-underscore characters (got \"%s\")", SchemaConstants.NAME_LENGTH, keyspace); + if (!isValidCharsName(keyspace)) + except("Keyspace name must not be empty or contain non-alphanumeric-underscore characters (got \"%s\")", SchemaConstants.NAME_LENGTH, keyspace); - if (!isNameValid(name)) - except("Table name must not be empty, more than %s characters long, or contain non-alphanumeric-underscore characters (got \"%s\")", SchemaConstants.NAME_LENGTH, name); + if (!isValidCharsName(name)) + except("Table name must not be empty or contain non-alphanumeric-underscore characters (got \"%s\")", SchemaConstants.NAME_LENGTH, name); params.validate(); diff --git a/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java b/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java index cf970de6e7e9..c23c6b24ecb9 100644 --- a/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java +++ b/test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java @@ -20,6 +20,7 @@ import org.apache.cassandra.cql3.CQLTester; import org.apache.cassandra.cql3.UntypedResultSet; +import org.apache.cassandra.cql3.functions.types.ParseUtils; import org.apache.cassandra.exceptions.ConfigurationException; import org.apache.cassandra.exceptions.InvalidRequestException; @@ -105,7 +106,7 @@ public void testCreateTableWithMissingClusteringColumn() @Test public void testCreatingTableWithLongName() throws Throwable { - String keyspace = "g38373639353166362d356631322d343864652d393063362d653862616534343165333764_tpch"; + String keyspace = "\"38373639353166362d356631322d343864652d393063362d653862616534343165333764_tpch\""; String table = "test_create_k8yq1r75bpzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz"; execute(String.format("CREATE KEYSPACE %s with replication = " + @@ -116,7 +117,7 @@ public void testCreatingTableWithLongName() throws Throwable "val int)", keyspace, table)); execute(String.format("INSERT INTO %s.%s (key,val) VALUES (1,1)", keyspace, table)); - flush(keyspace, table); + flush(ParseUtils.unDoubleQuote(keyspace), table); UntypedResultSet result = execute(String.format("SELECT * from %s.%s", keyspace, table)); assertThat(result.size()).isEqualTo(1); } diff --git a/test/unit/org/apache/cassandra/schema/IndexMetadataTest.java b/test/unit/org/apache/cassandra/schema/IndexMetadataTest.java index c9e0d52a0ebe..c445f3c659d7 100644 --- a/test/unit/org/apache/cassandra/schema/IndexMetadataTest.java +++ b/test/unit/org/apache/cassandra/schema/IndexMetadataTest.java @@ -25,27 +25,28 @@ import org.apache.cassandra.cql3.ColumnIdentifier; +import static org.apache.cassandra.schema.SchemaConstants.isValidCharsName; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; public class IndexMetadataTest { @Test - public void testIsNameValidPositive() + public void testIsValidCharsNamePositive() { - assertTrue(IndexMetadata.isNameValid("abcdefghijklmnopqrstuvwxyz")); - assertTrue(IndexMetadata.isNameValid("ABCDEFGHIJKLMNOPQRSTUVWXYZ")); - assertTrue(IndexMetadata.isNameValid("_01234567890")); + assertTrue(isValidCharsName("abcdefghijklmnopqrstuvwxyz")); + assertTrue(isValidCharsName("ABCDEFGHIJKLMNOPQRSTUVWXYZ")); + assertTrue(isValidCharsName("_01234567890")); } @Test - public void testIsNameValidNegative() + public void testIsValidCharsNameNegative() { - assertFalse(IndexMetadata.isNameValid(null)); - assertFalse(IndexMetadata.isNameValid("")); - assertFalse(IndexMetadata.isNameValid(" ")); - assertFalse(IndexMetadata.isNameValid("@")); - assertFalse(IndexMetadata.isNameValid("!")); + assertFalse(isValidCharsName(null)); + assertFalse(isValidCharsName("")); + assertFalse(isValidCharsName(" ")); + assertFalse(isValidCharsName("@")); + assertFalse(isValidCharsName("!")); } @Test diff --git a/test/unit/org/apache/cassandra/schema/MigrationManagerTest.java b/test/unit/org/apache/cassandra/schema/MigrationManagerTest.java index 78f9ad9eea96..240f20d6d7ba 100644 --- a/test/unit/org/apache/cassandra/schema/MigrationManagerTest.java +++ b/test/unit/org/apache/cassandra/schema/MigrationManagerTest.java @@ -138,11 +138,11 @@ public void testInvalidNames() { String[] valid = {"1", "a", "_1", "b_", "__", "1_a"}; for (String s : valid) - assertTrue(SchemaConstants.isValidName(s)); + assertTrue(SchemaConstants.isNameSafeForFilename(s)); String[] invalid = {"b@t", "dash-y", "", " ", "dot.s", ".hidden"}; for (String s : invalid) - assertFalse(SchemaConstants.isValidName(s)); + assertFalse(SchemaConstants.isNameSafeForFilename(s)); } @Test diff --git a/test/unit/org/apache/cassandra/schema/ValidationTest.java b/test/unit/org/apache/cassandra/schema/ValidationTest.java index 8eb1247c5b0c..db06b971fa51 100644 --- a/test/unit/org/apache/cassandra/schema/ValidationTest.java +++ b/test/unit/org/apache/cassandra/schema/ValidationTest.java @@ -32,19 +32,19 @@ public class ValidationTest @Test public void testIsNameValidPositive() { - assertTrue(SchemaConstants.isValidName("abcdefghijklmnopqrstuvwxyz")); - assertTrue(SchemaConstants.isValidName("ABCDEFGHIJKLMNOPQRSTUVWXYZ")); - assertTrue(SchemaConstants.isValidName("_01234567890")); + assertTrue(SchemaConstants.isNameSafeForFilename("abcdefghijklmnopqrstuvwxyz")); + assertTrue(SchemaConstants.isNameSafeForFilename("ABCDEFGHIJKLMNOPQRSTUVWXYZ")); + assertTrue(SchemaConstants.isNameSafeForFilename("_01234567890")); } @Test public void testIsNameValidNegative() { - assertFalse(SchemaConstants.isValidName(null)); - assertFalse(SchemaConstants.isValidName("")); - assertFalse(SchemaConstants.isValidName(" ")); - assertFalse(SchemaConstants.isValidName("@")); - assertFalse(SchemaConstants.isValidName("!")); + assertFalse(SchemaConstants.isNameSafeForFilename(null)); + assertFalse(SchemaConstants.isNameSafeForFilename("")); + assertFalse(SchemaConstants.isNameSafeForFilename(" ")); + assertFalse(SchemaConstants.isNameSafeForFilename("@")); + assertFalse(SchemaConstants.isNameSafeForFilename("!")); } private static Set primitiveTypes =