Re: [SR-Users] decode_mime_type error
Hi Marius, You are right.. It will fail for parse_accept_body... I hope you will be able to open this issue on bug tracker.. Thanks Jijo On Fri, Aug 19, 2011 at 10:12 PM, Bucur Marius bucur_marius_ovi...@yahoo.com wrote: Hello Jijo, About the example I pointed, the parse_accept_body will fail and let me explain why. For the second invocation, 'some value' is not a valid mime type (a mime type is of form 'type/subtype'). When decode_mime_type will not find the '/' separator (see parse_content.c:310) it will fail, making parse_accept_body to fail too. If you want to test, just call parse_accept_body with the example I pointed, and you will see it returns -1 for my example (a valid accept header). This is, of course, bad behavior. Best regards, Marius -- *From:* Jijo realj...@gmail.com *To:* Bucur Marius bucur_marius_ovi...@yahoo.com; sr-users@lists.sip-router.org *Sent:* Friday, August 19, 2011 11:17 AM *Subject:* Re: [SR-Users] decode_mime_type error Hi Marius, The existing implementation is done considering the limitation of decode_mime_type but somehow missed the change in parse_content_type_hdr. I agree with you that its better to do the change in decode_mime_type as it is common for lot of other callers.. Regarding the example you pointed, According to my understanding from the code, I believe that existing implementation for parse_accept_body shall work as the mime types are predefined. So in the example you pointed for second invocation 'some value' is not a defined mime type as it not matching with predefined types. The decode_mime_type returns end pointer and parse_accept_body shall process only with the valid mime type text/plain. another example with multiple mime types.., Accept: text/html, multipart/mixed In this case parse_accept_body returns 2 mime types as both are predefined in the list. Thanks Jijo On Thu, Aug 18, 2011 at 5:58 PM, Bucur Marius bucur_marius_ovi...@yahoo.com wrote: Hi Jijo, In my opinion, decode_mime_type is broken, and parse_accept_body does not work as expected. For example, this is a valid accept header: accept: text/plain;param=,some value. but the parse_accept_body will return -1 because the first return value of decode_mime_type is ,some value\. The second invocation will think some value\ is a mime type, which obviously isn't. I must say I never saw such an accept header, but the standard permits it. Indeed, a quick fix would be to ignore the return code when comma is found. This must be done for other functions that call decode_mime_type like has_body(mime) in textops. This is just my opinion, but I think this solution is a bit hackish since the convention, the promise that decode_mime_type should respect is to return a non-NULL, non-end pointer ONLY when there are more mime types in the body. And it doesn't. A lot of functions that call it rely on this. I think it is easier/more elegant to fix the bug in the decode_mime_type then to change the function convention in all callers. Also, this might work for most callers, but parse_accept_body will still be buggy, as I explained in the example above. Cheers, Marius From: Jijo realj...@gmail.com To: Bucur Marius bucur_marius_ovi...@yahoo.com Sent: Thursday, August 18, 2011 10:48 AM Subject: Re: [SR-Users] decode_mime_type error Hi Marius, Right, the same function is used for Accept and Content-Type. I don't think there is any change required for parsing Accept. In case of Accept the function decode_mime_type is called in a loop until it find all the media types. On the returning from decode_mime_type(), the main function parse_accept_xxx() checks the pointer is 'comma' or not. if its comma, then the function decode_mime_type called again and find the remaining media types. So i think the existing implementation for Parsing Accept is fine. The only change required here is not return error for comma while parsing the Content-Type. Thanks Jijo On Thu, Aug 18, 2011 at 12:32 PM, Bucur Marius bucur_marius_ovi...@yahoo.com wrote: Hello Jijo, You are right, multiple mime-types are not allowed. But it seems like decode_mime_type had the purpose of also parsing the Accept header: Accept: text/html, image/jpeg, multipart hence the search for commas :). The only function that uses this feature and calls decode_mime_type in a loop is parse_accept_body. The other functions that call decode_mime_type just fail when returning ret != end, and since multiple mime types in content-type are not allowed the behavior is fine. The bad thing is the Accept header can have mime parameters too (which can contain comma). Accept= Accept HCOLON [ accept-range *(COMMA accept-range) ] accept-range = media-range *(SEMI accept-param) media-range= ( */* / ( m-type SLASH * ) / ( m-type SLASH m-subtype ) ) *( SEMI m-parameter ) So I
Re: [SR-Users] decode_mime_type error
Hello, On 8/19/11 2:09 AM, Bucur Marius wrote: Hi Jijo, In my opinion, decode_mime_type is broken, and parse_accept_body does not work as expected. For example, this is a valid accept header: accept: text/plain;param=,some value. but the parse_accept_body will return -1 because the first return value of decode_mime_type is ,some value\. The second invocation will think some value\ is a mime type, which obviously isn't. I must say I never saw such an accept header, but the standard permits it. Indeed, a quick fix would be to ignore the return code when comma is found. This must be done for other functions that call decode_mime_type like has_body(mime) in textops. This is just my opinion, but I think this solution is a bit hackish since the convention, the promise that decode_mime_type should respect is to return a non-NULL, non-end pointer ONLY when there are more mime types in the body. And it doesn't. A lot of functions that call it rely on this. I think it is easier/more elegant to fix the bug in the decode_mime_type then to change the function convention in all callers. Also, this might work for most callers, but parse_accept_body will still be buggy, as I explained in the example above. this has to be fixed, the safest way not to forget about this is to open an issue on bug tracker. Cheers, Daniel Cheers, Marius From: Jijorealj...@gmail.com To: Bucur Mariusbucur_marius_ovi...@yahoo.com Sent: Thursday, August 18, 2011 10:48 AM Subject: Re: [SR-Users] decode_mime_type error Hi Marius, Right, the same function is used for Accept and Content-Type. I don't think there is any change required for parsing Accept. In case of Accept the function decode_mime_type is called in a loop until it find all the media types. On the returning from decode_mime_type(), the main function parse_accept_xxx() checks the pointer is 'comma' or not. if its comma, then the function decode_mime_type called again and find the remaining media types. So i think the existing implementation for Parsing Accept is fine. The only change required here is not return error for comma while parsing the Content-Type. Thanks Jijo On Thu, Aug 18, 2011 at 12:32 PM, Bucur Mariusbucur_marius_ovi...@yahoo.com wrote: Hello Jijo, You are right, multiple mime-types are not allowed. But it seems like decode_mime_type had the purpose of also parsing the Accept header: Accept: text/html, image/jpeg, multipart hence the search for commas :). The only function that uses this feature and calls decode_mime_type in a loop is parse_accept_body. The other functions that call decode_mime_type just fail when returning ret != end, and since multiple mime types in content-type are not allowed the behavior is fine. The bad thing is the Accept header can have mime parameters too (which can contain comma). Accept= Accept HCOLON [ accept-range *(COMMA accept-range) ] accept-range = media-range *(SEMI accept-param) media-range= ( */* / ( m-type SLASH * ) / ( m-type SLASH m-subtype ) ) *( SEMI m-parameter ) So I think the best thing would be to change decode_mime_type so that it does a more thorough parsing (e.g. check for parameter) hence a parameter value can contain commas (only if quoted). Cheers, Marius From: Jijorealj...@gmail.com To: Bucur Mariusbucur_marius_ovi...@yahoo.com; SIP Router - Kamailio (OpenSER) and SIP Express Router (SER) - Users Mailing Listsr-users@lists.sip-router.org Sent: Thursday, August 18, 2011 8:04 AM Subject: Re: [SR-Users] decode_mime_type error Hi Marius, Thanks for the response.. I checked the other functions, but they don't have the check for ret !=end, but they check the pointer and if it is comma then loop through again until it find all the media types. As per the RFC3261 multiple media-types are not supoorted in the Content-Type. So I'm not sure why the author used comma to determine multiple media types . Probably just followed like Route or Record-Route headers..?? Content-Type = ( Content-Type / c ) HCOLON media-type media-type = m-type SLASH m-subtype *(SEMI m-parameter) m-type = discrete-type / composite-type discrete-type= text / image / audio / video / application / extension-token composite-type = message / multipart / extension-token Record-Route = Record-Route HCOLON rec-route *(COMMA rec-route) Thanks Jijo On Thu, Aug 18, 2011 at 2:23 AM, Bucur Mariusbucur_marius_ovi...@yahoo.com wrote: Hello Jijo, It seems like the decode_mime_type is a somehow broken. The comma is very well allowed in boundary, as you said. The BNF specified in RFC2046 permits it. But, the decode_mime_type function ignores everything coming after comma. More than that, it notifies the function caller that this content type has multiple mime types. I think the author of the function thought of something like: Content-Type: text/html, image/jpeg // very weird, though
Re: [SR-Users] decode_mime_type error
Hello Jijo, About the example I pointed, the parse_accept_body will fail and let me explain why. For the second invocation, 'some value' is not a valid mime type (a mime type is of form 'type/subtype'). When decode_mime_type will not find the '/' separator (see parse_content.c:310) it will fail, making parse_accept_body to fail too. If you want to test, just call parse_accept_body with the example I pointed, and you will see it returns -1 for my example (a valid accept header). This is, of course, bad behavior. Best regards, Marius From: Jijo realj...@gmail.com To: Bucur Marius bucur_marius_ovi...@yahoo.com; sr-users@lists.sip-router.org Sent: Friday, August 19, 2011 11:17 AM Subject: Re: [SR-Users] decode_mime_type error Hi Marius, The existing implementation is done considering the limitation of decode_mime_type but somehow missed the change in parse_content_type_hdr. I agree with you that its better to do the change in decode_mime_type as it is common for lot of other callers.. Regarding the example you pointed, According to my understanding from the code, I believe that existing implementation for parse_accept_body shall work as the mime types are predefined. So in the example you pointed for second invocation 'some value' is not a defined mime type as it not matching with predefined types. The decode_mime_type returns end pointer and parse_accept_body shall process only with the valid mime type text/plain. another example with multiple mime types.., Accept: text/html, multipart/mixed In this case parse_accept_body returns 2 mime types as both are predefined in the list. Thanks Jijo On Thu, Aug 18, 2011 at 5:58 PM, Bucur Marius bucur_marius_ovi...@yahoo.com wrote: Hi Jijo, In my opinion, decode_mime_type is broken, and parse_accept_body does not work as expected. For example, this is a valid accept header: accept: text/plain;param=,some value. but the parse_accept_body will return -1 because the first return value of decode_mime_type is ,some value\. The second invocation will think some value\ is a mime type, which obviously isn't. I must say I never saw such an accept header, but the standard permits it. Indeed, a quick fix would be to ignore the return code when comma is found. This must be done for other functions that call decode_mime_type like has_body(mime) in textops. This is just my opinion, but I think this solution is a bit hackish since the convention, the promise that decode_mime_type should respect is to return a non-NULL, non-end pointer ONLY when there are more mime types in the body. And it doesn't. A lot of functions that call it rely on this. I think it is easier/more elegant to fix the bug in the decode_mime_type then to change the function convention in all callers. Also, this might work for most callers, but parse_accept_body will still be buggy, as I explained in the example above. Cheers, Marius From: Jijo realj...@gmail.com To: Bucur Marius bucur_marius_ovi...@yahoo.com Sent: Thursday, August 18, 2011 10:48 AM Subject: Re: [SR-Users] decode_mime_type error Hi Marius, Right, the same function is used for Accept and Content-Type. I don't think there is any change required for parsing Accept. In case of Accept the function decode_mime_type is called in a loop until it find all the media types. On the returning from decode_mime_type(), the main function parse_accept_xxx() checks the pointer is 'comma' or not. if its comma, then the function decode_mime_type called again and find the remaining media types. So i think the existing implementation for Parsing Accept is fine. The only change required here is not return error for comma while parsing the Content-Type. Thanks Jijo On Thu, Aug 18, 2011 at 12:32 PM, Bucur Marius bucur_marius_ovi...@yahoo.com wrote: Hello Jijo, You are right, multiple mime-types are not allowed. But it seems like decode_mime_type had the purpose of also parsing the Accept header: Accept: text/html, image/jpeg, multipart hence the search for commas :). The only function that uses this feature and calls decode_mime_type in a loop is parse_accept_body. The other functions that call decode_mime_type just fail when returning ret != end, and since multiple mime types in content-type are not allowed the behavior is fine. The bad thing is the Accept header can have mime parameters too (which can contain comma). Accept = Accept HCOLON [ accept-range *(COMMA accept-range) ] accept-range = media-range *(SEMI accept-param) media-range= ( */* / ( m-type SLASH * ) / ( m-type SLASH m-subtype ) ) *( SEMI m-parameter ) So I think the best thing would be to change decode_mime_type so that it does a more thorough parsing (e.g. check for parameter) hence a parameter value can contain commas (only if quoted). Cheers, Marius From: Jijo realj...@gmail.com To: Bucur Marius
Re: [SR-Users] decode_mime_type error
Hello Jijo,
It seems like the decode_mime_type is a somehow broken. The comma is very well
allowed in boundary, as you said. The BNF specified in RFC2046 permits it.
But, the decode_mime_type function ignores everything coming after comma. More
than that, it notifies the function caller that this content type has multiple
mime types.
I think the author of the function thought of something like:
Content-Type: text/html, image/jpeg // very weird, though
This is wrong, hence the comma can be in a parameter (like in your case).
I think the best fix, as you said is to remove that check, hence a non-NULL
return value doesn't mean anything (that the content type has multiple mime
types).
Another fix is to write a good decode_mime_type, that checks if the comma is
inside a parameter. I don't know if effort is worth it.
One thing we need to do is to check if there are any functions in Kamailio that
call decode_mime_type and also perform this check (ret != end).
Cheers,
Marius
From: Jijo realj...@gmail.com
To: sr-users@lists.sip-router.org; sr-...@lists.sip-router.org
Sent: Wednesday, August 17, 2011 10:54 AM
Subject: [SR-Users] decode_mime_type error
Hi All,
The function parse_content_type_hdr() is failing in decode_mime_type() when
Content-Type parameter boundary has value comma as below. The error message
is ERROR:parse_content_type_hdr: CONTENT_TYPE hdr contains
more then one mime type :-(!
Content Type Header is as below, It works fine if the boundary value is without
comma.
Content-Type: multipart/mixed;boundary=,AW
The parse_content_type_hdr() is failing in the following code,
ret = decode_mime_type(msg-content_type-body.s, end , mime);
if (ret==0)
goto error;
if (ret!=end) {
LOG(L_INFO,ERROR:parse_content_type_hdr: CONTENT_TYPE hdr contains
more then one mime type :-(!\n);
goto error ;
}
I thought of fixing this issue by commenting the code for condition ret !=end.
I'm not sure why we we need that check, as mime variable has the information
to process the content.
Please let me know if you see any impact.
Thanks
Jijo
___
SIP Express Router (SER) and Kamailio (OpenSER) - sr-users mailing list
sr-users@lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users
___
SIP Express Router (SER) and Kamailio (OpenSER) - sr-users mailing list
sr-users@lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users
Re: [SR-Users] decode_mime_type error
Hi Marius,
Thanks for the response.. I checked the other functions, but they don't have
the check for ret !=end, but they check the pointer and if it is comma then
loop through again until it find all the media types.
As per the RFC3261 multiple media-types are not supoorted in the
Content-Type. So I'm not sure why the author used comma to determine
multiple media types . Probably just followed like Route or Record-Route
headers..??
Content-Type http://rfc-ref.org/RFC-TEXTS/3261/kw-content_type.html
= ( Content-Type
http://rfc-ref.org/RFC-TEXTS/3261/kw-content_type.html / c )
HCOLON media-type
http://rfc-ref.org/RFC-TEXTS/3261/kw-media-type.html
media-type http://rfc-ref.org/RFC-TEXTS/3261/kw-media-type.html
= m-type SLASH m-subtype *(SEMI m-parameter)
m-type = discrete-type / composite
http://rfc-ref.org/RFC-TEXTS/3261/kw-composite.html-type
discrete-type= text / image
http://rfc-ref.org/RFC-TEXTS/3261/kw-image.html / audio
http://rfc-ref.org/RFC-TEXTS/3261/kw-audio.html / video
/ application / extension-token
http://rfc-ref.org/RFC-TEXTS/3261/kw-token.html
composite http://rfc-ref.org/RFC-TEXTS/3261/kw-composite.html-type
= message / multipart / extension-token
http://rfc-ref.org/RFC-TEXTS/3261/kw-token.html
Record-Route = Record-Route HCOLON rec-route *(COMMA rec-route)
Thanks
Jijo
On Thu, Aug 18, 2011 at 2:23 AM, Bucur Marius bucur_marius_ovi...@yahoo.com
wrote:
Hello Jijo,
It seems like the decode_mime_type is a somehow broken. The comma is very
well allowed in boundary, as you said. The BNF specified in RFC2046 permits
it.
But, the decode_mime_type function ignores everything coming after comma.
More than that, it notifies the function caller that this content type has
multiple mime types.
I think the author of the function thought of something like:
Content-Type: text/html, image/jpeg // very weird, though
This is wrong, hence the comma can be in a parameter (like in your case).
I think the best fix, as you said is to remove that check, hence a non-NULL
return value doesn't mean anything (that the content type has multiple mime
types).
Another fix is to write a good decode_mime_type, that checks if the comma
is inside a parameter. I don't know if effort is worth it.
One thing we need to do is to check if there are any functions in Kamailio
that call decode_mime_type and also perform this check (ret != end).
Cheers,
Marius
From: Jijo realj...@gmail.com
To: sr-users@lists.sip-router.org; sr-...@lists.sip-router.org
Sent: Wednesday, August 17, 2011 10:54 AM
Subject: [SR-Users] decode_mime_type error
Hi All,
The function parse_content_type_hdr() is failing in decode_mime_type() when
Content-Type parameter boundary has value comma as below. The error
message is ERROR:parse_content_type_hdr: CONTENT_TYPE hdr contains
more then one mime type :-(!
Content Type Header is as below, It works fine if the boundary value is
without comma.
Content-Type: multipart/mixed;boundary=,AW
The parse_content_type_hdr() is failing in the following code,
ret = decode_mime_type(msg-content_type-body.s, end , mime);
if (ret==0)
goto error;
if (ret!=end) {
LOG(L_INFO,ERROR:parse_content_type_hdr: CONTENT_TYPE hdr
contains
more then one mime type :-(!\n);
goto error ;
}
I thought of fixing this issue by commenting the code for condition ret
!=end. I'm not sure why we we need that check, as mime variable has the
information to process the content.
Please let me know if you see any impact.
Thanks
Jijo
___
SIP Express Router (SER) and Kamailio (OpenSER) - sr-users mailing list
sr-users@lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users
___
SIP Express Router (SER) and Kamailio (OpenSER) - sr-users mailing list
sr-users@lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users
___
SIP Express Router (SER) and Kamailio (OpenSER) - sr-users mailing list
sr-users@lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users
Re: [SR-Users] decode_mime_type error
Hi Jijo, In my opinion, decode_mime_type is broken, and parse_accept_body does not work as expected. For example, this is a valid accept header: accept: text/plain;param=,some value. but the parse_accept_body will return -1 because the first return value of decode_mime_type is ,some value\. The second invocation will think some value\ is a mime type, which obviously isn't. I must say I never saw such an accept header, but the standard permits it. Indeed, a quick fix would be to ignore the return code when comma is found. This must be done for other functions that call decode_mime_type like has_body(mime) in textops. This is just my opinion, but I think this solution is a bit hackish since the convention, the promise that decode_mime_type should respect is to return a non-NULL, non-end pointer ONLY when there are more mime types in the body. And it doesn't. A lot of functions that call it rely on this. I think it is easier/more elegant to fix the bug in the decode_mime_type then to change the function convention in all callers. Also, this might work for most callers, but parse_accept_body will still be buggy, as I explained in the example above. Cheers, Marius From: Jijo realj...@gmail.com To: Bucur Marius bucur_marius_ovi...@yahoo.com Sent: Thursday, August 18, 2011 10:48 AM Subject: Re: [SR-Users] decode_mime_type error Hi Marius, Right, the same function is used for Accept and Content-Type. I don't think there is any change required for parsing Accept. In case of Accept the function decode_mime_type is called in a loop until it find all the media types. On the returning from decode_mime_type(), the main function parse_accept_xxx() checks the pointer is 'comma' or not. if its comma, then the function decode_mime_type called again and find the remaining media types. So i think the existing implementation for Parsing Accept is fine. The only change required here is not return error for comma while parsing the Content-Type. Thanks Jijo On Thu, Aug 18, 2011 at 12:32 PM, Bucur Marius bucur_marius_ovi...@yahoo.com wrote: Hello Jijo, You are right, multiple mime-types are not allowed. But it seems like decode_mime_type had the purpose of also parsing the Accept header: Accept: text/html, image/jpeg, multipart hence the search for commas :). The only function that uses this feature and calls decode_mime_type in a loop is parse_accept_body. The other functions that call decode_mime_type just fail when returning ret != end, and since multiple mime types in content-type are not allowed the behavior is fine. The bad thing is the Accept header can have mime parameters too (which can contain comma). Accept = Accept HCOLON [ accept-range *(COMMA accept-range) ] accept-range = media-range *(SEMI accept-param) media-range= ( */* / ( m-type SLASH * ) / ( m-type SLASH m-subtype ) ) *( SEMI m-parameter ) So I think the best thing would be to change decode_mime_type so that it does a more thorough parsing (e.g. check for parameter) hence a parameter value can contain commas (only if quoted). Cheers, Marius From: Jijo realj...@gmail.com To: Bucur Marius bucur_marius_ovi...@yahoo.com; SIP Router - Kamailio (OpenSER) and SIP Express Router (SER) - Users Mailing List sr-users@lists.sip-router.org Sent: Thursday, August 18, 2011 8:04 AM Subject: Re: [SR-Users] decode_mime_type error Hi Marius, Thanks for the response.. I checked the other functions, but they don't have the check for ret !=end, but they check the pointer and if it is comma then loop through again until it find all the media types. As per the RFC3261 multiple media-types are not supoorted in the Content-Type. So I'm not sure why the author used comma to determine multiple media types . Probably just followed like Route or Record-Route headers..?? Content-Type = ( Content-Type / c ) HCOLON media-type media-type = m-type SLASH m-subtype *(SEMI m-parameter) m-type = discrete-type / composite-type discrete-type= text / image / audio / video / application / extension-token composite-type = message / multipart / extension-token Record-Route = Record-Route HCOLON rec-route *(COMMA rec-route) Thanks Jijo On Thu, Aug 18, 2011 at 2:23 AM, Bucur Marius bucur_marius_ovi...@yahoo.com wrote: Hello Jijo, It seems like the decode_mime_type is a somehow broken. The comma is very well allowed in boundary, as you said. The BNF specified in RFC2046 permits it. But, the decode_mime_type function ignores everything coming after comma. More than that, it notifies the function caller that this content type has multiple mime types. I think the author of the function thought of something like: Content-Type: text/html, image/jpeg // very weird, though This is wrong, hence the comma can be in a parameter (like in your case). I think the best fix, as you said is to remove that check, hence a non-NULL return
[SR-Users] decode_mime_type error
Hi All,
The function parse_content_type_hdr() is failing in decode_mime_type() when
Content-Type parameter boundary has value comma as below. The error
message is ERROR:parse_content_type_hdr: CONTENT_TYPE hdr contains
more then one mime type :-(!
Content Type Header is as below, It works fine if the boundary value is
without comma.
Content-Type: multipart/mixed;boundary=*,*AW
The parse_content_type_hdr() is failing in the following code,
ret = decode_mime_type(msg-content_type-body.s, end , mime);
if (ret==0)
goto error;
if (ret!=end) {
LOG(L_INFO,ERROR:parse_content_type_hdr: CONTENT_TYPE hdr contains
more then one mime type :-(!\n);
goto error ;
}
I thought of fixing this issue by commenting the code for condition ret
!=end. I'm not sure why we we need that check, as mime variable has the
information to process the content.
Please let me know if you see any impact.
Thanks
Jijo
___
SIP Express Router (SER) and Kamailio (OpenSER) - sr-users mailing list
sr-users@lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users
