diff --git a/CHANGES.txt b/CHANGES.txt index 296d41f2b2..2543f44e85 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.0.29 + * Fix incorrect resource name in LIST PERMISSION output (CASSANDRA-17848) * Suppress CVE-2022-41854 and similar (CASSANDRA-18083) * Fix running Ant rat targets without git (CASSANDRA-17974) * Fix intermittent failure in nodetool toppartitions (CASSANDRA-17254) diff --git a/src/java/org/apache/cassandra/auth/FunctionResource.java b/src/java/org/apache/cassandra/auth/FunctionResource.java index 9a6c6ab877..2981d91a13 100644 --- a/src/java/org/apache/cassandra/auth/FunctionResource.java +++ b/src/java/org/apache/cassandra/auth/FunctionResource.java @@ -160,15 +160,23 @@ public class FunctionResource implements IResource /** * Parses a resource name into a FunctionResource instance. + * A valid resource name for function should be in the follow format: + * functions/KEYSPACE/FUNCTION_NAME[FUNCTION_ARGS] + * Note that + * 1. FUNCTION_NAME could contain any character due to the use of quoted text in CQL. + * 2. FUNCTION_ARGS could be empty. If it is not empty, it is expressed in this format: + * FUNCTION_ARG1^FUNCTION_ARG2... where ^ is the delimiter for arguments * * @param name Name of the function resource. * @return FunctionResource instance matching the name. */ public static FunctionResource fromName(String name) { - String[] parts = StringUtils.split(name, '/'); + // Split the name into at most 3 parts. + // The last part is the function name + args list, the name might contains '/' + String[] parts = StringUtils.split(name, "/", 3); - if (!parts[0].equals(ROOT_NAME) || parts.length > 3) + if (!parts[0].equals(ROOT_NAME)) throw new IllegalArgumentException(String.format("%s is not a valid function resource name", name)); if (parts.length == 1) @@ -179,8 +187,17 @@ public class FunctionResource implements IResource if (!name.matches("^.+\\[.*\\]$")) throw new IllegalArgumentException(String.format("%s is not a valid function resource name. It must end with \"[]\"", name)); - String[] nameAndArgs = StringUtils.split(parts[2], "[|]"); - return function(parts[1], nameAndArgs[0], nameAndArgs.length > 1 ? argsListFromString(nameAndArgs[1]) : Collections.emptyList()); + + String function = parts[2]; + // The name must end with '[...]' block + int lastStartingBracketIndex = function.lastIndexOf('['); + String functionName = StringUtils.substring(function, 0, lastStartingBracketIndex); + String functionArgs = StringUtils.substring(function, + // excludes the wrapping brackets [ ] + lastStartingBracketIndex + 1, + function.length() - 1); + + return function(parts[1], functionName, functionArgs.isEmpty() ? Collections.emptyList() : argsListFromString(functionArgs)); } /** diff --git a/src/java/org/apache/cassandra/cql3/functions/FunctionName.java b/src/java/org/apache/cassandra/cql3/functions/FunctionName.java index d732efae92..6fb3faa1e1 100644 --- a/src/java/org/apache/cassandra/cql3/functions/FunctionName.java +++ b/src/java/org/apache/cassandra/cql3/functions/FunctionName.java @@ -17,12 +17,20 @@ */ package org.apache.cassandra.cql3.functions; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + import com.google.common.base.Objects; import org.apache.cassandra.db.SystemKeyspace; public final class FunctionName { + private static final Set DISALLOWED_CHARACTERS = Collections.unmodifiableSet( + new HashSet<>(Arrays.asList('/', '[', ']'))); + public final String keyspace; public final String name; @@ -31,6 +39,24 @@ public final class FunctionName return new FunctionName(SystemKeyspace.NAME, name); } + /** + * Validate the function name, e.g. contains no disallowed characters + * @param name + * @return true if name is valid; otherwise, false + */ + public static boolean isNameValid(String name) + { + for (int i = 0; i < name.length(); i++) + { + char c = name.charAt(i); + if (DISALLOWED_CHARACTERS.contains(c)) + { + return false; + } + } + return true; + } + public FunctionName(String keyspace, String name) { assert name != null : "Name parameter must not be null"; diff --git a/src/java/org/apache/cassandra/cql3/statements/CreateAggregateStatement.java b/src/java/org/apache/cassandra/cql3/statements/CreateAggregateStatement.java index 9d91693008..2210a2d19b 100644 --- a/src/java/org/apache/cassandra/cql3/statements/CreateAggregateStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/CreateAggregateStatement.java @@ -80,6 +80,9 @@ public final class CreateAggregateStatement extends SchemaAlteringStatement public Prepared prepare(ClientState clientState) { + if (!FunctionName.isNameValid(functionName.name)) + throw new InvalidRequestException(String.format("Aggregate name '%s' is invalid", functionName.name)); + argTypes = new ArrayList<>(argRawTypes.size()); for (CQL3Type.Raw rawType : argRawTypes) argTypes.add(prepareType("arguments", rawType)); diff --git a/src/java/org/apache/cassandra/cql3/statements/CreateFunctionStatement.java b/src/java/org/apache/cassandra/cql3/statements/CreateFunctionStatement.java index dfe522b5c2..fc65804f02 100644 --- a/src/java/org/apache/cassandra/cql3/statements/CreateFunctionStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/CreateFunctionStatement.java @@ -78,6 +78,9 @@ public final class CreateFunctionStatement extends SchemaAlteringStatement public Prepared prepare(ClientState clientState) throws InvalidRequestException { + if (!FunctionName.isNameValid(functionName.name)) + throw new InvalidRequestException(String.format("Function name '%s' is invalid", functionName.name)); + if (new HashSet<>(argNames).size() != argNames.size()) throw new InvalidRequestException(String.format("duplicate argument names for given function %s with argument names %s", functionName, argNames)); diff --git a/test/unit/org/apache/cassandra/auth/FunctionResourceTest.java b/test/unit/org/apache/cassandra/auth/FunctionResourceTest.java index 54da393edc..f075e6cd69 100644 --- a/test/unit/org/apache/cassandra/auth/FunctionResourceTest.java +++ b/test/unit/org/apache/cassandra/auth/FunctionResourceTest.java @@ -18,6 +18,8 @@ package org.apache.cassandra.auth; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.List; import org.junit.Test; @@ -36,7 +38,7 @@ public class FunctionResourceTest private static final String varType = "org.apache.cassandra.db.marshal.UTF8Type"; @Test - public void testFunction() throws Exception + public void testFunction() { FunctionResource expected = FunctionResource.root(); FunctionResource actual = FunctionResource.fromName(func); @@ -45,7 +47,7 @@ public class FunctionResourceTest } @Test - public void testFunctionKeyspace() throws Exception + public void testFunctionKeyspace() { FunctionResource expected = FunctionResource.keyspace(ks); FunctionResource actual = FunctionResource.fromName(String.format("%s/%s", func, ks)); @@ -54,7 +56,7 @@ public class FunctionResourceTest } @Test - public void testFunctionWithSingleInputParameter() throws Exception + public void testFunctionWithSingleInputParameter() { List> argTypes = new ArrayList<>(); argTypes.add(TypeParser.parse(varType)); @@ -65,7 +67,7 @@ public class FunctionResourceTest } @Test - public void testFunctionWithMultipleInputParameters() throws Exception + public void testFunctionWithMultipleInputParameter() { List> argTypes = new ArrayList<>(); argTypes.add(TypeParser.parse(varType)); @@ -77,7 +79,7 @@ public class FunctionResourceTest } @Test - public void testFunctionWithoutInputParameters() throws Exception + public void testFunctionWithoutInputParameter() { List> argTypes = new ArrayList<>(); FunctionResource expected = FunctionResource.function(ks, name, argTypes); @@ -104,12 +106,21 @@ public class FunctionResourceTest @Test public void testFunctionWithInvalidInput() { - String expected = String.format("%s/%s/%s[%s]/test is not a valid function resource name", func, ks, name, varType); - assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> FunctionResource.fromName(String.format("%s/%s/%s[%s]/test", - func, - ks, - name, - varType))) + String invalidInput = String.format("%s/%s/%s[%s]/test", func, ks, name, varType); + String expected = String.format("%s is not a valid function resource name. It must end with \"[]\"", invalidInput); + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> FunctionResource.fromName(invalidInput)) .withMessage(expected); } + + @Test + public void testFunctionWithSpecialCharacter() + { + for (String funcName : Arrays.asList("my/fancy/func", "my_other[fancy]func")) + { + String input = String.format("%s/%s/%s[%s]", func, ks, funcName, varType); + FunctionResource actual = FunctionResource.fromName(input); + FunctionResource expected = FunctionResource.function(ks, funcName, Collections.singletonList(TypeParser.parse(varType))); + assertEquals(expected, actual); + } + } } diff --git a/test/unit/org/apache/cassandra/cql3/validation/entities/UFTest.java b/test/unit/org/apache/cassandra/cql3/validation/entities/UFTest.java index cb7686774e..e9c26d364f 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/entities/UFTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/entities/UFTest.java @@ -19,6 +19,7 @@ package org.apache.cassandra.cql3.validation.entities; import java.nio.ByteBuffer; import java.util.ArrayList; +import java.util.Arrays; import java.util.Date; import java.util.List; @@ -37,13 +38,14 @@ import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.exceptions.SyntaxException; import org.apache.cassandra.schema.KeyspaceMetadata; import org.apache.cassandra.service.ClientState; -import org.apache.cassandra.transport.Event; -import org.apache.cassandra.transport.Server; import org.apache.cassandra.transport.Event.SchemaChange.Change; import org.apache.cassandra.transport.Event.SchemaChange.Target; +import org.apache.cassandra.transport.Server; import org.apache.cassandra.transport.messages.ResultMessage; import org.apache.cassandra.utils.ByteBufferUtil; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + public class UFTest extends CQLTester { @Test @@ -996,4 +998,21 @@ public class UFTest extends CQLTester " return a;\n" + " $$"); } + + @Test + public void testRejectInvalidFunctionNamesOnCreation() + { + for (String funcName : Arrays.asList("my/fancy/func", "my_other[fancy]func")) + { + assertThatThrownBy(() -> { + createFunctionOverload(String.format("%s.\"%s\"", KEYSPACE_PER_TEST, funcName), "int", + "CREATE OR REPLACE FUNCTION %s(val int) " + + "RETURNS NULL ON NULL INPUT " + + "RETURNS int " + + "LANGUAGE JAVA\n" + + "AS 'return val;'"); + }).hasRootCauseInstanceOf(InvalidRequestException.class) + .hasRootCauseMessage("Function name '%s' is invalid", funcName); + } + } } diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/AggregationTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/AggregationTest.java index 9f2bdb2b3d..439bd4e90d 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/AggregationTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/AggregationTest.java @@ -61,6 +61,7 @@ import org.apache.cassandra.transport.Event.SchemaChange.Target; import org.apache.cassandra.transport.messages.ResultMessage; import static ch.qos.logback.core.CoreConstants.RECONFIGURE_ON_CHANGE_TASK; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -2135,4 +2136,20 @@ public class AggregationTest extends CQLTester for (int i = 1; i <= 17; i++) execute("insert into %s (bucket, v1, v2, v3) values (?, ?, ?, ?)", i, (float) (i / 10.0), i / 10.0, BigDecimal.valueOf(i / 10.0)); } + + @Test + public void testRejectInvalidAggregateNamesOnCreation() + { + for (String funcName : Arrays.asList("my/fancy/aggregate", "my_other[fancy]aggregate")) + { + assertThatThrownBy(() -> { + createAggregateOverload(String.format("%s.\"%s\"", KEYSPACE_PER_TEST, funcName), "int", + " CREATE AGGREGATE IF NOT EXISTS %s(text, text)\n" + + " SFUNC func\n" + + " STYPE map\n" + + " INITCOND { };"); + }).hasRootCauseInstanceOf(InvalidRequestException.class) + .hasRootCauseMessage("Aggregate name '%s' is invalid", funcName); + } + } }