Skip to content

fix: throw OgnlException for out-of-range integer literals#590

Open
SAY-5 wants to merge 2 commits into
orphan-oss:mainfrom
SAY-5:fix-integer-literal-overflow
Open

fix: throw OgnlException for out-of-range integer literals#590
SAY-5 wants to merge 2 commits into
orphan-oss:mainfrom
SAY-5:fix-integer-literal-overflow

Conversation

@SAY-5

@SAY-5 SAY-5 commented Jun 9, 2026

Copy link
Copy Markdown

Fixes #583. Integer literals that overflow their target type (decimal above Integer.MAX_VALUE, long above Long.MAX_VALUE, and all hex literals in the upper half of the unsigned range) currently let a raw NumberFormatException escape parseExpression, which breaks the documented OgnlException contract that callers like Struts rely on. This wraps the failure in makeInt() as a TokenMgrError so it surfaces through the existing parseExpression handler as an OgnlException. Added a parameterized test covering the decimal, long, and hex overflow cases.

Signed-off-by: Sai Asish Y <say.apm35@gmail.com>

@lukaszlenart lukaszlenart left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, @SAY-5 — the core approach is spot on. Wrapping the switch in makeInt() and rethrowing NumberFormatException as a TokenMgrError(LEXICAL_ERROR) is exactly the right fix, since parseExpression already converts TokenMgrError into an OgnlException. A few suggestions before merge:

1. SonarCloud (java:S1166) — preserve/use the caught exception. The catch (NumberFormatException e) never references e, and TokenMgrError has no cause-accepting constructor, so the original detail is dropped. Our quality gate will likely flag this on new code. Folding the cause into the message resolves it and keeps useful diagnostics:

} catch (NumberFormatException e) {
    // Surface as a lexical error so callers see the documented OgnlException
    // instead of a raw NumberFormatException. TokenMgrError has no
    // cause-accepting constructor, so the original message is folded in.
    throw new TokenMgrError("Integer literal out of range: " + image
            + " (" + e.getMessage() + ")", TokenMgrError.LEXICAL_ERROR);
}

2. Add the -2147483648 case. The issue calls this out specifically: the lexer tokenises the leading - separately, so makeInt() sees the bare 2147483648. It's worth a test to lock in that it surfaces as OgnlException.

3. Assert the valid boundaries still parse. A couple of "does not throw" cases (2147483647, 9223372036854775807L) guard against an over-eager future change rejecting in-range values.

(Optional) 4. A note on parseExpression's javadoc that it throws OgnlException for out-of-range numeric literals, and a quick confirmation test that the float path (1e100) is unaffected, would round it out nicely.

The parameterized test reads well and the ognl.test package placement is consistent with the rest of the suite. With the S1166 tweak and the -2147483648 case, this is good to go. Thanks again!

Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
@SAY-5

SAY-5 commented Jun 18, 2026

Copy link
Copy Markdown
Author

Thanks for the careful review. Pushed all four: the catch now folds e.getMessage() into the message (resolves S1166 while keeping the diagnostic), added the -2147483648 case, added does-not-throw assertions for 2147483647 / 9223372036854775807L / 0x7FFFFFFF, and noted the out-of-range behaviour on parseExpression's javadoc with a 1e100 case to confirm the float path is untouched.

@SAY-5

SAY-5 commented Jun 18, 2026

Copy link
Copy Markdown
Author

Done, thanks. Folded the NumberFormatException message into the TokenMgrError so the cause is preserved (resolves S1166), added the -2147483648 case, and added does-not-throw assertions for 2147483647, 9223372036854775807L, 0x7FFFFFFF and the 1e100 float path. Also noted the out-of-range behaviour on parseExpression's javadoc. Full test class passes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ognl.parseExpression throws unchecked NumberFormatException for integer literals exceeding Integer.MAX_VALUE and for all hexadecimal literals

2 participants