Spring SpEL vararg method invocation fails if string literal contains a comma

Affects: 4.1.2 and up

I'm trying to use my own matching functions with spring-expression, but I came across a possible bug.

Here's some code for context:

import org.springframework.expression.spel.support.StandardEvaluationContext;

public class RuleEvaluationContext extends StandardEvaluationContext {

    public RuleEvaluationContext(Object rootObject) {
        super(rootObject);
        registerFunctions();
    }

    public RuleEvaluationContext() {
        super();
        registerFunctions();
    }

    private void registerFunctions() {
        for (Functions fun : Functions.values()) {
            this.registerFunction(fun.funcName(), fun.method());
        }
    }

}

enum Functions {
/* Functions commented for the sake of simplicity

    CONTAINS(Contains.FUNC_NAME,
        getDeclaredMethod(Contains.class, Contains.FUNC_NAME, String.class, String[].class)),

    STARTS_WITH(StartsWith.FUNC_NAME,
        getDeclaredMethod(StartsWith.class, StartsWith.FUNC_NAME, String.class, String[].class)),
*/

    MATCHES(Matches.FUNC_NAME,
        getDeclaredMethod(Matches.class, Matches.FUNC_NAME, String.class, String[].class)),
/*
    EQUALS(Equals.FUNC_NAME,
        getDeclaredMethod(Equals.class, Equals.FUNC_NAME, String.class, String.class)),

    GREATER(GreaterThan.FUNC_NAME,
        getDeclaredMethod(GreaterThan.class, GreaterThan.FUNC_NAME, String.class, String.class)),

    LESS_THAN(LessThan.FUNC_NAME,
        getDeclaredMethod(LessThan.class, LessThan.FUNC_NAME, String.class, String.class));
*/

    private final String funcName;
    private final Method method;

    Functions(String funcName, Method method) {
        this.funcName = funcName;
        this.method = method;
    }

    public String funcName() {
        return funcName;
    }

    public Method method() {
        return method;
    }

    private static Method getDeclaredMethod(Class c, String name, Class<?>... parameterTypes) {
        try {
            return c.getDeclaredMethod(name, parameterTypes);
        } catch (NoSuchMethodException e) {
            throw new FunctionsException(e);
        }
    }

}

This is one of the registered functions:

public class Matches {

    public static final String FUNC_NAME = "matches";

    private Matches() {
        super();
    }

    public static boolean matches(String text, String... regexps) {
        if (text != null) {
            for (String regex : regexps) {
                Pattern pattern = Pattern.compile(regex, Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CASE);
                Matcher matcher = pattern.matcher(text);
                if (matcher.find()) {
                    return true;
                }
            }
        }
        return false;
    }
}

@Test
public void test() {
    final RuleEvaluationContext context = new RuleEvaluationContext(new FakeObject());
    String originalRule = "#matches(prop, 'xyz,xyz')";
    Expression expression = parser.parseExpression(originalRule);
    Boolean result = expression.getValue(context, Boolean.class);
    assertThat(result, is(false));
}

private final ExpressionParser parser = new SpelExpressionParser();

private static class FakeObject {
    private String prop = "xyz";

    public String getProp() {
        return prop;
    }
}

This test fails when it should pass. Upon further inspection I've found that the problem is that spring-expression breaks the 'xyz,xyz' into ["xyz", "xyz"] as can be seen below.

libnova

However I've also found that using old versions of the library (pre 4.1.2), this doesn't happen.

libantiga

As does putting more strings inside the argument:

@Test
public void test() {
    final RuleEvaluationContext context = new RuleEvaluationContext(new FakeObject());
    String originalRule = "#matches(prop, 'abc', 'xyz,xyz')";
    Expression expression = parser.parseExpression(originalRule);
    Boolean result = expression.getValue(context, Boolean.class);
    assertThat(result, is(false));
}

libnovasembug

Comment From: sbrannen

Hi @bmoraes-axur,

Thanks for creating your first issue for the Spring Framework! 👍

We'll investigate.

Related issues:

16119
16931
16933
20670

Comment From: sbrannen

I have confirmed that this is a bug which applies to method invocations in general (not just to custom functions).

Comment From: xixingya

hello, I have find it is because if just two args, the second args will have another logic

Comment From: xixingya

first into org.springframework.expression.spel.ast.FunctionReference#executeFunctionJLRMethod.
and in ReflectionHelper.convertAllArguments. org.springframework.expression.spel.support.ReflectionHelper#convertArguments


if (varargsPosition == arguments.length - 1) {
                // If the target is varargs and there is just one more argument
                // then convert it here
                TypeDescriptor targetType = new TypeDescriptor(methodParam);
                Object argument = arguments[varargsPosition];
                TypeDescriptor sourceType = TypeDescriptor.forObject(argument);
                arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetType);
                // Three outcomes of that previous line:
                // 1) the input argument was already compatible (ie. array of valid type) and nothing was done
                // 2) the input argument was correct type but not in an array so it was made into an array
                // 3) the input argument was the wrong type and got converted and put into an array
                if (argument != arguments[varargsPosition] &&
                        !isFirstEntryInArray(argument, arguments[varargsPosition])) {
                    conversionOccurred = true; // case 3
                }
            }

Comment From: xixingya

but I have no idea to fix the problem, in the doc say it is a feature if just one argument should convert // If the target is varargs and there is just one more argument // then convert it here

Comment From: sbrannen

Thanks for sharing your thoughts, @xixingya.

I already had a fix in place and pushed it to 5.3.x and main.

Comment From: sbrannen

@bmoraes-axur, feel free to try this out in the next 5.3.13 snapshot and let us know if you run into any issues.

