This is an automated email from the ASF dual-hosted git repository. robbie pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/qpid-jms.git
The following commit(s) were added to refs/heads/master by this push: new 0cf96df QPIDJMS-522: add a 'jms.validateSelector' URI option to toggle the local validation of message selector strings 0cf96df is described below commit 0cf96dfb9de4b5ea4ebc204b0a0cd650ee065adf Author: Robbie Gemmell <rob...@apache.org> AuthorDate: Thu Dec 10 12:13:05 2020 +0000 QPIDJMS-522: add a 'jms.validateSelector' URI option to toggle the local validation of message selector strings --- .../java/org/apache/qpid/jms/JmsConnection.java | 8 +++ .../org/apache/qpid/jms/JmsConnectionFactory.java | 15 ++++ .../main/java/org/apache/qpid/jms/JmsSession.java | 27 +++---- .../apache/qpid/jms/meta/JmsConnectionInfo.java | 10 +++ .../jms/integration/SessionIntegrationTest.java | 84 +++++++++++++--------- .../qpid/jms/meta/JmsConnectionInfoTest.java | 3 + qpid-jms-docs/Configuration.md | 1 + 7 files changed, 103 insertions(+), 45 deletions(-) diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnection.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnection.java index 27a5f66..ad75e6b 100644 --- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnection.java +++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnection.java @@ -993,6 +993,14 @@ public class JmsConnection implements AutoCloseable, Connection, TopicConnection connectionInfo.setValidatePropertyNames(validatePropertyNames); } + public boolean isValidateSelector() { + return connectionInfo.isValidateSelector(); + } + + public void setValidateSelector(boolean validateSelector) { + connectionInfo.setValidateSelector(validateSelector); + } + public JmsPrefetchPolicy getPrefetchPolicy() { return connectionInfo.getPrefetchPolicy(); } diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnectionFactory.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnectionFactory.java index 1bf65d9..c05da14 100644 --- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnectionFactory.java +++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnectionFactory.java @@ -95,6 +95,7 @@ public class JmsConnectionFactory extends JNDIStorable implements ConnectionFact private String queuePrefix = null; private String topicPrefix = null; private boolean validatePropertyNames = true; + private boolean validateSelector = true; private boolean awaitClientID = true; private boolean useDaemonThread = false; private long sendTimeout = JmsConnectionInfo.DEFAULT_SEND_TIMEOUT; @@ -543,6 +544,20 @@ public class JmsConnectionFactory extends JNDIStorable implements ConnectionFact this.validatePropertyNames = validatePropertyNames; } + public boolean isValidateSelector() { + return validateSelector; + } + + /** + * Sets whether local validation is performed of a consumers message selector string + * conforming to the JMS selector syntax. Default is true. + * + * @param validateSelector whether to validate consumer message selector strings + */ + public void setValidateSelector(boolean validateSelector) { + this.validateSelector = validateSelector; + } + /** * Gets the currently set close timeout. * diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsSession.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsSession.java index d9c2c42..dc4b303 100644 --- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsSession.java +++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsSession.java @@ -474,7 +474,7 @@ public class JmsSession implements AutoCloseable, Session, QueueSession, TopicSe public MessageConsumer createConsumer(Destination destination, String messageSelector, boolean noLocal) throws JMSException { checkClosed(); checkDestination(destination); - messageSelector = checkSelector(messageSelector); + messageSelector = checkSelector(messageSelector, connection.isValidateSelector()); JmsDestination dest = JmsMessageTransformation.transformDestination(connection, destination); JmsMessageConsumer result = new JmsMessageConsumer(getNextConsumerId(), this, dest, messageSelector, noLocal); result.init(); @@ -501,7 +501,7 @@ public class JmsSession implements AutoCloseable, Session, QueueSession, TopicSe public QueueReceiver createReceiver(Queue queue, String messageSelector) throws JMSException { checkClosed(); checkDestination(queue); - messageSelector = checkSelector(messageSelector); + messageSelector = checkSelector(messageSelector, connection.isValidateSelector()); JmsDestination dest = JmsMessageTransformation.transformDestination(connection, queue); JmsQueueReceiver result = new JmsQueueReceiver(getNextConsumerId(), this, dest, messageSelector); result.init(); @@ -523,7 +523,7 @@ public class JmsSession implements AutoCloseable, Session, QueueSession, TopicSe public QueueBrowser createBrowser(Queue destination, String messageSelector) throws JMSException { checkClosed(); checkDestination(destination); - messageSelector = checkSelector(messageSelector); + messageSelector = checkSelector(messageSelector, connection.isValidateSelector()); JmsDestination dest = JmsMessageTransformation.transformDestination(connection, destination); JmsQueueBrowser result = new JmsQueueBrowser(this, dest, messageSelector); return result; @@ -544,7 +544,7 @@ public class JmsSession implements AutoCloseable, Session, QueueSession, TopicSe public TopicSubscriber createSubscriber(Topic topic, String messageSelector, boolean noLocal) throws JMSException { checkClosed(); checkDestination(topic); - messageSelector = checkSelector(messageSelector); + messageSelector = checkSelector(messageSelector, connection.isValidateSelector()); JmsDestination dest = JmsMessageTransformation.transformDestination(connection, topic); JmsTopicSubscriber result = new JmsTopicSubscriber(getNextConsumerId(), this, dest, noLocal, messageSelector); result.init(); @@ -567,7 +567,7 @@ public class JmsSession implements AutoCloseable, Session, QueueSession, TopicSe checkClosed(); checkDestination(topic); checkClientIDWasSetExplicitly(); - messageSelector = checkSelector(messageSelector); + messageSelector = checkSelector(messageSelector, connection.isValidateSelector()); JmsDestination dest = JmsMessageTransformation.transformDestination(connection, topic); JmsTopicSubscriber result = new JmsDurableTopicSubscriber(getNextConsumerId(), this, dest, name, noLocal, messageSelector); result.init(); @@ -621,7 +621,7 @@ public class JmsSession implements AutoCloseable, Session, QueueSession, TopicSe public MessageConsumer createSharedConsumer(Topic topic, String name, String selector) throws JMSException { checkClosed(); checkDestination(topic); - selector = checkSelector(selector); + selector = checkSelector(selector, connection.isValidateSelector()); JmsDestination dest = JmsMessageTransformation.transformDestination(connection, topic); JmsMessageConsumer result = new JmsSharedMessageConsumer(getNextConsumerId(), this, dest, name, selector); result.init(); @@ -644,7 +644,7 @@ public class JmsSession implements AutoCloseable, Session, QueueSession, TopicSe public MessageConsumer createSharedDurableConsumer(Topic topic, String name, String selector) throws JMSException { checkClosed(); checkDestination(topic); - selector = checkSelector(selector); + selector = checkSelector(selector, connection.isValidateSelector()); JmsDestination dest = JmsMessageTransformation.transformDestination(connection, topic); JmsMessageConsumer result = new JmsSharedDurableMessageConsumer(getNextConsumerId(), this, dest, name, selector); result.init(); @@ -1109,18 +1109,21 @@ public class JmsSession implements AutoCloseable, Session, QueueSession, TopicSe } } - static String checkSelector(String selector) throws InvalidSelectorException { + static String checkSelector(String selector, boolean validate) throws InvalidSelectorException { if (selector != null) { if (selector.trim().length() == 0) { return null; } - try { - SelectorParser.parse(selector); - } catch (FilterException e) { - throw new InvalidSelectorException(e.getMessage()); + if (validate) { + try { + SelectorParser.parse(selector); + } catch (FilterException e) { + throw new InvalidSelectorException(e.getMessage()); + } } } + return selector; } diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/meta/JmsConnectionInfo.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/meta/JmsConnectionInfo.java index 6b08fc3..0dc0dc7 100644 --- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/meta/JmsConnectionInfo.java +++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/meta/JmsConnectionInfo.java @@ -65,6 +65,7 @@ public final class JmsConnectionInfo extends JmsAbstractResource implements Comp private boolean forceSyncSend; private boolean forceAsyncAcks; private boolean validatePropertyNames = true; + private boolean validateSelector = true; private boolean receiveLocalOnly; private boolean receiveNoWaitLocalOnly; private boolean localMessagePriority; @@ -118,6 +119,7 @@ public final class JmsConnectionInfo extends JmsAbstractResource implements Comp copy.topicPrefix = topicPrefix; copy.connectTimeout = connectTimeout; copy.validatePropertyNames = validatePropertyNames; + copy.validateSelector = validateSelector; copy.useDaemonThread = useDaemonThread; copy.closeLinksThatFailOnReconnect = closeLinksThatFailOnReconnect; copy.messageIDPolicy = getMessageIDPolicy().copy(); @@ -217,6 +219,14 @@ public final class JmsConnectionInfo extends JmsAbstractResource implements Comp this.validatePropertyNames = validatePropertyNames; } + public boolean isValidateSelector() { + return validateSelector; + } + + public void setValidateSelector(boolean validateSelector) { + this.validateSelector = validateSelector; + } + public long getCloseTimeout() { return closeTimeout; } diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/SessionIntegrationTest.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/SessionIntegrationTest.java index 4222a9a..a69624e 100644 --- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/SessionIntegrationTest.java +++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/SessionIntegrationTest.java @@ -242,9 +242,12 @@ public class SessionIntegrationTest extends QpidJmsTestCase { Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); - testPeer.expectReceiverAttach(); + SourceMatcher sourceMatcher = new SourceMatcher(); + sourceMatcher.withFilter(nullValue()); + + testPeer.expectReceiverAttach(notNullValue(), sourceMatcher); testPeer.expectLinkFlow(); - testPeer.expectReceiverAttach(); + testPeer.expectReceiverAttach(notNullValue(), sourceMatcher); testPeer.expectLinkFlow(); testPeer.expectClose(); @@ -270,9 +273,12 @@ public class SessionIntegrationTest extends QpidJmsTestCase { Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); - testPeer.expectReceiverAttach(); + SourceMatcher sourceMatcher = new SourceMatcher(); + sourceMatcher.withFilter(nullValue()); + + testPeer.expectReceiverAttach(notNullValue(), sourceMatcher); testPeer.expectLinkFlow(); - testPeer.expectReceiverAttach(); + testPeer.expectReceiverAttach(notNullValue(), sourceMatcher); testPeer.expectLinkFlow(); testPeer.expectClose(); @@ -289,18 +295,55 @@ public class SessionIntegrationTest extends QpidJmsTestCase { } @Test(timeout = 20000) + public void testCreateConsumerWithInvalidSelector() throws Exception { + try (TestAmqpPeer testPeer = new TestAmqpPeer();) { + Connection connection = testFixture.establishConnecton(testPeer); + connection.start(); + + testPeer.expectBegin(); + Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); + + Topic destination = session.createTopic(getTestName()); + + try { + session.createConsumer(destination, "3+5"); + fail("Should have thrown a invalid selector exception"); + } catch (InvalidSelectorException jmsse) { + // Expected + } + + testPeer.expectClose(); + connection.close(); + + testPeer.waitForAllHandlersToComplete(1000); + } + } + + @Test(timeout = 20000) public void testCreateConsumerWithSimpleSelector() throws Exception { - doCreateConsumerWithSelectorTestImpl("myvar=42"); + doCreateConsumerWithSelectorTestImpl("myvar=42", false); } @Test(timeout = 20000) public void testCreateConsumerWithQuotedVariableSelector() throws Exception { - doCreateConsumerWithSelectorTestImpl("\"my.quoted-var\"='some-value'"); + doCreateConsumerWithSelectorTestImpl("\"my.quoted-var\"='some-value'", false); + } + + @Test(timeout = 20000) + public void testCreateConsumerWithInvalidSelectorAndDisableValidation() throws Exception { + // Verifies that with the local validation disabled, the selector filter is still created + // and sent on the source terminus, containing the desired non-JMS selector string. + doCreateConsumerWithSelectorTestImpl("my.invalid-var > 'string-value'", true); } - private void doCreateConsumerWithSelectorTestImpl(String messageSelector) throws Exception { + private void doCreateConsumerWithSelectorTestImpl(String messageSelector, boolean disableValidation) throws Exception { try (TestAmqpPeer testPeer = new TestAmqpPeer();) { - Connection connection = testFixture.establishConnecton(testPeer); + String options = null; + if(disableValidation) { + options = "jms.validateSelector=false"; + } + + Connection connection = testFixture.establishConnecton(testPeer, options); connection.start(); testPeer.expectBegin(); @@ -1002,31 +1045,6 @@ public class SessionIntegrationTest extends QpidJmsTestCase { } @Test(timeout = 20000) - public void testInvalidSelector() throws Exception { - try (TestAmqpPeer testPeer = new TestAmqpPeer();) { - Connection connection = testFixture.establishConnecton(testPeer); - connection.start(); - - testPeer.expectBegin(); - Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); - - String topicName = "myTopic"; - Topic destination = session.createTopic(topicName); - - try { - session.createConsumer(destination, "3+5"); - fail("Should have thrown a invalid selector exception"); - } catch (InvalidSelectorException jmsse) { - } - - testPeer.expectClose(); - connection.close(); - - testPeer.waitForAllHandlersToComplete(1000); - } - } - - @Test(timeout = 20000) public void testCreateProducerTargetContainsQueueCapability() throws Exception { doCreateProducerTargetContainsCapabilityTestImpl(Queue.class); } diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/meta/JmsConnectionInfoTest.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/meta/JmsConnectionInfoTest.java index d39fca2..2baf223 100644 --- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/meta/JmsConnectionInfoTest.java +++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/meta/JmsConnectionInfoTest.java @@ -80,6 +80,8 @@ public class JmsConnectionInfoTest { info.setUsername("user"); boolean validatePropertyNames = !info.isValidatePropertyNames(); info.setValidatePropertyNames(validatePropertyNames); + boolean validateSelector = !info.isValidateSelector(); + info.setValidateSelector(validateSelector); boolean awaitClientID = !info.isAwaitClientID(); info.setAwaitClientID(awaitClientID); info.setMessageIDPolicy(new JmsDefaultMessageIDPolicy()); @@ -103,6 +105,7 @@ public class JmsConnectionInfoTest { assertEquals("topic", copy.getTopicPrefix()); assertEquals("user", copy.getUsername()); assertEquals(validatePropertyNames, copy.isValidatePropertyNames()); + assertEquals(validateSelector, copy.isValidateSelector()); assertNotSame(info.getPrefetchPolicy(), copy.getPrefetchPolicy()); assertNotSame(info.getPresettlePolicy(), copy.getPresettlePolicy()); diff --git a/qpid-jms-docs/Configuration.md b/qpid-jms-docs/Configuration.md index 3cc6830..7b31b93 100644 --- a/qpid-jms-docs/Configuration.md +++ b/qpid-jms-docs/Configuration.md @@ -91,6 +91,7 @@ The options apply to the behaviour of the JMS objects such as Connection, Sessio + **jms.localMessageExpiry** Controls whether MessageConsumer instances will locally filter expired Messages or deliver them. By default this value is set to true and expired messages will be filtered. + **jms.localMessagePriority** If enabled prefetched messages are reordered locally based on their given Message priority value. Default is false. + **jms.validatePropertyNames** If message property names should be validated as valid Java identifiers. Default is true. ++ **jms.validateSelector** Controls whether local validation is performed on consumer message selector strings. Default is true. + **jms.receiveLocalOnly** If enabled receive calls with a timeout will only check a consumers local message buffer, otherwise the remote peer is checked to ensure there are really no messages available if the local timeout expires before a message arrives. Default is false, the remote is checked. + **jms.receiveNoWaitLocalOnly** If enabled receiveNoWait calls will only check a consumers local message buffer, otherwise the remote peer is checked to ensure there are really no messages available. Default is false, the remote is checked. + **jms.queuePrefix** Optional prefix value added to the name of any Queue created from a JMS Session. --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org For additional commands, e-mail: commits-h...@qpid.apache.org