[Wireshark-bugs] [Bug 12184] MATE AVPL match modes "Loose" and "Every" do not work correctly

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12184

--- Comment #10 from Peter Wu  ---
The patch has been merged now, I guess this bug can be closed after fixing the
documentation on the wiki?

Or do you want to keep it open for adding (a) new mode(s) of match semantics?

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12184] MATE AVPL match modes "Loose" and "Every" do not work correctly

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12184

--- Comment #9 from Peter Wu  ---
At the moment it looks like a misunderstanding of the intended functionality.
The important point is that Every and Strict perform pair-wise matching,
returning a result AVPL with a length of at most the number of condition AVPs.

This means that a documentation update is necessary, but the functionality
should probably be preserved. Can you review the commit message at 
https://code.wireshark.org/review/1 for inaccuracies?

As for your test cases, you have to modify it as follows for the expected
result:

Transform show_loose {
// Does not work because "Loose" always matches
//Match Loose (rsp_code!100,con_type) Replace (loose=1);
// Instead use this, it will return on a successful match:
Match Strict (con_type) Replace (loose=1);
Match Strict (rsp_code!100) Replace (loose=1);
};

The Every case is now also fixed in the patch by using sorting by string value
instead of by a random memory address.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12184] MATE AVPL match modes "Loose" and "Every" do not work correctly

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12184

--- Comment #8 from Peter Wu  ---
Hmm, I am still not fully certain how multiple AVPs with the same name have to
be handled (for matching data AVPs and returning results).

In plugins/mate/examples/web.mate there is this snippet (old syntax):
Action=Transform; Name=rm_client_from_dns_resp; Mode=Replace; Match=Every;
dns_resp=1; client_addr; .dns_resp=1;

Here you can see that the intention of "Every" is that every condition
(dns_resp=1, client_addr) must match and that it is replaced by (dns_resp=1)
(effectively dropping client_addr).

In mms.mate we can see a case where "Loose" with zero conditions is allowed, it
is expected to match (and apply the replacement). Rewritten to the new syntax
(for clarity) it becomes:

Transform mms_start {
// yes, really no attributes and apparently no value for the AVP
either?
Match Loose () Insert (mms_start);
}
Pdu mmse_over_wsp_pdu Proto wsp Transport ip {
// (Match omitted)
Transform mms_start;
}

In the same file we find (translated and annotated):

Transform rm_client_from_http_resp1 {
// If "http_rq" is set, match and return from this transform.
Match Strict (http_rq) Insert ();
// could probably be "Strict" as well?
Match Every (addr) Insert (not_rq);
}
Transform rm_client_from_http_resp2 {
// huh, why add not_rq above and replace it here? Why not just:
// Match Strict (addr) Insert (ue); above?
Match Strict (not_rq) Replace (ue);
}
Pdu mmse_over_http ... {
Extract addr From ip.addr;
Extract http_rq From http.request;
...
Transform rm_client_from_http_resp1;
Transform rm_client_from_http_resp2;
}

In matelib/radius.mate:

Transform radius_same_port {
// If there are already two ports, do nothing
Match Strict (radius_port, radius_port) Insert ();
// else add a new port?
Match Every (radius_port) Insert (radius_port=0);
}
Pdu radius_pdu Proto radius Transport udp/ip {
Match radius_addr From ip.addr;
Match radius_port From udp.port;
Match radius_id From radius.id;
Match radius_code From radius.code;
Transform radius_same_port;
}
Gop radius_req On radius_pdu (radius_id, radius_addr, radius_addr,
radius_port, radius_port) {
// not sure if right syntax
Start (radius_code|1|4|7);
Stop (radius_code|2|3|5|8|9);
}

None of these examples really help understanding what should be done for Match
(what to return for a match).

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12184] MATE AVPL match modes "Loose" and "Every" do not work correctly

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12184

--- Comment #7 from Peter Wu  ---
I have pushed some buggy WIP to implement the (wrong) idea following the
updated wiki/bug comments, it will be modified as things become clearer.

The current code has one fatal flaw, it compares attributes by their memory
address. Should we have data (a=1,b=2) then it is possible that Loose (b=2)
does not match depending on whether  <  or  >  (where '&' denotes
address-of operator). After fixing this, the Loose case seems fixed.
(Additionally, I no longer return an empty list if the match fails, it now
returns NULL (=failed match)).

From
https://wiki.wireshark.org/Mate/Reference?action=recall=27#Attribute.2FValue_Pair_List_.28AVPL.29
:
>> PDUS, GoPs and GoGs use an AVPL to contain the tracing information.
>> An AVPL is an unsorted set of AVPs that can be matched against other AVPLs.

