Projects tigase _server server-core Issues #1271
NPE in JDBCMsgRepository.storeMessage due to missing 'from' (#1271)
wojciech.kapcia@tigase.net opened 3 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)
wojciech.kapcia@tigase.net commented 3 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:

  1. 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)
  2. 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.net commented 3 years ago

Changes applied per Andrzej's comments.

issue 1 of 1
Type
Bug
Priority
Normal
Assignee
Version
Candidate for next minor release, tigase-server-8.2.0
Spent time
9h
Issue Votes (0)
Watchers (0)
Reference
tigase/_server/server-core#1271
Please wait...
Page is in error, reload to recover