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 &lt;realjijo(a)gmail.com&gt; *To:* Bucur Marius &lt;bucur_marius_ovidiu(a)yahoo.com&gt;om>; sr-users(a)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(a)yahoo.com&gt; 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 &lt;realjijo(a)gmail.com&gt; To: Bucur Marius &lt;bucur_marius_ovidiu(a)yahoo.com&gt; 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(a)yahoo.com&gt; wrote: Hello Jijo, decode_mime_type had the purpose of also parsing the Accept header: 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. contain comma). ] m-subtype ) ) *( SEMI m-parameter ) does a more thorough parsing (e.g. check for parameter) hence a parameter value can contain commas (only if quoted). (OpenSER) and SIP Express Router (SER) - Users Mailing List < sr-users(a)lists.sip-router.org&gt; 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. 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..?? bucur_marius_ovidiu(a)yahoo.com&gt; wrote: well allowed in boundary, as you said. The BNF specified in RFC2046 permits it. More than that, it notifies the function caller that this content type has multiple mime types. non-NULL return value doesn't mean anything (that the content type has multiple mime types). comma is inside a parameter. I don't know if effort is worth it. Kamailio that call decode_mime_type and also perform this check (ret != end). when Content-Type parameter "boundary" has value comma as below. The error message is "ERROR:parse_content_type_hdr: CONTENT_TYPE hdr contains " without comma. contains " !=end. I'm not sure why we we need that check, as mime variable has the information to process the content.