diff --git a/CHANGES.txt b/CHANGES.txt index fc0d9fb2c6..4dfc269f6a 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -6,6 +6,7 @@ Merged from 3.11: * Fix Splitter sometimes creating more splits than requested (CASSANDRA-18013) Merged from 3.0: + * 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) diff --git a/src/java/org/apache/cassandra/auth/FunctionResource.java b/src/java/org/apache/cassandra/auth/FunctionResource.java index bdec768dd1..ef7d99a5c6 100644 --- a/src/java/org/apache/cassandra/auth/FunctionResource.java +++ b/src/java/org/apache/cassandra/auth/FunctionResource.java @@ -170,15 +170,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) @@ -189,8 +197,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 472a7ffe67..bc70ed93ed 100644 --- a/src/java/org/apache/cassandra/cql3/functions/FunctionName.java +++ b/src/java/org/apache/cassandra/cql3/functions/FunctionName.java @@ -17,6 +17,11 @@ */ 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.cql3.CqlBuilder; @@ -24,6 +29,9 @@ import org.apache.cassandra.schema.SchemaConstants; public final class FunctionName { + private static final Set DISALLOWED_CHARACTERS = Collections.unmodifiableSet( + new HashSet<>(Arrays.asList('/', '[', ']'))); + // We special case the token function because that's the only function which name is a reserved keyword private static final FunctionName TOKEN_FUNCTION_NAME = FunctionName.nativeFunction("token"); @@ -35,6 +43,24 @@ public final class FunctionName return new FunctionName(SchemaConstants.SYSTEM_KEYSPACE_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/schema/CreateAggregateStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/CreateAggregateStatement.java index cc9a96b42c..e567021421 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/CreateAggregateStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/CreateAggregateStatement.java @@ -91,6 +91,9 @@ public final class CreateAggregateStatement extends AlterSchemaStatement if (ifNotExists && orReplace) throw ire("Cannot use both 'OR REPLACE' and 'IF NOT EXISTS' directives"); + if (!FunctionName.isNameValid(aggregateName)) + throw ire("Aggregate name '%s' is invalid", aggregateName); + rawArgumentTypes.stream() .filter(raw -> !raw.isTuple() && raw.isFrozen()) .findFirst() diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/CreateFunctionStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/CreateFunctionStatement.java index 18d8479bfd..3a1eb9fc01 100644 --- a/src/java/org/apache/cassandra/cql3/statements/schema/CreateFunctionStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/schema/CreateFunctionStatement.java @@ -91,6 +91,9 @@ public final class CreateFunctionStatement extends AlterSchemaStatement UDFunction.assertUdfsEnabled(language); + if (!FunctionName.isNameValid(functionName)) + throw ire("Function name '%s' is invalid", functionName); + if (new HashSet<>(argumentNames).size() != argumentNames.size()) throw ire("Duplicate argument names for given function %s with argument names %s", functionName, argumentNames); diff --git a/test/unit/org/apache/cassandra/auth/FunctionResourceTest.java b/test/unit/org/apache/cassandra/auth/FunctionResourceTest.java index 07a4b10274..8da811d0f2 100644 --- a/test/unit/org/apache/cassandra/auth/FunctionResourceTest.java +++ b/test/unit/org/apache/cassandra/auth/FunctionResourceTest.java @@ -19,6 +19,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; @@ -37,7 +39,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); @@ -46,7 +48,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)); @@ -55,7 +57,7 @@ public class FunctionResourceTest } @Test - public void testFunctionWithSingleInputParameter() throws Exception + public void testFunctionWithSingleInputParameter() { List> argTypes = new ArrayList<>(); argTypes.add(TypeParser.parse(varType)); @@ -66,7 +68,7 @@ public class FunctionResourceTest } @Test - public void testFunctionWithMultipleInputParameter() throws Exception + public void testFunctionWithMultipleInputParameter() { List> argTypes = new ArrayList<>(); argTypes.add(TypeParser.parse(varType)); @@ -78,7 +80,7 @@ public class FunctionResourceTest } @Test - public void testFunctionWithoutInputParameter() throws Exception + public void testFunctionWithoutInputParameter() { List> argTypes = new ArrayList<>(); FunctionResource expected = FunctionResource.function(ks, name, argTypes); @@ -105,13 +107,22 @@ 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 d4712737a4..76b83241c3 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; @@ -26,7 +27,8 @@ import com.google.common.reflect.TypeToken; import org.junit.Assert; import org.junit.Test; -import com.datastax.driver.core.*; +import com.datastax.driver.core.TypeTokens; +import com.datastax.driver.core.UDTValue; import com.datastax.driver.core.exceptions.InvalidQueryException; import org.apache.cassandra.cql3.CQLTester; import org.apache.cassandra.cql3.QueryProcessor; @@ -45,6 +47,8 @@ import org.apache.cassandra.transport.ProtocolVersion; 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 @@ -1006,4 +1010,21 @@ public class UFTest extends CQLTester assertRows(execute("SELECT " + fNameICC + "(empty_int) FROM %s"), row(0)); assertRows(execute("SELECT " + fNameICN + "(empty_int) FROM %s"), row(new Object[]{ null })); } + + @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 c5ee079c55..4b8e5e9c21 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/AggregationTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/AggregationTest.java @@ -58,6 +58,7 @@ import org.apache.cassandra.transport.ProtocolVersion; 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; @@ -2124,4 +2125,20 @@ public class AggregationTest extends CQLTester assertRows(execute("select sum(v1), sum(v2), sum(v3) from %s;"), row((float) 15.3, 15.3, BigDecimal.valueOf(15.3))); } + + @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); + } + } }