Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mod_privacy: honor the 'order' attribute #2530

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

crosser
Copy link

@crosser crosser commented Jul 15, 2018

When checking a packet against the active privacy list, make sure that
the items in the list are sorted according to the 'order' attribute.

Bug-url: #2529
Signed-off-by: Eugene Crosser [email protected]

@zinid
Copy link
Contributor

zinid commented Jul 15, 2018

No, this is not efficient. The list should be sorted during iq-set.

@crosser
Copy link
Author

crosser commented Jul 15, 2018

Then you would need to guarantee that the backend preserves the order. Is there such guarantee, including possible future backends?

In any case, my concern is the bug #2529 . If it is fixed in some other way, I'll be perfectly happy.

@crosser
Copy link
Author

crosser commented Jul 16, 2018

@zinid , I think that I can make ordering O(N) and still used at application-time. Will that be acceptable? If so, I'll resubmit this PR.

(I think pre-sorting is not a good design. Imagine that you keep it in an SQL and someone decides to modify the data directly in the backend. Then sorting will be lost. I actually did that before, because user-side privacy list editor(s?) suck.)

@crosser
Copy link
Author

crosser commented Jul 16, 2018

@zinid , I pushed a new version of the patch that does not use lists:sort(). (But the code is more difficult to read, unfortunately :( ). Please see if this is more acceptable. Thanks.

@crosser crosser force-pushed the order-privacy-lists branch 3 times, most recently from 851c569 to b3dd089 Compare July 28, 2018 09:42
When checking a packet against the active privacy list, make sure that
the matching item with the lowest 'order' attribute is used.

Bug-url: processone#2529
Signed-off-by: Eugene Crosser [email protected]
@p1bot
Copy link
Collaborator

p1bot commented Jan 21, 2019

Hi @crosser, many thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes.

@p1bot p1bot added the cla-missing Contributor needs to sign Contribution License Agreement label Jan 21, 2019
@p1bot
Copy link
Collaborator

p1bot commented Jan 21, 2019

You did it @crosser!

Thank you for signing the ProcessOne Contribution License Agreement.

We will have a look at your contribution!

@p1bot p1bot removed the cla-missing Contributor needs to sign Contribution License Agreement label Jan 21, 2019
@Neustradamus
Copy link
Contributor

@prefiks: What do you think about this PR and Issue?

@mremond mremond added this to the ejabberd 23.xx milestone Jul 20, 2023
@badlop badlop modified the milestones: ejabberd 23.10, ejabberd 23.xx Oct 17, 2023
@badlop badlop modified the milestone: ejabberd 24.xx Jan 22, 2024
@badlop badlop modified the milestones: ejabberd 24.xx, Parking Lot Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants