URI comparison logic

[mykhailopopivniak]

RFC 3261 describes the rules for URI comparison the section 19.1.4 URI Comparison

  o  URI uri-parameter components are compared as follows:

     -  Any uri-parameter appearing in both URIs must match.

     -  A user, ttl, or method uri-parameter appearing in only one
        URI never matches, even if it contains the default value.

     -  A URI that includes an maddr parameter will not match a URI
        that contains no maddr parameter.

     -  All other uri-parameters appearing in only one URI are
        ignored when comparing the URIs.

However, it looks like resiprocate does not consider URI parameters at all in the Uri::operator< method. Take for example this case:

Uri uri1("sip:test@192.168.31.45:5065;ttl=4");
Uri uri2("sip:test@192.168.31.45:5065;ttl=8");

(uri1 < uri2) == false
(uri2 < uri1) == false

Then, logically, we expect uri1 == uri2 to be true. However, Uri::operator== method does consider URI params, so we get (uri1 == uri2) == false. To me it seems to be breaking search logic in ordered container, like map or set, which use operator< for comparison.

Another thing is that Uri::operator== method mostly follows the RFC, but the Any uri-parameter appearing in both URIs must match point is implemented only for unknown (extension or not defined) parameters and a subset of defined. So, when checking for equallity resiprocate uses only the following defined params:

• user;
• ttl;
• method;
• maddr;
• transport;
• lr.

Other defined params for URI that are not considered include:

• ob
• gr
• comp
• duration
• ext
• sigcomp-id

I believe these two points are bugs that could have significant implications. I would like to improve this point, yet, it would be nice to get the insight from the maintainers on this topic. @sgodin, what do you think about this case? Should this be improved or left as it is? My concern is that the Uri class is used everywhere and the NameAddr class equality and comparison operators also depend on the ones from Uri. Such changes could potentially break compatibility and impact many users.

[sgodin]

Hmm that is a good question. Unfortunately I did not write the code in the first place so I can only speculate. One speculation is that it wasn't easy at the time to loop through all known parameters, but I'm not certain. I do share the same concern as you about making such potentially breaking changes. ie: We would want to ensure we don't break repro registration matching logic. I am also concerned about what other RFC's might have defined for "newer" uri parameters and what they say about equality check rules that might override what is/was written in RFC3261.

[mykhailopopivniak]

I am not very familiar with repro, but from what I can see in the code, registration matching uses only operator== for URI comparison of contacts. AORs are stored in a map and find() method is used, so operator< is also used for comparisons. However, AORs come without any parameters. Therefore, it seems that considering URI parameters in operator< should not affect repro's functionality.

In my opinion, adhering to this potentially incorrect logic is not preferable to potentially breaking something that depends on it. Since both operator< and operator== are used for comparisons, they should implement consistent logic by considering URI parameters.

Regarding "newer" URI parameters, I can say for sure only about gr param. RFC 5627 does not define new comparison rules and instead references the rules in RFC3261.

RFC 5627

5.4. Creation of a GRUU

However, if two GRUUs are associated with different AORs or different instance IDs or both, the GRUUs MUST be different based on URI equality comparison. A GRUU in a domain MUST NOT be equivalent, based on URI comparison, to any AOR in a domain except for the one associated with the GRUU.

A public GRUU will always be equivalent to the AOR based on URI equality rules. The reason is that the rules in RFC 3261 [1] cause URI parameters that are in one URI, but not in the other, to be ignored for equality purposes. Since a public GRUU differs from an AOR only by the presence of the "gr" URI parameter, the two URIs are equivalent based on those rules.

6.1. Request Targeting

If it contains the "gr" URI parameter but is not equivalent, based on URI comparison, to a currently valid GRUU within the domain, it SHOULD be rejected with a 404 (Not Found) response; this is the same behavior a proxy would exhibit for any other URI within the domain that is not valid. If the Request-URI contains the "gr" URI parameter and is equivalent, based on URI comparison, to a GRUU which is currently valid within the domain, processing proceeds as it would for any other URI present in the location service, as defined in Section 16.5 of RFC 3261 [1], except that the "gr" URI parameter is not removed as part of the canonicalization process.

I did not find any new rules defined for other parameters while skimming through their RFCs, but one would need to read them thoroughly to say for sure.

I have prepared a related pull request #385, but I think the issue can remain open until someone checks and adds other URI parameters.

[sgodin]

That all makes sense to me - thanks for doing the extra leg work!