[jira] [Commented] (AMQ-5281) Incorrect handling of unknown values in selectors

["Grigroy Sobko (JIRA)" <ji...@apache.org>]

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

Grigroy Sobko commented on AMQ-5281:
------------------------------------

The is caused by org.apache.activemq.filter.LogicExpression, in this lines:

{code}
    public static BooleanExpression createAND(BooleanExpression lvalue, BooleanExpression rvalue) {
        return new LogicExpression(lvalue, rvalue) {

            public Object evaluate(MessageEvaluationContext message) throws JMSException {

                Boolean lv = (Boolean)left.evaluate(message);

               // Can we do an AND shortcut??
                if (lv == null) {
                    return null;
                }
                if (!lv.booleanValue()) {
                    return Boolean.FALSE;
                }

                Boolean rv = (Boolean)right.evaluate(message);
                return rv == null ? null : rv;
            }

            public String getExpressionSymbol() {
                return "AND";
            }
        };
    }
{code}

You see comment "//Can we do an AND shortcut??"?
That's the cause of that bug.

When left value is unknown method returns null. That's incorrect. You should check if right value is false and return false in that case.

> Incorrect handling of unknown values in selectors
> -------------------------------------------------
>
>                 Key: AMQ-5281
>                 URL: https://issues.apache.org/jira/browse/AMQ-5281
>             Project: ActiveMQ
>          Issue Type: Bug
>    Affects Versions: 5.8.0, 5.x
>            Reporter: Grigroy Sobko
>
> Due to JmsMessage specification :
> http://docs.oracle.com/javaee/1.4/api/javax/jms/Message.html
> There are rules how unknown Values evaluates. 
> There how AND operator should handle unknown:
> - UNKNOWN AND FALSE => FALSE 
> - FALSE AND UNKNOWN => FALSE 
> And that's how it is handled in ActiveMQ:
> - UNKNOWN AND FALSE => UNKNOWN (!!!)
> - FALSE AND UNKNOWN => FALSE 
> I've wrote test to reproduce this:
> {code}
> package org.activemq.test;
> import org.apache.activemq.command.ActiveMQMessage;
> import org.apache.activemq.command.ActiveMQTopic;
> import org.apache.activemq.filter.BooleanExpression;
> import org.apache.activemq.filter.MessageEvaluationContext;
> import org.apache.activemq.selector.SelectorParser;
> import org.junit.Before;
> import org.junit.Test;
> import javax.jms.JMSException;
> import javax.jms.Message;
> import static org.junit.Assert.assertEquals;
> import static org.junit.Assert.assertTrue;
> public class SelectorUnknownHandlingTest {
>     private Message message;
>     @Before
>     public void setUp() throws Exception {
>         message = createMessage();
>     }
>     @Test
>     public void testUnknown() throws Exception {
>         // Some unset property with gt operator => unknown
>         assertSelectorEvaluatesToUnknown(message, "(unknownProp > 0)");
>     }
>     @Test
>     public void testUnknownAndFalse() throws Exception {
>         // false and unknown => false
>         assertSelectorEvaluatesToFalse(message, "(falseProp AND unknownProp > 0)");
>         // THIS ASSERTION FAILS !! IT EVALUATES TO UNKNOWN INSTEAD
>         // unknown and false => false
>         assertSelectorEvaluatesToFalse(message, "(unknownProp > 0 AND falseProp)");
>     }
>     @Test
>     public void testUnknownOrTrue() throws Exception {
>         // unknown or true => true
>         assertSelectorEvaluatesToTrue(message, "(unknownProp > 0 OR trueProp)");
>         // true or unknown => true
>         assertSelectorEvaluatesToTrue(message, "(trueProp OR unknownProp > 0)");
>     }
>     private void assertSelectorEvaluatesToUnknown(Message message, String selector) throws JMSException {
>         assertSelector(message, selector, false);
>         assertSelector(message, "not(" + selector + ")", false);
>     }
>     private void assertSelectorEvaluatesToFalse(Message message, String selector) throws JMSException {
>         assertSelector(message, selector, false);
>         assertSelector(message, "not(" + selector + ")", true);
>     }
>     private void assertSelectorEvaluatesToTrue(Message message, String selector) throws JMSException {
>         assertSelector(message, selector, true);
>         assertSelector(message, "not(" + selector + ")", false);
>     }
>     protected Message createMessage() throws JMSException {
>         Message message = createMessage("FOO.BAR");
>         message.setJMSType("selector-test");
>         message.setJMSMessageID("connection:1:1:1:1");
>         message.setBooleanProperty("trueProp", true);
>         message.setBooleanProperty("falseProp", false);
>         return message;
>     }
>     protected void assertSelector(Message message, String text, boolean expected) throws JMSException {
>         BooleanExpression selector = SelectorParser.parse(text);
>         assertTrue("Created a valid selector", selector != null);
>         MessageEvaluationContext context = new MessageEvaluationContext();
>         context.setMessageReference((org.apache.activemq.command.Message)message);
>         boolean value = selector.matches(context);
>         assertEquals("Selector for: " + text, expected, value);
>     }
>     protected Message createMessage(String subject) throws JMSException {
>         ActiveMQMessage message = new ActiveMQMessage();
>         message.setJMSDestination(new ActiveMQTopic(subject));
>         return message;
>     }
> }
> {code}



--
This message was sent by Atlassian JIRA
(v6.2#6252)