[asterisk-commits] rizzo: trunk r46026 - /trunk/channels/chan_sip.c

[asterisk-commits at lists.digium.com]

Author: rizzo
Date: Mon Oct 23 14:52:43 2006
New Revision: 46026

URL: http://svn.digium.com/view/asterisk?rev=46026&view=rev
Log:
get_header() always returns a non-NULL value, so checking
for NULL is certainly wrong and usually disables the
checks that we want to make instead.

This commit fixes a number of the above bugs where the result
of get_header() is immediately checked for NULL.

This is certainly a candidate for merging into 1.4



Modified:
    trunk/channels/chan_sip.c

Modified: trunk/channels/chan_sip.c
URL: http://svn.digium.com/view/asterisk/trunk/channels/chan_sip.c?rev=46026&r1=46025&r2=46026&view=diff
==============================================================================
--- trunk/channels/chan_sip.c (original)
+++ trunk/channels/chan_sip.c Mon Oct 23 14:52:43 2006
@@ -4305,7 +4305,10 @@
 	const char *to = get_header(req, "To");
 	const char *cseq = get_header(req, "Cseq");
 
-	if (!callid || !to || !from || !cseq)		/* Call-ID, to, from and Cseq are required by RFC 3261. (Max-forwards and via too - ignored now) */
+	/* Call-ID, to, from and Cseq are required by RFC 3261. (Max-forwards and via too - ignored now) */
+	/* get_header always returns non-NULL so we must use ast_strlen_zero() */
+	if (ast_strlen_zero(callid) || ast_strlen_zero(to) ||
+			ast_strlen_zero(from) || ast_strlen_zero(cseq))
 		return NULL;	/* Invalid packet */
 
 	if (pedanticsipchecking) {
@@ -7750,7 +7753,7 @@
 	
 	/* Save User agent */
 	useragent = get_header(req, "User-Agent");
-	if (useragent && strcasecmp(useragent, peer->useragent)) {
+	if (strcasecmp(useragent, peer->useragent)) {	/* XXX copy if they are different ? */
 		ast_copy_string(peer->useragent, useragent, sizeof(peer->useragent));
 		if (option_verbose > 3)
 			ast_verbose(VERBOSE_PREFIX_3 "Saved useragent \"%s\" for peer %s\n", peer->useragent, peer->name);  
@@ -8576,7 +8579,8 @@
 	if (!req)
 		req = &transferer->initreq;
 
-	if (!(p_refer_to = get_header(req, "Refer-To"))) {
+	p_refer_to = get_header(req, "Refer-To");
+	if (ast_strlen_zero(p_refer_to)) {
 		ast_log(LOG_WARNING, "Refer-To Header missing. Skipping transfer.\n");
 		return -2;	/* Syntax error */
 	}
@@ -8592,7 +8596,8 @@
 	refer_to += 4;			/* Skip sip: */
 
 	/* Get referred by header if it exists */
-	if ((p_referred_by = get_header(req, "Referred-By"))) {
+	p_referred_by = get_header(req, "Referred-By");
+	if (!ast_strlen_zero(p_referred_by)) {
 		char *lessthan;
 		h_referred_by = ast_strdupa(p_referred_by);
 		if (pedanticsipchecking)
@@ -11943,6 +11948,8 @@
 		/* according to section 6.13 of RFC, contact headers override
 		   expires headers, so check those first */
 		expires = 0;
+
+		/* XXX todo: try to save the extra call */
 		if (!ast_strlen_zero(get_header(req, "Contact"))) {
 			const char *contact = NULL;
 			const char *tmptmp = NULL;
@@ -13032,13 +13039,13 @@
 	/* Find out what they support */
 	if (!p->sipoptions) {
 		const char *supported = get_header(req, "Supported");
-		if (supported)
+		if (!ast_strlen_zero(supported))
 			parse_sip_options(p, supported);
 	}
 
 	/* Find out what they require */
 	required = get_header(req, "Require");
-	if (required && !ast_strlen_zero(required)) {
+	if (!ast_strlen_zero(required)) {
 		required_profile = parse_sip_options(NULL, required);
 		if (required_profile && required_profile != SIP_OPT_REPLACES) {
 			/* At this point we only support REPLACES */
@@ -13070,7 +13077,8 @@
 		return 0;
 	}
 
-	if ((p_replaces = get_header(req, "Replaces")) && !ast_strlen_zero(p_replaces)) {
+	p_replaces = get_header(req, "Replaces");
+	if (!ast_strlen_zero(p_replaces)) {
 		/* We have a replaces header */
 		char *ptr;
 		char *fromtag = NULL;