[Asterisk-code-review] core_local: Fix local channel parsing with slashes (asterisk[master])

N A asteriskteam at digium.com
Tue Apr 26 14:47:06 CDT 2022


Attention is currently required from: Joshua Colp, Richard Mudgett.
N A has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/18369 )

Change subject: core_local: Fix local channel parsing with slashes
......................................................................


Patch Set 3:

(1 comment)

File main/core_local.c:

https://gerrit.asterisk.org/c/asterisk/+/18369/comment/de59998f_dc98f15b 
PS3, Line 907: 	if ((opts = strrchr(parse, '/')) && really_local_options(opts)) {
> That's certainly possible.
Okay, so it seems the problem with this is that local_devicestate gets called without any options (as there shouldn't be any). However, it has a check to remove everything after the last slash, so even with strrchr, it will do something like this:

[2022-04-26 19:40:01] NOTICE[25922]: core_local.c:319 local_devicestate: exten was PJSIP/ATAxGrandstream1 at astrex-outgoing
[2022-04-26 19:40:01] NOTICE[25922]: core_local.c:324 local_devicestate: exten was PJSIP
[2022-04-26 19:40:01] WARNING[25922]: core_local.c:328 local_devicestate: Someone used Local/PJSIP/ATAxGrandstream1 at astrex-outgoing somewhere without a @context. This is bad.

The actual dialing part works all right with the dummy /z option at the end, but the devicestate function isn't getting that.


My main thought right now is that if there are options, it should appear after the '@'. However, there is also a check to see that there is an '@' in the first place. In other words, we can't rely on there being either an '/' or an '@' present, apparently.

So to avoid doing anything hacky here, I'm thinking something like:

IF an '@' is present, then check for options starting at the '@', to the end of the string. (Right now, the reverse is done, first check for '/', then '@')
Else, '@' is not present, so we're malformed anyways. So perhaps just do it the way we do now, since this is an edge case as it is already that means something has gone wrong.

Any thoughts?



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/18369
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I8e85bc14072e452d0537a801aba17779569d0360
Gerrit-Change-Number: 18369
Gerrit-PatchSet: 3
Gerrit-Owner: N A <mail at interlinked.x10host.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Attention: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Tue, 26 Apr 2022 19:47:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Joshua Colp <jcolp at sangoma.com>
Comment-In-Reply-To: N A <mail at interlinked.x10host.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220426/f90899b3/attachment.html>


More information about the asterisk-code-review mailing list