[Home]

Summary:ASTERISK-11875: [patch] loopback switch does not allow modification of extension
Reporter:David Chappell (chappell)Labels:
Date Opened:2008-04-18 09:21:00Date Closed:2008-06-03 19:27:35
Priority:MinorRegression?No
Status:Closed/CompleteComponents:PBX/pbx_loopback
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) pbx_loopback.c.diff
Description:Due to incorrect use of the AST_LIST_INSERT_HEAD() macro the loopback switch cannot perform any translation on the extension number before searching for it in the target context.


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

I have attached a patch which fixes this.

In the patch I have also renamed a couple of the functions whose names seemed to be out of sync with their current behavior and edited the embedded documentation for clarity.

I have marked this bug as "minor" because I believe that this feature has never worked.
Comments:By: Tilghman Lesher (tilghman) 2008-04-25 11:02:58

What's the reason for the final section of the patch, where you're removing 3 lines?
By: David Chappell (chappell) 2008-05-02 10:59:25

I fixed this bug about 11 months ago so my memory of why I removed those three lines is a little hazy, but I do remember that I removed them after extensive debugging.  After studying the code for a while I believe it is because they represent an test which is invalid in context.

You will note that the surrounding functions such as loopback_canmatch() have a similar test.  In those functions the purpose of the test is to prevent the switch from firing unless the source extension matches the extramatch pattern.  The test is appropriate in those functions, though for reasons of efficiency the two tests should probably be applied in the opposite order, like so:

static int loopback_exists(struct ast_channel *chan, const char *context, const char *exten, int priority, co
{
       LOOPBACK_COMMON;
callerid);
       if (newpattern && !ast_extension_match(newpattern, exten))
               res = 0;
       else
               res = ast_exists_extension(chan, newcontext, newexten, newpriority, callerid);
       return res;
}

But, if I understand the role of loopback_exec() correctly, at the point at which it applies the test the switch has _already_ fired and the current extension (to which the test is now applied) is the _target_ extension. Thus the extramatch test is now erroneously applied to the target extension which, if the extension has been modified, may not match.  Setting res to -1 causes something bad to happen.  As I recall, the bad thing is an erroneous hangup of the channel.

Not only is the test incorrectly applied, it appears to be unnecessary since at the point at which loopback_exec() is called it has already been determined that the switch should fire.

So, my patch fixes two bugs.  Both cause things like this:

[dundi-e164-customers]
lswitch => Loopback/${EXTEN:1}@dids/_1.

to fail.  (The purpose of this dialplan fragment is to advertise a context full of 10-digit NANP numbers using DUNDi.)  The first bug prevents the switch from ever firing because ${EXTEN:1} evaluations to "" (due to incorrect construction of the linked list) which is replaced with ${EXTEN} (by code which does not use the defective linked list) which doesn't match anything in context [dids].  Once we fix the first bug, the second bug causes a hangup because after the switch rewrites say 18605551212 to 8605551212 the rewritten number no longer matches _1.
By: Digium Subversion (svnbot) 2008-06-03 17:34:38

Repository: asterisk
Revision: 120226

U   branches/1.4/pbx/pbx_loopback.c

------------------------------------------------------------------------
r120226 | tilghman | 2008-06-03 17:34:37 -0500 (Tue, 03 Jun 2008) | 8 lines

Due to incorrect use of the AST_LIST_INSERT_HEAD() macro the loopback switch
cannot perform any translation on the extension number before searching for it
in the target context.
(closes issue ASTERISK-11875)
Reported by: chappell
Patches:
      pbx_loopback.c.diff uploaded by chappell (license 8)

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

http://svn.digium.com/view/asterisk?view=rev&revision=120226
By: Digium Subversion (svnbot) 2008-06-03 17:35:32

Repository: asterisk
Revision: 120227

_U  trunk/
U   trunk/pbx/pbx_loopback.c

------------------------------------------------------------------------
r120227 | tilghman | 2008-06-03 17:35:32 -0500 (Tue, 03 Jun 2008) | 16 lines

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

........
r120226 | tilghman | 2008-06-03 17:41:04 -0500 (Tue, 03 Jun 2008) | 8 lines

Due to incorrect use of the AST_LIST_INSERT_HEAD() macro the loopback switch
cannot perform any translation on the extension number before searching for it
in the target context.
(closes issue ASTERISK-11875)
Reported by: chappell
Patches:
      pbx_loopback.c.diff uploaded by chappell (license 8)

........

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

http://svn.digium.com/view/asterisk?view=rev&revision=120227
By: Digium Subversion (svnbot) 2008-06-03 17:36:09

Repository: asterisk
Revision: 120228

_U  branches/1.6.0/
U   branches/1.6.0/pbx/pbx_loopback.c

------------------------------------------------------------------------
r120228 | tilghman | 2008-06-03 17:36:08 -0500 (Tue, 03 Jun 2008) | 24 lines

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

