h323ep.cxx - Number translation problems.
A little background info: We use yate with h323plus to make calls, currently h323+ 1.25.0, yate 5.5.
In our system we use '#' extensively to delimit routing info ( as we use variable length prefixes, this makes them unique and we can distinguish between 10+347... and 103+47....).
But one day we used ## in a special prefix and the calls began to fail. We saw yate thar when dialing "0##666#88820334661" we ended up with the following ( every phne number redacted, cut last six numbers of phones for privacy ) pdu in the gatekeeper:
admissionRequest { requestSeqNum = 32114 callType = pointToPoint <<null>> endpointIdentifier = 12 characters { .... } destinationInfo = 1 entries { [0]=url_ID "666%2388820334661@0" } ..... }
which lead to a rejected call. We expected something on the line ( from another PDU without double hash ):
admissionRequest { ... destinationInfo = 1 entries { [0]=dialedDigits "100#00543414" } ... }
like:
destinationInfo = 1 entries { [0]=dialedDigits "0##666#88820334661" }
an url instead of "0#34666666666" to the gatekeeper. After further examination we found the following in h323+ 1.25.0:
[src]$ fgrep -R -C7 '"##"' h323plus/ h323plus/src/h323ep.cxx-PBoolean H323EndPoint::ParsePartyName(const PString & _remoteParty, h323plus/src/h323ep.cxx- PString & alias, h323plus/src/h323ep.cxx- H323TransportAddress & address) h323plus/src/h323ep.cxx-{ h323plus/src/h323ep.cxx- PString remoteParty = _remoteParty; h323plus/src/h323ep.cxx- h323plus/src/h323ep.cxx- // Support [address]##[Alias] dialing h323plus/src/h323ep.cxx: PINDEX hash = _remoteParty.Find("##"); h323plus/src/h323ep.cxx- if (hash != P_MAX_INDEX) { h323plus/src/h323ep.cxx- remoteParty = "h323:" + _remoteParty.Mid(hash+2) + "@" + _remoteParty.Left(hash); h323plus/src/h323ep.cxx- PTRACE(4, "H323\tConverted " << _remoteParty << " to " << remoteParty); h323plus/src/h323ep.cxx- } h323plus/src/h323ep.cxx- h323plus/src/h323ep.cxx- //check if IPv6 address h323plus/src/h323ep.cxx- PINDEX ipv6 = remoteParty.Find("::");
Well, it seems some function which should ( IMO ) be in the gatekeeper or the dialer as been grafted inside the generic endpoint code. Not too big a problem. We just developed a patch to zap the code patch and followed on. But then, we had a look ar h323plus 1.25.4 and found:
PString remoteParty = _remoteParty; // Support [address]##[Alias] dialing PINDEX hash = _remoteParty.Find("##"); if (hash != P_MAX_INDEX) { remoteParty = "h323:" + _remoteParty.Mid(hash+2) + "@" + _remoteParty.Left(hash); PTRACE(4, "H323\tConverted " << _remoteParty << " to " << remoteParty); } PINDEX phash = _remoteParty.Find("#"); if (phash != P_MAX_INDEX) { remoteParty.Replace("#","%"); PTRACE(4, "H323\tAdjusted " << remoteParty); } //check if IPv6 address PINDEX ipv6 = remoteParty.Find("::");
Which is worst, now we can even use a single #. I wonder what is the rationale for doing this kind of transformations. We are developing another patch for this, but fear the future. For me it looks like this is trying to overcome the limitations of some device so the user can dial URLs ? IP ? from the phone keyboard, but on the way they are destroying our routing, and as they are buried deep inside the lib ( where I personally think they shouldn't be ) they are tricky to detect and fix.
Shouldn't this kind of legal-number-destroying functionality be relegated to an overridable method, or just delegated to the client code? Is this going to go on ( I mean, more exotic translations on otherwise legal dialedDigits ) ? And I'm not the only one using '#' in the digits, some of our providers do it too.
Regards. Francisco Olarte.
Hi Francisco,
I would agree that this kind of transformation should not be forced onto every application.
My suggestion would be that you make a patch so applications can decide if they want the transformation or not. And then we add the patch into H323Plus instead of you keeping a permanent diff.
Simon, maybe you can weight in. I'm pretty sure you know applications that rely on the current behavior. ;-)
Regards, Jan
Jan
Yes you are correct. "##" is commonly used by polycom devices to delimitate extensions behind a gatekeeper or gateway.
So you might dial 567890##1234 which indicates you are dialing extension 1234 behind gateway/gatekeeper at number 567890.
I'm seriously loathed to change the default behavior because it is widely used however I might add a setting to disable that behavior if required. I will try to have something checked in this weekend.
Simon
-----Original Message----- From: h323plus [mailto:h323plus-bounces@lists.packetizer.com] On Behalf Of Jan Willamowius Sent: Thursday, July 23, 2015 12:51 AM To: h323plus@lists.packetizer.com Subject: Re: [h323plus] h323ep.cxx - Number translation problems.
Hi Francisco,
I would agree that this kind of transformation should not be forced onto every application.
My suggestion would be that you make a patch so applications can decide if they want the transformation or not. And then we add the patch into H323Plus instead of you keeping a permanent diff.
Simon, maybe you can weight in. I'm pretty sure you know applications that rely on the current behavior. ;-)
Regards, Jan
-- Jan Willamowius, jan@willamowius.de, http://www.gnugk.org/
Francisco Olarte wrote:
A little background info: We use yate with h323plus to make calls, currently h323+ 1.25.0, yate 5.5.
In our system we use '#' extensively to delimit routing info ( as we use variable length prefixes, this makes them unique and we can distinguish between 10+347... and 103+47....).
But one day we used ## in a special prefix and the calls began to fail. We saw yate thar when dialing "0##666#88820334661" we ended up with the following ( every phne number redacted, cut last six numbers of phones for privacy ) pdu in the gatekeeper:
admissionRequest { requestSeqNum = 32114 callType = pointToPoint <<null>> endpointIdentifier = 12 characters { .... } destinationInfo = 1 entries { [0]=url_ID "666%2388820334661@0" } ..... }
which lead to a rejected call. We expected something on the line ( from another PDU without double hash ):
admissionRequest { ... destinationInfo = 1 entries { [0]=dialedDigits "100#00543414" } ... }
like:
destinationInfo = 1 entries { [0]=dialedDigits "0##666#88820334661" }
an url instead of "0#34666666666" to the gatekeeper. After further examination we found the following in h323+ 1.25.0:
[src]$ fgrep -R -C7 '"##"' h323plus/ h323plus/src/h323ep.cxx-PBoolean H323EndPoint::ParsePartyName(const PString & _remoteParty, h323plus/src/h323ep.cxx- PString & alias, h323plus/src/h323ep.cxx- H323TransportAddress & address) h323plus/src/h323ep.cxx-{ h323plus/src/h323ep.cxx- PString remoteParty = _remoteParty; h323plus/src/h323ep.cxx- h323plus/src/h323ep.cxx- // Support [address]##[Alias] dialing h323plus/src/h323ep.cxx: PINDEX hash = _remoteParty.Find("##"); h323plus/src/h323ep.cxx- if (hash != P_MAX_INDEX) { h323plus/src/h323ep.cxx- remoteParty = "h323:" + _remoteParty.Mid(hash+2) + "@" + _remoteParty.Left(hash); h323plus/src/h323ep.cxx- PTRACE(4, "H323\tConverted " << _remoteParty << " to " << remoteParty); h323plus/src/h323ep.cxx- } h323plus/src/h323ep.cxx- h323plus/src/h323ep.cxx- //check if IPv6 address h323plus/src/h323ep.cxx- PINDEX ipv6 = remoteParty.Find("::");
Well, it seems some function which should ( IMO ) be in the gatekeeper or the dialer as been grafted inside the generic endpoint code. Not too big a problem. We just developed a patch to zap the code patch and followed on. But then, we had a look ar h323plus 1.25.4 and found:
PString remoteParty = _remoteParty; // Support [address]##[Alias]
dialing
PINDEX hash = _remoteParty.Find("##"); if (hash != P_MAX_INDEX) { remoteParty = "h323:" + _remoteParty.Mid(hash+2) + "@" + _remoteParty.Left(hash); PTRACE(4, "H323\tConverted " << _remoteParty << " to " << remoteParty); } PINDEX phash = _remoteParty.Find("#"); if (phash != P_MAX_INDEX) { remoteParty.Replace("#","%"); PTRACE(4, "H323\tAdjusted " << remoteParty); } //check if IPv6 address PINDEX ipv6 = remoteParty.Find("::");
Which is worst, now we can even use a single #. I wonder what is the rationale for doing this kind of transformations. We are developing another patch for this, but fear the future. For me it looks like this is trying to overcome the limitations of some device so the user can dial URLs ? IP ? from the phone keyboard, but on the way they are destroying our routing, and as they are buried deep inside the lib ( where I personally think they shouldn't be ) they are tricky to detect and fix.
Shouldn't this kind of legal-number-destroying functionality be relegated to an overridable method, or just delegated to the client code? Is this going to go on ( I mean, more exotic translations on otherwise legal dialedDigits ) ? And I'm not the only one using '#' in the digits, some of our providers do it too.
Regards. Francisco Olarte.
Simon,
different kinds of applications have different needs: Softphone type applications will probably want the rewrite, while gateway type applications probably don't (and will be somewhat surprised by the new 'feature'). Both use the Endpoint class. Thus we'll need a way to select the behavior.
Regards, Jan
Simon Horne wrote:
Jan
Yes you are correct. "##" is commonly used by polycom devices to delimitate extensions behind a gatekeeper or gateway.
So you might dial 567890##1234 which indicates you are dialing extension 1234 behind gateway/gatekeeper at number 567890.
I'm seriously loathed to change the default behavior because it is widely used however I might add a setting to disable that behavior if required. I will try to have something checked in this weekend.
Simon
-----Original Message----- From: h323plus [mailto:h323plus-bounces@lists.packetizer.com] On Behalf Of Jan Willamowius Sent: Thursday, July 23, 2015 12:51 AM To: h323plus@lists.packetizer.com Subject: Re: [h323plus] h323ep.cxx - Number translation problems.
Hi Francisco,
I would agree that this kind of transformation should not be forced onto every application.
My suggestion would be that you make a patch so applications can decide if they want the transformation or not. And then we add the patch into H323Plus instead of you keeping a permanent diff.
Simon, maybe you can weight in. I'm pretty sure you know applications that rely on the current behavior. ;-)
Regards, Jan
-- Jan Willamowius, jan@willamowius.de, http://www.gnugk.org/
Francisco Olarte wrote:
A little background info: We use yate with h323plus to make calls, currently h323+ 1.25.0, yate 5.5.
In our system we use '#' extensively to delimit routing info ( as we use variable length prefixes, this makes them unique and we can distinguish between 10+347... and 103+47....).
But one day we used ## in a special prefix and the calls began to fail. We saw yate thar when dialing "0##666#88820334661" we ended up with the following ( every phne number redacted, cut last six numbers of phones for privacy ) pdu in the gatekeeper:
admissionRequest { requestSeqNum = 32114 callType = pointToPoint <<null>> endpointIdentifier = 12 characters { .... } destinationInfo = 1 entries { [0]=url_ID "666%2388820334661@0" } ..... }
which lead to a rejected call. We expected something on the line ( from another PDU without double hash ):
admissionRequest { ... destinationInfo = 1 entries { [0]=dialedDigits "100#00543414" } ... }
like:
destinationInfo = 1 entries { [0]=dialedDigits "0##666#88820334661" }
an url instead of "0#34666666666" to the gatekeeper. After further examination we found the following in h323+ 1.25.0:
[src]$ fgrep -R -C7 '"##"' h323plus/ h323plus/src/h323ep.cxx-PBoolean H323EndPoint::ParsePartyName(const PString & _remoteParty, h323plus/src/h323ep.cxx- PString & alias, h323plus/src/h323ep.cxx- H323TransportAddress & address) h323plus/src/h323ep.cxx-{ h323plus/src/h323ep.cxx- PString remoteParty = _remoteParty; h323plus/src/h323ep.cxx- h323plus/src/h323ep.cxx- // Support [address]##[Alias] dialing h323plus/src/h323ep.cxx: PINDEX hash = _remoteParty.Find("##"); h323plus/src/h323ep.cxx- if (hash != P_MAX_INDEX) { h323plus/src/h323ep.cxx- remoteParty = "h323:" + _remoteParty.Mid(hash+2) + "@" + _remoteParty.Left(hash); h323plus/src/h323ep.cxx- PTRACE(4, "H323\tConverted " << _remoteParty << " to " << remoteParty); h323plus/src/h323ep.cxx- } h323plus/src/h323ep.cxx- h323plus/src/h323ep.cxx- //check if IPv6 address h323plus/src/h323ep.cxx- PINDEX ipv6 = remoteParty.Find("::");
Well, it seems some function which should ( IMO ) be in the gatekeeper or the dialer as been grafted inside the generic endpoint code. Not too big a problem. We just developed a patch to zap the code patch and followed on. But then, we had a look ar h323plus 1.25.4 and found:
PString remoteParty = _remoteParty; // Support [address]##[Alias]
dialing
PINDEX hash = _remoteParty.Find("##"); if (hash != P_MAX_INDEX) { remoteParty = "h323:" + _remoteParty.Mid(hash+2) + "@" + _remoteParty.Left(hash); PTRACE(4, "H323\tConverted " << _remoteParty << " to " << remoteParty); } PINDEX phash = _remoteParty.Find("#"); if (phash != P_MAX_INDEX) { remoteParty.Replace("#","%"); PTRACE(4, "H323\tAdjusted " << remoteParty); } //check if IPv6 address PINDEX ipv6 = remoteParty.Find("::");
Which is worst, now we can even use a single #. I wonder what is the rationale for doing this kind of transformations. We are developing another patch for this, but fear the future. For me it looks like this is trying to overcome the limitations of some device so the user can dial URLs ? IP ? from the phone keyboard, but on the way they are destroying our routing, and as they are buried deep inside the lib ( where I personally think they shouldn't be ) they are tricky to detect and fix.
Shouldn't this kind of legal-number-destroying functionality be relegated to an overridable method, or just delegated to the client code? Is this going to go on ( I mean, more exotic translations on otherwise legal dialedDigits ) ? And I'm not the only one using '#' in the digits, some of our providers do it too.
Regards. Francisco Olarte.
Hi Jan:
On Wed, Jul 22, 2015 at 7:20 PM, Jan Willamowius jan@willamowius.de wrote:
Simon, different kinds of applications have different needs: Softphone type applications will probably want the rewrite, while gateway type applications probably don't (and will be somewhat surprised by the new 'feature'). Both use the Endpoint class. Thus we'll need a way to select the behavior.
That was what I was trying to explain / inquiry about.
Even in a softphone, I do not see the point, softphones have a keyboard and can directly dial a@B instead of B##a. If the softphone wants a 'polycom mode' because polycom phones do that translation when dialing on a keypad, it should do it before passing the number to the H323endpoint ( it may derive & wrap, but I do not think that is correct ).
I do not even understand where the @ is used. If it is used in some kind of polycom gateway/pbx then this chunk of code would only be useful to be able to dial into it from an h323+ app which receives pure dialed digits ( like, let's say, an h323-isdn gw ). This application could very easily do it before passing the remoteParty into makeCall. I fail to see a real use case for this.
And, if a gateway is using it, it should do the translation by itself.
I find this kind of magic transformations deeply disturbing, as they forbid by default ( and in the current case, unless patched ) the usage of perfectly legal ( and used, at least we've been routing all calls here using them ) combinations. Luckily for me I could manage to redesign my routing prefixes to avoid ## when I was bitten. But 1.26 also forbids #, which it translates to %. I doubt I can transform all the routing patterns we've been putting in place during the last 15 years to avoid '#'. I could try '*', but it may be the next victim.
My position would be more along the way of putting a couple of empty virtual hooks in the H323Endpoint class and, as a concession to the polycom users, include a H323polycomEndpoint derived class with the magic in place, so library users are perfectly aware of what they can expect. This seem like we are tailoring the library, mutilating it to accomodate some special cases. To me it feels like if we patched libC to make strlen ( and strcmp/cpy ) ignore trailing spaces because some users do not know how to rtrim() their strings.
Regards. Francisco Olarte.
Hi,
I can understand the reasoning behind adding the rewrite for softphones.
The room@IP (or IP##room) syntax is used a lot when dialing into MCUs for example (a lot as as in 80% of what some users dial). Since there is no consensus between the major vendors what is the 'right' syntax, most end users are utterly confused what to use. So even when the are able to dial the @, they might dial ## because they always did before...
Changing existing behavior in a framework is a very tricky decision. And H323Plus has always put a lot of effort into maintaining compatibility. But it is also a good thing if not every application has to code the same thing and the framework provides these bits and pieces that many applications want.
My preferred solution would be to factor rewrites as this one into a separate endpoint method ("ConvenienceRewrites()" ?) and have a switch if its called or not. I would default the switch to not do the rewrite, but since none of my apps is hurt either way, I'll leave it to you guys to battle out the implementation. ;-)
Regards, Jan
Francisco Olarte wrote:
Hi Jan:
On Wed, Jul 22, 2015 at 7:20 PM, Jan Willamowius jan@willamowius.de wrote:
Simon, different kinds of applications have different needs: Softphone type applications will probably want the rewrite, while gateway type applications probably don't (and will be somewhat surprised by the new 'feature'). Both use the Endpoint class. Thus we'll need a way to select the behavior.
That was what I was trying to explain / inquiry about.
Even in a softphone, I do not see the point, softphones have a keyboard and can directly dial a@B instead of B##a. If the softphone wants a 'polycom mode' because polycom phones do that translation when dialing on a keypad, it should do it before passing the number to the H323endpoint ( it may derive & wrap, but I do not think that is correct ).
I do not even understand where the @ is used. If it is used in some kind of polycom gateway/pbx then this chunk of code would only be useful to be able to dial into it from an h323+ app which receives pure dialed digits ( like, let's say, an h323-isdn gw ). This application could very easily do it before passing the remoteParty into makeCall. I fail to see a real use case for this.
And, if a gateway is using it, it should do the translation by itself.
I find this kind of magic transformations deeply disturbing, as they forbid by default ( and in the current case, unless patched ) the usage of perfectly legal ( and used, at least we've been routing all calls here using them ) combinations. Luckily for me I could manage to redesign my routing prefixes to avoid ## when I was bitten. But 1.26 also forbids #, which it translates to %. I doubt I can transform all the routing patterns we've been putting in place during the last 15 years to avoid '#'. I could try '*', but it may be the next victim.
My position would be more along the way of putting a couple of empty virtual hooks in the H323Endpoint class and, as a concession to the polycom users, include a H323polycomEndpoint derived class with the magic in place, so library users are perfectly aware of what they can expect. This seem like we are tailoring the library, mutilating it to accomodate some special cases. To me it feels like if we patched libC to make strlen ( and strcmp/cpy ) ignore trailing spaces because some users do not know how to rtrim() their strings.
Regards. Francisco Olarte.
Hi Jan:
On Wed, Jul 22, 2015 at 8:30 PM, Jan Willamowius jan@willamowius.de wrote:
I can understand the reasoning behind adding the rewrite for softphones. The room@IP (or IP##room) syntax is used a lot when dialing into MCUs for example (a lot as as in 80% of what some users dial). Since there is no consensus between the major vendors what is the 'right' syntax, most end users are utterly confused what to use. So even when the are able to dial the @, they might dial ## because they always did before...
I understand a little bit more. And I fully agree into having that translation into a softphone, and if I had to make a softphone I'll probably put it ( in the softphone ).
And I could even agree to have it into a library, but conveniently isolated and not into the base class, specially when it conflicts with some legal uses of ##. And that is not the major offender, current versions ( since 1.25.4 ) change # to % if I read the code correctly.
Changing existing behavior in a framework is a very tricky decision. And H323Plus has always put a lot of effort into maintaining compatibility.
Not so much. According to our bisections since h323+ 1.25.4 we have:
PINDEX phash = _remoteParty.Find("#"); if (phash != P_MAX_INDEX) { remoteParty.Replace("#","%"); PTRACE(4, "H323\tAdjusted " << remoteParty); }
So I cannot use # in any place in any number ( as soon as I put one, even if it is a sacrificial one at the front, it's going to get changed to % and my remoteParty is no longer able to fit into the dialedDigits choice of an AliasAddress.
We have not hit it because we base our updates in the current version. Last one was in 1.25.0. Current one is trying to be based in 1.26.5 ( we couldn't do it cause we hit another bug in the way the gkclient ras code manages ip address, due to a change which we are preparing to report ), and we have it frozen till we finish the ## / # patches, as the second one is changing existing behaviour and destroying us.
..But it is also a good thing if not every application has to code the same thing and the framework provides these bits and pieces that many applications want.
Completely agreed. Having a slew of properly maintained methods in the library is nice, keeps errors out, pervents duplicate efforts, but...,
My preferred solution would be to factor rewrites as this one into a separate endpoint method ("ConvenienceRewrites()" ?) and have a switch if its called or not. I would default the switch to not do the rewrite, but since none of my apps is hurt either way, I'll leave it to you guys to battle out the implementation. ;-)
Will do. I would also prefer the naked library to not rewrite, in fact I would default the method to a hook ( called but NOP one ), and make a derived 'softphoneEndpoint' with the fleshed out hook and some settings ( as we have three behaviours, no-rewrite, 1.25 mode, 1.26 mode, setting that would need IMO an 'applied rewrites' with some commonly used predefined values, implemented in any sensible way )
Also, as I previously stated, the hook method seems like a great point to put it for me, even if the default one destroys #. This means I can maintain a single patch to deactivate itand be reasonable confindent any future number destroying backwards incompatible transformation have a place to be put which will not need new patches ( as I would patch away the method call or derive and zap it ).
I'll wait for some feedback from Simon and do what I can to have a better, greater and prettier h323+, as I think this kind of changes are bad for its future ( they steer it towards being a niche library for softphones and users who are incapable of transforming the numbers in the proper place ( I mean 'softphone developers who are not able to apply the transformations before passing alias/numbers to the library' ), I'm all for helping them as long as it does not hurt other users ).
Francisco Olarte.
Simon:
On Wed, Jul 22, 2015 at 5:05 PM, Simon Horne s.horne@packetizer.com wrote:
Jan Yes you are correct. "##" is commonly used by polycom devices to delimitate extensions behind a gatekeeper or gateway.
So you might dial 567890##1234 which indicates you are dialing extension 1234 behind gateway/gatekeeper at number 567890.
I do not understand the use case. You dial that on a polycom phone to access something behind a ¿ polycom ? gateway/gatekeeper ? or the polycom devices automatically dial this? Because h323ep is for use in something like softphones. In softphones you can dial @ easily.
I'm seriously loathed to change the default behavior because it is widely used however I might add a setting to disable that behavior if required. I will try to have something checked in this weekend.
I will maintain my diff meanwhile, as I need to dial # ( not ## now, but # is changed to % too ).
I'm strongly against forcing this kind of behaviour on every user by default. I mean, it seems to be acommodating some devices/programs which cannot dial 1234@567890, which I understand, but on the way, on its current incarnation, is making it imposible to use ## ( and, in 1.26.5 at least, a single # ) as dialed digits, which I use, know other people that uses and think its a legitimate use of the non-digit possible values of dialedDigits ( in fact, if I hadn't look at the 1.26 sources to see if ## translation was still there ALL my routing would have failed if I had upgraded to 1.26, or 1.25.5. It seems h323+ is being tailored to some special needs disregarding its use as a building block ).
Having said it, if you want it there, and enabled by default, I would ask for a change along the lines of isolating the special handling of remoteParty into a virtual method, something like calling 'remoteParty=specialPartyNameTransformer(remoteParty)'. This makes it easy to patch it away ( which I will need to do ) or to override it in a derived class ( which, in the case I found the problem, compiling Yate with h323+, can also be done with a little patch by overriding the transformer method to return the argument in the YateH323Endpoint class ). I also think this would make the code more clear and modular. I can try to make the patch if you are too busy, I'm not as familiar as you with the H323ep inner workings but this is just string transformation stuff.
I would still like to have the code factored out even if you add a setting to disable the behaviour, as I need a ( patched ) library which respect my numbers to work with, as not all the programs I use can be easily patched / modified to change the setting. Also, if the setting is just used to conditionalize the remoteParty transformations it is easy to get confused and do something like '*#*' => '.@.' in the future and put it outside the conditional path. And also, I've found that for this kind of transformation a method is better, as once you hit the pattern of transformation you are looking for you can return the transformed value if further patterns need not be applied, or must be avoided.
Regards. Francisco Olarte.
participants (3)
-
Francisco Olarte
-
Jan Willamowius
-
Simon Horne