[Home]

Summary:ASTERISK-13415: [patch] X-Asterisk-Hangupcause header only in challenged BYEs
Reporter:klaus3000 (klaus3000)Labels:
Date Opened:2009-01-21 04:41:14.000-0600Date Closed:2009-01-29 04:35:51.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) bug14294.diff
( 1) bug14294b.diff
Description:Hi!

The X-Asterisk-Hangupcause is added in transmit_request_with_auth() function, but not in transmit_request(). Why?

IMO, the Hangupcause header should be added to all BYE requests

****** ADDITIONAL INFORMATION ******

Actually I still have not achieved to have a SIP message with X-Asterisk-Hangupcause header added automatically by Asterisk, but reviewing the code this looks like a bug to me.

Problem exists in 1.4.22 and trunk.
Comments:By: Olle Johansson (oej) 2009-01-23 08:49:51.000-0600

Try this patch.
By: Olle Johansson (oej) 2009-01-23 08:57:58.000-0600

This patch should propably be merged with the one in 13385 that solves a few issues and adds a Reason header option.
By: klaus3000 (klaus3000) 2009-01-23 09:21:41.000-0600

just a question for understanding:

new, the headers are added in transmit_request() AND transmit_request_with_auth().

Doesn't this cause the header to be present in case of challenged BYE?
By: klaus3000 (klaus3000) 2009-01-23 09:58:11.000-0600

Actually I am not sure anymore if this is needed at all. Because in sip_hangup() the transmit_request_with_auth() is used to send the BYE.

MAybe the problem is somewhere else.
By: klaus3000 (klaus3000) 2009-01-23 10:07:22.000-0600

I think I found a problem:

scenario:
     Asterisk
INVITE-->
        ---INVITE->
        <---200----        
<---200----

        <---BYE----        
       ast_hangup()
   transmit_request_with_auth()
     
in transmit_request_with_auth() p->owner=NULL, thus no headers added (tested on 1.4.23)
       
<---BYE----
By: Olle Johansson (oej) 2009-01-26 10:26:43.000-0600

Klaus, please try this patch on 1.4 if you have time. THanks.
By: klaus3000 (klaus3000) 2009-01-28 08:58:19.000-0600

works for me, thanks. I think a logical next step would be to
- make these headers a config option
- support for reading this headers with a config option (avoids loss of hangupcause information due to multiple cause code mappings)
By: Olle Johansson (oej) 2009-01-29 02:25:25.000-0600

OK, I think we should move away from X-Asterisk to reason codes and also make it optional. Agreed.

The related bug has a patch for Reason: header support.
By: Digium Subversion (svnbot) 2009-01-29 02:47:32.000-0600

Repository: asterisk
Revision: 172169

U   branches/1.4/channels/chan_sip.c

------------------------------------------------------------------------
r172169 | oej | 2009-01-29 02:47:31 -0600 (Thu, 29 Jan 2009) | 16 lines

Make sure that we always add the hangupcause headers. In some cases, the owner was disconnected before we checked for the cause.
This patch implements a temporary storage in the pvt and use that instead.

The code is based on ideas from code from Adomjan in issue ASTERISK-12652 (Add support for Reason: header)
Thanks to Klaus Darillion for testing!

(closes issue ASTERISK-13415)
related to issue ASTERISK-12652

Reported by: klaus3000 and adomjan
Patches:
     bug14294b.diff uploaded by oej (license 306)
     Based on 20080829_chan_sip.c-q850reason_header.patch uploaded by adomjan (license 487)
Tested by: oej, klaus3000


------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=172169
By: Digium Subversion (svnbot) 2009-01-29 03:17:17.000-0600

Repository: asterisk
Revision: 172173

_U  trunk/
U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r172173 | oej | 2009-01-29 03:17:16 -0600 (Thu, 29 Jan 2009) | 24 lines

Merged revisions 172169 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r172169 | oej | 2009-01-29 09:48:18 +0100 (Tor, 29 Jan 2009) | 16 lines

Make sure that we always add the hangupcause headers. In some cases, the owner was disconnected before we checked for the cause.
This patch implements a temporary storage in the pvt and use that instead.

The code is based on ideas from code from Adomjan in issue ASTERISK-12652 (Add support for Reason: header)
Thanks to Klaus Darillion for testing!

(closes issue ASTERISK-13415)
related to issue ASTERISK-12652

Reported by: klaus3000 and adomjan
Patches:
     bug14294b.diff uploaded by oej (license 306)
     Based on 20080829_chan_sip.c-q850reason_header.patch uploaded by adomjan (license 487)
Tested by: oej, klaus3000


........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=172173
By: Digium Subversion (svnbot) 2009-01-29 03:56:34.000-0600

Repository: asterisk
Revision: 172217

_U  branches/1.6.0/
U   branches/1.6.0/channels/chan_sip.c

------------------------------------------------------------------------
r172217 | oej | 2009-01-29 03:56:33 -0600 (Thu, 29 Jan 2009) | 32 lines

Merged revisions 172173 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r172173 | oej | 2009-01-29 10:18:01 +0100 (Tor, 29 Jan 2009) | 24 lines

Merged revisions 172169 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r172169 | oej | 2009-01-29 09:48:18 +0100 (Tor, 29 Jan 2009) | 16 lines

Make sure that we always add the hangupcause headers. In some cases, the owner was disconnected before we checked for the cause.
This patch implements a temporary storage in the pvt and use that instead.

The code is based on ideas from code from Adomjan in issue ASTERISK-12652 (Add support for Reason: header)
Thanks to Klaus Darillion for testing!

(closes issue ASTERISK-13415)
related to issue ASTERISK-12652

Reported by: klaus3000 and adomjan
Patches:
     bug14294b.diff uploaded by oej (license 306)
     Based on 20080829_chan_sip.c-q850reason_header.patch uploaded by adomjan (license 487)
Tested by: oej, klaus3000


........

................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=172217
By: Digium Subversion (svnbot) 2009-01-29 04:35:51.000-0600

Repository: asterisk
Revision: 172231

_U  branches/1.6.1/
U   branches/1.6.1/channels/chan_sip.c

------------------------------------------------------------------------
r172231 | oej | 2009-01-29 04:35:50 -0600 (Thu, 29 Jan 2009) | 32 lines

Merged revisions 172173 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r172173 | oej | 2009-01-29 10:18:01 +0100 (Tor, 29 Jan 2009) | 24 lines

Merged revisions 172169 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r172169 | oej | 2009-01-29 09:48:18 +0100 (Tor, 29 Jan 2009) | 16 lines

Make sure that we always add the hangupcause headers. In some cases, the owner was disconnected before we checked for the cause.
This patch implements a temporary storage in the pvt and use that instead.

The code is based on ideas from code from Adomjan in issue ASTERISK-12652 (Add support for Reason: header)
Thanks to Klaus Darillion for testing!

(closes issue ASTERISK-13415)
related to issue ASTERISK-12652

Reported by: klaus3000 and adomjan
Patches:
     bug14294b.diff uploaded by oej (license 306)
     Based on 20080829_chan_sip.c-q850reason_header.patch uploaded by adomjan (license 487)
Tested by: oej, klaus3000


........

................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=172231