Hi Marius,<br><br>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..<br>
<br>Regarding the example you pointed,<br>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.<br>
<br>another example with multiple mime types.., <br>Accept: text/html, multipart/mixed<br>In this case parse_accept_body returns 2 mime types as both are predefined in the list.<br><br>Thanks<br>Jijo<br><br><br><div class="gmail_quote">
On Thu, Aug 18, 2011 at 5:58 PM, Bucur Marius <span dir="ltr"><<a href="mailto:bucur_marius_ovidiu@yahoo.com" target="_blank">bucur_marius_ovidiu@yahoo.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">Hi Jijo,<br>
<br>
In my opinion, decode_mime_type is broken, and parse_accept_body does not work as expected.<br>
For example, this is a valid accept header:<br>
<br>
accept: text/plain;param=",some value".<br>
<br>
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.<br>
<br>
I must say I never saw such an accept header, but the standard permits it.<br>
Indeed, a quick fix would be to ignore the return code when comma is found.<br>
This must be done for other functions that call decode_mime_type like has_body(mime) in textops.<br>
<br>
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.<br>
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.<br>
Also, this might work for most callers, but parse_accept_body will still be buggy, as I explained in the example above.<br>
<div><br>
Cheers,<br>
Marius<br>
<br>
________________________________<br>
From: Jijo <<a href="mailto:realjijo@gmail.com" target="_blank">realjijo@gmail.com</a>><br>
To: Bucur Marius <<a href="mailto:bucur_marius_ovidiu@yahoo.com" target="_blank">bucur_marius_ovidiu@yahoo.com</a>><br>
</div>Sent: Thursday, August 18, 2011 10:48 AM<br>
<div><div></div><div>Subject: Re: [SR-Users] decode_mime_type error<br>
<br>
<br>
Hi Marius,<br>
<br>
Right, the same function is used for Accept and Content-Type.<br>
<br>
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.<br>
<br>
The only change required here is not return error for comma while parsing the Content-Type.<br>
<br>
Thanks<br>
Jijo<br>
<br>
<br>
On Thu, Aug 18, 2011 at 12:32 PM, Bucur Marius <<a href="mailto:bucur_marius_ovidiu@yahoo.com" target="_blank">bucur_marius_ovidiu@yahoo.com</a>> wrote:<br>
<br>
Hello Jijo,<br>
><br>
>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:<br>
><br>
>Accept: text/html, image/jpeg, multipart<br>
><br>
>hence the search for commas :).<br>
>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.<br>
>The bad thing is the Accept header can have mime parameters too (which can contain comma).<br>
><br>
>Accept = "Accept" HCOLON [ accept-range *(COMMA accept-range) ]<br>
>accept-range = media-range *(SEMI accept-param)<br>
>media-range = ( "*/*" / ( m-type SLASH "*" ) / ( m-type SLASH m-subtype ) ) *( SEMI m-parameter )<br>
><br>
>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).<br>
><br>
><br>
>Cheers,<br>
><br>
>Marius<br>
>________________________________<br>
>From: Jijo <<a href="mailto:realjijo@gmail.com" target="_blank">realjijo@gmail.com</a>><br>
>To: Bucur Marius <<a href="mailto:bucur_marius_ovidiu@yahoo.com" target="_blank">bucur_marius_ovidiu@yahoo.com</a>>; SIP Router - Kamailio (OpenSER) and SIP Express Router (SER) - Users Mailing List <<a href="mailto:sr-users@lists.sip-router.org" target="_blank">sr-users@lists.sip-router.org</a>><br>
>Sent: Thursday, August 18, 2011 8:04 AM<br>
>Subject: Re: [SR-Users] decode_mime_type error<br>
><br>
><br>
><br>
>Hi Marius,<br>
><br>
>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.<br>
><br>
>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..??<br>
><br>
>Content-Type = ( "Content-Type" / "c" ) HCOLON media-type<br>
>media-type = m-type SLASH m-subtype *(SEMI m-parameter)<br>
>m-type = discrete-type / composite-type<br>
>discrete-type = "text" / "image" / "audio" / "video"<br>
>/ "application" / extension-token<br>
>composite-type = "message" / "multipart" / extension-token<br>
><br>
>Record-Route = "Record-Route" HCOLON rec-route *(COMMA rec-route)<br>
><br>
>Thanks<br>
>Jijo<br>
><br>
><br>
><br>
>On Thu, Aug 18, 2011 at 2:23 AM, Bucur Marius <<a href="mailto:bucur_marius_ovidiu@yahoo.com" target="_blank">bucur_marius_ovidiu@yahoo.com</a>> wrote:<br>
><br>
>Hello Jijo,<br>
>><br>
>>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.<br>
>>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.<br>
>>I think the author of the function thought of something like:<br>
>><br>
>>Content-Type: text/html, image/jpeg // very weird, though<br>
>><br>
>>This is wrong, hence the comma can be in a parameter (like in your case).<br>
>>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).<br>
>>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.<br>
>><br>
>>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).<br>
>><br>
>>Cheers,<br>
>>Marius<br>
>><br>
>><br>
>>________________________________<br>
>>From: Jijo <<a href="mailto:realjijo@gmail.com" target="_blank">realjijo@gmail.com</a>><br>
>>To: <a href="mailto:sr-users@lists.sip-router.org" target="_blank">sr-users@lists.sip-router.org</a>; <a href="mailto:sr-dev@lists.sip-router.org" target="_blank">sr-dev@lists.sip-router.org</a><br>
>>Sent: Wednesday, August 17, 2011 10:54 AM<br>
>>Subject: [SR-Users] decode_mime_type error<br>
>><br>
>><br>
>><br>
>>Hi All,<br>
>><br>
>>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 "<br>
>> "more then one mime type :-(!<br>
>><br>
>>Content Type Header is as below, It works fine if the boundary value is without comma.<br>
>><br>
>>Content-Type: multipart/mixed;boundary=",AW"<br>
>><br>
>>The parse_content_type_hdr() is failing in the following code,<br>
>><br>
>> ret = decode_mime_type(msg->content_type->body.s, end , &mime);<br>
>> if (ret==0)<br>
>> goto error;<br>
>> if (ret!=end) {<br>
>><br>
>> LOG(L_INFO,"ERROR:parse_content_type_hdr: CONTENT_TYPE hdr contains "<br>
>> "more then one mime type :-(!\n");<br>
>> goto error ;<br>
>> }<br>
>><br>
>>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.<br>
>><br>
>>Please let me know if you see any impact.<br>
>><br>
>>Thanks<br>
>>Jijo<br>
>><br>
>><br>
>><br>
>><br>
>><br>
>>_______________________________________________<br>
>>SIP Express Router (SER) and Kamailio (OpenSER) - sr-users mailing list<br>
>><a href="mailto:sr-users@lists.sip-router.org" target="_blank">sr-users@lists.sip-router.org</a><br>
>><a href="http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users" target="_blank">http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users</a><br>
>><br>
>>_______________________________________________<br>
>>SIP Express Router (SER) and Kamailio (OpenSER) - sr-users mailing list<br>
>><a href="mailto:sr-users@lists.sip-router.org" target="_blank">sr-users@lists.sip-router.org</a><br>
>><a href="http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users" target="_blank">http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users</a><br>
>> <br>
><br>
</div></div></blockquote></div><br>