Fail consumer creation when the remote will not apply the selector - #75
Open
ansd wants to merge 1 commit into
Open
Fail consumer creation when the remote will not apply the selector#75ansd wants to merge 1 commit into
ansd wants to merge 1 commit into
Conversation
A consumer created with a message selector silently behaved as if it had no selector at all when the remote declined to apply the requested filter. Every message on the destination was handed to the application, including the ones the selector was supposed to exclude. AMQP 1.0 negotiates a filter in both directions. Section 3.5.3 defines the source filter field as: > The receiving endpoint sets its desired filter, the sending endpoint sets the filter actually in place (including any filters defaulted at the node). The receiving endpoint MUST check that the filter in place meets its needs and take responsibility for detaching if it does not. Section 3.5.1 adds that a filter is a "restriction ... to only deliver a subset of messages" and that the values in the filter-set are the filters the sending endpoint has applied. A peer that will not honour a filter therefore leaves it out of the filter-set it sends in the attach response, and that response is the only way a client learns which filters took effect. We set the jms-selector filter on the source of the attach we send, but never looked at what came back, so we could not tell a peer that applies the selector from one that declines it. This matters wherever a peer supports selectors on some destinations and not others: RabbitMQ, for instance, applies a selector on its JMS queue type but not on classic queues, and correctly omits the filter from the attach response in the latter case. Against such a destination a JMS application asking for "myProp = 'foo'" received everything. Silently widening a selector is a correctness fault: the application cannot detect it, and Jakarta Messaging gives it no reason to expect it. Check the attach response instead, and if the requested selector filter is not in the filter-set actually in place, abort the consumer with a ProviderUnsupportedOperationException, detaching a durable subscription rather than closing it so an existing subscription is not destroyed on the way out. This mirrors how the shared-subscription link capability is already validated in afterOpened(). Tests cover both shapes a declining peer can send: no filter section at all, and an empty filter-set, which is what a broker produces when it strips a filter it will not apply from the set it echoes back. TestAmqpPeer gains a public expectReceiverAttachWithResponseSource so a test can control the filters the peer reports; by default the peer still echoes the source it received, so existing selector tests are unaffected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A consumer created with a message selector silently behaved as if it had no selector at all when the remote declined to apply the requested filter. Every message on the destination was handed to the application, including the ones the selector was supposed to exclude.
AMQP 1.0 negotiates a filter in both directions. Section 3.5.3 defines the source filter field as:
Section 3.5.1 adds that a filter is a "restriction ... to only deliver a subset of messages" and that the values in the filter-set are the filters the sending endpoint has applied. A peer that will not honour a filter therefore leaves it out of the filter-set it sends in the attach response, and that response is the only way a client learns which filters took effect.
We set the jms-selector filter on the source of the attach we send, but never looked at what came back, so we could not tell a peer that applies the selector from one that declines it. This matters wherever a peer supports selectors on some destinations and not others: RabbitMQ, for instance, applies a selector on its JMS queue type but not on classic queues, and correctly omits the filter from the attach response in the latter case. Against such a destination a JMS application asking for "myProp = 'foo'" received everything.
Silently widening a selector is a correctness fault: the application cannot detect it, and Jakarta Messaging gives it no reason to expect it. Check the attach response instead, and if the requested selector filter is not in the filter-set actually in place, abort the consumer with a ProviderUnsupportedOperationException, detaching a durable subscription rather than closing it so an existing subscription is not destroyed on the way out. This mirrors how the shared-subscription link capability is already validated in afterOpened().
Tests cover both shapes a declining peer can send: no filter section at all, and an empty filter-set, which is what a broker produces when it strips a filter it will not apply from the set it echoes back. TestAmqpPeer gains a public expectReceiverAttachWithResponseSource so a test can control the filters the peer reports; by default the peer still echoes the source it received, so existing selector tests are unaffected.