Suppressed echoes in chat lobbies by:

- adding a time stamp to lobby events (new item tag)
- controllign time stamp of lobby msg and event: drop packets if time is older than cache duration.
- moved connexion challenge code to lobby management loop
- added new type of lobby invite to handle connexion challenges and avoid false invitations in the GUI

New lobby event format is not backward compatible -> "peer typing" and "peer joined/left" will need 
the new version. Messages are still compatible)



git-svn-id: http://svn.code.sf.net/p/retroshare/code/trunk@4859 b45a01b8-16f6-495d-af2f-9b41ad6348cc

libretroshare/src/retroshare/rsmsgs.h

@@ -68,8 +68,9 @@
 #define RS_MSGTAGTYPE_LATER      5
 #define RS_MSGTAGTYPE_USER       100
 
-#define RS_CHAT_LOBBY_PRIVACY_LEVEL_PUBLIC  1	/* lobby is visible by friends. Friends can connect.*/
+#define RS_CHAT_LOBBY_PRIVACY_LEVEL_CHALLENGE  	0	/* Used to accept connexion challenges only. */
-#define RS_CHAT_LOBBY_PRIVACY_LEVEL_PRIVATE 2	/* lobby invisible by friends. Peers on invitation only .*/
+#define RS_CHAT_LOBBY_PRIVACY_LEVEL_PUBLIC  		1	/* lobby is visible by friends. Friends can connect.*/
+#define RS_CHAT_LOBBY_PRIVACY_LEVEL_PRIVATE 		2	/* lobby invisible by friends. Peers on invitation only .*/
 
 typedef uint64_t 		ChatLobbyId ;
 typedef uint64_t 		ChatLobbyMsgId ;

libretroshare/src/serialiser/rsmsgitems.cc

@@ -46,7 +46,7 @@ std::ostream& RsChatMsgItem::print(std::ostream &out, uint16_t indent)
 	out << "QblogMs " << chatFlags << std::endl;
 
 	printIndent(out, int_Indent);
-	out << "sendTime:  " << sendTime  << std::endl;
+	out << "sendTime:  " << sendTime  << " (" << time(NULL)-sendTime << " secs ago)" << std::endl;
 
 	printIndent(out, int_Indent);
 
@@ -97,6 +97,7 @@ std::ostream& RsChatLobbyEventItem::print(std::ostream &out, uint16_t indent)
 	RsChatLobbyBouncingObject::print(out,indent) ;
 	printIndent(out, indent); out << "Event type  : " << event_type  << std::endl;
 	printIndent(out, indent); out << "String param: " << string1  << std::endl;
+	printIndent(out, indent); out << "Send time: " << sendTime  << " (" << time(NULL)-sendTime << " secs ago)" << std::endl;
 	printRsItemEnd(out, "RsChatLobbyEventItem", indent);
 	return out;
 } 
@@ -148,7 +149,7 @@ std::ostream& RsPrivateChatMsgConfigItem::print(std::ostream &out, uint16_t inde
 	out << "QblogMs " << configFlags << std::endl;
 
 	printIndent(out, int_Indent);
-	out << "sendTime:  " << sendTime  << std::endl;
+	out << "sendTime:  " << sendTime  << " (" << time(NULL)-sendTime << " secs ago)" << std::endl;
 
 	printIndent(out, int_Indent);
 
@@ -271,6 +272,7 @@ uint32_t RsChatLobbyEventItem::serial_size()
 	s += RsChatLobbyBouncingObject::serial_size() ;
 	s += 1 ; // event_type
 	s += GetTlvStringSize(string1) ;	// string1
+	s += 4 ; // send time
 
 	return s ;
 }
@@ -506,6 +508,7 @@ bool RsChatLobbyEventItem::serialise(void *data, uint32_t& pktsize)
 	ok &= RsChatLobbyBouncingObject::serialise(data,tlvsize,offset) ;		// first, serialize parent
 	ok &= setRawUInt8(data, tlvsize, &offset, event_type);
 	ok &= SetTlvString(data, tlvsize, &offset, TLV_TYPE_STR_NAME, string1);
+	ok &= setRawUInt32(data, tlvsize, &offset, sendTime);
 
 	pktsize = tlvsize ;
 
@@ -752,7 +755,7 @@ RsChatMsgItem::RsChatMsgItem(void *data,uint32_t /*size*/,uint8_t subtype)
 #ifdef CHAT_DEBUG
 	std::cerr << "Building new chat msg item." << std::endl ;
 #endif
