Hi Marius,
You are right.. It will fail for parse_accept_body... I hope you will be able to open this issue on bug tracker..
Thanks Jijo
On Fri, Aug 19, 2011 at 10:12 PM, Bucur Marius < bucur_marius_ovidiu@yahoo.com> wrote:
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 realjijo@gmail.com *To:* Bucur Marius bucur_marius_ovidiu@yahoo.com; sr-users@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@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 realjijo@gmail.com To: Bucur Marius bucur_marius_ovidiu@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@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@gmail.com To: Bucur Marius bucur_marius_ovidiu@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_ovidiu@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@gmail.com To: sr-users@lists.sip-router.org; sr-dev@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 hdrcontains "
"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