[jira] [Commented] (JEXL-256) Jexl should not try to resolve a variable from Context when Context.has() returns false
[
https://issues.apache.org/jira/browse/JEXL-256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16435349#comment-16435349
]
Dmitri Blinov commented on JEXL-256:
Thanks for suggestion. I'm going to use {{JexlException.Assignment}} as it more
close to the case. Now I think we may close the issue. Thanks for your time
once again!
> 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)
[jira] [Commented] (JEXL-256) Jexl should not try to resolve a variable from Context when Context.has() returns false
[
https://issues.apache.org/jira/browse/JEXL-256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16434564#comment-16434564
]
Henri Biestro commented on JEXL-256:
Instead of IllegalArgumentException, you may throw a JexlException from your
overridden set method, that'll do the trick. I've committed a test to verify
the behavior in the trunk in Issues200Test.test256(...).
> 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)
[jira] [Commented] (JEXL-256) Jexl should not try to resolve a variable from Context when Context.has() returns false
[
https://issues.apache.org/jira/browse/JEXL-256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16433583#comment-16433583
]
Dmitri Blinov commented on JEXL-256:
I have figured out how to overcome this issue without changing Jexl, I simply
need to restrict the usage of context variable names that, in my case, are
well-known prefixes of my ant-ish variables, which makes sence to do anyway.
So, for example, when {{java.lang.Double}} is need to be resolved, I should
restrict the use of {{java}} variable name, and return {{null}} in {{get()}}
and {{false}} in {{has()}}. The only problem now is that I also want to restict
assigning anything to such a variable, so {{java = 1}} would be illegal to use
inside script. I tried to throw {{IllegalArgumentException}} inside
{{Context.set()}} but somehow Jexl does not catch this and passes this
exception without wrapping it with JexlException, which makes me suspect this
is not right thing to do. Is there a better way we can prevent context from
setting values to certain names?
> 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)
[jira] [Commented] (JEXL-256) Jexl should not try to resolve a variable from Context when Context.has() returns false
[
https://issues.apache.org/jira/browse/JEXL-256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)
[jira] [Commented] (JEXL-256) Jexl should not try to resolve a variable from Context when Context.has() returns false
[
https://issues.apache.org/jira/browse/JEXL-256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16429401#comment-16429401
]
Gary Gregory commented on JEXL-256:
---
IIRC, you cannot put a bull key or value in a ConcurrentMap.
> 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)
[jira] [Commented] (JEXL-256) Jexl should not try to resolve a variable from Context when Context.has() returns false
[
https://issues.apache.org/jira/browse/JEXL-256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16429319#comment-16429319
]
Henri Biestro commented on JEXL-256:
Side comment, ConcurrentHashMap throws NPE for null key on get .
Anyway, the current behavior is inspired by handling maps that allow null
values; on get, if the result is null, the only way to know why is to call
containsKey. And this disambiguation is not needed in the general most frequent
case. Since determining if a key exists is as costly as trying to get the
value, the runtime cost / disputable functional gain seems unwarranted.
If you’ve noticed odd behaviors wrt antish vars, a test case would be much
appreciated.
> 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)
[jira] [Commented] (JEXL-256) Jexl should not try to resolve a variable from Context when Context.has() returns false
[
https://issues.apache.org/jira/browse/JEXL-256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16428367#comment-16428367
]
Dmitri Blinov commented on JEXL-256:
I understand your logic, but I thought that if JexlContext has defined two
methods for variables, like {{get()}} and {{has()}}, that would mean if we
check for a variable, we should first call {{has()}} and only if it returns
{{true}} we should call {{get()}}. Your explanation has your reason, that we
optimistically try to {{get()}} first and if {{null}} has returned, we use
{{has()}} to check if there is the variable missing or its value is {{nul}}l. I
have implemented the context {{get()}} method to log an error message if there
is an attempt to address the variable that does not exist within context. And
as wrote previously, I noticed that there are log messages about missing
context variables which names are parts of a longer antish style variable
names. The idea is to check for {{has()}} first, so that developers writing
their own context implementation could rely on {{has()}} as a guarding method.
I think this is similar to {{ConcurrentHashMap}} implementation where {{get()}}
method can throw NPE if there is no key, so we should rely on {{containsKey()}}
before we actually try to get anything from the map
> 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)
[jira] [Commented] (JEXL-256) Jexl should not try to resolve a variable from Context when Context.has() returns false
[
https://issues.apache.org/jira/browse/JEXL-256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16428226#comment-16428226
]
Henri Biestro commented on JEXL-256:
I must be missing the point; the current code only checks 'has' if the 'get'
returns null. This allows having only 1 call to 'get' in the general (non null)
case. If your context returns a non-null value to a 'get' when it returns false
for the same 'has' call, I'm tempted to say embed the logic in your context
implementation...And we probably should document that there is an expectation
that 'has' can not return false when 'get' does not return null.
I'm obviously not understanding your case correctly... :-/
> 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)
