SpEL vararg method invocation fails if string literal contains a comma

[bmoraes-axur]

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.

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

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));
}

[sbrannen]

Hi @bmoraes-axur,

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

We'll investigate.

Related issues:

• array varargs conversion error in SpEL expression [SPR-11494] #16119
• VerifyError when trying to compile constructor invocation with SpEL [SPR-12326] #16931
• Extend SpEL compiler for few more constructs: inline lists, string concatenation and varargs invocation [SPR-12328] #16933
• SpEL method invocation with varargs on proxy [SPR-16122] #20670

[sbrannen]

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

[xixingya]

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

[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
				}
			}

[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

[sbrannen]

Thanks for sharing your thoughts, @xixingya.

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

[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.

[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!

[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')").

[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.

[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 {
    }
}

[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).

[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	✅
#formatString('Hello %s', null)	null	✅

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'	❌
#formatString('Hello %s', '')	Hello	✅

Test 3: single string with comma

expression	result	expected
#formatAny('Hello %s', 'a, b')	Hello a	❌
#formatString('Hello %s', 'a, b')	Hello a, b	✅

Test 4: double string with comma

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

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.

[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

[weiw005]

@sbrannen Sounds good. I have created #33013