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

Gordon Sim commented on QPID-4969:
----------------------------------

This also breaks the unit tests, though in that case it is the test itself that 
is really at fault. Since unbind works only on the binding key (not the args), 
I think it is generally good to disallow different bindings to the same queue 
with the same key. Any use case that relied on *different* args and the same 
key would I think already be subtly (sometimes not so subtly!) broken.

The issue with the change as it stands is I think simply that a bind is no 
longer idempotent, i.e. you should not prevent the same binding being requested 
(you don't actually need to do anything for it though, since it is already in 
place). So the thrid command in the reproducer above _should_ fail, but just 
reissuing the second command should not.

Suggested change:

{noformat}
Index: src/qpid/broker/HeadersExchange.cpp
===================================================================
--- src/qpid/broker/HeadersExchange.cpp (revision 1498354)
+++ src/qpid/broker/HeadersExchange.cpp (working copy)
@@ -200,8 +200,10 @@
 
         Bindings::ConstPtr p = bindings.snapshot();
         if (p.get()) {
+            MatchArgs matchArgs(queue, &extra_args);
+            MatchKey matchKey(queue, bindingKey);
             for (std::vector<BoundKey>::const_iterator i = p->begin(); i != 
p->end(); ++i) {
-                if (queue == i->binding->queue && bindingKey == 
i->binding->key) {
+                if (matchKey(*i) && matchArgs(*i)) {
                     throw InternalErrorException(QPID_MSG("Exchange: " << 
getName()
                         << ", binding key: " << bindingKey
                         << " Duplicate binding key not allowed." ));
@@ -372,7 +374,7 @@
 //---------
 HeadersExchange::MatchArgs::MatchArgs(Queue::shared_ptr q, const 
qpid::framing::FieldTable* a) : queue(q), args(a) {}
 
-bool HeadersExchange::MatchArgs::operator()(BoundKey & bk)
+bool HeadersExchange::MatchArgs::operator()(const BoundKey & bk)
 {
     return bk.binding->queue == queue && bk.binding->args == *args;
 }
@@ -380,7 +382,7 @@
 //---------
 HeadersExchange::MatchKey::MatchKey(Queue::shared_ptr q, const std::string& k) 
: queue(q), key(k) {}
 
-bool HeadersExchange::MatchKey::operator()(BoundKey & bk)
+bool HeadersExchange::MatchKey::operator()(const BoundKey & bk)
 {
     return bk.binding->queue == queue && bk.binding->key == key;
 }
Index: src/qpid/broker/HeadersExchange.h
===================================================================
--- src/qpid/broker/HeadersExchange.h   (revision 1498354)
+++ src/qpid/broker/HeadersExchange.h   (working copy)
@@ -48,7 +48,7 @@
         const Queue::shared_ptr queue;        
         const qpid::framing::FieldTable* args;
         MatchArgs(Queue::shared_ptr q, const qpid::framing::FieldTable* a);
-        bool operator()(BoundKey & bk);
+        bool operator()(const BoundKey & bk);
     };
     
     struct MatchKey
@@ -56,7 +56,7 @@
         const Queue::shared_ptr queue;
         const std::string& key;
         MatchKey(Queue::shared_ptr q, const std::string& k);
-        bool operator()(BoundKey & bk);
+        bool operator()(const BoundKey & bk);
     };
 
     struct FedUnbindModifier
{noformat}


                
> C++ Broker headers exchange allows creation of bindings with duplicate keys
> ---------------------------------------------------------------------------
>
>                 Key: QPID-4969
>                 URL: https://issues.apache.org/jira/browse/QPID-4969
>             Project: Qpid
>          Issue Type: Bug
>          Components: C++ Broker
>    Affects Versions: 0.22
>            Reporter: Chuck Rolke
>            Assignee: Chuck Rolke
>             Fix For: 0.23
>
>
> The test case:
> {code}
> qpid-config add queue MyQueue --durable
> qpid-config bind amq.match MyQueue SomeKey any property1=value1
> qpid-config bind amq.match MyQueue SomeKey all property1=value1
> {code}
> Causes a management error as two bindings are created with 
> amq.match,MyQueue,SomeKey managementId.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org

Reply via email to