Hello,
On 8/19/11 2:09 AM, Bucur Marius 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.
this has to be fixed, the safest way not to forget
about this is to open
an issue on bug tracker.
Cheers,
Daniel
Cheers,
Marius
________________________________
From: Jijo<realjijo(a)gmail.com>
To: Bucur Marius<bucur_marius_ovidiu(a)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(a)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(a)gmail.com>
To: Bucur Marius<bucur_marius_ovidiu(a)yahoo.com>om>; SIP Router - Kamailio (OpenSER)
and SIP Express Router (SER) - Users Mailing List<sr-users(a)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(a)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(a)gmail.com>
> 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
> 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
> 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
sr-users(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users
--
Daniel-Constantin Mierla --
http://www.asipto.com
Kamailio Advanced Training, Oct 10-13, Berlin:
http://asipto.com/u/kat
http://linkedin.com/in/miconda --
http://twitter.com/miconda