-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18423/#review35450
-----------------------------------------------------------
A simplification could be:
* Keep changes to QueueFactory.cpp, .h
* In Broker.cpp change .isSet to tests for zero, don't use .value
* Leave QueueSettings.cpp, .h unchanged
Logically the new code would do several things:
* Only send MAXPAGES and MAXPAGEFACTOR to ACL when paging is true.
* Only send MAXQUEUECOUNT to ACL if max_count is specified.
* Only send MAXFILECOUNT and MAXFILESIZE to ACL if durable is true.
* For MAXPAGES, MAXPAGEFACTOR, MAXQUEUESIZE, MAXQUEUECOUNT never send
0 to ACL, rather send the queue creation default values.
ACL rule writers need to know:
* Rules specifying filemax{size|count}{lower|upper}limit will only match
when client sets durable=true.
* Rules specifying {pages|pagefactor}{lower|upper}limit will only match
when client sets paging=true.
* Rules specifying queuemaxcount{upper|lower}limit will only match when
client sets qpid.max_count option.
- Chug Rolke
On Feb. 24, 2014, 2:11 p.m., Pavel Moravec wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18423/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2014, 2:11 p.m.)
>
>
> Review request for qpid, Chug Rolke and Gordon Sim.
>
>
> Bugs: QPID-5561 and QPID-5565
> https://issues.apache.org/jira/browse/QPID-5561
> https://issues.apache.org/jira/browse/QPID-5565
>
>
> Repository: qpid
>
>
> Description
> -------
>
> This patch fixes both:
>
> QPID-5561: C++ Broker queue creation ACL checks against wrong limit values
> QPID-5565: [C++ broker] ACL queue create rules to take zero as unlimited value
>
> The patch:
> - passes to ACL persistency-related / paging queue options only if the queue
> is marked as durable / with paging
> - fills proper default values
>
>
> Potential issues:
> - I had to copy constant value "static const u_int32_t defJrnlFileSizePgs =
> 24" and also "defNumJrnlFiles = 8" as there is no simple way (known to me) to
> test if legacy store module is loaded
> - anyway as legacy store is not supposed to be changed (due to
> linearstore), I see this a acceptable
> - ACL line like:
>
> acl allow all create queue filemaxcountupperlimit=32
>
> is _not_ matched for a transient queue. That differs from current behaviour,
> anyway I see it as an improvement rather (as is, the above ACL is meaningless
> for transient queues that dont have filecount limit applicable)
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1571198
> /trunk/qpid/cpp/src/qpid/broker/QueueFactory.h 1571198
> /trunk/qpid/cpp/src/qpid/broker/QueueFactory.cpp 1571198
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1571198
> /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1571198
> /trunk/qpid/cpp/src/tests/acl.py 1571198
>
> Diff: https://reviews.apache.org/r/18423/diff/
>
>
> Testing
> -------
>
> - different variants of use cases (esp. from the JIRAs) passed
> - automated tests passed
>
>
> Thanks,
>
> Pavel Moravec
>
>
