| Summary: | ASTERISK-16226: [patch] response_refer() does not have a default case, so a 400 final response stalls in the Transfer() application | ||
| Reporter: | David Woolley (davidw) | Labels: | |
| Date Opened: | 2010-06-08 11:34:16 | Date Closed: | 2010-08-18 08:11:36 |
| Priority: | Minor | Regression? | No |
| Status: | Closed/Complete | Components: | Channels/chan_sip/Transfers |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| Environment: | Attachments: | ( 0) handle_response_refer-fix1.diff ( 1) Issue17486-counterbid.diff.txt | |
| Description: | For an outgoing refer request, generated by the Transfer() application, chan_sip.c's response_refer routine only recognizes specific response codes, which don't include "400 Bad Request", the generic 400 response code. As a result, if it gets an unrecognized final status, it does not wake up the Transfer application to allow it to return a FAILURE response to the dialplan. Cisco CCM 6 can generate this response to REFER, although not necessarily to the REFERs generated by the standard Asterisk code. ****** ADDITIONAL INFORMATION ****** I'm being naughty here in that we are actually working with a heavily modified copy of 1.6.1.0, including backported code, which attempts to implement outgoing attended transfers (they work for one brand of PABX already) and I am reporting this against the trunk version because I've confirmed by code reading that there is no default case there. Also we only get this response for REFER replaces, which, of course, the standard Asterisk will not initiate. However, I've looked at RFC 3515 and it mandates the use of 400 responses in some cases, and I can see no reason why there should not be general 4xx or even 2xx responses. A visual check of the code easily confirms that it doesn't handle these. We are still experimenting with the Cisco, and if we need to we will fix this locally, but I'm not sure when I'll be allowed the time to submit any change back (although it looks like I might be able to submit it independent of our other changes). I've only classed this as minor because I don't have a real life example of it failing on the standard code with a real life remote SIP UA. I should be able to provide a full sip trace, but given this is ultimately based on code reading of a standard version, and execution of a non-standard one, I'll just include the actual response, for the moment. <--- SIP read from UDP://192.168.10.10:5060 ---> SIP/2.0 400 Bad Request Date: Tue, 08 Jun 2010 15:23:09 GMT From: <sip:6100@192.168.130.107>;tag=as1632869c Content-Length: 0 To: "Xxxx Xxxxxxx 6906" <sip:6906@192.168.10.10>;tag=dafadd12-25b2-40d3-958c-bc3d033e7ed0-29935189 Contact: <sip:6906@192.168.10.10:5060> Call-ID: aa152200-c0e1603d-fac08-a0aa8c0@192.168.10.10 Via: SIP/2.0/UDP 192.168.130.107:5060;branch=z9hG4bK7e41d88f;rport CSeq: 103 REFER <-------------> --- (9 headers 0 lines) --- -- Got SIP response 400 "Bad Request" back from 192.168.10.10 | ||
| Comments: | By: Matthew Nicholson (mnicholson) 2010-07-30 11:46:32 Test with the patch I just uploaded and see if it resolves the issue for you. By: David Woolley (davidw) 2010-07-30 12:10:14 I'm afraid I'm on leave next week and it is the end of the working day in the UK. However, my initial reaction is that you missed the fact that these responses never actually get as far as response_refer because the first level response handler doesn't know about their being relevant to REFER. Also, it still has the problem that it is enumerating specific possibilities, when the RFCs don't actually constrain the possible responses. I have actually done some rather more radical surgery for this, but it is entangled with with changes to support outgoing REFER/Replaces and I haven't had time to allocate to separating the bug fixes from the new feature code, or to forward port either to the relevant maintained versions. Actually, it may not be too entwined, but it is against a modified 1.6.1.0. I'll attach what we have at the moment. By: Digium Subversion (svnbot) 2010-08-11 16:11:52 Repository: asterisk Revision: 281874 U branches/1.8/channels/chan_sip.c ------------------------------------------------------------------------ r281874 | mnicholson | 2010-08-11 16:11:52 -0500 (Wed, 11 Aug 2010) | 10 lines handle all possible responses to REFER requests (closes issue ASTERISK-16226) Reported by: davidw Patches: Issue17486-counterbid.diff.txt uploaded by davidw (license 780) Tested by: davidw Review: https://reviewboard.asterisk.org/r/837/ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=281874 By: Digium Subversion (svnbot) 2010-08-11 16:12:24 Repository: asterisk Revision: 281876 _U trunk/ U trunk/channels/chan_sip.c ------------------------------------------------------------------------ r281876 | mnicholson | 2010-08-11 16:12:24 -0500 (Wed, 11 Aug 2010) | 17 lines Merged revisions 281874 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.8 ........ r281874 | mnicholson | 2010-08-11 16:11:54 -0500 (Wed, 11 Aug 2010) | 10 lines handle all possible responses to REFER requests (closes issue ASTERISK-16226) Reported by: davidw Patches: Issue17486-counterbid.diff.txt uploaded by davidw (license 780) Tested by: davidw Review: https://reviewboard.asterisk.org/r/837/ ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=281876 By: Digium Subversion (svnbot) 2010-08-18 08:10:37 Repository: asterisk Revision: 282639 U branches/1.8/channels/chan_sip.c ------------------------------------------------------------------------ r282639 | mnicholson | 2010-08-18 08:10:37 -0500 (Wed, 18 Aug 2010) | 13 lines Properly handle 200 and unknown responses conatined in NOTIFY requests received in response to REFER requests. This patch fixes the way asterisk handles NOTIFY requests received in response to REFER requests. These changes to NOTIFY handler were first introduced in r217482. This new change properly handles the 200 response by queueing an AST_TRANSFER_SUCCESS control frame and also prevents that control frame from being queued when provisional and unknown responses are received. (issue ASTERISK-16226) Reported by: davidw Tested by: mnicholson (issue ASTERISK-12074) Reported by: davidw Review: https://reviewboard.asterisk.org/r/860/ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=282639 By: Digium Subversion (svnbot) 2010-08-18 08:11:36 Repository: asterisk Revision: 282640 _U trunk/ U trunk/channels/chan_sip.c ------------------------------------------------------------------------ r282640 | mnicholson | 2010-08-18 08:11:36 -0500 (Wed, 18 Aug 2010) | 20 lines Merged revisions 282639 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.8 ........ r282639 | mnicholson | 2010-08-18 08:10:39 -0500 (Wed, 18 Aug 2010) | 13 lines Properly handle 200 and unknown responses conatined in NOTIFY requests received in response to REFER requests. This patch fixes the way asterisk handles NOTIFY requests received in response to REFER requests. These changes to NOTIFY handler were first introduced in r217482. This new change properly handles the 200 response by queueing an AST_TRANSFER_SUCCESS control frame and also prevents that control frame from being queued when provisional and unknown responses are received. (issue ASTERISK-16226) Reported by: davidw Tested by: mnicholson (issue ASTERISK-12074) Reported by: davidw Review: https://reviewboard.asterisk.org/r/860/ ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=282640 | ||