rls(k): update_rlsubs() broken in two funtions

- deletion from hash table of expired subscriptions is done
separately now in remove_expired_rlsubs()
- should allow proper notification in case of un-SUBSCRIBE events,
first doing updated of rlsubs, then sending NOTIFY requests and then
deleting from hash table
- reported by Peter Dunkley
- added several safety checks about size of built attributes, used
  variables, trimmed whitespaces from attributes added in bodies to save
  some bytes on the network

modules_k/rls/notify.c

@@ -34,6 +34,7 @@
 #include "../../ut.h"
 #include "../../str.h"
 #include "../../dprint.h"
+#include "../../trim.h"
 #include "../../data_lump_rpl.h"
 #include "../../parser/msg_parser.h"
 #include "../../parser/parse_event.h"
@@ -84,7 +85,11 @@ int send_full_notify(subs_t* subs, xmlNodePtr rl_node, int version, str* rl_uri,
 	LM_DBG("start\n");
 	/* query in alfabetical order */
 	
-	CONSTR_RLSUBS_DID(subs, &rlsubs_did);
+	if(CONSTR_RLSUBS_DID(subs, &rlsubs_did)<0)
+	{
+		LM_ERR("cannot build rls subs did\n");
+		goto error;
+	}
 
 	query_cols[0]= &str_rlsubs_did_col;
 	query_vals[0].type = DB1_STR;
@@ -492,6 +497,7 @@ str* constr_multipart_body(db1_res_t* result, char** cid_array,
 	
 		body.s= (char*)row_vals[pres_state_col].val.string_val;
 		body.len= strlen(body.s);
+		trim(&body);
 		ctype.s = (char*)row_vals[content_type_col].val.string_val;
 		ctype.len = strlen(ctype.s);
 

modules_k/rls/resource_notify.c

@@ -33,6 +33,7 @@
 #include "../../parser/parse_from.h"
 #include "../../lib/kcore/cmpapi.h"
 #include "../../lib/kcore/hash_func.h"
+#include "../../trim.h"
 #include "../pua/hash.h"
 #include "rls.h"
 #include "notify.h"
@@ -192,6 +193,7 @@ int rls_handle_notify(struct sip_msg* msg, char* c1, char* c2)
 		}
 		pto = &TO;
 	}
