You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Chug Rolke <cr...@redhat.com> on 2012/07/10 17:56:33 UTC
Re: Review Request: Acl publish exchange fastpath lookup uses TopicExchange
binding string match rules
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5836/
-----------------------------------------------------------
(Updated July 10, 2012, 3:56 p.m.)
Review request for qpid, Alan Conway, Gordon Sim, and Ted Ross.
Changes
-------
Adding more reviewers.
Description
-------
Acl match rules with single '*' at end of a string are inadequate for expressing topic exchange key matches. This patch uses the actual topic exchange match logic for matching the Acl rule against a user's run-time publish request.
Note that each Acl rule contains a topic exchange node tree with only one key in it. The topic exchange match then returns match or no-match for a lookup on that rule. Although the topic exchange node trees may have many nodes in them, Acl logic uses one tree-with-one-key per rule so that allow and deny rules and broad and narrow key specifications may be intermixed in the Acl rule file and still produce correct matches.
This patch also improves a run-time issue by parsing the 'publish exchange' rules' property list once at rule-creation time. The required routing key and exchange name are pulled out of the property list and placed as members of the rule. When run-time publish authorizations are performed the n the lookup code uses these members directly.
This addresses bug QPID-3892.
https://issues.apache.org/jira/browse/QPID-3892
Diffs
-----
trunk/qpid/cpp/src/qpid/acl/AclData.h 1359193
trunk/qpid/cpp/src/qpid/acl/AclData.cpp 1359193
trunk/qpid/cpp/src/qpid/acl/AclReader.h 1359193
trunk/qpid/cpp/src/qpid/acl/AclReader.cpp 1359193
trunk/qpid/cpp/src/qpid/acl/AclTopicMatch.h PRE-CREATION
trunk/qpid/cpp/src/tests/acl.py 1359193
Diff: https://reviews.apache.org/r/5836/diff/
Testing
-------
The topic key node request self test patterns are replicated through Acl files and the same tests are run.
New tests are added to check that Acl user '*' works with the same results as named users.
Acl tests do not include multiple-node-per-tree tests since that's not how Acl code uses the node trees.
Thanks,
Chug Rolke
Re: Review Request: Acl publish exchange fastpath lookup uses TopicExchange
binding string match rules
Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5836/#review9057
-----------------------------------------------------------
trunk/qpid/cpp/src/qpid/acl/AclData.h
<https://reviews.apache.org/r/5836/#comment19204>
Type names should be capitalized. I realize the names weren't introduced by this change but would be nice to clean it up.
trunk/qpid/cpp/src/qpid/acl/AclData.h
<https://reviews.apache.org/r/5836/#comment19205>
Why so many public data members?
trunk/qpid/cpp/src/qpid/acl/AclData.cpp
<https://reviews.apache.org/r/5836/#comment19206>
I'm not familiar with the logic here so can't really comment on correcness.
- Alan Conway
On July 10, 2012, 3:56 p.m., Chug Rolke wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5836/
> -----------------------------------------------------------
>
> (Updated July 10, 2012, 3:56 p.m.)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, and Ted Ross.
>
>
> Description
> -------
>
> Acl match rules with single '*' at end of a string are inadequate for expressing topic exchange key matches. This patch uses the actual topic exchange match logic for matching the Acl rule against a user's run-time publish request.
>
> Note that each Acl rule contains a topic exchange node tree with only one key in it. The topic exchange match then returns match or no-match for a lookup on that rule. Although the topic exchange node trees may have many nodes in them, Acl logic uses one tree-with-one-key per rule so that allow and deny rules and broad and narrow key specifications may be intermixed in the Acl rule file and still produce correct matches.
>
> This patch also improves a run-time issue by parsing the 'publish exchange' rules' property list once at rule-creation time. The required routing key and exchange name are pulled out of the property list and placed as members of the rule. When run-time publish authorizations are performed the n the lookup code uses these members directly.
>
>
> This addresses bug QPID-3892.
> https://issues.apache.org/jira/browse/QPID-3892
>
>
> Diffs
> -----
>
> trunk/qpid/cpp/src/qpid/acl/AclData.h 1359193
> trunk/qpid/cpp/src/qpid/acl/AclData.cpp 1359193
> trunk/qpid/cpp/src/qpid/acl/AclReader.h 1359193
> trunk/qpid/cpp/src/qpid/acl/AclReader.cpp 1359193
> trunk/qpid/cpp/src/qpid/acl/AclTopicMatch.h PRE-CREATION
> trunk/qpid/cpp/src/tests/acl.py 1359193
>
> Diff: https://reviews.apache.org/r/5836/diff/
>
>
> Testing
> -------
>
> The topic key node request self test patterns are replicated through Acl files and the same tests are run.
> New tests are added to check that Acl user '*' works with the same results as named users.
> Acl tests do not include multiple-node-per-tree tests since that's not how Acl code uses the node trees.
>
>
> Thanks,
>
> Chug Rolke
>
>