All matching functions however expect sorted AVPLs (this is enforced at
insertion time). Perhaps it refers to the MATE configuration file where
"Gop sess On pdu_name (a, a, b)" is the same as
"Gop sess On pdu_name (a, b, a)".

"Strict" seems to have a totally diferent meaning than assumed before.
Apparently it only consumes the first AVP from the data AVPL (such that it
cannot be matched by other conditions). With this assumption,
"Match Strict (a, a)" would fail on (a=1), but succeed on (a=1,a=2).

The current (modified) wording of the wiki is contradictory (or at least
confusing). The original revision says:
>> "Every" Match: Will match if none of the AVPs of the operator AVPL fails to
>> match a present AVP in the operand AVPL, even if not all of the operator's
>> AVPs have a match. If it matches it will return an AVPL containing all AVPs
>> from the operand AVPL that did match one AVP in the operator AVPL.

I think this is the same as the "Strict" behavior I described before.

Maybe you could jump on #wireshark on Freenode IRC for some faster feedback?

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12184] MATE AVPL match modes "Loose" and "Every" do not work correctly

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12184

--- Comment #6 from Gerrit Code Review  ---
Change 1 had a related patch set uploaded by Peter Wu:
[WIP] mate: document more parts, try to fix matching logic

https://code.wireshark.org/review/1

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12184] MATE AVPL match modes "Loose" and "Every" do not work correctly

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12184

--- Comment #5 from Pavel Sindelka  ---
(In reply to Peter Wu from comment #4)
> What about adding this to
> https://wiki.wireshark.org/Mate/Reference#Every_Match
> 
>...
> 
> The description on the wiki (while it might technically correct) is hard to
> parse. Your comment is already an improvement, ...

To the best of my memory, I was only changing the syntax of the examples, and
replacing Luis' "operator" and "operand" names by "configuration AVP(L)" and
"data AVP(L)" respectively. So I didn't dare to add my own examples or change
the wording towards a more precise one back then, fearing that I didn't
understand the author's intention deep enough.

But luckily there is a better memory than mine - the wiki's own one. Revision
27 is the last one before I've started editing it. So I can see now that I may
have shifted the meaning of Strict a bit.

The original text was:
>> A Strict match will return if and only if every AVP in the operator has at
>> least one match in the operand and none fails.

My edit has changed it to:
>> A Strict match between AVPLs succeeds if and only if every AVP in the
>> configuration AVPL has at least one counterpart in the data AVPL and none
>> of the AVP matches fails. The result AVPL contains all the data AVPs that
>> matched.

My current understanding of how it should work is that for Strict to succeed,
two (different from each other by definition) values of the same attribute may
be present in the data AVPL, but it is enough that only one of them matches the
condition associated to that attribute in configuration. The fact that the
other value does not match the condition doesn't prevent Strict from
succeeding, yet this instance of that attribute will not be part of the result
AVPL.

If this is also what the current code says, I will make the Wiki clear (and add
some examples). The "and none fails" part makes me uncertain whether this was
author's intention. But where two instances of the same attribute are part of
GoP's key AVPL, each of the values in the GoP's key matches only one of the
values in the candidate PDU's AVPL and the Strict match is still considered
successful, so I guess such reading is correct.

> ..., but for the comment in the source code I will be writing:
> 
> There is a match if and only if for all conditions, either
> (1) there are no data avps with the same attribute name or
> (2) there exists a data avp with the same attribute name which matches.
> (If there are no conditions, the result will be a match.)

Especially the last line confirms that we share the some doubt in the Every
case. The way Every is described on the Wiki, this statement is true. But while
it is unlikely someone would use any type of match with an empty list of
conditions, people might be surprised that condition (a=1,b=2) matches data
(c=3,d=4,e=5) although it yields no result AVPL. So the question is whether an
Every match with an empty result AVPL should really be considered successful or
not.

So yes, I will add more examples to the Wiki, but we must first be sure that
they match the real behaviour.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12184] MATE AVPL match modes "Loose" and "Every" do not work correctly

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12184

--- Comment #4 from Peter Wu  ---
What about adding this to https://wiki.wireshark.org/Mate/Reference#Every_Match

(a=1, a=2) Match Every (a=1) ==> (a=1)
(a=1, a=2) Match Every (a=1, a=2) ==> (a=1, a=2)
(a=1, a=2) Match Every (a=1, a=3) ==> No match  // a found, but not a=3
(a=1, a=2) Match Every (a!1) ==> (a=2)
(a=1, a=2) Match Every (b?) ==> ()  // match with empty result

The description on the wiki (while it might technically correct) is hard to
parse. Your comment is already and improvement, but for the comment in the
source code I will be writing:

There is a match if and only if for all conditions, either
(1) there are no data avps with the same attribute name or
(2) there exists a data avp with the same attribute name which matches.
(If there are no conditions, the result will be a match.)

