[Devel] [patch] Nathelper - very minor bug

[John Riordan]

Hi,

Yes, I remember you from astricon last year. Small world.

BTW: We think the openser effort it great. We're migrating from ser.

Anyway, I'm already doing as you suggest. The problem is there.

This is a snipit from nathelper.c starting line 909 in replace_sdp_ip()


	for(;;) {
		if (extract_mediaip(&body1, &oldip, &pf,line) == -1)
			break;
	...
	}

The loop iterates calling extract_mediaip to finding the next c= line
on each iteration and replacing ips. It does so until extract_mediaip
cannot find another c= line - indicated when it returns -1.

Unfortunately, extract_mediaip spits out an error message when it can't
find another c= line. That is, it ALWAYS spits out a message at L_ERR
even in messages with sdp. It spits out a message when there are multiple
c= lines in the sdp.

The ser 0.9.4 code logs it correctly

in ser 0.9.4 extract_mediaip()

	if (cp1 == NULL) {
		LOG(L_DBG, "ERROR: extract_mediaip: no `c=' in SDP\n");
		return -1;
	}

in openser dev extract_mediaip()

	if (cp1 == NULL) {
		LOG(L_ERR, "ERROR: extract_mediaip: no `%s' in SDP\n",line);
		return -1;
	}

Regards,

John


Glenn Dalgliesh wrote:
> I don't think this is a bug I think you are trying to apply 
> fix_nated_sdp to all sip messages and not just the ones with SDP in 
> them. For example you need to apply to INVITE & Session Progess but not 
> trying OK and TRYING.
> You need to pair down the messages that it gets applied to using if 
> statements. Below is an example of one way to do this....
> 
> FYI: I think we meet at astricon last year in Atlanta. I work with David 
> Troy, founder or ToadNet
> 
> if(nat_uac_test("3"))
> {
>  if((method == "REGISTER") || !(search("^Record-Route:")))
>  {
>   log("LOG:Someone trying to register from private IP, rewriting\n");
> 
>   fix_nated_contact();
>   if(method == "INVITE")
>   {
>    fix_nated_sdp("1");
>    force_rtp_proxy();
>    t_on_reply("1");
>   };
>   force_rport();
>   setflag(6);
>  };
> };
> 
> 
> onreply_route[1]
> {
> if(status =~ "(183)|2[0-9][0-9]")
> {
>  fix_nated_contact();
>  force_rtp_proxy();
> };
> }
> 
> 
> 
> 1.5.2. fix_nated_sdp(mode)
> Alters the SDP information in orer to facilitate NAT traversal. What 
> changes to be performed may be controled via the "parameter".
> 
> The parameter value may be a bitwise OR of the following flags:
> 
> 
>  a.. 0x01 - adds "a=direction:active" SDP line;
> 
>  b.. 0x02 - rewrite media IP address (c=) with source address of the 
> message.
> 
>  c.. 0x04 - adds "a=nortpproxy:yes" SDP line;
> 
>  d.. 0x08 - rewrite IP from origin description (o=) with source address 
> of the message.
> 
> Example 1-13. fix_nated_sdp usage
> 
> ...
> if (search("User-Agent: Cisco ATA.*") {fix_nated_sdp("3");};
> ...----- Original Message ----- From: "John Riordan" 
> <john at junctionnetworks.com>
> To: <devel at openser.org>
> Cc: <john at junctionnetworks.com>
> Sent: Saturday, October 08, 2005 10:48 AM
> Subject: [Devel] [patch] Nathelper - very minor bug
> 
> 
>> Hi,
>>
>> extract_mediaip is currently logging failure to find a c= line
>> as an ERROR. However, the way this function is used (in replace_sdp_ip
>> and elsewhere) the expectation is that failure may not be an ERROR,
>> but simply an indication that all c= lines have been dealt with.
>>
>> The upshot is that an ERROR is getting logged whenever fix_nated_sdp
>> is used - regardless of there being a c= line in the SDP or not.
>>
>> The error looks like:
>>
>> Oct  8 09:58:34 host /usr/local/sbin/openser[10370]: ERROR: 
>> extract_mediaip: no `c=' in SDP
>>
>> The attached patch is simply changes the logged message to DBG.
>>
>> Regards,
>>
>> John
>>
> 
> 
> -------------------------------------------------------------------------------- 
> 
> 
> 
>> --- modules/nathelper/nathelper.c.~1.5.~ 2005-07-15 00:26:56.000000000 
>> -0400
>> +++ modules/nathelper/nathelper.c 2005-10-08 10:14:59.252490355 -0400
>> @@ -1020,7 +1020,7 @@
>>  cp = cp1 + 2;
>>  }
>>  if (cp1 == NULL) {
>> - LOG(L_ERR, "ERROR: extract_mediaip: no `%s' in SDP\n",line);
>> + LOG(L_DBG, "DEBUG: extract_mediaip: no `%s' in SDP\n",line);
>>  return -1;
>>  }
>>  mediaip->s = cp1 + 2;
>>
> 
> 
> -------------------------------------------------------------------------------- 
> 
> 
> 
>> _______________________________________________
>> Devel mailing list
>> Devel at openser.org
>> http://openser.org/cgi-bin/mailman/listinfo/devel
>>
> 
>