Hello,
during testing SER at SIPit 24 I discovered something what I believe is a major bug in the To and From header parser of SER.
When you send a request with a To or From header like this: To: sip:nils@invalid.com;foo the To header parameter parser fails, and the whole To header is not parsed. According to the BNF of 3261 generic parameters without parameters are allowed in To and From header.
So I created the attached patch, which seems to work for me. Please review carefully and let me know if I overlooked something in the problem statement or the bug fix.
Cheers Nils
Index: parse_to.c =================================================================== RCS file: /cvsroot/ser/sip_router/parser/parse_to.c,v retrieving revision 1.22 diff -a -u -r1.22 parse_to.c --- parse_to.c 23 Jun 2008 17:09:57 -0000 1.22 +++ parse_to.c 20 May 2009 05:04:40 -0000 @@ -122,6 +122,11 @@ case TAG3: param->type=TAG_PARAM; case PARA_NAME: + param->value.len = tmp-param->value.s; + saved_status = E_PARA_VALUE; + status = F_LF; + add_param( param , to_b ); + break; case TAG1: case TAG2: param->name.len = tmp-param->name.s; @@ -161,6 +166,11 @@ case TAG3: param->type=TAG_PARAM; case PARA_NAME: + param->name.len = tmp-param->name.s; + saved_status = E_PARA_VALUE; + status = F_CR; + add_param( param , to_b ); + break; case TAG1: case TAG2: param->name.len = tmp-param->name.s; @@ -277,7 +287,13 @@ } #endif case PARA_VALUE_TOKEN: - param->value.len=tmp-param->value.s; + case PARA_NAME: + if (status == PARA_VALUE_TOKEN) { + param->value.len=tmp-param->value.s; + } + else { + param->name.len = tmp-param->name.s; + } add_param(param,to_b); case E_PARA_VALUE: param = (struct to_param*)
Hi,
as I received so far no feedback at all for this issue I'm wondering if everybody is just fine with the fact that sip-router can not parse parameter only values in To and From header (and maybe other headers which use the parse_to function). Or is everybody just still puzzled by this fact and still reviewing my patch?
Regards Nils
El Jueves, 28 de Mayo de 2009, Juha Heinanen escribió:
i personally have not yet seen to/from params without values.
But they are perfectly valid :(
Am 28.05.2009 21:17 Uhr, schrieb Juha Heinanen:
Right. If someone would have stumbeled about this, I guess we should have received reports/complaints earlier. So it is more a theoretical problem then a real one, which results in lower priority. But still it seems that we agree that it should be fixed.
Nils
On May 28, 2009 at 17:44, Nils Ohlmeier nils@iptel.org wrote:
There were some problems with your patch. You assumed that CR or LF will mark the end of the parameter, but you can have the parameter continued on the next line. There was also a mixed value with name. I have a different patch, which I'll commit into andrei/to_parser_fix branch. It removes the special PINGTEL cases and fixes another theoretical problem when a parameter is at the end of string (not terminated by CR or LF, but by the end of the string/message).
However I did only _very_ little testing. Could you run your tests on it? I don't want to merge the to_fix unless we are reasonably sure that it's good and it does not introduce some other bug.
Andrei
Am 28.05.2009 20:46 Uhr, schrieb Andrei Pelinescu-Onciul:
Ok I will give detailed testing. But that could take some time.
BTW the To parser has even with this patch some more problems, e.g. null characters in display names are not treated correctly. So maybe we could take this branch to improve the To parser, give it real hard testing and then merge it later.
Nils
2009/5/28 Nils Ohlmeier nils@iptel.org:
Hi Nils, I'm trying it in Kamailio 1.5 (sending a request with "To: sip:nils@invalid.com;foo") and no error occurs. Does it just affect to SR?
Hi,
I would think that Kamailio is affected as well, because To header parameter parser code is in the SER CVS since 2002 and was basically not touched since then. So if not somebody in the Kamilio project fixed this it should be the same. During my tests I found out that it is important that you are using a configuration which actually tries to parse the To header at all. For example if you send an OPTIONS request to the iptel.org server (not a user), the To header can be as ugly as you want (you will always get a 200 OK), because the configuration does not care the To header at all. But if you send a REGISTER with such a To header you will not get any response at all.
Regards Nils
El Viernes, 29 de Mayo de 2009, Nils Ohlmeier escribió:
Nils, I'm sending REGISTER with "To: sip:anonimous@domain.org;qwe" and Kamailio gives no error (and stores the user location right.
-
Andrei Pelinescu-Onciul -
Iñaki Baz Castillo -
jh＠tutpro.com -
Nils Ohlmeier -
Nils Ohlmeier