Comment From: bmoraes-axur

@bmoraes-axur, feel free to try this out in the next 5.3.13 snapshot and let us know if you run into any issues.

Will do, thank you very much!

Comment From: Jos31fr

Warning: I had a regression on my web app I think due to this correction. I think it's correct but some persons like me can be impacted with a minor evolution of spring (12 -> 13).

In Spring Security, I was using @PreAuthorize("hasAnyRole('ROLE_X, ROLE_Y')") without quoting each ROLE. I must now use @PreAuthorize("hasAnyRole('ROLE_X', 'ROLE_Y')").

Comment From: sbrannen

Warning: I had a regression on my web app I think due to this correction. I think it's correct but some persons like me can be impacted with a minor evolution of spring (12 -> 13).

That's a good point. If you were relying on this bug, you'll need to switch to the proper syntax for multiple vararg values.

In Spring Security, I was using @PreAuthorize("hasAnyRole('ROLE_X, ROLE_Y')") without quoting each ROLE. I must now use @PreAuthorize("hasAnyRole('ROLE_X', 'ROLE_Y')").

That is indeed the correct way to use hasAnyRole() as documented in the Spring Security reference docs.

Comment From: bbbush

In Spring Security, I was using @PreAuthorize("hasAnyRole('ROLE_X, ROLE_Y')") without quoting each ROLE. I must now use @PreAuthorize("hasAnyRole('ROLE_X', 'ROLE_Y')").

we encountered the same regression when upgrading from earlier versions. We use a property to store the list of authorized roles or groups, and now due to this change, entire list is treated as one role -- and "hasAnyAuthority" is failing because of that.

@PreAuthorize("hasAnyAuthority(@propertyResolver.getProperty('admin.authority'))")

Because a property is a string, this regression only impacts varargs uses like hasAnyAuthority

some workaround suggested from @yabqiu: split the array in the expression, or add another wrapper to help infer the correct type.

@SpringBootApplication
public class Main implements CommandLineRunner {

    static {
        System.setProperty("property1", "{'a','b'}");
    }

    public static void main(String[] args) {
        SpringApplication.run(Main.class, args);
    }

    public static int setProperty(String... args) {
        return args.length;
    }

    public static String[] identity(String[] arr) {
        return arr;
    }

    @Autowired
    public void printLength(@Value("#{@main.setProperty(systemProperties['property1'])}") int value) {
        System.out.println("property length: " + value);
    }

    @Autowired
    public void printLengthSplit(@Value("#{T(test.Main).setProperty(systemProperties['property1'].split(','))}") int value) {
        System.out.println("length with split: " + value);
    }

    @Autowired
    public void printLengthId(@Value("#{T(test.Main).setProperty(T(test.Main).identity(systemProperties['property1'] ))}") int value) {
        System.out.println("length with type cast: " + value);
    }

    @Override
    public void run(String... args) throws Exception {
    }
}

Comment From: manoharank

@sbrannen Still facing the issue when the method is print(Object... args) and invoking print(name) and name has comma. No issue when invoking print(name, age) (more than one arguments).

Comment From: weiw005

I've been encountering some issues that seem to be related to this. It seems to me that the behavior to handle vararg parameters is inconsistent.

To explain, below are a couple of Kotlin functions that I used to test:

object Functions {
    @JvmStatic
    fun formatAny(format: String, vararg args: Any?): String? {
        if (args.any { it == null }) return null
        return format.format(*args)
    }

    @JvmStatic
    fun formatString(format: String, vararg args: String?): String? {
        if (args.any { it == null }) return null
        return format.format(*args)
    }
}

Both functions accept vararg parameters. The only difference between the two is that the first function can work with non-string objects (which is normally what people would want for the string formatting use cases).

I did a few tests with the latest version that I can find in maven, which is org.springframework:spring-expression:6.1.8. See the results below.

Test 1: single null argument

expression	result	expected
`#formatAny('Hello %s', null)`	`null`	:white_check_mark:
`#formatString('Hello %s', null)`	`null`	:white_check_mark:

Test 2: single empty string

expression	result	expected
`#formatAny('Hello %s', '')`	throws `SpelEvaluationException: EL1023E: A problem occurred whilst attempting to invoke the function 'formatAny': 'null'`	:x:
`#formatString('Hello %s', '')`	`Hello`	:white_check_mark:

Test 3: single string with comma

expression	result	expected
`#formatAny('Hello %s', 'a, b')`	`Hello a`	:x:
`#formatString('Hello %s', 'a, b')`	`Hello a, b`	:white_check_mark:

Test 4: double string with comma

expression	result	expected
`#formatAny('Hello %s', 'a, b', 'c, d')`	`Hello a, b, c, d`	:white_check_mark:
`#formatString('Hello %s', 'a, b', 'c, d')`	`Hello a, b, c, d`	:white_check_mark:

My conclusion is that: - When argument type is specified for vararg (e.g. vararg args: String?), things seem to always work correctly. - However, if argument type is Any? (Object in java), the behavior is inconsistent depending on whether it's single argument or multiple argument. - When it's single argument, the original bug described in this issue still exists.

Comment From: sbrannen

Hi @manoharank and @weiw005,

Please note that this issue was closed almost three years ago.

If you are still encountering issues against supported versions of Spring Framework, please create a new GitHub issue and provide a sample application that reproduces the issue which we can run ourselves -- for example, a public Git repository or a ZIP-file attached to the issue.

Thanks

Comment From: weiw005

@sbrannen Sounds good. I have created https://github.com/spring-projects/spring-framework/issues/33013

Spring SpEL vararg method invocation fails if string literal contains a comma

16119

16931

16933

20670