[Devel] [ openser-Bugs-1684036 ] PRESENCE module and multiple
transports issue
SourceForge.net
noreply at sourceforge.net
Thu Mar 22 17:16:31 CET 2007
Bugs item #1684036, was opened at 2007-03-20 00:26
Message generated for change (Comment added) made by anca_vamanu
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=743020&aid=1684036&group_id=139143
Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: modules
Group: ver 1.2.x
Status: Closed
Resolution: Fixed
Priority: 5
Private: No
Submitted By: Aron Rosenberg (amr42)
Assigned to: anca (anca_vamanu)
Summary: PRESENCE module and multiple transports issue
Initial Comment:
The new PRESENCE module does not seem to take into account the transport, socket or port that a user iniated the SUBSCRIBE on. Consider the following two scenarios that appear to be broken
Server Setup:
IP: 1.1.1.1 udp port 5060
IP: 1.1.1.1 udp port 5062
IP: 1.1.1.1 tcp port 5060
User A connects to the server on 1.1.1.1 udp port 5060, and requests a SUBSCRIBE to User A (himself) and User B
User B connects to the server on 1.1.1.1 tcp port 5060, and requests a SUBSCRIBE to USER A and USER B (himself)
If User A sends a PUBLISH (with 1.1.1.1 udp port 5060), then the subsequent NOTIFY's (to A and B) will go out on 1.1.1.1 udp port 5060 even though User B requested the SUBSCRIPTION via tcp port 5060.
If User B sends a PUBLISH (with 1.1.1.1 tcp port 5060), then both subsequent NOTIFY's will be sent via 1.1.1.1 tcp port 5060.
Summary: Presence module is not saving the transport that the subscribe arrived on and is not using that detail to send out the NOTIFY.
Solution: Add a new column to the active_watchers table called socket (exactly like REGISTRAR module has with location table) and store the incoming socket for the SUBSCRIBE request. This socket would then need to be used when sending out a NOTIFY request.
----------------------------------------------------------------------
>Comment By: anca (anca_vamanu)
Date: 2007-03-22 16:16
Message:
Logged In: YES
user_id=1614776
Originator: NO
Hi,
Indeed there were some errors there. They are fixed now.
Thanks.
----------------------------------------------------------------------
Comment By: Aron Rosenberg (amr42)
Date: 2007-03-21 17:51
Message:
Logged In: YES
user_id=43318
Originator: YES
Sorry Anca, the thing i was refering was to your contact changes in
addition to Kobi's patch.
svn head has
subscribe.c:1134
if(port== 5060)
strncpy(contact.s+contact.len, ":5060;transport=" , 16);
else
strncpy(contact.s+ contact.len, ":5062;transport=", 16);
contact.len+=16;
wouldn't it cleaner (and support ports other than 5060, and 5062) if it
were something like:
contact.len+=sprintf(contact.s+ contact.len,":%d;transport=",port)
Also, there is a typo in
subscribe.c:1106 proto= "upd"; should be proto= "udp";
One more question - We don't use TLS, but is that taken into account with
how you are dynamically building the contact header?
----------------------------------------------------------------------
Comment By: anca (anca_vamanu)
Date: 2007-03-21 17:18
Message:
Logged In: YES
user_id=1614776
Originator: NO
Aron,
I don't understand what error you are refering to.
The line 1137 in subscribe.c is added by me. I searched the patch and
found no record of 5062.
The fix does not work for you?
----------------------------------------------------------------------
Comment By: Aron Rosenberg (amr42)
Date: 2007-03-21 16:18
Message:
Logged In: YES
user_id=43318
Originator: YES
Anca,
Kobi's patch has one small thing which should be fixed - He has a
hardcoded port 5062 in the subscribe.c line 1137. This was from our inhouse
testing, where we were running on a second UDP port (5062) to do testing.
----------------------------------------------------------------------
Comment By: anca (anca_vamanu)
Date: 2007-03-21 14:55
Message:
Logged In: YES
user_id=1614776
Originator: NO
Hello,
The transport problems are now fixed on svn. Since the solution implies
alterations in the database table we will wait a bit before applying the
changes to v1.2.
Aron thanks for reporting and pointing out the solution.
Thank you for the patches, Kobi. I had the same solution in my mind for
received sockets. I used your patches with small modifications.
For the contact in replies and Notify, the 'server_address' parameter,
which has became optional, if set, has priority. Otherwise it is extracted
also from msg->rcv.
Best regards,
Anca Vamanu
----------------------------------------------------------------------
Comment By: Kobi Eshun (ekobi)
Date: 2007-03-21 06:31
Message:
Logged In: YES
user_id=1039134
Originator: NO
Hi,
I implemented a patch that stores the required socket_info to the
active_watchers table on SUBSCRIBE, and then uses that for all subsequent
NOTIFY requests on that dialog. This ALTER query adds the new column to the
table:
ALTER TABLE `active_watchers` ADD `socket_info` VARCHAR( 128 ) NOT
NULL ;
There is still the issue of setting the correct port in the contact header
when responding to the SUBSCRIBE request, instead of the current,
statically defined server address. I cheated by adding the correct header
at the script layer, and (in a different patch) checking for duplicate
contact header in the module.
--
kobi
Index: modules/presence/notify.c
===================================================================
--- modules/presence/notify.c (revision 1855)
+++ modules/presence/notify.c (working copy)
@@ -38,6 +38,7 @@
#include "../../db/db.h"
#include "../../db/db_val.h"
#include "../tm/tm_load.h"
+#include "../../socket_info.h"
#include "presence.h"
#include "notify.h"
@@ -636,6 +637,19 @@
&td->route_set);
td->state= DLG_CONFIRMED ;
+
+ if (subs->sockinfo_str.len) {
+ int port, proto;
+ str host;
+ if (parse_phostport (
+ subs->sockinfo_str.s,subs->sockinfo_str.len,&host.s,
+ &host.len,&port, &proto )) {
+ LOG (L_ERR,"PRESENCE:build_dlg_t:bad sockinfo string\n");
+ goto error;
+ }
+ td->send_sock = grep_sock_info (
+ &host, (unsigned short) port, (unsigned short) proto);
+ }
return td;
@@ -647,6 +661,7 @@
}
if(td!=NULL)
free_tm_dlg(td);
+
return NULL;
}
@@ -659,7 +674,7 @@
db_key_t query_cols[6];
db_op_t query_ops[6];
db_val_t query_vals[6];
- db_key_t result_cols[15];
+ db_key_t result_cols[16];
int n_result_cols = 0, n_query_cols = 0;
db_row_t *row ;
db_val_t *row_vals ;
@@ -667,9 +682,11 @@
int size= 0;
str from_user, from_domain, to_tag, from_tag;
str event_id, callid, record_route, contact, status;
+ str sockinfo_str;
int from_user_col, from_domain_col, to_tag_col, from_tag_col;
int expires_col= 0,callid_col, cseq_col, i, status_col =0, event_id_col
= 0;
int version_col = 0, record_route_col = 0, contact_col = 0;
+ int sockinfo_col = 0;
if (pa_dbf.use_table(pa_db, active_watchers_table) < 0)
{
@@ -713,6 +730,7 @@
result_cols[contact_col=n_result_cols++] = "contact";
result_cols[expires_col=n_result_cols++] = "expires";
result_cols[status_col=n_result_cols++] = "status";
+ result_cols[sockinfo_col=n_result_cols++] = "socket_info";
if(strlen(event)== strlen( "presence.winfo"))
{
@@ -792,10 +810,13 @@
status.s= NULL;
status.len= 0;
}
+
+ sockinfo_str.s = row_vals[sockinfo_col].val.str_val.s;
+ sockinfo_str.len = sockinfo_str.s?strlen (sockinfo_str.s):0;
size= sizeof(subs_t)+ (p_user->len+ p_domain->len+ from_user.len+
from_domain.len+ event_id.len+ + strlen(event)+ to_tag.len+
- from_tag.len+ callid.len+ record_route.len+ contact.len)*
sizeof(char);
+ from_tag.len+ callid.len+ record_route.len+ contact.len
+sockinfo_str.len)* sizeof(char);
if(force_active== 0)
size+= status.len* sizeof(char);
@@ -891,6 +912,11 @@
}
}
+ subs->sockinfo_str.s =(char*)subs+ size;
+ memcpy(subs->sockinfo_str.s, sockinfo_str.s, sockinfo_str.len);
+ subs->sockinfo_str.len = sockinfo_str.len;
+ size+= sockinfo_str.len;
+
subs->cseq = row_vals[cseq_col].val.int_val;
subs->expires = row_vals[expires_col].val.int_val -
(int)time(NULL);
Index: modules/presence/subscribe.c
===================================================================
--- modules/presence/subscribe.c (revision 1855)
+++ modules/presence/subscribe.c (working copy)
@@ -424,7 +424,18 @@
n_query_cols++;
}
+ /* */
+ /* Save receive socket also -- ke. */
+ {
+ struct socket_info *si = msg->rcv.bind_address;
+ query_cols[n_query_cols] = "socket_info";
+ query_vals[n_query_cols].type = DB_STR;
+ query_vals[n_query_cols].val.str_val = si->sock_str;
+ query_vals[n_query_cols].nul = 0;
+ n_query_cols++;
+ }
+
DBG("PRESENCE:update_subscribtion:Inserting into database:"
"\nn_query_cols:%d\n",n_query_cols);
for(i = 0;i< n_query_cols-2; i++)
Index: modules/presence/subscribe.h
===================================================================
--- modules/presence/subscribe.h (revision 1855)
+++ modules/presence/subscribe.h (working copy)
@@ -15,6 +15,7 @@
str to_tag;
str from_tag;
str callid;
+ str sockinfo_str;
unsigned int cseq;
str contact;
str record_route;
----------------------------------------------------------------------
Comment By: Aron Rosenberg (amr42)
Date: 2007-03-20 17:33
Message:
Logged In: YES
user_id=43318
Originator: YES
Anca,
There is a socket member for the dlg_t* that you can pass that would seem
to specify which socket to use.
----------------------------------------------------------------------
Comment By: anca (anca_vamanu)
Date: 2007-03-20 17:27
Message:
Logged In: YES
user_id=1614776
Originator: NO
Hello,
The api in tm will have to be extended for this.
Thanks for reporting.
Best regards,
Anca Vamanu
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=743020&aid=1684036&group_id=139143
More information about the Devel
mailing list