[Asterisk-Dev] H.323 channel driver problems

[joachim]

Thank you for your contributions!!!!
Please post all of this on bugs.digium.com (look at the bug and diff
guidelines), this way jeremy and others will be able to reply to your
findings and fixes.

Thanks,

Zoa.


Chih-Wei Huang wrote:

>Hi all,
>This is my first mail to the mailing list.
>I'm a developer of VoIP, especially in H.323 field,
>but I'm totally new to Asterisk.
>Recently I began to play Asterisk, including the H323 driver.
>I could play IVR, transfer a call, and make an H323 call to
>SIP client, and vice versa. What a nice project!!
>
>The version I have tested is 1.0.5.
>As Jeremy(the H323 driver author) suggested, I began
>to play the CVS head version.
>But the latest H323 driver seems totally broken.
>Neither inbound nor outbound call can work.
>I feel silly about the fact. Here are the problems I found:
>
>* Outbound call problem
>
>There are several problems in
>oh323_call of chan_h323.c. First of all, the line
>
>        memcpy(addr, dest, strlen(addr));
>
>strlen(addr) should be strlen(dest), or addr will always be
>empty and Open323 stack will complain that an invalid URL.
>Furthermore, using memcpy in this way is not good since
>it's a potential buffer overflow problem.
>I suggest use strncpy:
>
>	strncpy(addr, dest, INET_ADDRSTRLEN - 1);
>
>(In fact I doubt why copies dest. It seems totally unnecessary)
>
>Second, the following block has many problems:
>
>	if (pvt->exten) {
>		// case 1
>                snprintf(called_addr, sizeof(called_addr), "%s@%s:%d",
>pvt->exten, addr, pvt->options.port);
>        } else {
>		// case 2
>                snprintf(called_addr, sizeof(called_addr), "%s:%d",addr,
>pvt->options.port);
>        }
>
>The if condition is strange. As I tested, pvt->exten was not null but
>pointed to an empty string. So you would get a called_addr like
>'@addr:port', which is invalid to Openh323. I think it should be
>
>	if (!ast_strlen_zero(pvt->exten)) { // non empty extention
>
>Further, both case 1 and 2 form the destination in this form
>'addr:port', which is only valid if addr is an IP address.
>However, in my tested cases, addr(original dest) is not an IP,
>but an extention number(like 601). So it is not a valid destination.
>
>Moreover, the form 'addr:port' is not an appropriate destination
>when using with a gatekeeper. The port should be determined by the
>gatekeeper in ARQ/ACF procedure. The client should not specified
>the port itself. In fact if you use this form of destination in ARQ,
>you will get ARJ with calledPartyNotRegistered.
>
>Since I don't know what exact the logic and duty that block wants to do,
>I could not provide a proper patch. I just mentioned the problems I
>observed: you have to consider
>
> * whether addr is a valid IP address
> * whether using with a gatekeeper
>
>to form the final destination.
>
>In fact, I just change the final destination to original dest
>and everything works fine:
>
>   res = h323_make_call(dest, &(pvt->cd), &pvt->options);
>
>
>* Inbound call problem
>
>It will never succeed because setup_incoming_call
>doesn't return a valid pointer in some cases. The function structured
>like this:
>
>        if ((!strcasecmp(cd.sourceIp, gatekeeper) ... ) { // usingGK
>		if (...) {
>			// case 1
>		} else {
>			// case 2
>		}
>	} else {
>		if (...) {
>			// case 3
>		} else {
>			// case 4
>		}
>	}
>
>Only in case 4, a proper pointer call_options is returned.
>Since I'm using a gatekeeper (case 1 & 2), I can never receive a call.
>I don't know how to generate a valid pointer call_options.
>I just tried to add the line to case 1 & 2 and it seems work.
>
>	call_options = &pvt->options
>
>I hope someone can explain the logic so a proper fix can be found.
>
>
>To be continued...
>
>
>
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : http://lists.digium.com/pipermail/asterisk-dev/attachments/20050317/29ab0faf/signature.pgp