Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SpEL, error parsing big InlineMap [SPR-17605] #22137

Closed
spring-issuemaster opened this issue Dec 18, 2018 · 2 comments
Closed

SpEL, error parsing big InlineMap [SPR-17605] #22137

spring-issuemaster opened this issue Dec 18, 2018 · 2 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Dec 18, 2018

Davide Pallaoro opened SPR-17605 and commented

Hi,

In the project where I am working now, one of the components uses SpEL for parsing formulas stored in files.

Some of those formulas could be InlineMap, and I have encountered a problem when parsing pretty big maps (tens of thousands of elements).

 

Here you can find a short program that simulates the issue. If you, place the following files [^yes-works.spel] and [^not-works.spel] in /tmp/spel-test and run the following java class, using org.springframework:spring-expression 5.1.3-RELEASE

package test;

import org.springframework.expression.Expression;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.testng.annotations.Test;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Map;

public class LoadBigMap {

    @Test
    public void notWork() throws IOException {
        readMap("not-works.spel");
    }

    @Test
    public void yesWork() throws IOException {
        readMap("yes-works.spel");
    }

    private static void readMap(String path) throws IOException {
        String spelString = readFile(path);
        Expression expression = new SpelExpressionParser().parseExpression(spelString);
        Map value = (Map) expression.getValue();
        System.out.println(value.size());
    }

    private static String readFile(String path) throws IOException {
        Path currentDir = Paths.get(".");
        System.out.println(currentDir.toAbsolutePath());
        return new String(Files.readAllBytes(Paths.get("/tmp/spel-test", path)));
    }

}

yesWork() will parse the map correctly and notWork() will raise the following exception

java.lang.IllegalArgumentException: Pos must not be 0

	at org.springframework.util.Assert.isTrue(Assert.java:118)
	at org.springframework.expression.spel.ast.SpelNodeImpl.<init>(SpelNodeImpl.java:73)
	at org.springframework.expression.spel.ast.Literal.<init>(Literal.java:40)
	at org.springframework.expression.spel.ast.StringLiteral.<init>(StringLiteral.java:37)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.maybeEatLiteral(InternalSpelExpressionParser.java:868)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatStartNode(InternalSpelExpressionParser.java:505)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatPrimaryExpression(InternalSpelExpressionParser.java:351)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatUnaryExpression(InternalSpelExpressionParser.java:345)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatPowerIncDecExpression(InternalSpelExpressionParser.java:304)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatProductExpression(InternalSpelExpressionParser.java:282)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatSumExpression(InternalSpelExpressionParser.java:264)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatRelationalExpression(InternalSpelExpressionParser.java:218)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatLogicalAndExpression(InternalSpelExpressionParser.java:205)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatLogicalOrExpression(InternalSpelExpressionParser.java:192)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatExpression(InternalSpelExpressionParser.java:153)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.maybeEatInlineListOrMap(InternalSpelExpressionParser.java:669)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatStartNode(InternalSpelExpressionParser.java:521)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatPrimaryExpression(InternalSpelExpressionParser.java:351)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatUnaryExpression(InternalSpelExpressionParser.java:345)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatPowerIncDecExpression(InternalSpelExpressionParser.java:304)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatProductExpression(InternalSpelExpressionParser.java:282)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatSumExpression(InternalSpelExpressionParser.java:264)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatRelationalExpression(InternalSpelExpressionParser.java:218)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatLogicalAndExpression(InternalSpelExpressionParser.java:205)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatLogicalOrExpression(InternalSpelExpressionParser.java:192)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.eatExpression(InternalSpelExpressionParser.java:153)
	at org.springframework.expression.spel.standard.InternalSpelExpressionParser.doParseExpression(InternalSpelExpressionParser.java:131)
	at org.springframework.expression.spel.standard.SpelExpressionParser.doParseExpression(SpelExpressionParser.java:61)
	at org.springframework.expression.spel.standard.SpelExpressionParser.doParseExpression(SpelExpressionParser.java:33)
	at org.springframework.expression.common.TemplateAwareExpressionParser.parseExpression(TemplateAwareExpressionParser.java:52)
	at org.springframework.expression.common.TemplateAwareExpressionParser.parseExpression(TemplateAwareExpressionParser.java:43)
	at test.LoadBigMap.readMap(LoadBigMap.java:28)
	at test.LoadBigMap.notWork(LoadBigMap.java:17)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:80)
	at org.testng.internal.Invoker.invokeMethod(Invoker.java:673)
	at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:842)
	at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1166)
	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:125)
	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:109)
	at org.testng.TestRunner.runWorkers(TestRunner.java:1178)
	at org.testng.TestRunner.privateRun(TestRunner.java:757)
	at org.testng.TestRunner.run(TestRunner.java:608)
	at org.testng.SuiteRunner.runTest(SuiteRunner.java:334)
	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:329)
	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:291)
	at org.testng.SuiteRunner.run(SuiteRunner.java:240)
	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1158)
	at org.testng.TestNG.runSuitesLocally(TestNG.java:1083)
	at org.testng.TestNG.run(TestNG.java:999)
	at org.testng.IDEARemoteTestNG.run(IDEARemoteTestNG.java:72)
	at org.testng.RemoteTestNGStarter.main(RemoteTestNGStarter.java:123)

The files [^not-works.spel] and [^yes-works.spel] differ only for a space in position 655350.

It would be great that as long as the 2 maps contain the same items, both of them would be parsed correctly.

 

Looking into the code of spring I’ve noticed that the method

org.springframework.expression.spel.standard.InternalSpelExpressionParser#toPos(org.springframework.expression.spel.standard.Token)

compress startPos and endPos into one single integer(32 bits), the problem here is that 655350 is bigger than (16bit) and therefore the result is not the expected one.

Moreover, I have noticed that in case of a InlineMap, it looks like that the value of pos is never read.

 

Let me know if everything is clear or you need some further details.

I’m pretty new into the Spring world, and I hope that I followed all the correct process in order to submit this issue, if not, please let me know.

 

Many thanks and kind regards.


Affects: 4.3.21, 5.0.11, 5.1.3

Attachments:

Issue Links:

  • #22146 SpEL, InlineMap with negative values are not cached
  • #22157 Revisit restrictions in SpEL position handling

Referenced from: commits b2756f5, c02446c, 15f255a

Backported to: 5.0.12, 4.3.22

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Dec 25, 2018

Juergen Hoeller commented

I've relaxed that assertion for the time being, with the position indicating 0 in an overflow scenario. #22157 is meant to revise this towards proper handling of larger position values in Spring Framework 5.2.

@devDavide

This comment has been minimized.

Copy link

@devDavide devDavide commented Jan 23, 2019

Thank you very much for the fixing this issue, now it works perfectly :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.