NPE in JDBCMsgRepository.storeMessage due to missing 'from' (#1271)

Wojciech Kapcia (Tigase) opened 4 years ago

We already had some packets that seemed to not be stamped with correct "from" in tigase.xmpp.impl.BindResource#preProcess

[2021-07-16 20:57:25:218] [SEVERE  ] [        in_29-sess-man ] AbstractMessageReceiver$QueueListener.run(): [in_29-sess-man] Exception during packet processing: from=c2s@ip-172-31-20-109.us-west-2.compute.internal/172.20.0.2_5222_172.31.4.176_53150, to=sess-man@ip-172-31-20-109.us-west-2.compute.internal, DATA=<message type="chat" xmlns="jabber:client" to="…@sure.im"><received xmlns="urn:xmpp:receipts" id="NTQwNjljYmQtYjQ3NC…jZhYjVhNg=="/><origin-id xmlns="urn:xmpp:sid:0" id="7811B228-…-72B4605869EA"/></message>, SIZE=333, XMLNS=jabber:client, PRIORITY=NORMAL, PERMISSION=NONE, TYPE=chat, STABLE_ID=d61e3637-…-a80ad6818b16
java.lang.NullPointerException
	at tigase.server.amp.db.JDBCMsgRepository.storeMessage(JDBCMsgRepository.java:391)
	at jdk.internal.reflect.GeneratedMethodAccessor114.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
	at java.base/java.lang.reflect.Method.invoke(Unknown Source)
	at tigase.stats.StatisticsInvocationHandler.invoke(StatisticsInvocationHandler.java:75)
	at com.sun.proxy.$Proxy44.storeMessage(Unknown Source)
	at tigase.server.amp.db.MsgRepository$MsgRepositoryMDBean.storeMessage(MsgRepository.java:402)
	at tigase.xmpp.impl.OfflineMessages.savePacketForOffLineUser(OfflineMessages.java:383)
	at tigase.xmpp.impl.MessageAmp.postProcess(MessageAmp.java:114)
	at tigase.server.xmppsession.SessionManager.processPacket(SessionManager.java:1872)
	at tigase.cluster.SessionManagerClustered.processPacket(SessionManagerClustered.java:282)
	at tigase.cluster.SessionManagerClustered.processPacket(SessionManagerClustered.java:268)
	at tigase.server.AbstractMessageReceiver$QueueListener.run(AbstractMessageReceiver.java:1404)

Related
- NPE in StreamManagementIOProcessor (#1188) Closed

Activities

Wojciech Kapcia (Tigase) commented 4 years ago
As per discussion, most likely majority of our issues recently were caused by packet's not being stamped correctly by BindResource#preProcess. We co remove from from the incoming packets in tigase.server.xmppclient.ClientConnectionManager#processSocketData as per specification but instead of stamping it already in CM we do it in SessionManager. Due to higher disconnection rate recently due to 'mobile-first' approach it may happen that certain packets would arrive to Session Manager after the session is gone and wouldn't be stamped correctly causing issues. Unfortunately due to current operation of (Default) Clustering strategy setting stanza from in CM is not possible (would cause duplications)

After discussing this with @andrzej.wojcik I tackled the issue in two ways:

I added additional check in BindResource#preProcess that would simply drop packets that arrive from Client Connection Manager with the missing session (as we can't process them properly in majority of cases either way)

thinking that this could still cause some problems (dropping packets acked by SM came to mind) I decided to temporarily extend Packet API with setServerAuthorisedStanzaFrom field that would hold authenticated user JID (that Connection Manager is already aware of) and would be used later on by BindResource pre-processor. Benefit of it would be even faster processing in BindResource#preProcess (even less checks) and, after reviewing the APIs in 8.3.0/9.0.0 we can resolve the issue properly by setting stanza from attribute directly in Connection Manager.
Wojciech Kapcia (Tigase) commented 4 years ago

Changes applied per Andrzej's comments.
Login to comment

Type	Bug
Priority	Normal
Assignee	Wojciech Kapcia (Tigase)
Version	Candidate for next minor release, tigase-server-8.2.0
Spent time	0

Issue Votes (0)

Watchers (0)

Reference

tigase/_server/server-core#1271