Summary: | ASTERISK-16472: [patch] Asterisk just reads the first "Accept" header | ||
Reporter: | Iñaki Baz Castillo (ibc) | Labels: | |
Date Opened: | 2010-07-29 16:31:02 | Date Closed: | 2010-08-27 17:39:47 |
Priority: | Minor | Regression? | No |
Status: | Closed/Complete | Components: | Channels/chan_sip/General |
Versions: | Frequency of Occurrence | ||
Related Issues: | |||
Environment: | Attachments: | ( 0) multiple_accept_headers_1.4.diff | |
Description: | When a SUBSCRIBE "Event: dialog" comes to Asterisk it takes the first "Accept" header value as the only one, discarding all the rest, so it replies "489 Bad Event" if it doesn't recognize the first "Accept" value. Real example: This is a SUBSCRIBE coming from a Linksys SPA phone without using the specific "asterisk BLF mode" (a mode which shouldn't exist as "event" subscription is a RFC standard): SUBSCRIBE sip:8888@222.222.19.1 SIP/2.0' Via: SIP/2.0/UDP 192.168.1.14:25060;branch=z9hG4bK-f443274c;rport' From: <sip:cliente8888@222.222.19.1>;tag=a833e3091d78eb5' To: <sip:8888@222.222.19.1>' Call-ID: 7645f40c-c2e2f289@192.168.1.14' CSeq: 63626 SUBSCRIBE' Max-Forwards: 70' Contact: <sip:cliente8888@192.168.1.14:25060>' Accept: multipart/related' Accept: application/rlmi+xml' Accept: application/dialog-info+xml' Expires: 1800' Event: dialog' User-Agent: Linksys/SPA942-6.1.5(a)' Content-Length: 0' Supported: replaces, eventlist' Asterisk replies 489 Bad Event (format multipart/related). This is a SUBSCRIBE coming from the same SPA phone by using "asterisk BLF mode": SUBSCRIBE sip:8888@222.222.19.1 SIP/2.0' Via: SIP/2.0/UDP 192.168.1.14:25060;branch=z9hG4bK-3733da3b;rport' From: <sip:cliente8169@212.230.19.1>;tag=d041391aac7ac1b5' To: <sip:8888@222.222.19.1>' Call-ID: 70dd34be-24845a61@192.168.1.14' CSeq: 50627 SUBSCRIBE' Max-Forwards: 70' Contact: <sip:cliente8169@192.168.1.14:25060>' Accept: application/dialog-info+xml' Expires: 1800' Event: dialog' User-Agent: Linksys/SPA942-6.1.5(a)' Content-Length: 0 In this case Asterisk replies "200 OK" as it understands application/dialog-info+xml. The differences are: 1) Accept: multipart/related' Accept: application/rlmi+xml' Accept: application/dialog-info+xml' Supported: replaces, eventlist' 2) Accept: application/dialog-info+xml' Of course, the expected behavior is Asterisk to inspect *all* the "Accept" headers and *all* the possible comma separated values in each "Accept" header. There is no "preference" based on the Accept value position as Asterisk should select its preferred value. | ||
Comments: | By: Paul Belanger (pabelanger) 2010-07-29 20:08:13 I suspect this may be a feature request. That aside, do you mind trying with 1.8 or even trunk? I'd like to see if it is supported there. By: Olle Johansson (oej) 2010-07-30 01:46:22 It is clearly a bug when we can't parse a standard SIP message. By: Olle Johansson (oej) 2010-07-30 01:46:47 ...and I wasn't aware of this being possible when I wrote that part of the code... By: Iñaki Baz Castillo (ibc) 2010-07-30 02:54:03 > @pabelanger: I suspect this may be a feature request. IMHO doing a correct parsing of a protocol message can never be a feature request but a requeriment. As a side note, during an INVITE it seems that Asterisk doesn't inspect Accept value(s) in any way. All the following cases make the INVITE arriving to asterisk to work: 1) No "Accept" header (so "application/sdp" is used by default). 2) Multiple Accept headers: Accept: application/lalalalala Accept: application/sdp 3) Multiple header values: Accept: application/lalalalala, application/sdp 4) A unssuported value: Accept: application/lalalalalaa Perhaps case 4) shouldn't work. By: Paul Belanger (pabelanger) 2010-07-30 08:48:21 Sorry, should have been more verbose. Did you get a chance to try with trunk? By: Iñaki Baz Castillo (ibc) 2010-07-30 09:02:10 Honestly I don't test Asterisk trunk version, neither 1.6 series, I have no enough time for it. By: Leif Madsen (lmadsen) 2010-08-05 14:40:59 This is likely still an issue in 1.6.2 and 1.8 because I'm not aware of a ton of work being done there to resolve anything in that area. However, it would be ideal for someone to verify this either with a code review or testing, so I'm setting this to Feedback until we get that information. By: David Vossel (dvossel) 2010-08-19 17:30:59 I can verify we do no look at multiple "Accept" headers. I'm writing a patch for this and should have it up for testing soon. By: David Vossel (dvossel) 2010-08-19 17:46:03 I uploaded a patch for Trunk. I'll make a similar one for 1.4. If you can test the trunk one that would be great though. By: David Vossel (dvossel) 2010-08-23 17:26:46 I uploaded the 1.4 patch and took down the Trunk patch (I found some things I needed to fix in it while testing, but the 1.4 patch should work). Please test the 1.4 patch and report your results. By: Digium Subversion (svnbot) 2010-08-27 17:17:24 Repository: asterisk Revision: 283960 U branches/1.4/channels/chan_sip.c ------------------------------------------------------------------------ r283960 | dvossel | 2010-08-27 17:17:24 -0500 (Fri, 27 Aug 2010) | 8 lines Parse all "Accept" headers for SIP SUBSCRIBE requests. (closes issue ASTERISK-16472) Reported by: ibc Patches: multiple_accept_headers_1.4.diff uploaded by dvossel (license 671) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=283960 By: Digium Subversion (svnbot) 2010-08-27 17:27:48 Repository: asterisk Revision: 284002 _U branches/1.6.2/ U branches/1.6.2/channels/chan_sip.c ------------------------------------------------------------------------ r284002 | dvossel | 2010-08-27 17:27:48 -0500 (Fri, 27 Aug 2010) | 14 lines Merged revisions 283960 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r283960 | dvossel | 2010-08-27 17:17:26 -0500 (Fri, 27 Aug 2010) | 8 lines Parse all "Accept" headers for SIP SUBSCRIBE requests. (closes issue ASTERISK-16472) Reported by: ibc Patches: multiple_accept_headers_1.4.diff uploaded by dvossel (license 671) ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=284002 By: Digium Subversion (svnbot) 2010-08-27 17:37:10 Repository: asterisk Revision: 284032 _U branches/1.8/ U branches/1.8/channels/chan_sip.c ------------------------------------------------------------------------ r284032 | dvossel | 2010-08-27 17:37:10 -0500 (Fri, 27 Aug 2010) | 21 lines Merged revisions 284002 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.6.2 ................ r284002 | dvossel | 2010-08-27 17:27:50 -0500 (Fri, 27 Aug 2010) | 14 lines Merged revisions 283960 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r283960 | dvossel | 2010-08-27 17:17:26 -0500 (Fri, 27 Aug 2010) | 8 lines Parse all "Accept" headers for SIP SUBSCRIBE requests. (closes issue ASTERISK-16472) Reported by: ibc Patches: multiple_accept_headers_1.4.diff uploaded by dvossel (license 671) ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=284032 By: Digium Subversion (svnbot) 2010-08-27 17:39:46 Repository: asterisk Revision: 284033 _U trunk/ U trunk/channels/chan_sip.c ------------------------------------------------------------------------ r284033 | dvossel | 2010-08-27 17:39:46 -0500 (Fri, 27 Aug 2010) | 28 lines Merged revisions 284032 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.8 ................ r284032 | dvossel | 2010-08-27 17:37:11 -0500 (Fri, 27 Aug 2010) | 21 lines Merged revisions 284002 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.6.2 ................ r284002 | dvossel | 2010-08-27 17:27:50 -0500 (Fri, 27 Aug 2010) | 14 lines Merged revisions 283960 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r283960 | dvossel | 2010-08-27 17:17:26 -0500 (Fri, 27 Aug 2010) | 8 lines Parse all "Accept" headers for SIP SUBSCRIBE requests. (closes issue ASTERISK-16472) Reported by: ibc Patches: multiple_accept_headers_1.4.diff uploaded by dvossel (license 671) ........ ................ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=284033 |