-	if (getRsItemSubType(getRsItemId(data)) == subtype && offset != rssize)
+	if (getRsItemSubType(getRsItemId(data)) == RS_PKT_SUBTYPE_DEFAULT && offset != rssize)
 		std::cerr << "Size error while deserializing." << std::endl ;
 	if (!ok)
 		std::cerr << "Unknown error while deserializing." << std::endl ;
@@ -841,6 +844,7 @@ RsChatLobbyEventItem::RsChatLobbyEventItem(void *data,uint32_t /*size*/)
 
 	ok &= getRawUInt8(data, rssize, &offset, &event_type);
 	ok &= GetTlvString(data, rssize, &offset, TLV_TYPE_STR_NAME, string1);
+	ok &= getRawUInt32(data, rssize, &offset, &sendTime);
 
 #ifdef CHAT_DEBUG
 	std::cerr << "Building new chat lobby status item." << std::endl ;

libretroshare/src/serialiser/rsmsgitems.h

@@ -56,11 +56,12 @@ const uint8_t RS_PKT_SUBTYPE_CHAT_LOBBY_INVITE_DEPREC = 0x07 ;	// don't use ! De
 const uint8_t RS_PKT_SUBTYPE_CHAT_LOBBY_ACCEPT     	= 0x08 ;
 const uint8_t RS_PKT_SUBTYPE_CHAT_LOBBY_CHALLENGE  	= 0x09 ;
 const uint8_t RS_PKT_SUBTYPE_CHAT_LOBBY_UNSUBSCRIBE	= 0x0A ;
-const uint8_t RS_PKT_SUBTYPE_CHAT_LOBBY_EVENT      	= 0x0B ;
+const uint8_t RS_PKT_SUBTYPE_CHAT_LOBBY_EVENT_DEPREC  = 0x0B ;	// don't use ! Deprecated
 const uint8_t RS_PKT_SUBTYPE_CHAT_LOBBY_MSG        	= 0x0C ;
 const uint8_t RS_PKT_SUBTYPE_CHAT_LOBBY_LIST_REQUEST 	= 0x0D ;
 const uint8_t RS_PKT_SUBTYPE_CHAT_LOBBY_LIST         	= 0x0E ;
 const uint8_t RS_PKT_SUBTYPE_CHAT_LOBBY_INVITE       	= 0x0F ;
+const uint8_t RS_PKT_SUBTYPE_CHAT_LOBBY_EVENT       	= 0x10 ;
 
 // for defining tags themselves and msg tags
 const uint8_t RS_PKT_SUBTYPE_MSG_TAG_TYPE 	= 0x03;
@@ -167,6 +168,7 @@ class RsChatLobbyEventItem: public RsChatItem, public RsChatLobbyBouncingObject
 		//
 		uint8_t event_type ;		// used for defining the type of event.
 		std::string string1;		// used for any string
+		uint32_t sendTime;		// used to check for old looping messages
 };
 
 class RsChatLobbyListRequestItem: public RsChatItem

libretroshare/src/services/p3chatservice.cc

@@ -39,7 +39,6 @@
 /****
  * #define CHAT_DEBUG 1
  ****/
-#define CHAT_DEBUG 1
 
 static const int 		CONNECTION_CHALLENGE_MAX_COUNT 	  =   20 ; // sends a connexion challenge every 20 messages
 static const int 		CONNECTION_CHALLENGE_MIN_DELAY 	  =   15 ; // sends a connexion at most every 15 seconds
@@ -708,6 +707,15 @@ void p3ChatService::handleRecvChatLobbyEventItem(RsChatLobbyEventItem *item)
 	std::cerr << "Received ChatLobbyEvent item of type " << (int)(item->event_type) << ", and string=" << item->string1 << std::endl;
 #endif
 
+	if(time(NULL)+100 > item->sendTime + MAX_KEEP_MSG_RECORD)	// the message is older than the max cache keep minus 100 seconds ! It's too old, and is going to make an echo!
+	{
+		std::cerr << "Received severely outdated lobby event item! Dropping it!" << std::endl;
+		std::cerr << "Message item is:" << std::endl;
+		item->print(std::cerr) ;
+		std::cerr << std::endl;
+		return ;
+	}
+
 	if(! bounceLobbyObject(item,item->PeerId()))
 		return ;
 
@@ -811,6 +819,15 @@ bool p3ChatService::handleRecvChatMsgItem(RsChatMsgItem *ci)
 				return true ;
 		}
 
