[ 
https://issues.apache.org/jira/browse/JEXL-256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16430277#comment-16430277
 ] 

Dmitri Blinov commented on JEXL-256:
------------------------------------

Ok, the comparison with {{ConcurrentHashMap}} was not a good one, mia culpa. 
Antish vars themselves are working, I can't say they are not evaluated 
correctly or something. That's why I didn't tag this issue as a bug, but as an 
improvement. I'm not the one to tell whether the runtime cost for everyone is 
neglectable in 2018 or not. But the functional gain I see here is more clear 
API for Context implementors and thus lesser bugs. Context may not neccesserily 
be as simple as HashMap. For example, I hope I haven't missed the point here 
either, I see in Jexl there is a {{JexlScriptEngine.JexlContextWrapper}} class 
which promises only to give access to {{ENGINE_SCOPE}} binding, but in fact its 
{{get()}} method returns variables from both {{ENGINE_SCOPE}} and 
{{GLOBAL_SCOPE}}. Its {{has()}} method correctly restricts variables to 
{{ENGINE_SCOPE}} but is never called if {{get()}} wrongly returns a non-null 
variable from {{GLOBAL_SCOPE}}.

> Jexl should not try to resolve a variable from Context when Context.has() 
> returns false
> ---------------------------------------------------------------------------------------
>
>                 Key: JEXL-256
>                 URL: https://issues.apache.org/jira/browse/JEXL-256
>             Project: Commons JEXL
>          Issue Type: Improvement
>    Affects Versions: 3.1
>            Reporter: Dmitri Blinov
>            Priority: Major
>
> I have bumped into the problem when my {{Context.get()}} sometimes reports 
> access to variables that are reported by the {{Context.has()}} method as not 
> existent, and are not supposed to be in the context, mainly parts of ant-ish 
> variable paths. I assume that once {{Context.has()}} have reported {{false}} 
> no attempt from the Jexl to call {{Context.get()}} with the same parameter 
> should be made.
> I think the problem lies in {{Interpreter.java}} which first calls 
> {{Context.get()}} and only if it returns null, which should not necceserily 
> mean the variable does not exist, checks {{Context.has()}}. 
> {code}
>     @Override
>     protected Object visit(ASTIdentifier node, Object data) {
>         cancelCheck(node);
>         String name = node.getName();
>         if (data == null) {
>             int symbol = node.getSymbol();
>             if (symbol >= 0) {
>                 return frame.get(symbol);
>             }
>             Object value = context.get(name);
>             if (value == null
>                     && !(node.jjtGetParent() instanceof ASTReference)
>                     && !context.has(name)
>                     && !node.isTernaryProtected()) {
>                 return unsolvableVariable(node, name, true);
>             }
>             return value;
>         } else {
>             return getAttribute(data, name, node);
>         }
>     }
> {code}
> So I suggest to change the code to something like this
> {code}
>     @Override
>     protected Object visit(ASTIdentifier node, Object data) {
>         cancelCheck(node);
>         String name = node.getName();
>         if (data == null) {
>             int symbol = node.getSymbol();
>             if (symbol >= 0) {
>                 return frame.get(symbol);
>             }
>             if (!context.has(name)
>                     && !(node.jjtGetParent() instanceof ASTReference)
>                     && !node.isTernaryProtected()) {
>                 return unsolvableVariable(node, name, true);
>             }
>             return context.get(name);
>         } else {
>             return getAttribute(data, name, node);
>         }
>     }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to