Joe Maimon <jmaimon@ttec.com> wrote:
The same thing goes for "==". Having the attribute not exist is technically an exception, not a failed match.
When you put it that way, it actually becomes clearer.
Yeah, I spent a while doing similar rule-based approaches at a previous company.
So to != should only return match if there was no attribute that existed and compared successfully with != value.
I'm not sure that makes sense.. == means "exists AND matches" By boolean inversion, != means "does not exist OR does not match" That makes sense. It gets more difficult once multiple attributes of the same name are in a list. I'll have to think about that some more.
Does this mean you think the patch would be a good idea otherwise?
Yes.
With some changes it can avoid changing the behavior of != !~ when no pairs are found.
But is that a good idea?
Perhaps a small poll to find out if anyone present relies on the current behavior of != ?
Sure. Since it's not really documented, I'm not sure what the existing systems *expect*.
So far I have seen -= , != , !* , !~ that could possibly be changed.
I also have added -* , -~ operators that I find usefull, especialy in the policy module.
The use there is probably better done by something like: ... reply -= { foo == bar, ... } i.e. "remove from reply attributes that match these conditions". The behavior of '==' and '=~' is always the same, which helps.
The policy module, while I managed to get it to work for my own purposes, wasnt being called complete or stable. And it stands to reason that paircompare could/should be used by all modules who wish to maintain comparable comparison rules as used in the users file and other modules.
I would prefer to fix the policy module so it's complete and stable. With a little bit of effort, it can solve many of the same problems that the existing modules do, in a more general manner.
Also the policy manages to duplicate a nice portion of the paircompare() functionality within evaluate_condition(). That means two sets of code need to be maintained to present as consistent an experience to the user as possible.
Yeah, that's bad. But the main reason is that paircompare() has it's problems. It's meant to support the "users" file, and extending it for any other purpose is a bad idea.
Additionaly, != is handled the same way in the policy module as in paircompare(), if it doesnt exist it returns FALSE.
So long as rlm_policy is documented and consistent, that shouldn't be a problem.
I guess what I am getting at is the impression that equaly important as being consistent with past usage should be internal consistency with current usage.
rlm_policy is new and experimental. I don't mind changing existing deployments if we can figure out what the heck is the right thing to do. Changing the interpretation of the "users" file, and paircompare() at this stage is not a good idea.
Not like my opinion actually matters much.
<shrug> See the number of bufs fixes that have gonr in recently based on your comments. Alan DeKok.