+		if(now+100 > cli->sendTime + MAX_KEEP_MSG_RECORD)	// the message is older than the max cache keep plus 100 seconds ! It's too old, and is going to make an echo!
+		{
+			std::cerr << "Received severely outdated message!Dropping it!" << std::endl;
+			std::cerr << "Message item is:" << std::endl;
+			cli->print(std::cerr) ;
+			std::cerr << std::endl;
+			return false ;
+		}
+
 		if(!bounceLobbyObject(cli,cli->PeerId()))	// forwards the message to friends, keeps track of subscribers, etc.
 			return false;
 
@@ -1615,88 +1632,72 @@ void p3ChatService::statusChange(const std::list<pqipeer> &plist)
 //
 bool p3ChatService::bounceLobbyObject(RsChatLobbyBouncingObject *item,const std::string& peer_id)
 {
-	bool send_challenge = false ;
+	time_t now = time(NULL) ;
-	ChatLobbyId send_challenge_lobby ;
+	RsStackMutex stack(mChatMtx); /********** STACK LOCKED MTX ******/
+#ifdef CHAT_DEBUG
+	locked_printDebugInfo() ; // debug
 
+	std::cerr << "Handling ChatLobbyMsg " << std::hex << item->msg_id << ", lobby id " << item->lobby_id << ", from peer id " << peer_id << std::endl;
+#endif
+
+	// send upward for display
+
+	std::map<ChatLobbyId,ChatLobbyEntry>::iterator it(_chat_lobbys.find(item->lobby_id)) ;
+
+	if(it == _chat_lobbys.end())
 	{
-		RsStackMutex stack(mChatMtx); /********** STACK LOCKED MTX ******/
+		std::cerr << "Chatlobby for id " << std::hex << item->lobby_id << " has no record. Dropping the msg." << std::dec << std::endl;
-
+		return false ;
-#ifdef CHAT_DEBUG
-		locked_printDebugInfo() ; // debug
-
-		std::cerr << "Handling ChatLobbyMsg " << std::hex << item->msg_id << ", lobby id " << item->lobby_id << ", from peer id " << peer_id << std::endl;
-#endif
-
-		// send upward for display
-
-		std::map<ChatLobbyId,ChatLobbyEntry>::iterator it(_chat_lobbys.find(item->lobby_id)) ;
-
-		if(it == _chat_lobbys.end())
-		{
-			std::cerr << "Chatlobby for id " << std::hex << item->lobby_id << " has no record. Dropping the msg." << std::dec << std::endl;
-			return false ;
-		}
-		ChatLobbyEntry& lobby(it->second) ;
-
-		time_t now = time(NULL) ;
-
-		// Adds the peer id to the list of friend participants, even if it's not original msg source
-
-		if(peer_id != mLinkMgr->getOwnId())
-			lobby.participating_friends.insert(peer_id) ;
-
-		lobby.nick_names[item->nick] = now ;
-
-		// Checks wether the msg is already recorded or not
-
-		std::map<ChatLobbyMsgId,time_t>::iterator it2(lobby.msg_cache.find(item->msg_id)) ;
-
-		if(it2 != lobby.msg_cache.end()) // found!
-		{
-#ifdef CHAT_DEBUG
-			std::cerr << "  Msg already received at time " << it2->second << ". Dropping!" << std::endl ;
-#endif
-			it2->second = now ;	// update last msg seen time, to prevent echos.
-			return false ;
-		}
-#ifdef CHAT_DEBUG
-		std::cerr << "  Msg already not received already. Adding in cache, and forwarding!" << std::endl ;
-#endif
-
-		lobby.msg_cache[item->msg_id] = now ;
-		lobby.last_activity = now ;
-
-		bool is_message =  (NULL != dynamic_cast<RsChatLobbyMsgItem*>(item)) ;
-
-		// Forward to allparticipating friends, except this peer.
-
-		for(std::set<std::string>::const_iterator it(lobby.participating_friends.begin());it!=lobby.participating_friends.end();++it)
-			if((*it)!=peer_id && mLinkMgr->isOnline(*it)) 
-			{
-				RsChatLobbyBouncingObject *obj2 = item->duplicate() ; // makes a copy
-				RsChatItem *item2 = dynamic_cast<RsChatItem*>(obj2) ;
-
-				assert(item2 != NULL) ;
-
-				item2->PeerId(*it) ;	// replaces the virtual peer id with the actual destination.
-
-				if(is_message)
-					checkSizeAndSendMessage(static_cast<RsChatLobbyMsgItem*>(item2)) ;
-				else
-					sendItem(item2);
-			}
-
-		if(++lobby.connexion_challenge_count > CONNECTION_CHALLENGE_MAX_COUNT && now > lobby.last_connexion_challenge_time + CONNECTION_CHALLENGE_MIN_DELAY) 
-		{
-			lobby.connexion_challenge_count = 0 ;
-			 send_challenge_lobby = item->lobby_id ;
-			 send_challenge = true ;
-			 lobby.last_connexion_challenge_time = now ;
-		}
 	}
+	ChatLobbyEntry& lobby(it->second) ;
 
-	if(send_challenge)
+	// Adds the peer id to the list of friend participants, even if it's not original msg source
-		sendConnectionChallenge(send_challenge_lobby) ;
+
+	if(peer_id != mLinkMgr->getOwnId())
+		lobby.participating_friends.insert(peer_id) ;
+
+	lobby.nick_names[item->nick] = now ;
+
+	// Checks wether the msg is already recorded or not
+
+	std::map<ChatLobbyMsgId,time_t>::iterator it2(lobby.msg_cache.find(item->msg_id)) ;
+
+	if(it2 != lobby.msg_cache.end()) // found!
+	{
+#ifdef CHAT_DEBUG
+		std::cerr << "  Msg already received at time " << it2->second << ". Dropping!" << std::endl ;
+#endif
+		it2->second = now ;	// update last msg seen time, to prevent echos.
+		return false ;
+	}
+#ifdef CHAT_DEBUG
+	std::cerr << "  Msg already not received already. Adding in cache, and forwarding!" << std::endl ;
+#endif
+
+	lobby.msg_cache[item->msg_id] = now ;
+	lobby.last_activity = now ;
+
+	bool is_message =  (NULL != dynamic_cast<RsChatLobbyMsgItem*>(item)) ;
+
+	// Forward to allparticipating friends, except this peer.
+
+	for(std::set<std::string>::const_iterator it(lobby.participating_friends.begin());it!=lobby.participating_friends.end();++it)
+		if((*it)!=peer_id && mLinkMgr->isOnline(*it)) 
+		{
+			RsChatLobbyBouncingObject *obj2 = item->duplicate() ; // makes a copy
+			RsChatItem *item2 = dynamic_cast<RsChatItem*>(obj2) ;
+
+			assert(item2 != NULL) ;
+
+			item2->PeerId(*it) ;	// replaces the virtual peer id with the actual destination.
+
+			if(is_message)
+				checkSizeAndSendMessage(static_cast<RsChatLobbyMsgItem*>(item2)) ;
+			else
+				sendItem(item2);
+		}
+
+	++lobby.connexion_challenge_count ;
 
 	return true ;
 }
