[SR-Users] decode_mime_type error

Bucur Marius bucur_marius_ovidiu at yahoo.com
Sat Aug 20 04:12:17 CEST 2011 

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 <realjijo at gmail.com>
To: Bucur Marius <bucur_marius_ovidiu at yahoo.com>; sr-users at 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_ovidiu at 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 <realjijo at gmail.com>
>To: Bucur Marius <bucur_marius_ovidiu at 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_ovidiu at 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 <realjijo at gmail.com>
>>To: Bucur Marius <bucur_marius_ovidiu at yahoo.com>; SIP Router - Kamailio (OpenSER) and SIP Express Router (SER) - Users Mailing List <sr-users at 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_ovidiu at 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 <realjijo at gmail.com>
>>>To: sr-users at lists.sip-router.org; sr-dev at 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 at 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 at lists.sip-router.org
>>>http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users
>>>      
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sip-router.org/pipermail/sr-users/attachments/20110820/deda31c3/attachment.htm>

More information about the sr-users mailing list