+	memset(&dialog, 0, sizeof(ua_pres_t));
 	dialog.watcher_uri= &pto->uri;
     if (pto->tag_value.s==NULL || pto->tag_value.len==0 )
 	{  
@@ -425,7 +427,7 @@ int parse_rlsubs_did(char* str_did, str* callid, str* from_tag, str* to_tag)
 {
 	char* smc= NULL;
 
-	smc= strstr(str_did, DID_SEP);
+	smc= strstr(str_did, RLS_DID_SEP);
 	if(smc== NULL)
 	{
 		LM_ERR("bad format for resource list Subscribe dialog"
@@ -435,8 +437,8 @@ int parse_rlsubs_did(char* str_did, str* callid, str* from_tag, str* to_tag)
 	callid->s= str_did;
 	callid->len= smc- str_did;
 			
-	from_tag->s= smc+ DID_SEP_LEN;
-	smc= strstr(from_tag->s, DID_SEP);
+	from_tag->s= smc+ RLS_DID_SEP_LEN;
+	smc= strstr(from_tag->s, RLS_DID_SEP);
 	if(smc== NULL)
 	{
 		LM_ERR("bad format for resource list Subscribe dialog"
@@ -445,8 +447,8 @@ int parse_rlsubs_did(char* str_did, str* callid, str* from_tag, str* to_tag)
 	}
 	from_tag->len= smc- from_tag->s;
 		
-	to_tag->s= smc+ DID_SEP_LEN;
-	to_tag->len= strlen(str_did)- 2* DID_SEP_LEN- callid->len- from_tag->len;
+	to_tag->s= smc+ RLS_DID_SEP_LEN;
+	to_tag->len= strlen(str_did)- 2* RLS_DID_SEP_LEN- callid->len- from_tag->len;
 
 	return 0;
 }
@@ -462,7 +464,8 @@ void timer_send_notify(unsigned int ticks,void *param)
 	char* prev_did= NULL, * curr_did= NULL;
 	db_row_t *row;	
 	db_val_t *row_vals;
-	char* resource_uri, *pres_state;
+	char* resource_uri;
+	str pres_state = {0, 0};
 	str callid, to_tag, from_tag;
 	xmlDocPtr rlmi_doc= NULL;
 	xmlNodePtr list_node= NULL, instance_node= NULL, resource_node;
@@ -554,7 +557,9 @@ void timer_send_notify(unsigned int ticks,void *param)
 		curr_did=     (char*)row_vals[did_col].val.string_val;
 		resource_uri= (char*)row_vals[resource_uri_col].val.string_val;
 		auth_state_flag=     row_vals[auth_state_col].val.int_val;
-		pres_state=   (char*)row_vals[pres_state_col].val.string_val;
+		pres_state.s=   (char*)row_vals[pres_state_col].val.string_val;
+		pres_state.len = strlen(pres_state.s);
+		trim(&pres_state);
 		
 		if(prev_did!= NULL && strcmp(prev_did, curr_did)) 
 		{
@@ -705,17 +710,18 @@ void timer_send_notify(unsigned int ticks,void *param)
 			/* add in the multipart buffer */
 			if(cid)
 			{
-				if(buf_len+ antet_len+ strlen(pres_state)+ 4 > size)
+				if(buf_len+ antet_len+ pres_state.len+ 4 > size)
 				{
 					REALLOC_BUF
 				}
-				buf_len+= sprintf(buf+ buf_len, "--%s\r\n\r\n", bstr.s);
+				buf_len+= sprintf(buf+ buf_len, "--%s\r\n", bstr.s);
 				buf_len+= sprintf(buf+ buf_len,
 						"Content-Transfer-Encoding: binary\r\n");
 				buf_len+= sprintf(buf+ buf_len, "Content-ID: <%s>\r\n", cid);
 				buf_len+= sprintf(buf+ buf_len, "Content-Type: %s\r\n\r\n",  
 						row_vals[content_type_col].val.string_val);
-				buf_len+= sprintf(buf+buf_len,"%s\r\n\r\n", pres_state);
+				buf_len+= sprintf(buf+buf_len,"%.*s\r\n\r\n", pres_state.len,
+						pres_state.s);
 			}
 
 			i++;
@@ -738,7 +744,9 @@ void timer_send_notify(unsigned int ticks,void *param)
 			}
 			resource_uri= (char*)row_vals[resource_uri_col].val.string_val;
 			auth_state_flag=     row_vals[auth_state_col].val.int_val;
-			pres_state=   (char*)row_vals[pres_state_col].val.string_val;
+			pres_state.s=   (char*)row_vals[pres_state_col].val.string_val;
+			pres_state.len= strlen(pres_state.s);
+			trim(&pres_state);
 		}
 
 		prev_did= curr_did;

modules_k/rls/rls.c

@@ -520,8 +520,14 @@ static void destroy(void)
 
 int handle_expired_record(subs_t* s)
 {
-	/* send Notify with state terminated*/	
-	if( rls_send_notify(s, NULL, NULL, NULL)< 0)
+	int ret;
+	int tmp;
+	/* send NOTIFY with state terminated - make sure exires value is 0 */
+	tmp = s->expires;
+	s->expires = 0;
+	ret = rls_send_notify(s, NULL, NULL, NULL);
+	s->expires = tmp;
+	if(ret <0)
 	{
 		LM_ERR("in function send_notify\n");
 		return -1;

modules_k/rls/rls.h

@@ -168,18 +168,25 @@ extern str str_doc_type_col;
 extern str str_etag_col;
 extern str str_doc_col;
 
-#define DID_SEP_LEN   strlen(DID_SEP)
-#define DID_SEP       ";"
-#define DID_INIT_LEN  (2* sizeof(DID_SEP))
+#define RLS_DID_SEP       ";"
+#define RLS_DID_SEP_LEN   strlen(RLS_DID_SEP)
+#define RLS_DID_INIT_LEN  (2* sizeof(RLS_DID_SEP))
+#define RLS_DID_MAX_LEN	255
 
-/* did_str= *callid*DID_SEP*from_tag*DID_SEP*to_tag* */
+/* did_str= *callid*RLS_DID_SEP*from_tag*RLS_DID_SEP*to_tag* */
 
 static inline int CONSTR_RLSUBS_DID(subs_t* subs, str *did)
 {
 	int len;
 
-	len= (DID_INIT_LEN+ subs->callid.len+ subs->to_tag.len+
+	len= (RLS_DID_INIT_LEN+ subs->callid.len+ subs->to_tag.len+
 			subs->from_tag.len+ 10)* sizeof(char);
+	if(len > RLS_DID_MAX_LEN)
+	{
+		LM_ERR("new DID size is too big [%d > %d]\n",
+				len, RLS_DID_MAX_LEN);
+		return -1;
+	}
 	did->s= (char*)pkg_malloc(len);
 	if(did->s== NULL) 
 	{
@@ -187,8 +194,8 @@ static inline int CONSTR_RLSUBS_DID(subs_t* subs, str *did)
 	}
 	
 	did->len= sprintf(did->s, "%.*s%s%.*s%s%.*s", subs->callid.len, 
-			subs->callid.s, DID_SEP,subs->from_tag.len, subs->from_tag.s, 
-			DID_SEP, subs->to_tag.len, subs->to_tag.s);
+			subs->callid.s, RLS_DID_SEP,subs->from_tag.len, subs->from_tag.s,
+			RLS_DID_SEP, subs->to_tag.len, subs->to_tag.s);
 
 	if(did->len>= len)
 	{

modules_k/rls/subscribe.c

@@ -66,6 +66,7 @@ subs_t* constr_new_subs(struct sip_msg* msg, struct to_body *pto,
 int resource_subscriptions(subs_t* subs, xmlNodePtr rl_node);
 
 int update_rlsubs( subs_t* subs,unsigned int hash_code);
+int remove_expired_rlsubs( subs_t* subs,unsigned int hash_code);
 
 /**
  * return the XML node for rls-services matching uri
@@ -406,7 +407,6 @@ int reply_489(struct sip_msg * msg)
 int rls_handle_subscribe(struct sip_msg* msg, char* s1, char* s2)
 {
 	subs_t subs;
-	str resource_list = {0, 0};
 	pres_ev_t* event = NULL;
 	int err_ret = -1;
 	str* contact = NULL;
@@ -630,6 +630,7 @@ int rls_handle_subscribe(struct sip_msg* msg, char* s1, char* s2)
 		LM_ERR("failed sending subscribe requests to resources in list\n");
 		goto error;
 	}
+	remove_expired_rlsubs(&subs, hash_code);
 
 done:
 	if(contact!=NULL)
@@ -677,14 +678,57 @@ error:
 
 	if(doc!=NULL)
 		xmlFreeDoc(doc);
-	if(resource_list.s!=NULL)
-		pkg_free(resource_list.s);
 	return err_ret;
 }
 
-int update_rlsubs( subs_t* subs, unsigned int hash_code)
+int remove_expired_rlsubs( subs_t* subs, unsigned int hash_code)
 {
 	subs_t* s, *ps;
+	int found= 0;
+
+	if(subs->expires!=0)
+		return 0;
+
+	/* search the record in hash table */
+	lock_get(&rls_table[hash_code].lock);
+
+	s= pres_search_shtable(rls_table, subs->callid,
+			subs->to_tag, subs->from_tag, hash_code);
+	if(s== NULL)
+	{
+		LM_DBG("record not found in hash table\n");
+		lock_release(&rls_table[hash_code].lock);
+		return -1;
+	}
+	/* delete record from hash table */
+	ps= rls_table[hash_code].entries;
+	while(ps->next)
+	{
+		if(ps->next== s)
+		{
+			found= 1;
+			break;
+		}
+		ps= ps->next;
+	}
+	if(found== 0)
+	{
+		LM_ERR("record not found\n");
+		lock_release(&rls_table[hash_code].lock);
+		return -1;
+	}
+	ps->next= s->next;
+	shm_free(s);
+
+	lock_release(&rls_table[hash_code].lock);
+
+	return 0;
+
+}
+
+int update_rlsubs( subs_t* subs, unsigned int hash_code)
+{
+	subs_t* s;
 
 	/* search the record in hash table */
 	lock_get(&rls_table[hash_code].lock);
@@ -735,29 +779,6 @@ int update_rlsubs( subs_t* subs, unsigned int hash_code)
 	subs->local_cseq= s->local_cseq;
 	subs->version= s->version;
 
-	if(subs->expires== 0)
-	{
-		/* delete record from hash table */
-		ps= rls_table[hash_code].entries;
-		int found= 0;
-		while(ps->next)
-		{
-			if(ps->next== s)
-			{
-				found= 1;
-				break;
-			}
-			ps= ps->next;
-		}
-		if(found== 0)
-		{
-			LM_ERR("record not found\n");
-			goto error;
-		}
-		ps->next= s->next;
-		shm_free(s);
-	}
-	
 	lock_release(&rls_table[hash_code].lock);
 
 	return 0;
@@ -788,15 +809,18 @@ int resource_subscriptions(subs_t* subs, xmlNodePtr xmlnode)
 	char* uri= NULL;
 	subs_info_t s;
 	str wuri= {0, 0};
-	static char buf[256];
 	str extra_headers;
 	str did_str= {0, 0};
 		
 	/* if is initial send an initial Subscribe 
 	 * else search in hash table for a previous subscription */
 
-	CONSTR_RLSUBS_DID(subs, &did_str);
-	
+	if(CONSTR_RLSUBS_DID(subs, &did_str)<0)
+	{
+		LM_ERR("cannot build rls subs did\n");
+		goto error;
+	}
+
 	memset(&s, 0, sizeof(subs_info_t));
 
 	if(uandd_to_uri(subs->from_user, subs->from_domain, &wuri)<0)
@@ -817,8 +841,12 @@ int resource_subscriptions(subs_t* subs, xmlNodePtr xmlnode)
 	s.source_flag = RLS_SUBSCRIBE;
 	if(rls_outbound_proxy.s)
 		s.outbound_proxy = &rls_outbound_proxy;
-	extra_headers.s = buf;
-	extra_headers.len = sprintf(extra_headers.s, "Supported: eventlist\r\n");
+	extra_headers.s = "Supported: eventlist\r\n"
+				"Accept: application/pidf+xml, application/rlmi+xml,"
+					" application/watcherinfo+xml,"
+					" multipart/related\r\n";
+	extra_headers.len = strlen(extra_headers.s);
+
 	s.extra_headers = &extra_headers;
 	
 	if(process_list_and_exec(xmlnode, send_resource_subs, (void*)(&s))<0)