@@ -1737,6 +1738,7 @@ void p3ChatService::sendLobbyStatusItem(const ChatLobbyId& lobby_id,int type,con
 
 		item.event_type = type ;
 		item.string1 = status_string ;
+		item.sendTime = time(NULL) ;
 	}
 	std::string ownId = mLinkMgr->getOwnId();
 	bounceLobbyObject(&item,ownId) ;
@@ -1841,7 +1843,7 @@ void p3ChatService::handleConnectionChallenge(RsChatLobbyConnectChallengeItem *i
 	}
 
 	if(found) // send invitation. As the peer already has the lobby, the invitation will most likely be accepted.
-		invitePeerToLobby(lobby_id, item->PeerId()) ;
+		invitePeerToLobby(lobby_id, item->PeerId(),true) ;
 #ifdef CHAT_DEBUG
 	else
 		std::cerr << "    Challenge denied: no existing cached msg has matching Id." << std::endl;
@@ -1917,10 +1919,13 @@ void p3ChatService::getChatLobbyList(std::list<ChatLobbyInfo>& linfos)
 	for(std::map<ChatLobbyId,ChatLobbyEntry>::const_iterator it(_chat_lobbys.begin());it!=_chat_lobbys.end();++it)
 		linfos.push_back(it->second) ;
 }
