Should connection be closed when receiving error?

Smack Smack Dev
1

Smack, by default, closes the connection (‘close with error’) when a stanza parsing error occurs (this behavior is configurable).

Should Smack do this when it fails to parse an error IQ stanza that reflects a bad stanza?

For example, receiving the stanza below causes Smack to close the connection. It is indeed true that the stanza is not correct (foobar isn’t a valid value for the subscription attribute), but should Smack be closing the connection when that’s part of the other party erroring out on exactly that?

<iq type="error" id="CVQDW-72" from="smack-inttest-one-t1i2s@example.org" to="smack-inttest-one-t1i2s@example.org/one-t1i2s">
  <query xmlns="jabber:iq:roster">
    <item jid="test-target-mtm3m@example.org" subscription="foobar"/>
  </query>
  <error code="500" type="wait">
    <internal-server-error xmlns="urn:ietf:params:xml:ns:xmpp-stanzas"/>
  </error>
</iq>

May 01, 2025 12:55:16 PM org.jivesoftware.smack.AbstractXMPPConnection callConnectionClosedOnErrorListener
WARNING: Connection XMPPTCPConnection[smack-inttest-one-t1i2s@example.org/one-t1i2s] (1) closed with error
java.io.IOException: java.lang.IllegalArgumentException: No enum constant org.jivesoftware.smack.roster.packet.RosterPacket.ItemType.foobar
	at org.jivesoftware.smack.parsing.ExceptionThrowingCallback.handleUnparsableStanza(ExceptionThrowingCallback.java:36)
	at org.jivesoftware.smack.parsing.ExceptionThrowingCallbackWithHint.handleUnparsableStanza(ExceptionThrowingCallbackWithHint.java:42)
	at org.jivesoftware.smack.AbstractXMPPConnection.parseAndProcessStanza(AbstractXMPPConnection.java:1471)
	at org.jivesoftware.smack.tcp.XMPPTCPConnection.access$100(XMPPTCPConnection.java:132)
	at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader.parsePackets(XMPPTCPConnection.java:988)
	at org.jivesoftware.smack.tcp.XMPPTCPConnection$PacketReader$1.run(XMPPTCPConnection.java:953)
	at java.base/java.lang.Thread.run(Thread.java:840)
Caused by: java.lang.IllegalArgumentException: No enum constant org.jivesoftware.smack.roster.packet.RosterPacket.ItemType.foobar
	at java.base/java.lang.Enum.valueOf(Enum.java:273)
	at org.jivesoftware.smack.roster.packet.RosterPacket$ItemType.valueOf(RosterPacket.java:330)
	at org.jivesoftware.smack.roster.packet.RosterPacket$ItemType.fromString(RosterPacket.java:378)
	at org.jivesoftware.smack.roster.provider.RosterPacketProvider.parseItem(RosterPacketProvider.java:86)
	at org.jivesoftware.smack.roster.provider.RosterPacketProvider.parse(RosterPacketProvider.java:50)
	at org.jivesoftware.smack.roster.provider.RosterPacketProvider.parse(RosterPacketProvider.java:33)
	at org.jivesoftware.smack.provider.IqProvider.lambda$parse$0(IqProvider.java:105)
	at org.jivesoftware.smack.provider.AbstractProvider.wrapExceptions(AbstractProvider.java:100)
	at org.jivesoftware.smack.provider.IqProvider.parse(IqProvider.java:105)
	at org.jivesoftware.smack.util.PacketParserUtils.parseIQ(PacketParserUtils.java:555)
	at org.jivesoftware.smack.util.PacketParserUtils.parseStanza(PacketParserUtils.java:113)
	at org.jivesoftware.smack.AbstractXMPPConnection.parseAndProcessStanza(AbstractXMPPConnection.java:1463)
	... 4 more
2

Does it close on error irrespective of who sent this? Because that sounds like a DoS attack…

3

Hey, I would vote for not closing the connection from a single bogus message. Even if the connection gets re-created automatically soon after.
I see the chance that other (unrelated) messages might get lost during the connection close and re-creation. Also, I’m considering the connection re-creation as an relatively expensive operation. An evil mind might use this behavior for putting stress on the backend system that uses the Smack library or even break it.

You might want to check my recent bugreport, which is kind of related :slight_smile:

4

As the hoped-for advantage of this behavior has not materialized, I am going to change the default behavior to discard incoming non-compliant stream elements and log an error (or warning).

1 Like