Alternatively, it is important to note that "Every" refers to the condition and
not "every" (compatible) data avp. "Every" condition must match (but if there
is no compatible data to test against, it will still be considered a match).

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12184] MATE AVPL match modes "Loose" and "Every" do not work correctly

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12184

Peter Wu  changed:

   What|Removed |Added

 Status|CONFIRMED   |IN_PROGRESS

--- Comment #3 from Peter Wu  ---
I made a small mistake in describing Strict, it additionally needs the
condition that all values must match.

Effectively Strict is the same as Every, with the additional requirement that
conditions must apply to at least one data AVP. This matches with your data
example.

Currently in progress of reverse-engineering, documenting and modifying the
functional code.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12184] MATE AVPL match modes "Loose" and "Every" do not work correctly

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12184

--- Comment #2 from Pavel Sindelka  ---
(In reply to Peter Wu from comment #1)

Seems it is going to be a language issue, as I've never used English to discuss
matters of mathematics.

For Every and Strict, I've either misunderstood Luis or you, or maybe you've
just swapped the mapping between the names and the descriptions in your
Comment?

Loose is fine - there is at least one matching data AVP for at least one of the
conditions in the list, here we agree.

To me, what Luis wrote about "Every" means that "for each attribute on the
condition list which exists at least once also in the data list, at least one
of the values of that attribute on the data list must match the condition for
that attribute. Any attribute on the condition list which has no counterpart on
the data list is ignored".

So using the set theory vernacular, you examine the conditions only on the
intersection of the two lists, except that it is hard to talk about an
intersection here: if you take the complete elements, they are different in the
two sets so the intersection is always empty, if you take just their
"attribute" part, some elements may exist twice in each of the operand sets
which I doubt is compatible with set theory - at least it makes it impossible
to represent sets of finite counts of elements as bitmaps).

So let's describe it algorithmically instead: you set the "total result" bit to
1 and start a foreach walk through the list of conditions. For each condition
on the list, you first check whether there is any compatible data element
available; if not, you do nothing and move to the next condition. If data
elements compatible with that condition are available, you set the "local
result" bit to 0 and examine all of them for a match (i.e. you must not stop
the evaluation on first match). For each compatible data element which matches,
you "or" the local result with 1 and mark that data element as part of the
result list. When finished, you "and" the "total result" bit with the value of
the "local result". If the "total result" becomes 0, you may break the outer
foreach because if the total result is "no match", nobody cares about the match
list.

For Strict, at least one compatible data attribute must exist for each
condition, and at least one of all the compatible data attributes must match
its condition.

I haven't dug too deep in this case, but theoretically you can have the
following:

data list: (a=10, a=12, a=20)
conditions list: (a<15)

In this case, a Strict match succeeds, and the result list is (a=10, a=12)

If you'd add (b=7) to the conditions list, a Strict match would fail on the
same data list, while an Every match would still succeed, yielding the same
result list. Every would still succeed if you'd add b=7 to the data list, but
it would fail if you'd add b=10 instead.

Just for the case, two conditions dealing with the same attribute may exist on
the conditions list, so it may look like this:
(a<11,a>15)

Both Every and Strict would match on the data list above, yielding a result
list (a=10,a=20) and both would fail if you'd remove any of (a=10, a=20) from
the data list; a Loose match with the same conditions and data would still
succeed if you'd remove either a=10 or a=20 from the original list but fail if
you'd remove both.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe

[Wireshark-bugs] [Bug 12184] MATE AVPL match modes "Loose" and "Every" do not work correctly

[bugzilla-daemon]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12184

Peter Wu  changed:

   What|Removed |Added

 Status|UNCONFIRMED |CONFIRMED
 CC||pe...@lekensteyn.nl
 Ever confirmed|0   |1

--- Comment #1 from Peter Wu  ---
(disclaimer: I started learning how to use MATE today, the following is an
interpretation of the Mate/Manual wiki page)

Definitions:
An operation AVP (Attribute Value Pair) is basically a condition.
An operation AVPL is basically a list of conditions.
A data AVP is basically data extracted from packets.
Below, "value" refers to the value from the data AVP which have the same
attribute as its condition.

Expected results:
Loose - there exist a condition with a matching value.
Every - for each condition, for all values it must match.
Strict - for each condition, there exist a matching value.

("Every" seems to resemble "for all" from set theory, "Strict" seems to
resemble "existence")


The code is buggy, "Loose" always matches (verified with source code).
"Every" matches nothing (perhaps it behaves more like "Strict"?)

Will have a look at fixing the logic.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Sent via:Wireshark-bugs mailing list 
Archives:https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs
 mailto:wireshark-bugs-requ...@wireshark.org?subject=unsubscribe