I expect that tel2uri() was implemented based on the needs of initial developer as documented at:

* https://www.kamailio.org/docs/modules/devel/modules/siputils.html#siputils.f...

It can be improved if one needs/wants more.

Note also that specs are not implemented ad-litteram in a single function. The design of the application has been from the beginning to offer small tools (functions/statements) that can be blended in config script to get what you need. There are many functions and transformations that can help you replace/remove unwanted characters from user part of R-URI, such as subst_user(...), {re.subst} or {s...} transformations, etc. With all these I am pretty sure you can transfrorm tel to sip uri as per specs without using tel2sip() function.

To be clear: I am not against enhancing tel2sip() to do more that it does now, but wanted to highlight that many spec requirements can be done with a combination of functions and expressions. I am not the author of tel2sip() and actually I haven't used it many times. I do URI normalization with script operations.

@miconda, I have written a routine that puts the parameters for the tel: uri into an array with key/value separated, then uses qsort to sort. With this, they can be put into the sip: uri in correct sorted order. The sort is very esoteric, specific to the requirements for sip: uri when converting from a tel: uri, so, I would suggest we add it to the existing conversion routine. Though, it would be very easy to make it a separate callable script called before the existing routine, to sort the tel: uri. Also, if inside the current routine, at the same time that we have the parameters split up, we can easily check if phone-context is present, and, if it is a domain, we can delete the phone-context parameter, and, move the domian to the standard place after the @ symbol.

Please let me know how we can move forward. I can create a modified tel2sip() routine with the sorting of tel: parameters and a check to eliminate duplicate domains. Then, do testing here, and, finally send to you for your perusal before accepting.

Thank you very much!

At the link below, you can go to 19.1.6. It is a very strange ordering giving priority to two parameters that should be first, if present.

https://www.ietf.org/rfc/rfc3261.txt

19.1.6 Relating SIP URIs and tel URLs . . . . ``` To mitigate this problem, elements constructing telephone-subscriber fields to place in the userinfo part of a SIP or SIPS URI SHOULD fold any case-insensitive portion of telephone-subscriber to lower case, and order the telephone-subscriber parameters lexically by parameter name, excepting isdn-subaddress and post-dial, which occur first and in that order. (All components of a tel URL except for future- extension parameters are defined to be compared case-insensitive.) ```

@doncarr - ti can be also a new function, like tel_to_sip_uri(), so if one wants to use the old/lightweight version can still do it. I guess the new function is not going to share too much code with the old onw.

I have a fix just about ready for your review. As noted, there can be some weird things in the telephone world!

There are a couple of not well defined issues. They more or less state that the objective of the sip: uri, is that two equivalent uris will always be the equivalent string, thus the need to sort the parameters and converting to lower case. That much is very clear. But, applying this same reasoning to the "phone-context" parameter, if it is a domain, then, it should be moved after the '@' symbol and not repeated as a "phone-context" parameter, as to have an equivalent string to one without the phone-context parameter.

For example, the following two are obviously equivalent, but, are not the same string:

sip:491234567891;phone-context=ims.tel.org@ims.tel.org;user=phone sip:491234567891@ims.tel.org;user=phone

Also, they do not specifically state that visual separators should be removed from the "phone-context" parameter when it is a global phone number prefix, but, without removing them, we would again have the problem of equivalent sip uris not being equivalent strings.

For example, the following two are obviously equivalent, but, not the same string:

sip:1234567;phone-context=+52-33@ims.tel.org;user=phone sip:1234567;phone-context=+5233@ims.tel.org;user=phone

If that all sounds good to you, I will submit a fix after more testing.

I have not messed with checking for clearly bad use of visual separators, I just silently remove all visual separators wherever they appear, as was done with the existing code.

Please let me know if you want any changes to the reasoning. You could also give me example tel: uris you would like me to test before submitting the fix.

Thanks for everything!

Don.

@juha-h, @miconda ,

On making a "normalizing" function. I could do that to remove the visual separators from the "phone-context" field when it is a phone number, and, also, pre-sort the parameters in the tel: uri, so that when the current (light weight) function pulls them, the parameters are already in the sorted order, and there are no visual separators in the "phone-context" field.

A little more tricky, I could also delete the "phone-context" field from the tel: uri when it is a domain, assuming that the host name was already pulled out. But, if this was done before domain was extracted, it would cause an error (basically losing the domain). Currently, the domain in the "hostpart" variable is inserted after the "@" sign, and the contents in the "phone-context" field is inserted as is.

