[
https://issues.apache.org/jira/browse/JEXL-324?focusedWorklogId=377849&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-377849
]
ASF GitHub Bot logged work on JEXL-324:
---------------------------------------
Author: ASF GitHub Bot
Created on: 27/Jan/20 21:20
Start Date: 27/Jan/20 21:20
Worklog Time Spent: 10m
Work Description: davidcostanzo commented on pull request #20: JEXL-324 -
Change grammar to make "new" require at least one argument
URL: https://github.com/apache/commons-jexl/pull/20
This PR changes the JEXL grammar to force "new" to require at least one
argument. It also includes one new test case to cover the newly illegal syntax
(new with arguments is already covered, but more coverage could be added if
desired).
Another way to fix the NPE described in JEXL-324 would be to keep the
grammar as-is, but tolerate new without any arguments within the Debugger
class. This would have less of a compatibility impact.
This is my first PR to the JEXL project. I'm open to criticism.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 377849)
Remaining Estimate: 0h
Time Spent: 10m
> JexlEngine.createExpression("new()").getParsedText() throws NPE
> ---------------------------------------------------------------
>
> Key: JEXL-324
> URL: https://issues.apache.org/jira/browse/JEXL-324
> Project: Commons JEXL
> Issue Type: Bug
> Affects Versions: 3.1
> Reporter: David Costanzo
> Priority: Minor
> Time Spent: 10m
> Remaining Estimate: 0h
>
> JEXL is able to parse the expression "new()", but some parts of the system do
> not expect this. Specifically, if you try to get the parsed text, JEXL
> throws a NullPointerException instead of returning "new()".
> I expect {{JexlExpression.getParsedText()}} to return a String for any
> expression that was successfully parsed. So either {{createExpression()}}
> should throw an exception or {{getParsedText()}} should return "new()". My
> preference is for {{createExpression()}} to fail and I'll try to submit a PR
> for this.
>
> *Impact:*
> My program tries to fail fast with a clear error message when a user attempts
> to "register" a function using a reserved name (function registration happens
> outside of JEXL and is implemented by populating a JexlContext with a parsed
> expression). My program does this by probing for valid function names by
> parsing {{_FUNCTION_NAME_+"()"}} and checking for errors. Since "new()" is a
> legal expression, my program has a special case to disallow "new". If
> {{createExpression("new()")}} threw an exception, then I could remove the
> special case.
>
> *Steps to Reproduce:*
>
> {code:java}
> @Test
> public void testNew() throws IOException {
> JexlEngine jexl = new JexlBuilder().create();
> try {
> JexlExpression expression = jexl.createExpression("new()");
> Assert.assertEquals("new()", expression.getParsedText());
> } catch (JexlException.Parsing exception) {
> }
> }
> {code}
>
> *What Happens:*
> {{getParsedText()}} throws a {{NullPointerException}}.
> *Expected Result:*
> Either {{JexlEngine.createExpression()}} throws a {{JexlException.Parsing}}
> or {{expression.getParsedText()}} returns "new()".
> *Note:*
> This was found on JEXL 3.1, the latest official release. I reproduced it on a
> snapshot of JEXL 3.2 built from GitHub source.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
