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 <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 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