Also, in this case, there can be no logic in deciding which domain to select (the one in the "hostpart", or the one in the "phone-context" It seems that the hostpart is currently pulled using cscf_get_realm_from_uri(), so, theoretically, the variable "hostpart" and the contents of "phone-context" should always be the same.

Ok, in any case, I think it would be a little confusing to have a pre-filter that creates a special format of a tel: uri such that an existing translator will end up creating a standards compliant sip: uri.

I think it would be best to give a simple/fast version and a standards compliant version.

The existing function does try to do part of the standards in that it converts everything to lower case, and, removes visual separators from the phone number digits before the first semi-colon.

@miconda, thanks, that is exactly what I have now, so, will submit it when I can. Below is the code that I wrote with a new function named "std_tel2sip()", as a preview I used most of the old procedure where possible, and tried to keep the same style.

I have been given a task with a short deadline, so, will not be able to work on this anymore right now. I will get back to it as soon as possible.

Don.

``` /* * Compare function to sort tel: uri options acording to standard * before inserting into sip: uri * * See "RFC 3261 SIP: Session Initiation Protocol June 2002" * 19.1.6 Relating SIP URIs and tel URLs */ typedef struct { char *name; char *value; } tel_param_t; #define MAX_TEL_PARAMS (10)

int compare_tel_options(const void *v1, const void *v2) { tel_param_t *p1 = (tel_param_t *) v1; tel_param_t *p2 = (tel_param_t *) v2;

if (0 == strcasecmp(p1->name, "isdn-subaddress")) { return -1; } else if (0 == strcasecmp(p2->name, "isdn-subaddress")) { return 1; } else if (0 == strcasecmp(p1->name, "post-dial")) { return -1; } else if (0 == strcasecmp(p2->name, "post-dial")) { return 1; } else { return strcasecmp(p1->name, p2->name); } }

/* * Remove visual separators from the phone number * Assume it has been validated as a number containing * ONLY leading '+', digits, and visual separators. */ static void remove_visual_separators_from_phone(char *p) { char *p2; p2 = p; while (*p != '\0') { /* Skip all visual separators */ while ((*p != '\0') && ((*p == '.') || (*p == '-') || (*p == '(') || (*p == ')'))) { p++; } *p2 = *p; /* Until the first visual separator, these both point to the same place. */ /* but, more efficient to just do it than an if statement each time. */ /* If we arrived at a terminator in the inner loop, time to exit */ if (*p == '\0') return; /* Now we increment both pointers. */ p++; p2++; } *p2 = '\0'; /* Make sure that the string is terminated after the last valid digit. */ }

/* * Check if this is a phone number. * Assume possible leading '+' * Assume separators '.', '-', '(', or ')' could be present. */ static int is_number(const char *p) { if (*p == '+') p++; while (*p != '\0') { if ((!isdigit(*p)) && (*p != '.') && (*p != '-') && (*p != '(') && (*p != ')')) return 0; p++; } return 1; }

/* * Converts URI, if it is tel URI, to SIP URI. Returns 1, if * conversion succeeded or if no conversion was needed, i.e., URI was not * tel URI. Returns -1, if conversion failed. Takes SIP URI hostpart from * second parameter and (if needed) writes the result to third parameter. * This one attempts to be standards compliant and sort tel: uri parameters * copied to the sip: uri in the manner defined in the standard. It also * deletes the "phone-context" parameter if it is a domain, and, takes visual * separators from the "phone-context" parameter if it is a telephone number. */ int std_tel2sip(struct sip_msg* _msg, char* _uri, char* _hostpart, char* _res) { str uri, hostpart, tel_uri, sip_uri; char *at; int i, j, in_tel_parameters = 0; pv_spec_t *res; pv_value_t res_val;

/* get parameters */ if (get_str_fparam(&uri, _msg, (fparam_t*)_uri) < 0) { LM_ERR("failed to get uri value\n"); } if (get_str_fparam(&hostpart, _msg, (fparam_t*)_hostpart) < 0) { LM_ERR("failed to get hostpart value\n"); } res = (pv_spec_t *)_res;

/* check if anything needs to be done */ if (uri.len < 4) return 1; if (strncasecmp(uri.s, "tel:", 4) != 0) return 1;

/* reserve memory for clean tel uri */ tel_uri.s = pkg_malloc(uri.len+1); if (tel_uri.s == 0) { LM_ERR("no more pkg memory\n"); return -1; }

/* Remove visual separators before converting to SIP URI. Don't remove * visual separators in TEL URI parameters (after the first ";") */ for (i=0, j=0; i < uri.len; i++) { if (in_tel_parameters == 0) { if (uri.s[i] == ';') in_tel_parameters = 1; } if (in_tel_parameters == 0) { if ((uri.s[i] != '-') && (uri.s[i] != '.') && (uri.s[i] != '(') && (uri.s[i] != ')')) tel_uri.s[j++] = tolower(uri.s[i]); } else { tel_uri.s[j++] = tolower(uri.s[i]); } } tel_uri.s[j] = '\0'; tel_uri.len = strlen(tel_uri.s);

/*** Start Code to sort tel: params *******/ tel_param_t params[MAX_TEL_PARAMS]; char *tmp_ptr = tel_uri.s + 4; // skip tel:

int n_tel_params = 0; for (int i=0; i < MAX_TEL_PARAMS; i++) { tmp_ptr = strchr(tmp_ptr, ';'); if (tmp_ptr == NULL) { break; } *tmp_ptr = '\0'; tmp_ptr++; n_tel_params++; params[i].name = tmp_ptr; } for (int i=0; i < n_tel_params; i++) { tmp_ptr = strchr(params[i].name, '='); if (tmp_ptr == NULL) { params[i].value = ""; } else { *tmp_ptr = '\0'; tmp_ptr++; params[i].value = tmp_ptr; } if ((0 == strcasecmp(params[i].name, "phone-context")) && (is_number(params[i].value))) { remove_visual_separators_from_phone(params[i].value); }

} if (n_tel_params > 1) { qsort(&params[0], n_tel_params, sizeof(tel_param_t), compare_tel_options); } /*** End Code to sort tel: params ******/

/* reserve memory for resulting sip uri */ sip_uri.len = 4 + tel_uri.len - 4 + 1 + hostpart.len + 1 + 10; sip_uri.s = pkg_malloc(sip_uri.len+1); if (sip_uri.s == 0) { LM_ERR("no more pkg memory\n"); pkg_free(tel_uri.s); return -1; }

/* create resulting sip uri */ at = sip_uri.s; append_str(at, "sip:", 4); /** Original code tel: parameters NOT sorted append_str(at, tel_uri.s + 4, tel_uri.len - 4); *****/ /***** Start Changed Code for sorted tel: parameters ****/ append_str(at, tel_uri.s + 4, strlen(tel_uri.s + 4)); /* This string was terminated after the number */ /** Now we need to insert sorted tel: parameters **/ for (int i=0; i < n_tel_params; i++) { /* If the phone context is a domain, it has already been extracted and is in the "host part" */ if ((0 != strcasecmp(params[i].name, "phone-context")) || (is_number(params[i].value))) { append_chr(at, ';'); append_str(at, params[i].name, strlen(params[i].name)); append_chr(at, '='); append_str(at, params[i].value, strlen(params[i].value)); } } /***** End Changed Code for sort tel: parameters ****/ append_chr(at, '@'); append_str(at, hostpart.s, hostpart.len); append_chr(at, ';'); append_str(at, "user=phone", 10);

/* tel_uri is not needed anymore */ pkg_free(tel_uri.s);

/* set result pv value and write sip uri to result pv */ res_val.rs = sip_uri; res_val.flags = PV_VAL_STR; if (res->setf(_msg, &res->pvp, (int)EQ_T, &res_val) != 0) { LM_ERR("failed to set result pvar\n"); pkg_free(sip_uri.s); return -1; }

/* free allocated pkg memory and return */ pkg_free(sip_uri.s); return 1; } ```

Hey, very sorry, I got assigned another project and have not been able to get back to this. Hopefully I will have some time soon. I think it will be a good update that will avoid future problems. Don.

On Sun, Sep 3, 2017 at 3:51 AM, Daniel-Constantin Mierla < notifications@github.com> wrote:

Any chance for a PR sometime soon, so this item can be closed here?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kamailio/kamailio/issues/1173#issuecomment-326792506, or mute the thread https://github.com/notifications/unsubscribe-auth/AEqRXOTUiZGD7LMkrcoZYG64ANE4xjEpks5semiLgaJpZM4OJzVh .

Hey, thank you VERY much Carsten, that is really appreciated. Sorry that I did not have time to take care of it. Don

On Tue, Sep 12, 2017 at 9:27 AM, ng-voice GmbH notifications@github.com wrote:

Hi,

i've reviewed the code and made some minor improvements. I will commit a slightly modified version later this week.

Thanks, Carsten

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kamailio/kamailio/issues/1173#issuecomment-328864905, or mute the thread https://github.com/notifications/unsubscribe-auth/AEqRXIC-OqO-MnNwXGfhGtLtYEkoLdAtks5shpDogaJpZM4OJzVh .

Hi guys, I am no longer at Verizon, but, can still help with this issue if needed. Let me know. I do look forward to working on Kamailio again sometime in the future. Don.

On Sat, Mar 3, 2018 at 12:39 PM, henningw notifications@github.com wrote:

Hi Carsten, was this already commited? Somehow I did not found it, maybe I did not looked carefully enough.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kamailio/kamailio/issues/1173#issuecomment-370169902, or mute the thread https://github.com/notifications/unsubscribe-auth/AEqRXHk1tjs90G-PKaNpFE3N11Ewe9OKks5tauNrgaJpZM4OJzVh .

I've reviewed the code and tested it intensively. I will make patch of it soon. Due to a busy schedule, this patch did not make it upstream yet....

Close this one - please submit your code as a new pull request if you are still interested on working on this.