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 &lt;bucur_marius_ovidiu(a)yahoo.com > 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> > 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 &lt;realjijo(a)gmail.com&gt; > >To: Bucur Marius &lt;bucur_marius_ovidiu(a)yahoo.com&gt;om>; SIP Router - Kamailio > (OpenSER) and SIP Express Router (SER) - Users Mailing List < > sr-users(a)lists.sip-router.org&gt; > >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(a)yahoo.com> > > >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 &lt;realjijo(a)gmail.com&gt; > >>To: sr-users(a)lists.sip-router.org; sr-dev(a)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 > &gt;&gt;sr-users(a)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 > &gt;&gt;sr-users(a)lists.sip-router.org > >>http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users > >> > > >