-void p3ChatService::invitePeerToLobby(const ChatLobbyId& lobby_id, const std::string& peer_id) 
+void p3ChatService::invitePeerToLobby(const ChatLobbyId& lobby_id, const std::string& peer_id,bool connexion_challenge) 
 {
 #ifdef CHAT_DEBUG
-	std::cerr << "Sending invitation to peer " << peer_id << " to lobby "<< std::hex << lobby_id << std::dec << std::endl;
+	if(connexion_challenge)
+		std::cerr << "Sending connexion challenge accept to peer " << peer_id << " for lobby "<< std::hex << lobby_id << std::dec << std::endl;
+	else
+		std::cerr << "Sending invitation to peer " << peer_id << " to lobby "<< std::hex << lobby_id << std::dec << std::endl;
 #endif
 
 	RsChatLobbyInviteItem *item = new RsChatLobbyInviteItem ;
@@ -1938,7 +1943,7 @@ void p3ChatService::invitePeerToLobby(const ChatLobbyId& lobby_id, const std::st
 	}
 	item->lobby_id = lobby_id ;
 	item->lobby_name = it->second.lobby_name ;
-	item->lobby_privacy_level = it->second.lobby_privacy_level ;
+	item->lobby_privacy_level = connexion_challenge?RS_CHAT_LOBBY_PRIVACY_LEVEL_CHALLENGE:(it->second.lobby_privacy_level) ;
 	item->PeerId(peer_id) ;
 
 	sendItem(item) ;
@@ -1964,7 +1969,7 @@ void p3ChatService::handleRecvLobbyInvite(RsChatLobbyInviteItem *item)
 			std::cerr << "     privacy levels: " << item->lobby_privacy_level << " vs. " << it->second.lobby_privacy_level ;
 #endif
 
-			if(item->lobby_privacy_level != it->second.lobby_privacy_level)
+			if(item->lobby_privacy_level != RS_CHAT_LOBBY_PRIVACY_LEVEL_CHALLENGE && item->lobby_privacy_level != it->second.lobby_privacy_level)
 			{
 				std::cerr << " : Don't match. Cancelling." << std::endl;
 				return ;
@@ -1979,6 +1984,12 @@ void p3ChatService::handleRecvLobbyInvite(RsChatLobbyInviteItem *item)
 			it->second.participating_friends.insert(item->PeerId()) ;
 			return ;
 		}
+
+		// Don't record the invitation if it's a challenge response item or a lobby we don't have.
+		//
+		if(item->lobby_privacy_level == RS_CHAT_LOBBY_PRIVACY_LEVEL_CHALLENGE)	
+			return ;
+
 		// no, then create a new invitation entry in the cache.
 		
 		ChatLobbyInvite invite ;
@@ -2352,6 +2363,7 @@ void p3ChatService::cleanLobbyCaches()
 
 	std::list<ChatLobbyId> keep_alive_lobby_ids ;
 	std::list<ChatLobbyId> changed_lobbies ;
+	std::list<ChatLobbyId> send_challenge_lobbies ;
 
 	{
 		RsStackMutex stack(mChatMtx); /********** STACK LOCKED MTX ******/
@@ -2405,6 +2417,16 @@ void p3ChatService::cleanLobbyCaches()
 				keep_alive_lobby_ids.push_back(it->first) ;
 				it->second.last_keep_alive_packet_time = now ;
 			}
+
+			// 4 - look at lobby activity and possibly send connexion challenge
+			//
+			if(++it->second.connexion_challenge_count > CONNECTION_CHALLENGE_MAX_COUNT && now > it->second.last_connexion_challenge_time + CONNECTION_CHALLENGE_MIN_DELAY) 
+			{
+				it->second.connexion_challenge_count = 0 ;
+				it->second.last_connexion_challenge_time = now ;
+
+				send_challenge_lobbies.push_back(it->first);
+			}
 		}
 
 		// 2 - clean deprecated public chat lobby records
@@ -2432,6 +2454,11 @@ void p3ChatService::cleanLobbyCaches()
 	// update the gui
 	for(std::list<ChatLobbyId>::const_iterator it(changed_lobbies.begin());it!=changed_lobbies.end();++it)
 		rsicontrol->getNotify().notifyChatLobbyEvent(*it,RS_CHAT_LOBBY_EVENT_KEEP_ALIVE,"------------","") ;
+
+	// send connexion challenges
+	//
+	for(std::list<ChatLobbyId>::const_iterator it(send_challenge_lobbies.begin());it!=send_challenge_lobbies.end();++it)
+		sendConnectionChallenge(*it) ;
 }
 
 

libretroshare/src/services/p3chatservice.h

@@ -160,7 +160,7 @@ class p3ChatService: public p3Service, public p3Config, public pqiMonitor
 		bool acceptLobbyInvite(const ChatLobbyId& id) ;
 		void denyLobbyInvite(const ChatLobbyId& id) ;
 		void getPendingChatLobbyInvites(std::list<ChatLobbyInvite>& invites) ;
-		void invitePeerToLobby(const ChatLobbyId&, const std::string&) ;
+		void invitePeerToLobby(const ChatLobbyId&, const std::string& peer_id,bool connexion_challenge = false) ;
 		void unsubscribeChatLobby(const ChatLobbyId& lobby_id) ;
 		bool setNickNameForChatLobby(const ChatLobbyId& lobby_id,const std::string& nick) ;
 		bool getNickNameForChatLobby(const ChatLobbyId& lobby_id,std::string& nick) ;