................
r120227 | tilghman | 2008-06-03 17:42:03 -0500 (Tue, 03 Jun 2008) | 16 lines

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

........
r120226 | tilghman | 2008-06-03 17:41:04 -0500 (Tue, 03 Jun 2008) | 8 lines

Due to incorrect use of the AST_LIST_INSERT_HEAD() macro the loopback switch
cannot perform any translation on the extension number before searching for it
in the target context.
(closes issue ASTERISK-11875)
Reported by: chappell
Patches:
      pbx_loopback.c.diff uploaded by chappell (license 8)

........

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

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

http://svn.digium.com/view/asterisk?view=rev&revision=120228
By: Digium Subversion (svnbot) 2008-06-03 19:27:35

Repository: asterisk
Revision: 120279

_U  team/seanbright/resolve-shadow-warnings/
U   team/seanbright/resolve-shadow-warnings/CHANGES
U   team/seanbright/resolve-shadow-warnings/Makefile
U   team/seanbright/resolve-shadow-warnings/apps/app_queue.c
U   team/seanbright/resolve-shadow-warnings/channels/chan_iax2.c
D   team/seanbright/resolve-shadow-warnings/configs/pbx_realtime.conf
U   team/seanbright/resolve-shadow-warnings/funcs/func_channel.c
U   team/seanbright/resolve-shadow-warnings/include/asterisk/options.h
U   team/seanbright/resolve-shadow-warnings/main/asterisk.c
U   team/seanbright/resolve-shadow-warnings/main/config.c
U   team/seanbright/resolve-shadow-warnings/main/pbx.c
U   team/seanbright/resolve-shadow-warnings/pbx/pbx_loopback.c
U   team/seanbright/resolve-shadow-warnings/pbx/pbx_realtime.c
U   team/seanbright/resolve-shadow-warnings/res/res_agi.c

------------------------------------------------------------------------
r120279 | seanbright | 2008-06-03 19:27:31 -0500 (Tue, 03 Jun 2008) | 90 lines

Merged revisions 120166,120169,120171,120174,120227,120230 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r120166 | mmichelson | 2008-06-03 17:22:52 -0400 (Tue, 03 Jun 2008) | 13 lines

Adding two new queue log events. The ADDMEMBER event is logged when
a dynamic realtime queue member is added to the queue, and the
REMOVEMEMBER event is logged when a dynamic realtime member is
removed. Since no calling channel is associated with these events
the string "REALTIME" is placed where the channel's unique id is
normally placed.

(closes issue ASTERISK-12128)
Reported by: atis
Patches:
     queue_log_rt_members.patch uploaded by atis (license 242)


................
r120169 | russell | 2008-06-03 17:35:11 -0400 (Tue, 03 Jun 2008) | 12 lines

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

........
r120168 | russell | 2008-06-03 16:34:55 -0500 (Tue, 03 Jun 2008) | 4 lines

Fix another place where peer->callno could change at a very bad time, and also
fix a place where a peer was used after the reference was released.
(inspired by rev 120001)

........

................
r120171 | tilghman | 2008-06-03 18:05:16 -0400 (Tue, 03 Jun 2008) | 5 lines

Move compatibility options into asterisk.conf, default them to on for upgrades,
and off for new installations.  This includes the translation from pipes to commas
for pbx_realtime and the EXEC command for AGI, as well as the change to the Set
application not to support multiple variables at once.

................
r120174 | jpeeler | 2008-06-03 18:17:07 -0400 (Tue, 03 Jun 2008) | 14 lines

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

........
r120173 | jpeeler | 2008-06-03 17:15:33 -0500 (Tue, 03 Jun 2008) | 6 lines

(closes issue ASTERISK-11077)
Reported by: yem
Tested by: yem

This change decreases the buffer size allocated on the stack substantially in config_text_file_load when LOW_MEMORY is turned on. This change combined with the fix from revision 117462 (making mkintf not copy the zt_chan_conf structure) was enough to prevent the crash.

........

................
r120227 | tilghman | 2008-06-03 18:42:03 -0400 (Tue, 03 Jun 2008) | 16 lines

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

........
r120226 | tilghman | 2008-06-03 17:41:04 -0500 (Tue, 03 Jun 2008) | 8 lines

Due to incorrect use of the AST_LIST_INSERT_HEAD() macro the loopback switch
cannot perform any translation on the extension number before searching for it
in the target context.
(closes issue ASTERISK-11875)
Reported by: chappell
Patches:
      pbx_loopback.c.diff uploaded by chappell (license 8)

........

................
r120230 | tilghman | 2008-06-03 19:17:33 -0400 (Tue, 03 Jun 2008) | 7 lines

Add a function, CHANNELS(), which retrieves a list of all active channels.
(closes issue ASTERISK-10844)
Reported by: rain
Patches:
      func_channel-channel_list_function.diff uploaded by rain (license 327)
      (with some additional changes by me, mostly to meet coding guidelines)

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

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

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